[PATCH V6 3/3] ahci: imx: Add i.MX53 support

Shawn Guo shawn.guo at linaro.org
Tue Dec 10 08:37:27 EST 2013


On Tue, Dec 10, 2013 at 12:47:38PM +0100, Sascha Hauer wrote:
> > @@ -34,10 +34,21 @@ enum {
> >  	HOST_TIMER1MS = 0xe0,			/* Timer 1-ms */
> >  };
> >  
> > +enum ahci_imx_type {
> > +	AHCI_IMX53,
> > +	AHCI_IMX6Q,
> > +};
> 
> Please next time introduce a SoC specific struct to encode the
> differences between SoCs. This way you can abstract away the differences
> in flags and function callbacks and don't end up with functions which
> do completely different things for different SoCs like currently in
> imx_sata_clock_enable(). The if (type == SOC_XY) style doesn't scale in
> many drivers anymore.

Yea, I was thinking about the same thing when reviewing the patch in the
first time, but decided to not comment on that because currently the
added functions are doing the similar/related thing.  But I agree that
SoC specific structure should be some thing more scalable and we should
go for in the future.

Shawn




More information about the linux-arm-kernel mailing list