[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