[PATCH v2 3/3] net/fec: add imx6q enet support

Wolfram Sang w.sang at pengutronix.de
Wed Sep 21 08:26:33 EDT 2011


On Wed, Sep 21, 2011 at 07:58:22PM +0800, Shawn Guo wrote:
> On Wed, Sep 21, 2011 at 01:32:39PM +0200, Wolfram Sang wrote:
> > 
> > > +/* Controller has GBIT support */
> > > +#define FEC_QUIRK_HAS_GBIT		(1 << 3)
> > 
> > Heh, this is not really a quirk, but a nice feature :) I think we can
> > drop QUIRK if we see driver_data more as "flags" instead of "quirks"?
> > Minor, though.
> > 
> As you have told, all these FEC_QUIRK_* are just flags actually.  The
> name was pick to keep the consistency, as they are all used for the
> same purpose.

I think introducing FEC_FEATURE_HAS_GBIT would be consistent enough, but
as I said, I don't mind much.

> > Also minor, but the patch looks like a good oportunity to start
> > replacing magic values with proper defines?
> > 
> There are already so many magic numbers.  It really deserves a separated
> patch.

That is the other possibility, yes. Which sadly never happened.

> I heard that Uwe had a plan to do that some time ago.  He gives up
> now? :)

I don't know about this case. Also, it is not about blaming here. It is
totally okay for you to say that you don't want to change your patch to
start replacing the magic values. I mainly wanted to point out the
oportunity here.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110921/780cd0f7/attachment.sig>


More information about the linux-arm-kernel mailing list