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]

Date:	Wed, 7 Jan 2015 10:33:10 +0800
From:	Shawn Guo <shawn.guo@...aro.org>
To:	"fugang.duan@...escale.com" <fugang.duan@...escale.com>
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

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