netdev - RE: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function

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: <BLUPR03MB37385464C06052A231ABF19F5460@BLUPR03MB373.namprd03.prod.outlook.com>
Date:	Wed, 7 Jan 2015 02:40:33 +0000
From:	"fugang.duan@...escale.com" <fugang.duan@...escale.com>
To:	Shawn Guo <shawn.guo@...aro.org>
CC:	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"bhutchings@...arflare.com" <bhutchings@...arflare.com>,
	"stephen@...workplumber.org" <stephen@...workplumber.org>
Subject: RE: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback
 function

From: Shawn Guo <shawn.guo@...aro.org> Sent: Wednesday, January 07, 2015 10:33 AM
> To: Duan Fugang-B38611
> Cc: davem@...emloft.net; netdev@...r.kernel.org;
> bhutchings@...arflare.com; stephen@...workplumber.org
> Subject: Re: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode
> callback function
> 
> On Wed, Jan 07, 2015 at 01:41:33AM +0000, fugang.duan@...escale.com wrote:
> > From: Shawn Guo <shawn.guo@...aro.org> Sent: Tuesday, January 06, 2015
> > 7:48 PM
> > > To: Duan Fugang-B38611
> > > Cc: davem@...emloft.net; netdev@...r.kernel.org;
> > > bhutchings@...arflare.com; stephen@...workplumber.org
> > > Subject: Re: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode
> > > callback function
> > >
> > > On Wed, Dec 24, 2014 at 05:30:40PM +0800, Fugang Duan wrote:
> > > > i.MX6q/dl, i.MX6SX SOCs enet support sleep mode that magic packet
> > > > can wake up system in suspend status. For different SOCs, there
> > > > have some SOC specifical GPR register to set sleep on/off mode. So
> > > > add these to callback function for driver.
> > > >
> > > > Signed-off-by: Fugang Duan <B38611@...escale.com>
> > >
> > > I do not like this patch.  In the end, this is just a GRP register
> > > bit setup per FEC driver need.  Rather than messing up platform code
> > > for each SoC with the same pattern, I do not see why this can not be
> > > done by FEC driver itself.
> > >
> > > You can take a look at LDB driver (drivers/gpu/drm/imx/imx-ldb.c) to
> > > see how this can be done.
> > >
> > > Shawn
> > >
> >
> > Hi, Shawn,
> >
> > It is SOC related setting, not fec IP itself setting, and different SOC
> GPR setting is not different.
> > So I think it better to put it to platform code.
> 
> The GPR difference between SoCs can be encoded in device tree as well.
> It's pointless to repeat the same code pattern for every single platform,
> that need to set up GPR bits for enabling magic packet wake up, while the
> only difference is the register and bit offset.
> 
> The platform code will become quite messy and unmaintainable if every
> device driver dump their GPR register setup code into platform.
> 
> Sorry, but it's NACK from me.
> 
> Shawn

Yes, the platform code will become quite messy and tremendous.
I agree with your thinking.

But David applied the patches, how can I do it now ?

Thanks,
Andy
--
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