netdev - Re: [PATCH v2 05/10] net/fec: add dual fec support for mx28

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110104150733.GF2987@jasper.tkos.co.il>
Date:	Tue, 4 Jan 2011 17:07:33 +0200
From:	Baruch Siach <baruch@...s.co.il>
To:	Shawn Guo <shawn.guo@...escale.com>
Cc:	davem@...emloft.net, gerg@...pgear.com, eric@...rea.com,
	bryan.wu@...onical.com, r64343@...escale.com, B32542@...escale.com,
	u.kleine-koenig@...gutronix.de, lw@...o-electronics.de,
	w.sang@...gutronix.de, s.hauer@...gutronix.de,
	netdev@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 05/10] net/fec: add dual fec support for mx28

Hi Shawn,

On Tue, Jan 04, 2011 at 10:13:09PM +0800, Shawn Guo wrote:
> On Tue, Jan 04, 2011 at 11:59:16AM +0200, Baruch Siach wrote:
> > On Tue, Jan 04, 2011 at 05:24:11PM +0800, Shawn Guo wrote:

[snip]

> > > -#ifdef CONFIG_ARCH_MXC
> > > -#include <mach/hardware.h>
> > 
> > Since you now remove mach/hardware.h for ARCH_MXC, does this build for all 
> > i.MX variants?
> > 
> Did the test build for mx25, mx27, mx3 and mx51.

This is surprising. It means that this include was not needed in the first 
place. git blame says this was added in 196719ec (fec: Add support for 
Freescale MX27) by Sascha.

> > > +#ifdef CONFIG_SOC_IMX28
> > > +/*
> > > + * mx28 does not have MIIGSK registers
> > > + */
> > > +#undef FEC_MIIGSK_ENR
> > > +#include <mach/mxs.h>
> > > +#else
> > > +#define cpu_is_mx28()	(0)
> > > +#endif
> > 
> > This breaks kernels for multiple archs (e.g. i.MX28 and i.MX25). Please use 
> > run-time detection of CPU type, and do the MII/RMII etc. configuration 
> > accordingly.
> > 
> I do not find a good way to detect cpu type.  Neither adding a new
> platform data field nor using __machine_arch_type to enumerate all
> mx28 based machine (though there is only one currently) seems to be
> good for me.

How about:

#ifdef CONFIG_SOC_IMX28
#include <mach/mxs.h>
#else
#define cpu_is_mx28()    (0)
#endif

if (cpu_is_mx28() {
    /* Do i.MX28 stuff */
} else {
    /* Do other i.MX stuff */
}

Note that the '#ifdef FEC_MIIGSK_ENR' section in fec_restart() is there only 
to allow build for M5272 which does not have this define in fec.h. Physically, 
i.MX27 does not have this register either.

> I will try to manipulate some mx28 unique register to identify mx28
> from other i.mx variants.  Hopefully, it will work.
> 
> Thanks for the comments.

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@...s.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html