| Message ID | 20240603055248.3928469-3-fea.wang@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/dma: Add error handling for loading descriptions failing | expand |
On Mon, Jun 3, 2024 at 7:48 AM Fea.Wang <fea.wang@sifive.com> wrote: > When calling the loading a description function, it should be noticed > that the function may return a failure value. Breaking the loop is one > of the possible ways to handle it. > > Signed-off-by: Fea.Wang <fea.wang@sifive.com> > Looks good, a nitpick comment, I would either squash this with patch #1 or move the change to return of error code in stream_desc_load() to this patch. > --- > hw/dma/xilinx_axidma.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c > index 4b475e5484..b8ab5a423d 100644 > --- a/hw/dma/xilinx_axidma.c > +++ b/hw/dma/xilinx_axidma.c > @@ -291,7 +291,9 @@ static void stream_process_mem2s(struct Stream *s, > StreamSink *tx_data_dev, > } > > while (1) { > - stream_desc_load(s, s->regs[R_CURDESC]); > + if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) { > + break; > + } > > if (s->desc.status & SDESC_STATUS_COMPLETE) { > s->regs[R_DMASR] |= DMASR_HALTED; > @@ -348,7 +350,9 @@ static size_t stream_process_s2mem(struct Stream *s, > unsigned char *buf, > } > > while (len) { > - stream_desc_load(s, s->regs[R_CURDESC]); > + if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) { > + break; > + } > > if (s->desc.status & SDESC_STATUS_COMPLETE) { > s->regs[R_DMASR] |= DMASR_HALTED; > -- > 2.34.1 > >
Hi Edgar, thank you for the advice, I will squash them in the next version of the patch series. Sincerely, Fea On Mon, Jun 3, 2024 at 6:21 PM Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > On Mon, Jun 3, 2024 at 7:48 AM Fea.Wang <fea.wang@sifive.com> wrote: > >> When calling the loading a description function, it should be noticed >> that the function may return a failure value. Breaking the loop is one >> of the possible ways to handle it. >> >> Signed-off-by: Fea.Wang <fea.wang@sifive.com> >> > > > Looks good, a nitpick comment, I would either squash this with patch #1 > or move the change to return of error code in stream_desc_load() to this > patch. > > > > >> --- >> hw/dma/xilinx_axidma.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c >> index 4b475e5484..b8ab5a423d 100644 >> --- a/hw/dma/xilinx_axidma.c >> +++ b/hw/dma/xilinx_axidma.c >> @@ -291,7 +291,9 @@ static void stream_process_mem2s(struct Stream *s, >> StreamSink *tx_data_dev, >> } >> >> while (1) { >> - stream_desc_load(s, s->regs[R_CURDESC]); >> + if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) { >> + break; >> + } >> >> if (s->desc.status & SDESC_STATUS_COMPLETE) { >> s->regs[R_DMASR] |= DMASR_HALTED; >> @@ -348,7 +350,9 @@ static size_t stream_process_s2mem(struct Stream *s, >> unsigned char *buf, >> } >> >> while (len) { >> - stream_desc_load(s, s->regs[R_CURDESC]); >> + if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) { >> + break; >> + } >> >> if (s->desc.status & SDESC_STATUS_COMPLETE) { >> s->regs[R_DMASR] |= DMASR_HALTED; >> -- >> 2.34.1 >> >>
Reviewed-by: Frank Chang <frank.chang@sifive.com> Fea.Wang <fea.wang@sifive.com> 於 2024年6月3日 週一 下午1:48寫道： > > When calling the loading a description function, it should be noticed > that the function may return a failure value. Breaking the loop is one > of the possible ways to handle it. > > Signed-off-by: Fea.Wang <fea.wang@sifive.com> > --- > hw/dma/xilinx_axidma.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c > index 4b475e5484..b8ab5a423d 100644 > --- a/hw/dma/xilinx_axidma.c > +++ b/hw/dma/xilinx_axidma.c > @@ -291,7 +291,9 @@ static void stream_process_mem2s(struct Stream *s, StreamSink *tx_data_dev, > } > > while (1) { > - stream_desc_load(s, s->regs[R_CURDESC]); > + if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) { > + break; > + } > > if (s->desc.status & SDESC_STATUS_COMPLETE) { > s->regs[R_DMASR] |= DMASR_HALTED; > @@ -348,7 +350,9 @@ static size_t stream_process_s2mem(struct Stream *s, unsigned char *buf, > } > > while (len) { > - stream_desc_load(s, s->regs[R_CURDESC]); > + if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) { > + break; > + } > > if (s->desc.status & SDESC_STATUS_COMPLETE) { > s->regs[R_DMASR] |= DMASR_HALTED; > -- > 2.34.1 > >
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c index 4b475e5484..b8ab5a423d 100644 --- a/hw/dma/xilinx_axidma.c +++ b/hw/dma/xilinx_axidma.c @@ -291,7 +291,9 @@ static void stream_process_mem2s(struct Stream *s, StreamSink *tx_data_dev, } while (1) { - stream_desc_load(s, s->regs[R_CURDESC]); + if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) { + break; + } if (s->desc.status & SDESC_STATUS_COMPLETE) { s->regs[R_DMASR] |= DMASR_HALTED; @@ -348,7 +350,9 @@ static size_t stream_process_s2mem(struct Stream *s, unsigned char *buf, } while (len) { - stream_desc_load(s, s->regs[R_CURDESC]); + if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) { + break; + } if (s->desc.status & SDESC_STATUS_COMPLETE) { s->regs[R_DMASR] |= DMASR_HALTED;
When calling the loading a description function, it should be noticed that the function may return a failure value. Breaking the loop is one of the possible ways to handle it. Signed-off-by: Fea.Wang <fea.wang@sifive.com> --- hw/dma/xilinx_axidma.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)