Messages in this thread | | | Date | Tue, 16 Apr 2024 22:11:58 +0300 | From | Andy Shevchenko <> | Subject | Re: [PATCH 4/4] dmaengine: dw: Simplify max-burst calculation procedure |
| |
On Tue, Apr 16, 2024 at 07:28:58PM +0300, Serge Semin wrote: > In order to have a more coherent DW AHB DMA slave configuration method > let's simplify the source and destination channel max-burst calculation > procedure: > > 1. Create the max-burst verification method as it has been just done for > the memory and peripheral address widths. Thus the DWC DMA slave config
dwc_config() method
?
> method will turn to a set of the verification methods execution. > > 2. Since both the generic DW AHB DMA and Intel DMA32 engines support the
"i" in iDMA 32-bit stands for "integrated", so 'Intel iDMA 32-bit'
> power-of-2 bursts only, then the specified by the client driver max-burst > values can be converted to being power-of-2 right in the max-burst > verification method. > > 3. Since max-burst encoded value is required on the CTL_LO fields > calculation stage, the encode_maxburst() callback can be easily dropped > from the dw_dma structure meanwhile the encoding procedure will be > executed right in the CTL_LO register value calculation. > > Thus the update will provide the next positive effects: the internal > DMA-slave config structure will contain only the real DMA-transfer config > value, which will be encoded to the DMA-controller register fields only > when it's required on the buffer mapping; the redundant encode_maxburst() > callback will be dropped simplifying the internal HW-abstraction API; > DWC-config method will look more readable executing the verification
dwc_config() method
?
> functions one-by-one.
..
> +static void dwc_verify_maxburst(struct dma_chan *chan)
It's inconsistent to the rest of _verify methods. It doesn't verify as it doesn't return anything. Make it int or rename the function.
> +{ > + struct dw_dma_chan *dwc = to_dw_dma_chan(chan); > + > + dwc->dma_sconfig.src_maxburst = > + clamp(dwc->dma_sconfig.src_maxburst, 1U, dwc->max_burst); > + dwc->dma_sconfig.dst_maxburst = > + clamp(dwc->dma_sconfig.dst_maxburst, 1U, dwc->max_burst); > + > + dwc->dma_sconfig.src_maxburst = > + rounddown_pow_of_two(dwc->dma_sconfig.src_maxburst); > + dwc->dma_sconfig.dst_maxburst = > + rounddown_pow_of_two(dwc->dma_sconfig.dst_maxburst); > +}
..
> static int dwc_verify_p_buswidth(struct dma_chan *chan) > - reg_burst = rounddown_pow_of_two(dwc->dma_sconfig.src_maxburst); > + reg_burst = dwc->dma_sconfig.src_maxburst;
Seems you have a dependency, need a comment below that maxburst has to be "verified" [whatever] first.
..
> +static inline u8 dw_dma_encode_maxburst(u32 maxburst) > +{ > + /* > + * Fix burst size according to dw_dmac. We need to convert them as: > + * 1 -> 0, 4 -> 1, 8 -> 2, 16 -> 3. > + */ > + return maxburst > 1 ? fls(maxburst) - 2 : 0; > +}
Split these moves to another preparatory patch.
-- With Best Regards, Andy Shevchenko
| |