On Tue, Apr 16, 2024 at 07:28:57PM +0300, Serge Semin wrote:
> Currently the CTL LO fields are calculated on the platform-specific basis.
> It's implemented by means of the prepare_ctllo() callbacks using the
> ternary operator within the local variables init block at the beginning of
> the block scope. The functions code currently is relatively hard to
> comprehend and isn't that optimal since implies four conditional
> statements executed and two additional local variables defined. Let's
> simplify the DW AHB DMA prepare_ctllo() method by unrolling the ternary
> operators into the normal if-else statement, dropping redundant
> master-interface ID variables and initializing the local variables based
> on the singly evaluated DMA-transfer direction check. Thus the method will
> look much more readable since now the fields content can be easily
> inferred right from the if-else branch. Provide the same update in the
> Intel DMA32 core driver for sake of the driver code unification.
>
> Note besides of the effects described above this update is basically a
> preparation before dropping the max burst encoding callback. It will
> require calling the burst fields calculation methods right in the
> prepare_ctllo() callbacks, which would have made the later function code
> even more complex.
Yeah, this is inherited from the original driver where it used to be a macro.
...
> + if (dwc->direction == DMA_MEM_TO_DEV) {
> + sms = dwc->dws.m_master;
> + smsize = 0;
> + dms = dwc->dws.p_master;
> + dmsize = sconfig->dst_maxburst;
I would group it differently, i.e.
sms = dwc->dws.m_master;
dms = dwc->dws.p_master;
smsize = 0;
dmsize = sconfig->dst_maxburst;
> + } else if (dwc->direction == DMA_DEV_TO_MEM) {
> + sms = dwc->dws.p_master;
> + smsize = sconfig->src_maxburst;
> + dms = dwc->dws.m_master;
> + dmsize = 0;
> + } else /* DMA_MEM_TO_MEM */ {
> + sms = dwc->dws.m_master;
> + smsize = 0;
> + dms = dwc->dws.m_master;
> + dmsize = 0;
> + }
Ditto for two above cases.
> static u32 idma32_prepare_ctllo(struct dw_dma_chan *dwc)
> {
> struct dma_slave_config *sconfig = &dwc->dma_sconfig;
> - u8 smsize = (dwc->direction == DMA_DEV_TO_MEM) ? sconfig->src_maxburst : 0;
> - u8 dmsize = (dwc->direction == DMA_MEM_TO_DEV) ? sconfig->dst_maxburst : 0;
> + u8 smsize, dmsize;
> +
> + if (dwc->direction == DMA_MEM_TO_DEV) {
> + smsize = 0;
> + dmsize = sconfig->dst_maxburst;
> + } else if (dwc->direction == DMA_DEV_TO_MEM) {
> + smsize = sconfig->src_maxburst;
> + dmsize = 0;
> + } else /* DMA_MEM_TO_MEM */ {
> + smsize = 0;
> + dmsize = 0;
> + }
u8 smsize = 0, dmsize = 0;
if (dwc->direction == DMA_MEM_TO_DEV)
dmsize = sconfig->dst_maxburst;
else if (dwc->direction == DMA_DEV_TO_MEM)
smsize = sconfig->src_maxburst;
?
Something similar also can be done in the Synopsys case above, no?
--
With Best Regards,
Andy Shevchenko
