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

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:

quoted

quoted

+/* 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.

quoted

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/  |