On 21-07-18, 13:06, Paul Cercueil wrote: > +static inline void jz4780_dma_chan_enable(struct jz4780_dma_dev *jzdma, > + unsigned int chn) right justified and aligned with preceding please. While adding new code to a existing driver it is a good idea to conform to existing style > +{ > + if (jzdma->version == ID_JZ4770) > + jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKES, BIT(chn)); > +} > + > +static inline void jz4780_dma_chan_disable(struct jz4780_dma_dev *jzdma, > + unsigned int chn) > +{ > + if (jzdma->version == ID_JZ4770) > + jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DCKEC, BIT(chn)); so if another version has this feature we would do: if (jzdma->version == ID_JZ4770) || if (jzdma->version == ID_JZXXXX)) and so on.. why not add a value, clk_enable in the description and use that. For each controller it is set to true or false -- ~Vinod