From: Shawn Guo <redacted> Sent: Wednesday, January 07, 2015 10:33 AM

To: Duan Fugang-B38611
Cc: davem@davemloft.net; netdev@vger.kernel.org;
bhutchings@solarflare.com; stephen@networkplumber.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@freescale.com wrote:

quoted

From: Shawn Guo <redacted> Sent: Tuesday, January 06, 2015
7:48 PM

quoted

To: Duan Fugang-B38611
Cc: davem@davemloft.net; netdev@vger.kernel.org;
bhutchings@solarflare.com; stephen@networkplumber.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:

quoted

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

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.

quoted

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