Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures

On Thu, May 21, 2020 at 3:12 PM Serge Semin
[off-list ref] wrote:

On Thu, May 21, 2020 at 12:57:17PM +0300, Andy Shevchenko wrote:

quoted

On Thu, May 21, 2020 at 4:23 AM Serge Semin
[off-list ref] wrote:

...

quoted

Thanks for an update, but that's not what I asked for...

quoted

-static struct dw_dma_slave mid_dma_tx = { .dst_id = 1 };
-static struct dw_dma_slave mid_dma_rx = { .src_id = 0 };

quoted

 static int mid_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
 {
+       struct dw_dma_slave slave = {
+               .src_id = 0,
+               .dst_id = 0
+       };

quoted

quoted

-       struct dw_dma_slave *tx = dws->dma_tx;
-       struct dw_dma_slave *rx = dws->dma_rx;

May we simple do

struct dw_dma_slave tx = { .dst_id = 1 };
struct dw_dma_slave rx = { .src_id = 0 };

please?

Well, for me both solutions are equal

I don't think so.

except mine consumes less stack memory.

And brought confusion and less readability. :-(

The only reason why your solution might be better is that if DW DMA driver or
the DMA engine subsystem changed the dw_dma_slave structure instance passed to
the dma_request_channel() method, which non of them do. So I'll leave this for
Mark to decide. Mark, could you give us your final word about this?

I explained already why I prefer to see them in that form. Reader can
easily understand what request line is used for what channel.
In your code it's hidden somewhere and on top of that that _one_
structure on the stack adds more confusion.

-- 
With Best Regards,
Andy Shevchenko