Re: [PATCH] net: fec: add necessary defines to work on ARM64

[Lucas Stach]

Hi Andy,

Am Donnerstag, den 18.01.2018, 01:49 + schrieb Andy Duan:
> From: Lucas Stach <l.st...@pengutronix.de> Sent: Thursday, January
> 18, 2018 2:31 AM
> > The i.MX8 is a ARMv8 based SoC, that uses the same FEC IP as the
> > earlier,
> > ARMv7 based, i.MX SoCs. Allow the driver to work on ARM64.
> > 
> > Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
> > ---
[...]

> Kconfig also need to add ARM64 support.

No, not as far as I'm aware. I introduced the ARCH_MXC symbol for
ARM64, which unlocks the FEC config option when selected.

Regards,
Lucas

[PATCH] net: fec: add necessary defines to work on ARM64

[Lucas Stach]

The i.MX8 is a ARMv8 based SoC, that uses the same FEC IP as the
earlier, ARMv7 based, i.MX SoCs. Allow the driver to work on ARM64.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/net/ethernet/freescale/fec.h  | 5 +++--
 drivers/net/ethernet/freescale/fec_main.c | 8 +---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h 
b/drivers/net/ethernet/freescale/fec.h
index 5385074b3b7d..e7381f8ef89d 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -20,7 +20,8 @@
 #include 
 
 #if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || 
\
-defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM)
+defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM) || \
+defined(CONFIG_ARM64)
 /*
  * Just figures, Motorola would have to change the offsets for
  * registers in the same peripheral device on different models
@@ -195,7 +196,7 @@
  * Evidently, ARM SoCs have the FEC block generated in a
  * little endian mode so adjust endianness accordingly.
  */
-#if defined(CONFIG_ARM)
+#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
 #define fec32_to_cpu le32_to_cpu
 #define fec16_to_cpu le16_to_cpu
 #define cpu_to_fec32 cpu_to_le32
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index a74300a4459c..45eed37373a9 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -195,7 +195,8 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
  * account when setting it.
  */
 #if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || 
\
-defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM)
+defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM) || \
+defined(CONFIG_ARM64)
 #defineOPT_FRAME_SIZE  (PKT_MAXBUF_SIZE << 16)
 #else
 #defineOPT_FRAME_SIZE  0
@@ -2107,7 +2108,8 @@ static int fec_enet_get_regs_len(struct net_device *ndev)
 
 /* List of registers that can be safety be read to dump them with ethtool */
 #if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || 
\
-   defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM)
+   defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM) 
|| \
+   defined(CONFIG_ARM64)
 static u32 fec_enet_register_offset[] = {
FEC_IEVENT, FEC_IMASK, FEC_R_DES_ACTIVE_0, FEC_X_DES_ACTIVE_0,
FEC_ECNTRL, FEC_MII_DATA, FEC_MII_SPEED, FEC_MIB_CTRLSTAT, FEC_R_CNTRL,
@@ -3119,7 +3121,7 @@ static int fec_enet_init(struct net_device *ndev)
unsigned dsize_log2 = __fls(dsize);
 
WARN_ON(dsize != (1 << dsize_log2));
-#if defined(CONFIG_ARM)
+#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
fep->rx_align = 0xf;
fep->tx_align = 0xf;
 #else
-- 
2.11.0

Re: [PATCH 1/3] net: phy: add support to detect 100BASE-T1 capability

[Lucas Stach]

Am Donnerstag, den 14.12.2017, 10:46 +0100 schrieb Andrew Lunn:
> > > Hi Lucas
> > > 
> > > Why did you decide to do this, and not add a SUPPORTED_100baseT1?
> > > 
> > > Could a device support both 100-BASE-T and 100-BASE-T1?  If at
> > > some
> > > point we need to differentiate between them, it is going to be
> > > hard. Especially since this is part of the kernel ABI.
> > 
> > Networking and especially PHY isn't really my primary area of
> > expertise, so excuse my ignorance. My reasoning was that we don't
> > differentiate between 100BASE-T2 and 100BASE-T4 in the kernel
> > today, so
> > I thought it was fine to handle T1 the same way.
> > 
> > There are PHYs that can both do regular 100/1000 MBit Ethernet and
> > 100BASE-T1, but definitely not at the same time or over the same
> > electrical wiring. 100BASE-T1 is really different in that it uses
> > capacitive coupling, instead of magnetic like on regular Ethernet.
> > So
> > it is really a board level decision what gets used and is not
> > something
> > I would expect to change at runtime.
> 
> Hi Lucus
> 
> http://www.marvell.com/docs/automotive/assets/marvell-automotive-ethe
> rnet-88Q5050-product-brief-2017-07.pdf
> 
> This is a Marvell 8-port switch. It appears it can switch some of its
> ports between T1, TX, xMII, GMII and SGMII.
> 
> So maybe an end device is fixed to 100BASE-T1, but it looks like
> switches could be more flexible.

If you need this for the configuration of the switch in userspace, then
yes I agree that we should be able to differentiate between TX and T1. 

I'll just note that even while you can switch the PHY mode it won't
make much sense at runtime, as you won't be able to connect T1 to a
switch port that has standard Ethernet magnetics at this PHY port.

> So i think we should be able to differentiate between T1 and TX.
> We might also need an PHY_INTERFACE_MODE_100BASE_T1.

At least the PHYs I've looked at expose regular RGMII or (R)MII to the
MAC.

Again, if you need this for switch configuration, I'm happy to add it.

Regards,
Lucas

Re: [PATCH 1/3] net: phy: add support to detect 100BASE-T1 capability

[Lucas Stach]

Hi Andrew,

Am Mittwoch, den 13.12.2017, 21:11 +0100 schrieb Andrew Lunn:
> On Wed, Dec 13, 2017 at 06:37:49PM +0100, Lucas Stach wrote:
> > 100BASE-T1 is the automotive ethernet standard 802.3bw-2015.
> > Currently
> > we don't detect any valid modes for PHYs, which only support this
> > standard. Add support to detect the common 100Mbit full-duplex
> > mode.
> > 
> > Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
> > ---
> >  drivers/net/phy/phy_device.c | 2 ++
> >  include/uapi/linux/mii.h | 1 +
> >  2 files changed, 3 insertions(+)
> > 
> > diff --git a/drivers/net/phy/phy_device.c
> > b/drivers/net/phy/phy_device.c
> > index 67f25ac29025..8ef48b38d97b 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -1607,6 +1607,8 @@ int genphy_config_init(struct phy_device
> > *phydev)
> >     if (val < 0)
> >     return val;
> >  
> > +   if (val & ESTATUS_100T1_FULL)
> > +   features |= SUPPORTED_100baseT_Full;
> 
> Hi Lucas
> 
> Why did you decide to do this, and not add a SUPPORTED_100baseT1?
> 
> Could a device support both 100-BASE-T and 100-BASE-T1?  If at some
> point we need to differentiate between them, it is going to be
> hard. Especially since this is part of the kernel ABI.

Networking and especially PHY isn't really my primary area of
expertise, so excuse my ignorance. My reasoning was that we don't
differentiate between 100BASE-T2 and 100BASE-T4 in the kernel today, so
I thought it was fine to handle T1 the same way.

There are PHYs that can both do regular 100/1000 MBit Ethernet and
100BASE-T1, but definitely not at the same time or over the same
electrical wiring. 100BASE-T1 is really different in that it uses
capacitive coupling, instead of magnetic like on regular Ethernet. So
it is really a board level decision what gets used and is not something
I would expect to change at runtime.

I'll leave it to your judgment if this patch seems fine with the above
information in mind. Happy to rework if needed.

Regards,
Lucas

[PATCH 1/3] net: phy: add support to detect 100BASE-T1 capability

[Lucas Stach]

100BASE-T1 is the automotive ethernet standard 802.3bw-2015. Currently
we don't detect any valid modes for PHYs, which only support this
standard. Add support to detect the common 100Mbit full-duplex mode.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/net/phy/phy_device.c | 2 ++
 include/uapi/linux/mii.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 67f25ac29025..8ef48b38d97b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1607,6 +1607,8 @@ int genphy_config_init(struct phy_device *phydev)
if (val < 0)
return val;
 
+   if (val & ESTATUS_100T1_FULL)
+   features |= SUPPORTED_100baseT_Full;
if (val & ESTATUS_1000_TFULL)
features |= SUPPORTED_1000baseT_Full;
if (val & ESTATUS_1000_THALF)
diff --git a/include/uapi/linux/mii.h b/include/uapi/linux/mii.h
index b5c2fdcf23fd..eb5cc45d23fb 100644
--- a/include/uapi/linux/mii.h
+++ b/include/uapi/linux/mii.h
@@ -121,6 +121,7 @@
 #define EXPANSION_MFAULTS  0x0010  /* Multiple faults detected*/
 #define EXPANSION_RESV 0xffe0  /* Unused...   */
 
+#define ESTATUS_100T1_FULL 0x0080  /* Can do 100BASE-T1 Full  */
 #define ESTATUS_1000_TFULL 0x2000  /* Can do 1000BT Full  */
 #define ESTATUS_1000_THALF 0x1000  /* Can do 1000BT Half  */
 
-- 
2.11.0

[PATCH 2/3] net: phy: select sensible mode for non-autoneg PHYs on startup

[Lucas Stach]

Init speed and duplex to unknown, so phy_lookup_setting() knows that
it should select the mode only based on the PHY allowed link modes.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/net/phy/phy-core.c   | 5 +
 drivers/net/phy/phy_device.c | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 21f75ae244b3..149a4bab1e6f 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -149,6 +149,11 @@ phy_lookup_setting(int speed, int duplex, const unsigned 
long *mask,
const struct phy_setting *p, *match = NULL, *last = NULL;
int i;
 
+   if (!exact && speed == SPEED_UNKNOWN)
+   speed = INT_MAX;
+   if (!exact && duplex == DUPLEX_UNKNOWN)
+   duplex = DUPLEX_FULL;
+
for (i = 0, p = settings; i < ARRAY_SIZE(settings); i++, p++) {
if (p->bit < maxbit && test_bit(p->bit, mask)) {
last = p;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8ef48b38d97b..35278282259a 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1785,6 +1785,8 @@ static int phy_probe(struct device *dev)
phydev->supported = phydrv->features;
of_set_phy_supported(phydev);
phydev->advertising = phydev->supported;
+   phydev->speed = SPEED_UNKNOWN;
+   phydev->duplex = DUPLEX_UNKNOWN;
 
/* Get the EEE modes we want to prohibit. We will ask
 * the PHY stop advertising these mode later on
-- 
2.11.0

[PATCH 3/3] net: phy: sanitize autoneg in phy_start_aneg_priv

[Lucas Stach]

phy_sanitize_settings() is only called when autonegotiation has been
explicitly disabled. This breaks PHYs without any autonegotiation
capability, as the check for the capability happens inside this function.

Move the check out to the caller, so it is properly applied for those
PHYs.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/net/phy/phy.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 2b1e67bc1e73..433d859b6955 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -226,10 +226,6 @@ static void phy_sanitize_settings(struct phy_device 
*phydev)
const struct phy_setting *setting;
u32 features = phydev->supported;
 
-   /* Sanitize settings based on PHY capabilities */
-   if ((features & SUPPORTED_Autoneg) == 0)
-   phydev->autoneg = AUTONEG_DISABLE;
-
setting = phy_find_valid(phydev->speed, phydev->duplex, features);
if (setting) {
phydev->speed = setting->speed;
@@ -487,6 +483,10 @@ static int phy_start_aneg_priv(struct phy_device *phydev, 
bool sync)
 
mutex_lock(>lock);
 
+   /* Sanitize settings based on PHY capabilities */
+   if ((phydev->supported & SUPPORTED_Autoneg) == 0)
+   phydev->autoneg = AUTONEG_DISABLE;
+
if (AUTONEG_DISABLE == phydev->autoneg)
phy_sanitize_settings(phydev);
 
-- 
2.11.0

[PATCH 2/2] net: fec: optimize IRQ handler

[Lucas Stach]

fep->work_rx and fep->work_tx are both non-zero, as long as the NAPI
softirq hasn't finished its work. So if the current IRQ does not
signal any RX or TX completion, but some unrelated event, the path to
schedule the NAPI context is still entered.

The handler works correctly as in this case napi_schedule_prep() will
reject the scheduling attempt, but the flow can still be optimized by
not trying to schedule if the IRQ doesn't signal RX or TX completion.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/net/ethernet/freescale/fec_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index 0b70c07eb703..2043e140e9bd 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1587,14 +1587,14 @@ fec_enet_interrupt(int irq, void *dev_id)
int_events = readl_relaxed(fep->hwp + FEC_IEVENT) &
 readl_relaxed(fep->hwp + FEC_IMASK);
writel(int_events, fep->hwp + FEC_IEVENT);
-   fec_enet_collect_events(fep, int_events);
 
-   if ((fep->work_tx || fep->work_rx) && fep->link) {
+   if ((int_events & (FEC_ENET_RXF | FEC_ENET_TXF)) && fep->link) {
ret = IRQ_HANDLED;
 
if (napi_schedule_prep(>napi)) {
/* Disable the NAPI interrupts */
writel(FEC_NAPI_IMASK, fep->hwp + FEC_IMASK);
+   fec_enet_collect_events(fep, int_events);
__napi_schedule(>napi);
}
}
-- 
2.11.0

[PATCH 1/2] net: fec: don't ack masked interrupt events

[Lucas Stach]

The FEC doesn't have a real interrupt status register, that takes
into account the mask status of the IRQ. The driver reads the raw
interrupt event register, which also reports events for masked
IRQs.

The driver needs to apply the current mask itself, to avoid acking
IRQs that are currently masked, as NAPI relies on the masking to
hide the IRQs. The current behavior of just acking all interrupts
regardless of their mask status opens the driver up the "rotting
packet" race-window, as described in the original NAPI-HOWTO, which
has been observed in the wild.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/net/ethernet/freescale/fec_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index 610573855213..0b70c07eb703 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1584,7 +1584,8 @@ fec_enet_interrupt(int irq, void *dev_id)
uint int_events;
irqreturn_t ret = IRQ_NONE;
 
-   int_events = readl(fep->hwp + FEC_IEVENT);
+   int_events = readl_relaxed(fep->hwp + FEC_IEVENT) &
+readl_relaxed(fep->hwp + FEC_IMASK);
writel(int_events, fep->hwp + FEC_IEVENT);
fec_enet_collect_events(fep, int_events);
 
-- 
2.11.0

Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

[Lucas Stach]

Am Donnerstag, den 27.04.2017, 12:19 -0500 schrieb Bjorn Helgaas:
> [+cc Casey]
> 
> On Wed, Apr 26, 2017 at 09:18:33AM -0700, Alexander Duyck wrote:
> > On Wed, Apr 26, 2017 at 2:26 AM, Ding Tianhong  
> > wrote:
> > > Hi Amir:
> > >
> > > It is really glad to hear that the mlx5 will support RO mode this year, 
> > > if so, do you agree that enable it dynamic by ethtool -s xxx,
> > > we have try it several month ago but there was only one drivers would use 
> > > it at that time so the maintainer against it, it mlx5 would support RO,
> > > we could try to restart this solution, what do you think about it. :)
> > >
> > > Thanks
> > > Ding
> > 
> > Hi Ding,
> > 
> > Enabing relaxed ordering really doesn't have any place in ethtool. It
> > is a PCIe attribute that you are essentially wanting to enable.
> > 
> > It might be worth while to take a look at updating the PCIe code path
> > to handle this. Really what we should probably do is guarantee that
> > the architectures that need relaxed ordering are setting it in the
> > PCIe Device Control register and that the ones that don't are clearing
> > the bit. It's possible that this is already occurring, but I don't
> > know the state of handling those bits is in the kernel. Once we can
> > guarantee that we could use that to have the drivers determine their
> > behavior in regards to relaxed ordering. For example in the case of
> > igb/ixgbe we could probably change the behavior so that it will bey
> > default try to use relaxed ordering but if it is not enabled in PCIe
> > Device Control register the hardware should not request to use it. It
> > would simplify things in the drivers and allow for each architecture
> > to control things as needed in their PCIe code.
> 
> I thought Relaxed Ordering was an optimization.  Are there cases where
> it is actually required for correct behavior?

Yes, at least the Tegra 2 TRM claims that RO needs to be enabled on the
device side for correct operation with the following language:

"Tegra 2 requires relaxed ordering for responses to downstream requests
(responses can pass writes). It is possible in some circumstances for
PCIe transfers from an external bus masters (i.e. upstream transfers) to
become blocked by a downstream read or non-posted write. The responses
to these downstream requests are blocked by upstream posted writes only
when PCIe strict ordering is imposed. It is therefore necessary to never
impose strict ordering that would block a response to a downstream
NPW/read request and always set the relaxed ordering bit to 1. Only
devices that are capable of relaxed ordering may be used with Tegra 2
devices."

Regards,
Lucas

Re: [PATCH 2/2] ARM: dts: imx6: tag boards that have the HW workaround for ERR006687

[Lucas Stach]

Am Dienstag, den 07.06.2016, 12:46 -0700 schrieb Joshua Clayton:
> On Thu, 02 Jun 2016 10:25:06 +0200
> Lucas Stach <l.st...@pengutronix.de> wrote:
> 
> > Am Mittwoch, den 01.06.2016, 17:17 +0100 schrieb Russell King - ARM
> > Linux:
> > > On Wed, Jun 01, 2016 at 05:29:43PM +0200, Lucas Stach wrote:  
> > > > @@ -97,6 +97,7 @@
> > > > phy-reset-gpios = < 31 0>;
> > > > interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
> > > >   < 0 119 IRQ_TYPE_LEVEL_HIGH>;
> > > > +   fsl,err006687-war-present;  
> > > 
> 
> Why can't we just have the quirk detect the interrupts-extended
> property, which is not present on boards without the workaround.
> Wouldn't that be better? No dts change needed.
> 
I pondered that idea too, but have gone with the solution presented in
this patch series as it's more explicit and - with far more weight to it
- does not require the driver to touch DT properties that are outside of
its domain of responsibility. The "interrupts-extended" properties are
not meant to be parsed by the driver, but rather the IRQ subsystem.

Regards,
Lucas

Re: [PATCH v2 1/2] ARM: imx6: disable deeper idle states when FEC is active w/o HW workaround

[Lucas Stach]

Hi Fugang,

Am Montag, den 06.06.2016, 02:00 + schrieb Fugang Duan:
> From: Lucas Stach <l.st...@pengutronix.de> Sent: Saturday, June 04,
> 2016 12:31 AM
> > 
> > To: Shawn Guo <shawn...@kernel.org>; Fugang Duan <fugang.duan@nxp.c
> > om>
> > Cc: devicet...@vger.kernel.org; patchwork-...@pengutronix.de;
> > ker...@pengutronix.de; linux-arm-ker...@lists.infradead.org;
> > netdev@vger.kernel.org
> > Subject: [PATCH v2 1/2] ARM: imx6: disable deeper idle states when
> > FEC is
> > active w/o HW workaround
> > 
> > The i.MX6 Q/DL has an erratum (ERR006687) that prevents the FEC
> > from waking
> > the CPUs when they are in wait(unclocked) state. As the hardware
> > workaround
> > isn't applicable to all boards, disable the deeper idle state when
> > the workaround
> > isn't present and the FEC is in use.
> > 
> > This allows to safely run a kernel with CPUidle enabled on all
> > i.MX6 boards.
> > 
> > Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
> > Acked-by: David S. Miller <da...@davemloft.net> (for network
> > changes)
> > ---

[...]

> Hi, Lucas,
> 
> FEC irq cannot wake up CPUs when system is in wait mode. But we can
> use GPIO_6 for FEC interrupt that GPIO irq wake up CPUs.
> No need to disable wait mode as your such patches.
> 
> You just config the gpio irq like below patches:
> bc20a5d6da71 (ARM: dts: imx6qdl-sabreauto: use GPIO_6 for FEC
> interrupt.)
> 6261c4c8f13e (ARM: dts: imx6qdl-sabrelite: use GPIO_6 for FEC
> interrupt.)
> 
Please look at the description of this series again. The changes don't
disable the deeper idle states on boards where the HW waorkaround is
available. There is a large number of boards in the wild which can not
use the HW workaround, as they use GPIO_6 for other purposes. The aim
of this series is to have an automatic software workaround available
for those boards.

Regards,
Lucas

[PATCH v2 2/2] ARM: dts: imx6: tag boards that have the HW workaround for ERR006687

[Lucas Stach]

Add the DT property to all boards that have the hardware workaround
for erratum ERR006687 present. This allows the CPUidle driver to use
the deep idle states, even if the FEC is active.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
v2: Spell out "workaround" to avoid confusion.
---
 arch/arm/boot/dts/imx6dl-riotboard.dts   | 1 +
 arch/arm/boot/dts/imx6q-arm2.dts | 1 +
 arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi | 1 +
 arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi | 1 +
 arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi| 1 +
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 1 +
 arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 1 +
 arch/arm/boot/dts/imx6qdl-wandboard.dtsi | 1 +
 8 files changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/imx6dl-riotboard.dts 
b/arch/arm/boot/dts/imx6dl-riotboard.dts
index bfbed52ce1bd..2becd7cd6544 100644
--- a/arch/arm/boot/dts/imx6dl-riotboard.dts
+++ b/arch/arm/boot/dts/imx6dl-riotboard.dts
@@ -97,6 +97,7 @@
phy-reset-gpios = < 31 0>;
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-workaround-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6q-arm2.dts b/arch/arm/boot/dts/imx6q-arm2.dts
index d6515f7a56c4..0c401ab8dcb0 100644
--- a/arch/arm/boot/dts/imx6q-arm2.dts
+++ b/arch/arm/boot/dts/imx6q-arm2.dts
@@ -185,6 +185,7 @@
phy-mode = "rgmii";
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-workaround-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi 
b/arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi
index e456b5cc1b03..56c96aa82bf9 100644
--- a/arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi
@@ -250,6 +250,7 @@
txd3-skew-ps = <0>;
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-workaround-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi 
b/arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi
index 657da6b6ccd2..9677bf323823 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi
@@ -385,6 +385,7 @@
txd3-skew-ps = <0>;
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-workaround-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi 
b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
index 73915db704a0..97d9c333902b 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
@@ -287,6 +287,7 @@
txd3-skew-ps = <0>;
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-workaround-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi 
b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index d354d406954d..6aa193fb283f 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -155,6 +155,7 @@
phy-mode = "rgmii";
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-workaround-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi 
b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
index c47fe6c79b36..f65fdfc2536d 100644
--- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
@@ -273,6 +273,7 @@
txd3-skew-ps = <0>;
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-workaround-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi 
b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
index 8e7c40e114dd..096f64129e5e 100644
--- a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
@@ -211,6 +211,7 @@
phy-reset-gpios = < 29 0>;
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-workaround-present;
status = "okay";
 };
 
-- 
2.8.1

[PATCH v2 1/2] ARM: imx6: disable deeper idle states when FEC is active w/o HW workaround

[Lucas Stach]

The i.MX6 Q/DL has an erratum (ERR006687) that prevents the FEC from
waking the CPUs when they are in wait(unclocked) state. As the hardware
workaround isn't applicable to all boards, disable the deeper idle state
when the workaround isn't present and the FEC is in use.

This allows to safely run a kernel with CPUidle enabled on all i.MX6
boards.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
Acked-by: David S. Miller <da...@davemloft.net> (for network changes)
---
v2: Spell out "workaround" to avoid confusion.
---
 Documentation/devicetree/bindings/net/fsl-fec.txt |  3 +++
 arch/arm/mach-imx/cpuidle-imx6q.c | 16 +++
 drivers/net/ethernet/freescale/fec.h  |  2 ++
 drivers/net/ethernet/freescale/fec_main.c | 12 +++
 include/soc/imx/cpuidle.h | 25 +++
 5 files changed, 58 insertions(+)
 create mode 100644 include/soc/imx/cpuidle.h

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt 
b/Documentation/devicetree/bindings/net/fsl-fec.txt
index b037a9d78d93..a1e3693cca16 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -27,6 +27,9 @@ Optional properties:
   number to 1.
 - fsl,magic-packet : If present, indicates that the hardware supports waking
   up via magic packet.
+- fsl,err006687-workaround-present: If present indicates that the system has
+  the hardware workaround for ERR006687 applied and does not need a software
+  workaround.
 
 Optional subnodes:
 - mdio : specifies the mdio bus in the FEC, used as a container for phy nodes
diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c 
b/arch/arm/mach-imx/cpuidle-imx6q.c
index 353bb8774112..c3cc8ca8d2ff 100644
--- a/arch/arm/mach-imx/cpuidle-imx6q.c
+++ b/arch/arm/mach-imx/cpuidle-imx6q.c
@@ -62,6 +62,22 @@ static struct cpuidle_driver imx6q_cpuidle_driver = {
.safe_state_index = 0,
 };
 
+/*
+ * i.MX6 Q/DL has an erratum (ERR006687) that prevents the FEC from waking the
+ * CPUs when they are in wait(unclocked) state. As the hardware workaround 
isn't
+ * applicable to all boards, disable the deeper idle state when the workaround
+ * isn't present and the FEC is in use.
+ */
+void imx6q_cpuidle_fec_irqs_used(void)
+{
+   imx6q_cpuidle_driver.states[1].disabled = true;
+}
+
+void imx6q_cpuidle_fec_irqs_unused(void)
+{
+   imx6q_cpuidle_driver.states[1].disabled = false;
+}
+
 int __init imx6q_cpuidle_init(void)
 {
/* Set INT_MEM_CLK_LPM bit to get a reliable WAIT mode support */
diff --git a/drivers/net/ethernet/freescale/fec.h 
b/drivers/net/ethernet/freescale/fec.h
index f58f9ea51639..dc71a88e9c55 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -442,6 +442,8 @@ struct bufdesc_ex {
 #define FEC_QUIRK_SINGLE_MDIO  (1 << 11)
 /* Controller supports RACC register */
 #define FEC_QUIRK_HAS_RACC (1 << 12)
+/* Interrupt doesn't wake CPU from deep idle */
+#define FEC_QUIRK_ERR006687(1 << 13)
 
 struct bufdesc_prop {
int qid;
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index ca2594fd..8c2110b61684 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -60,6 +60,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -2820,6 +2821,9 @@ fec_enet_open(struct net_device *ndev)
if (ret)
goto err_enet_mii_probe;
 
+   if (fep->quirks & FEC_QUIRK_ERR006687)
+   imx6q_cpuidle_fec_irqs_used();
+
napi_enable(>napi);
phy_start(ndev->phydev);
netif_tx_start_all_queues(ndev);
@@ -2855,6 +2859,9 @@ fec_enet_close(struct net_device *ndev)
 
phy_disconnect(ndev->phydev);
 
+   if (fep->quirks & FEC_QUIRK_ERR006687)
+   imx6q_cpuidle_fec_irqs_unused();
+
fec_enet_clk_enable(ndev, false);
pinctrl_pm_select_sleep_state(>pdev->dev);
pm_runtime_mark_last_busy(>pdev->dev);
@@ -3294,6 +3301,11 @@ fec_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, ndev);
 
+   if ((of_machine_is_compatible("fsl,imx6q") ||
+of_machine_is_compatible("fsl,imx6dl")) &&
+   !of_property_read_bool(np, "fsl,err006687-workaround-present"))
+   fep->quirks |= FEC_QUIRK_ERR006687;
+
if (of_get_property(np, "fsl,magic-packet", NULL))
fep->wol_flag |= FEC_WOL_HAS_MAGIC_PACKET;
 
diff --git a/include/soc/imx/cpuidle.h b/include/soc/imx/cpuidle.h
new file mode 100644
index ..986a4823bce1
--- /dev/null
+++ b/include/soc/imx/cpuidle.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2016 Pengutronix, <ker...@pengutronix.de>
+ *
+ * This program is free software; you can redistr

Re: [PATCH 2/2] ARM: dts: imx6: tag boards that have the HW workaround for ERR006687

[Lucas Stach]

Am Mittwoch, den 01.06.2016, 17:17 +0100 schrieb Russell King - ARM
Linux:
> On Wed, Jun 01, 2016 at 05:29:43PM +0200, Lucas Stach wrote:
> > @@ -97,6 +97,7 @@
> > phy-reset-gpios = < 31 0>;
> > interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
> >   < 0 119 IRQ_TYPE_LEVEL_HIGH>;
> > +   fsl,err006687-war-present;
> 
> war?  Any reason not to spell it out, or use the more natural
> abbreviation "wa"?
> 
Apparently I've read too many documents where WAR is the abbreviation
for workaround, so it felt completely natural to me. I agree that it
would make sense to just spell it out to avoid any confusion.

Regards,
Lucas

[PATCH 1/2] ARM: imx6: disable deeper idle states when FEC is active w/o HW workaround

[Lucas Stach]

The i.MX6 Q/DL has an erratum (ERR006687) that prevents the FEC from
waking the CPUs when they are in wait(unclocked) state. As the hardware
workaround isn't applicable to all boards, disable the deeper idle state
when the workaround isn't present and the FEC is in use.

This allows to safely run a kernel with CPUidle enabled on all i.MX6
boards.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 Documentation/devicetree/bindings/net/fsl-fec.txt |  3 +++
 arch/arm/mach-imx/cpuidle-imx6q.c | 16 +++
 drivers/net/ethernet/freescale/fec.h  |  2 ++
 drivers/net/ethernet/freescale/fec_main.c | 12 +++
 include/soc/imx/cpuidle.h | 25 +++
 5 files changed, 58 insertions(+)
 create mode 100644 include/soc/imx/cpuidle.h

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt 
b/Documentation/devicetree/bindings/net/fsl-fec.txt
index b037a9d78d93..7d923cd2dba7 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -27,6 +27,9 @@ Optional properties:
   number to 1.
 - fsl,magic-packet : If present, indicates that the hardware supports waking
   up via magic packet.
+- fsl,err006687-war-present: If present indicates that the system has the
+  hardware workaround for ERR006687 applied and does not need a software
+  workaround.
 
 Optional subnodes:
 - mdio : specifies the mdio bus in the FEC, used as a container for phy nodes
diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c 
b/arch/arm/mach-imx/cpuidle-imx6q.c
index 353bb8774112..c3cc8ca8d2ff 100644
--- a/arch/arm/mach-imx/cpuidle-imx6q.c
+++ b/arch/arm/mach-imx/cpuidle-imx6q.c
@@ -62,6 +62,22 @@ static struct cpuidle_driver imx6q_cpuidle_driver = {
.safe_state_index = 0,
 };
 
+/*
+ * i.MX6 Q/DL has an erratum (ERR006687) that prevents the FEC from waking the
+ * CPUs when they are in wait(unclocked) state. As the hardware workaround 
isn't
+ * applicable to all boards, disable the deeper idle state when the workaround
+ * isn't present and the FEC is in use.
+ */
+void imx6q_cpuidle_fec_irqs_used(void)
+{
+   imx6q_cpuidle_driver.states[1].disabled = true;
+}
+
+void imx6q_cpuidle_fec_irqs_unused(void)
+{
+   imx6q_cpuidle_driver.states[1].disabled = false;
+}
+
 int __init imx6q_cpuidle_init(void)
 {
/* Set INT_MEM_CLK_LPM bit to get a reliable WAIT mode support */
diff --git a/drivers/net/ethernet/freescale/fec.h 
b/drivers/net/ethernet/freescale/fec.h
index f58f9ea51639..dc71a88e9c55 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -442,6 +442,8 @@ struct bufdesc_ex {
 #define FEC_QUIRK_SINGLE_MDIO  (1 << 11)
 /* Controller supports RACC register */
 #define FEC_QUIRK_HAS_RACC (1 << 12)
+/* Interrupt doesn't wake CPU from deep idle */
+#define FEC_QUIRK_ERR006687(1 << 13)
 
 struct bufdesc_prop {
int qid;
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index ca2594fd..8fb4f370dcc3 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -60,6 +60,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -2820,6 +2821,9 @@ fec_enet_open(struct net_device *ndev)
if (ret)
goto err_enet_mii_probe;
 
+   if (fep->quirks & FEC_QUIRK_ERR006687)
+   imx6q_cpuidle_fec_irqs_used();
+
napi_enable(>napi);
phy_start(ndev->phydev);
netif_tx_start_all_queues(ndev);
@@ -2855,6 +2859,9 @@ fec_enet_close(struct net_device *ndev)
 
phy_disconnect(ndev->phydev);
 
+   if (fep->quirks & FEC_QUIRK_ERR006687)
+   imx6q_cpuidle_fec_irqs_unused();
+
fec_enet_clk_enable(ndev, false);
pinctrl_pm_select_sleep_state(>pdev->dev);
pm_runtime_mark_last_busy(>pdev->dev);
@@ -3294,6 +3301,11 @@ fec_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, ndev);
 
+   if ((of_machine_is_compatible("fsl,imx6q") ||
+of_machine_is_compatible("fsl,imx6dl")) &&
+   !of_property_read_bool(np, "fsl,err006687-war-present"))
+   fep->quirks |= FEC_QUIRK_ERR006687;
+
if (of_get_property(np, "fsl,magic-packet", NULL))
fep->wol_flag |= FEC_WOL_HAS_MAGIC_PACKET;
 
diff --git a/include/soc/imx/cpuidle.h b/include/soc/imx/cpuidle.h
new file mode 100644
index ..986a4823bce1
--- /dev/null
+++ b/include/soc/imx/cpuidle.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2016 Pengutronix, <ker...@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Fo

[PATCH 2/2] ARM: dts: imx6: tag boards that have the HW workaround for ERR006687

[Lucas Stach]

Add the DT property to all boards that have the hardware workaround
for erratum ERR006687 present. This allows the CPUidle driver to use
the deep idle states, even if the FEC is active.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 arch/arm/boot/dts/imx6dl-riotboard.dts   | 1 +
 arch/arm/boot/dts/imx6q-arm2.dts | 1 +
 arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi | 1 +
 arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi | 1 +
 arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi| 1 +
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 1 +
 arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 1 +
 arch/arm/boot/dts/imx6qdl-wandboard.dtsi | 1 +
 8 files changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/imx6dl-riotboard.dts 
b/arch/arm/boot/dts/imx6dl-riotboard.dts
index bfbed52ce1bd..b10dc9513c6b 100644
--- a/arch/arm/boot/dts/imx6dl-riotboard.dts
+++ b/arch/arm/boot/dts/imx6dl-riotboard.dts
@@ -97,6 +97,7 @@
phy-reset-gpios = < 31 0>;
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-war-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6q-arm2.dts b/arch/arm/boot/dts/imx6q-arm2.dts
index d6515f7a56c4..fce60ef991ce 100644
--- a/arch/arm/boot/dts/imx6q-arm2.dts
+++ b/arch/arm/boot/dts/imx6q-arm2.dts
@@ -185,6 +185,7 @@
phy-mode = "rgmii";
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-war-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi 
b/arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi
index e456b5cc1b03..a74f396965ca 100644
--- a/arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi
@@ -250,6 +250,7 @@
txd3-skew-ps = <0>;
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-war-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi 
b/arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi
index 657da6b6ccd2..da76a128e5a9 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi
@@ -385,6 +385,7 @@
txd3-skew-ps = <0>;
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-war-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi 
b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
index 73915db704a0..dfcb5001a126 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
@@ -287,6 +287,7 @@
txd3-skew-ps = <0>;
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-war-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi 
b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index d354d406954d..e250da2a4caa 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -155,6 +155,7 @@
phy-mode = "rgmii";
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-war-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi 
b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
index c47fe6c79b36..d451ca242751 100644
--- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
@@ -273,6 +273,7 @@
txd3-skew-ps = <0>;
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-war-present;
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi 
b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
index 8e7c40e114dd..684c6a776565 100644
--- a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
@@ -211,6 +211,7 @@
phy-reset-gpios = < 29 0>;
interrupts-extended = < 6 IRQ_TYPE_LEVEL_HIGH>,
  < 0 119 IRQ_TYPE_LEVEL_HIGH>;
+   fsl,err006687-war-present;
status = "okay";
 };
 
-- 
2.8.1

[PATCH 0/2] Software workaround for i.MX6Q/DL ERR006687

[Lucas Stach]

Hi all,

this short series adds a software workaround for the i.MX6Q/DL erratum
ERR006687, where the FEC IRQ is unable to wake the CPU from deep idle
states.

Until now the only two options to avoid triggering the erratum was to
apply the hardware workround as described in the errata sheet, or to
disable CPU_IDLE in the kernel configuration. As the hardware workaround
isn't applicable on all boards, this left a fair amount of boards
suffering from the erratum, as CPU_IDLE is enabled in the i.MX6 and
multi-v7 defconfig.

The software workaround implemented here is to simply disable the deeper
CPU idle states on boards which don't have the HW workaround if the FEC
is active. Aside from enabling us to run a single kernel config across
all boards, this has the additional benefit that boards without the HW
workaround are still able to use the deeper idle states if the network
interface isn't active.

I would prefer if this series gets merged through the imx achitecture
tree with acks for the FEC changes from the network people.

Regards,
Lucas

Lucas Stach (2):
  ARM: imx6: disable deeper idle states when FEC is active w/o HW
workaround
  ARM: dts: imx6: tag boards that have the HW workaround for ERR006687

 Documentation/devicetree/bindings/net/fsl-fec.txt |  3 +++
 arch/arm/boot/dts/imx6dl-riotboard.dts|  1 +
 arch/arm/boot/dts/imx6q-arm2.dts  |  1 +
 arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi  |  1 +
 arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi  |  1 +
 arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi |  1 +
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi  |  1 +
 arch/arm/boot/dts/imx6qdl-sabrelite.dtsi  |  1 +
 arch/arm/boot/dts/imx6qdl-wandboard.dtsi  |  1 +
 arch/arm/mach-imx/cpuidle-imx6q.c | 16 +++
 drivers/net/ethernet/freescale/fec.h  |  2 ++
 drivers/net/ethernet/freescale/fec_main.c | 12 +++
 include/soc/imx/cpuidle.h | 25 +++
 13 files changed, 66 insertions(+)
 create mode 100644 include/soc/imx/cpuidle.h

-- 
2.8.1

Re: [PATCH] net: fec: only clear a queue's work bit if the queue was emptied

[Lucas Stach]

Am Dienstag, den 03.05.2016, 16:38 +0200 schrieb Uwe Kleine-König:
> In the receive path a queue's work bit was cleared unconditionally even
> if fec_enet_rx_queue only read out a part of the available packets from
> the hardware. This resulted in not reading any packets in the next napi
> turn and so packets were delayed or lost.
> 
> The obvious fix is to only clear a queue's bit when the queue was
> emptied.
> 
> Fixes: 4d494cdc92b3 ("net: fec: change data structure to support multiqueue")
> Signed-off-by: Uwe Kleine-König <u.kleine-koe...@pengutronix.de>

Reviewed-by: Lucas Stach <l.st...@pengutronix.de>

> ---
> Hello,
> 
> I created this patch against net/master. If you think it's to late to
> get it into 4.6 and it doesn't fit on net-next/master, just tell me and
> I will rebase.
> 
> Best regards
> Uwe
> 
> 
>  drivers/net/ethernet/freescale/fec_main.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c 
> b/drivers/net/ethernet/freescale/fec_main.c
> index 08243c2ff4b4..2a03857cca18 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1521,9 +1521,15 @@ fec_enet_rx(struct net_device *ndev, int budget)
>   struct fec_enet_private *fep = netdev_priv(ndev);
>  
>   for_each_set_bit(queue_id, >work_rx, FEC_ENET_MAX_RX_QS) {
> - clear_bit(queue_id, >work_rx);
> - pkt_received += fec_enet_rx_queue(ndev,
> + int ret;
> +
> + ret = fec_enet_rx_queue(ndev,
>   budget - pkt_received, queue_id);
> +
> + if (ret < budget - pkt_received)
> + clear_bit(queue_id, >work_rx);
> +
> + pkt_received += ret;
>   }
>   return pkt_received;
>  }

Re: [PATCH RFC] b43: stop hardcoding LED behavior

[Lucas Stach]

Am Montag, den 25.04.2016, 17:53 +0200 schrieb Michael Büsch:
> On Mon, 25 Apr 2016 09:40:51 +0200
> Lucas Stach <d...@lynxeye.de> wrote:
> 
> > 
> > On my system the SPROM correctly defines the only wired LED (radio)
> > but
> > skips all others, leading to the hardcode to register LEDs with RX
> > and TX
> > triggers.
> Hm ok. It probably is a good idea to change the condition from
> 
> if (sprom[led_index] == 0xFF)
> 
> to
> 
> if ((sprom[0] & sprom[1] & sprom[2] & sprom[3]) == 0xFF)
> 
> So the hardcoding only happens if there is no LED configured in the
> SPROM. (I think my card does this (see below), but I can check that
> later)
> 
> 
> > 
> > These triggers cause many uneccesary CPU wakeups to drive LEDs
> > that aren't even present in the system, reducing battery runtime.
> 
> Numbers please. Did you measure that is actually causes more
> _wakeups_?
> How many?
> The led work is placed in the mac80211 workqueue and LED updates only
> happen on behalf of mac80211 activities (by default). It only causes
> additional wakeups, if there's nothing else scheduled on the
> workqueue
> anyways (which might well be the case. So we need numbers. :)
> 
The blinking LEDs use a timer to enforce a constant blink rate at a
50ms on/off interval. While they are only triggered if there is some
RX/TX activity in the system, they cause up to 20 wakeups/second/led.
As the timers used for LED activity aren't deferrable, this hardcode is
causing 40 unnecessary CPU wakeups/s in my system.

> 
> > 
> > Remove the hardcode to stop it from doing any harm. If this code is
> > useful
> > for others it should probably be reworked as a quirk table
> > triggering only
> > for individual systems that need it.
> 
> There are cards that need it. I don't know how many that are, but I
> own
> an older 4306 PC-Card card that needs this.
> 
> So this effectively is a regression for this card.
> 
> So I don't think this is acceptable.
> You should at least make this configurable via module parameter or
> such.
> Or maybe the change from above already is enough. It should work for
> your case.
> 
There are some people that find those kinds of blinking LEDs
distracting, so a module parameter to disable them altogether might be
a good thing to have. Causing CPU wakeups in a system where those LEDs
aren't even physically populated is clearly undesired behavior.

If checking that the SPROM doesn't define any LED behavior is enough to
not regress your use case, I would be glad to rework the patch
accordingly.

Regards,
Lucas
> 
> > 
> > Signed-off-by: Lucas Stach <d...@lynxeye.de>
> > ---
> >  drivers/net/wireless/broadcom/b43/leds.c | 26 ++
> > 
> >  1 file changed, 2 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/broadcom/b43/leds.c
> > b/drivers/net/wireless/broadcom/b43/leds.c
> > index d79ab2a..77d2dad 100644
> > --- a/drivers/net/wireless/broadcom/b43/leds.c
> > +++ b/drivers/net/wireless/broadcom/b43/leds.c
> > @@ -224,31 +224,9 @@ static void b43_led_get_sprominfo(struct
> > b43_wldev *dev,
> >  
> >     if (sprom[led_index] == 0xFF) {
> >     /* There is no LED information in the SPROM
> > -    * for this LED. Hardcode it here. */
> > +    * for this LED. Keep it disabled. */
> >     *activelow = false;
> > -   switch (led_index) {
> > -   case 0:
> > -   *behaviour = B43_LED_ACTIVITY;
> > -   *activelow = true;
> > -   if (dev->dev->board_vendor ==
> > PCI_VENDOR_ID_COMPAQ)
> > -   *behaviour = B43_LED_RADIO_ALL;
> > -   break;
> > -   case 1:
> > -   *behaviour = B43_LED_RADIO_B;
> > -   if (dev->dev->board_vendor ==
> > PCI_VENDOR_ID_ASUSTEK)
> > -   *behaviour = B43_LED_ASSOC;
> > -   break;
> > -   case 2:
> > -   *behaviour = B43_LED_RADIO_A;
> > -   break;
> > -   case 3:
> > -   *behaviour = B43_LED_OFF;
> > -   break;
> > -   default:
> > -   *behaviour = B43_LED_OFF;
> > -   B43_WARN_ON(1);
> > -   return;
> > -   }
> > +   *behaviour = B43_LED_OFF;
> >     } else {
> >     *behaviour = sprom[led_index] & B43_LED_BEHAVIOUR;
> >     *activelow = !!(sprom[led_index] &
> > B43_LED_ACTIVELOW);
> 
> 
> 

[PATCH RFC] b43: stop hardcoding LED behavior

[Lucas Stach]

The code to hardcode the LED behavior is basically unchanged from when it
was first merged in 2007. It is likely wrong for many modern systems using
the b43 driver.

On my system the SPROM correctly defines the only wired LED (radio) but
skips all others, leading to the hardcode to register LEDs with RX and TX
triggers. These triggers cause many uneccesary CPU wakeups to drive LEDs
that aren't even present in the system, reducing battery runtime.

Remove the hardcode to stop it from doing any harm. If this code is useful
for others it should probably be reworked as a quirk table triggering only
for individual systems that need it.

Signed-off-by: Lucas Stach <d...@lynxeye.de>
---
 drivers/net/wireless/broadcom/b43/leds.c | 26 ++
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/leds.c 
b/drivers/net/wireless/broadcom/b43/leds.c
index d79ab2a..77d2dad 100644
--- a/drivers/net/wireless/broadcom/b43/leds.c
+++ b/drivers/net/wireless/broadcom/b43/leds.c
@@ -224,31 +224,9 @@ static void b43_led_get_sprominfo(struct b43_wldev *dev,
 
if (sprom[led_index] == 0xFF) {
/* There is no LED information in the SPROM
-* for this LED. Hardcode it here. */
+* for this LED. Keep it disabled. */
*activelow = false;
-   switch (led_index) {
-   case 0:
-   *behaviour = B43_LED_ACTIVITY;
-   *activelow = true;
-   if (dev->dev->board_vendor == PCI_VENDOR_ID_COMPAQ)
-   *behaviour = B43_LED_RADIO_ALL;
-   break;
-   case 1:
-   *behaviour = B43_LED_RADIO_B;
-   if (dev->dev->board_vendor == PCI_VENDOR_ID_ASUSTEK)
-   *behaviour = B43_LED_ASSOC;
-   break;
-   case 2:
-   *behaviour = B43_LED_RADIO_A;
-   break;
-   case 3:
-   *behaviour = B43_LED_OFF;
-   break;
-   default:
-   *behaviour = B43_LED_OFF;
-   B43_WARN_ON(1);
-   return;
-   }
+   *behaviour = B43_LED_OFF;
} else {
*behaviour = sprom[led_index] & B43_LED_BEHAVIOUR;
*activelow = !!(sprom[led_index] & B43_LED_ACTIVELOW);
-- 
2.5.5

net: fec: stable queue request

[Lucas Stach]

Hi David,

can you please add

b0c6ce24911fcb64715de9569f0f7b4f54d1d045
net: fec: Remove unneeded use of IS_ERR_VALUE() macro

42ea4457aea7aaeddf0c0b06724f297608f5e9d2
net: fec: normalize return value of pm_runtime_get_sync() in MDIO write

to your stable 4.2+ queue?

Those 2 commits will fix
https://bugzilla.kernel.org/show_bug.cgi?id=106171

Thanks,
Lucas
-- 
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 1/1] net: fec: add netif status check before set mac address

[Lucas Stach]

Am Mittwoch, den 09.09.2015, 10:42 +0800 schrieb Fugang Duan:
> There exist one issue by below case that case system hang:
> ifconfig eth0 down
> ifconfig eth0 hw ether 00:10:19:19:81:19
> 
> After eth0 down, all fec clocks are gated off. In the .fec_set_mac_address()
> function, it will set new MAC address to registers, which causes system hang.
> 
> So it needs to add netif status check to avoid registers access when clocks 
> are
> gated off. Until eth0 up the new MAC address are wrote into related registers.
> 
> Signed-off-by: Fugang Duan <b38...@freescale.com>
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c 
> b/drivers/net/ethernet/freescale/fec_main.c
> index 91925e3..cd09dbb 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3029,6 +3029,9 @@ fec_set_mac_address(struct net_device *ndev, void *p)
>   memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
>   }
>  
> + if (!netif_running(ndev))
> + return 0;
This deserves a comment in the code as to why it is needed and how it
still works.

Regards,
Lucas
> +
>   writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
>   (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24),
>   fep->hwp + FEC_ADDR_LOW);

-- 
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] Revert net: fec: Ensure clocks are enabled while using mdio bus

[Lucas Stach]

Am Freitag, den 14.08.2015, 08:25 + schrieb Peter Chen:
  
  
  Am Freitag, den 14.08.2015, 13:47 +0800 schrieb Peter Chen:
   It causes the i.mx6sx sdb board hang when using nfsroot during boots
   up at v4.2-rc6.
  
   This reverts commit 8fff755e9f8d0f70a595e79f248695ce6aef5cc3.
  
   Cc: netdev@vger.kernel.org
   Cc: Fugang Duan b38...@freescale.com
   Cc: shawn@linaro.org
   Cc: fabio.este...@freescale.com
   Cc: tyler.ba...@linaro.org
   Cc: Lucas Stach l.st...@pengutronix.de
   Cc: Andrew Lunn and...@lunn.ch
   Signed-off-by: Peter Chen peter.c...@freescale.com
   ---
  
   According to Fugang Duan, the i.mx series has different clock control
   sequence among SoCs, this patch may only consider certain SoCs.
  
  Sorry, but NACK.
  
  Please test current mainline (what will become v4.2-rc7). There is already a
  patch in that fixes i.MX27 and probably fixes the same problem on i.MX6SX.
  
 
 Would you help point to me which commit and at which tree?
 
Mainline, so Linus Torvalds tree.

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=14d2b7c1a96ef37eb571599c73d4a1a606b964d6

Regards,
Lucas

 Peter
 
drivers/net/ethernet/freescale/fec_main.c | 89
   +--
1 file changed, 13 insertions(+), 76 deletions(-)
  
   diff --git a/drivers/net/ethernet/freescale/fec_main.c
   b/drivers/net/ethernet/freescale/fec_main.c
   index 32e3807c..5e8b837 100644
   --- a/drivers/net/ethernet/freescale/fec_main.c
   +++ b/drivers/net/ethernet/freescale/fec_main.c
   @@ -24,7 +24,6 @@
#include linux/module.h
#include linux/kernel.h
#include linux/string.h
   -#include linux/pm_runtime.h
#include linux/ptrace.h
#include linux/errno.h
#include linux/ioport.h
   @@ -78,7 +77,6 @@ static void fec_enet_itr_coal_init(struct net_device
  *ndev);
#define FEC_ENET_RAEM_V  0x8
#define FEC_ENET_RAFL_V  0x8
#define FEC_ENET_OPD_V   0xFFF0
   -#define FEC_MDIO_PM_TIMEOUT  100 /* ms */
  
static struct platform_device_id fec_devtype[] = {
 {
   @@ -1769,13 +1767,7 @@ static void fec_enet_adjust_link(struct
   net_device *ndev)  static int fec_enet_mdio_read(struct mii_bus *bus,
   int mii_id, int regnum)  {
 struct fec_enet_private *fep = bus-priv;
   - struct device *dev = fep-pdev-dev;
 unsigned long time_left;
   - int ret = 0;
   -
   - ret = pm_runtime_get_sync(dev);
   - if (IS_ERR_VALUE(ret))
   - return ret;
  
 fep-mii_timeout = 0;
 init_completion(fep-mdio_done);
   @@ -1791,30 +1783,18 @@ static int fec_enet_mdio_read(struct mii_bus
  *bus, int mii_id, int regnum)
 if (time_left == 0) {
 fep-mii_timeout = 1;
 netdev_err(fep-netdev, MDIO read timeout\n);
   - ret = -ETIMEDOUT;
   - goto out;
   + return -ETIMEDOUT;
 }
  
   - ret = FEC_MMFR_DATA(readl(fep-hwp + FEC_MII_DATA));
   -
   -out:
   - pm_runtime_mark_last_busy(dev);
   - pm_runtime_put_autosuspend(dev);
   -
   - return ret;
   + /* return value */
   + return FEC_MMFR_DATA(readl(fep-hwp + FEC_MII_DATA));
}
  
static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int 
   regnum,
u16 value)
{
 struct fec_enet_private *fep = bus-priv;
   - struct device *dev = fep-pdev-dev;
 unsigned long time_left;
   - int ret = 0;
   -
   - ret = pm_runtime_get_sync(dev);
   - if (IS_ERR_VALUE(ret))
   - return ret;
  
 fep-mii_timeout = 0;
 init_completion(fep-mdio_done);
   @@ -1831,13 +1811,10 @@ static int fec_enet_mdio_write(struct mii_bus
  *bus, int mii_id, int regnum,
 if (time_left == 0) {
 fep-mii_timeout = 1;
 netdev_err(fep-netdev, MDIO write timeout\n);
   - ret  = -ETIMEDOUT;
   + return -ETIMEDOUT;
 }
  
   - pm_runtime_mark_last_busy(dev);
   - pm_runtime_put_autosuspend(dev);
   -
   - return ret;
   + return 0;
}
  
static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
   @@ -1849,6 +1826,9 @@ static int fec_enet_clk_enable(struct net_device
  *ndev, bool enable)
 ret = clk_prepare_enable(fep-clk_ahb);
 if (ret)
 return ret;
   + ret = clk_prepare_enable(fep-clk_ipg);
   + if (ret)
   + goto failed_clk_ipg;
 if (fep-clk_enet_out) {
 ret = clk_prepare_enable(fep-clk_enet_out);
 if (ret)
   @@ -1872,6 +1852,7 @@ static int fec_enet_clk_enable(struct net_device
  *ndev, bool enable)
 }
 } else {
 clk_disable_unprepare(fep-clk_ahb);
   + clk_disable_unprepare(fep-clk_ipg);
 if (fep-clk_enet_out)
 clk_disable_unprepare(fep-clk_enet_out);
 if (fep-clk_ptp) {
   @@ -1893,6 +1874,8 @@ failed_clk_ptp:
 if (fep-clk_enet_out)
 clk_disable_unprepare(fep-clk_enet_out);
failed_clk_enet_out:
   + clk_disable_unprepare

Re: [PATCH 1/1] Revert net: fec: Ensure clocks are enabled while using mdio bus

[Lucas Stach]

Am Freitag, den 14.08.2015, 13:47 +0800 schrieb Peter Chen:
 It causes the i.mx6sx sdb board hang when using nfsroot during boots
 up at v4.2-rc6.
 
 This reverts commit 8fff755e9f8d0f70a595e79f248695ce6aef5cc3.
 
 Cc: netdev@vger.kernel.org
 Cc: Fugang Duan b38...@freescale.com
 Cc: shawn@linaro.org
 Cc: fabio.este...@freescale.com
 Cc: tyler.ba...@linaro.org
 Cc: Lucas Stach l.st...@pengutronix.de
 Cc: Andrew Lunn and...@lunn.ch
 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
 
 According to Fugang Duan, the i.mx series has different clock control
 sequence among SoCs, this patch may only consider certain SoCs.
 
Sorry, but NACK.

Please test current mainline (what will become v4.2-rc7). There is
already a patch in that fixes i.MX27 and probably fixes the same problem
on i.MX6SX.

  drivers/net/ethernet/freescale/fec_main.c | 89 
 +--
  1 file changed, 13 insertions(+), 76 deletions(-)
 
 diff --git a/drivers/net/ethernet/freescale/fec_main.c 
 b/drivers/net/ethernet/freescale/fec_main.c
 index 32e3807c..5e8b837 100644
 --- a/drivers/net/ethernet/freescale/fec_main.c
 +++ b/drivers/net/ethernet/freescale/fec_main.c
 @@ -24,7 +24,6 @@
  #include linux/module.h
  #include linux/kernel.h
  #include linux/string.h
 -#include linux/pm_runtime.h
  #include linux/ptrace.h
  #include linux/errno.h
  #include linux/ioport.h
 @@ -78,7 +77,6 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
  #define FEC_ENET_RAEM_V  0x8
  #define FEC_ENET_RAFL_V  0x8
  #define FEC_ENET_OPD_V   0xFFF0
 -#define FEC_MDIO_PM_TIMEOUT  100 /* ms */
  
  static struct platform_device_id fec_devtype[] = {
   {
 @@ -1769,13 +1767,7 @@ static void fec_enet_adjust_link(struct net_device 
 *ndev)
  static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
  {
   struct fec_enet_private *fep = bus-priv;
 - struct device *dev = fep-pdev-dev;
   unsigned long time_left;
 - int ret = 0;
 -
 - ret = pm_runtime_get_sync(dev);
 - if (IS_ERR_VALUE(ret))
 - return ret;
  
   fep-mii_timeout = 0;
   init_completion(fep-mdio_done);
 @@ -1791,30 +1783,18 @@ static int fec_enet_mdio_read(struct mii_bus *bus, 
 int mii_id, int regnum)
   if (time_left == 0) {
   fep-mii_timeout = 1;
   netdev_err(fep-netdev, MDIO read timeout\n);
 - ret = -ETIMEDOUT;
 - goto out;
 + return -ETIMEDOUT;
   }
  
 - ret = FEC_MMFR_DATA(readl(fep-hwp + FEC_MII_DATA));
 -
 -out:
 - pm_runtime_mark_last_busy(dev);
 - pm_runtime_put_autosuspend(dev);
 -
 - return ret;
 + /* return value */
 + return FEC_MMFR_DATA(readl(fep-hwp + FEC_MII_DATA));
  }
  
  static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
  u16 value)
  {
   struct fec_enet_private *fep = bus-priv;
 - struct device *dev = fep-pdev-dev;
   unsigned long time_left;
 - int ret = 0;
 -
 - ret = pm_runtime_get_sync(dev);
 - if (IS_ERR_VALUE(ret))
 - return ret;
  
   fep-mii_timeout = 0;
   init_completion(fep-mdio_done);
 @@ -1831,13 +1811,10 @@ static int fec_enet_mdio_write(struct mii_bus *bus, 
 int mii_id, int regnum,
   if (time_left == 0) {
   fep-mii_timeout = 1;
   netdev_err(fep-netdev, MDIO write timeout\n);
 - ret  = -ETIMEDOUT;
 + return -ETIMEDOUT;
   }
  
 - pm_runtime_mark_last_busy(dev);
 - pm_runtime_put_autosuspend(dev);
 -
 - return ret;
 + return 0;
  }
  
  static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
 @@ -1849,6 +1826,9 @@ static int fec_enet_clk_enable(struct net_device *ndev, 
 bool enable)
   ret = clk_prepare_enable(fep-clk_ahb);
   if (ret)
   return ret;
 + ret = clk_prepare_enable(fep-clk_ipg);
 + if (ret)
 + goto failed_clk_ipg;
   if (fep-clk_enet_out) {
   ret = clk_prepare_enable(fep-clk_enet_out);
   if (ret)
 @@ -1872,6 +1852,7 @@ static int fec_enet_clk_enable(struct net_device *ndev, 
 bool enable)
   }
   } else {
   clk_disable_unprepare(fep-clk_ahb);
 + clk_disable_unprepare(fep-clk_ipg);
   if (fep-clk_enet_out)
   clk_disable_unprepare(fep-clk_enet_out);
   if (fep-clk_ptp) {
 @@ -1893,6 +1874,8 @@ failed_clk_ptp:
   if (fep-clk_enet_out)
   clk_disable_unprepare(fep-clk_enet_out);
  failed_clk_enet_out:
 + clk_disable_unprepare(fep-clk_ipg);
 +failed_clk_ipg:
   clk_disable_unprepare(fep-clk_ahb);
  
   return ret;
 @@ -2864,14 +2847,10 @@ fec_enet_open(struct net_device *ndev)
   struct fec_enet_private *fep = netdev_priv(ndev);
   int ret;
  
 - ret = pm_runtime_get_sync(fep-pdev-dev

Re: [PATCH 1/1] Revert net: fec: Ensure clocks are enabled while using mdio bus

[Lucas Stach]

Am Freitag, den 14.08.2015, 16:14 +0800 schrieb Peter Chen:
 On Fri, Aug 14, 2015 at 10:27:33AM +0200, Lucas Stach wrote:
  Am Freitag, den 14.08.2015, 08:25 + schrieb Peter Chen:


Am Freitag, den 14.08.2015, 13:47 +0800 schrieb Peter Chen:
 It causes the i.mx6sx sdb board hang when using nfsroot during boots
 up at v4.2-rc6.

 This reverts commit 8fff755e9f8d0f70a595e79f248695ce6aef5cc3.

 Cc: netdev@vger.kernel.org
 Cc: Fugang Duan b38...@freescale.com
 Cc: shawn@linaro.org
 Cc: fabio.este...@freescale.com
 Cc: tyler.ba...@linaro.org
 Cc: Lucas Stach l.st...@pengutronix.de
 Cc: Andrew Lunn and...@lunn.ch
 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---

 According to Fugang Duan, the i.mx series has different clock control
 sequence among SoCs, this patch may only consider certain SoCs.

Sorry, but NACK.

Please test current mainline (what will become v4.2-rc7). There is 
already a
patch in that fixes i.MX27 and probably fixes the same problem on 
i.MX6SX.

   
   Would you help point to me which commit and at which tree?
   
  Mainline, so Linus Torvalds tree.
  
  http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=14d2b7c1a96ef37eb571599c73d4a1a606b964d6
 
 It fixes my imx6sx-sdb board. It is interesting that there was
 no problem for some platforms, but with problem for others.
 Your fix is a common runtime PM fix.
 
That's because on i.MX6Q/DL IPG and AHB clock are the same clock, on
i.MX27 and apparently i.MX6SX they are different, so those chips fail if
RPM is disabling the clock at the wrong point in time.

 Again, why we need this as a bug-fix, not but as new feature for next
 rc1?

It fixes MDIO attached switches on Vybrid, but you are right this
probably could have waited until the next merge window. But this is the
wrong question to ask after we got in all the fixes to keep things from
regressing.

Seeing that people test those things pretty late (the original broken
patch got in with -rc2!) moving things to the next merge window would
just have people complaining during the v4.3 RC phase, instead of now
during the 4.2 RC phase.

Takeaway for everyone involved: test things more thoroughly and earlier.

Regards,
Lucas

-- 
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] can: flexcan: demote register output to debug level

[Lucas Stach]

This message isn't really helpful for the general reader of the kernel
logs, so should not be printed with info level. All other register
programming outputs in the flexcan driver already use the debug level.

Signed-off-by: Lucas Stach l.st...@pengutronix.de
---
 drivers/net/can/flexcan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index ad0a7e8c2c2b..95abd395cb0d 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -797,7 +797,7 @@ static void flexcan_set_bittiming(struct net_device *dev)
if (priv-can.ctrlmode  CAN_CTRLMODE_3_SAMPLES)
reg |= FLEXCAN_CTRL_SMP;
 
-   netdev_info(dev, writing ctrl=0x%08x\n, reg);
+   netdev_dbg(dev, writing ctrl=0x%08x\n, reg);
flexcan_write(reg, regs-ctrl);
 
/* print chip status */
-- 
2.4.6

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: fec: fix initial runtime PM refcount

[Lucas Stach]

Am Montag, den 03.08.2015, 14:28 -0400 schrieb Alan Stern:
 On Mon, 3 Aug 2015, Uwe [iso-8859-1] Kleine-Knig wrote:
 
  Hello,
  
  I have no clue about runtime-pm, but I added a few people to Cc: who
  should know better ...
  
  Best regards
  Uwe
  
  On Mon, Aug 03, 2015 at 06:15:54PM +0200, Andrew Lunn wrote:
   On Mon, Aug 03, 2015 at 05:50:11PM +0200, Lucas Stach wrote:
The clocks are initially active and thus the device is marked active.
This still keeps the PM refcount at 0, the pm_runtime_put_autosuspend()
call at the end of probe then leaves us with an invalid refcount of -1,
which in turn leads to the device staying in suspended state even though
netdev open had been called.

Fix this by initializing the refcount to be coherent with the initial
device status.

Fixes:
8fff755e9f8 (net: fec: Ensure clocks are enabled while using mdio bus)

Signed-off-by: Lucas Stach l.st...@pengutronix.de
---
Please apply this as a fix for 4.2
---
 drivers/net/ethernet/freescale/fec_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index 32e3807c650e..271bb5862346 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3433,6 +3433,7 @@ fec_probe(struct platform_device *pdev)
 
pm_runtime_set_autosuspend_delay(pdev-dev, 
FEC_MDIO_PM_TIMEOUT);
pm_runtime_use_autosuspend(pdev-dev);
+   pm_runtime_get_noresume(pdev-dev);
pm_runtime_set_active(pdev-dev);
pm_runtime_enable(pdev-dev);
   
   This might work, but is it the correct fix?
 
 It looks reasonable to me.  It might also make sense to move all of
 that pm_runtime_* stuff to the end of the probe routine.  Notice that
 they don't get undone if register_netdev() fails.
 
Unfortunately we can not move RPM enabling to the end of probe, as the
MDIO read/write functions that rely on RPM are already called while we
are still in the middle of the probe function.

I agree that we need better error handling here, but that comment
applies to the whole FEC probe function. I think that this might be
invasive enough to justify a delay to the next merge window, not really
material for the late RCs.

Regards,
Lucas

-- 
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: fec: fix initial runtime PM refcount

[Lucas Stach]

Am Montag, den 03.08.2015, 18:15 +0200 schrieb Andrew Lunn:
 On Mon, Aug 03, 2015 at 05:50:11PM +0200, Lucas Stach wrote:
  The clocks are initially active and thus the device is marked active.
  This still keeps the PM refcount at 0, the pm_runtime_put_autosuspend()
  call at the end of probe then leaves us with an invalid refcount of -1,
  which in turn leads to the device staying in suspended state even though
  netdev open had been called.
  
  Fix this by initializing the refcount to be coherent with the initial
  device status.
  
  Fixes:
  8fff755e9f8 (net: fec: Ensure clocks are enabled while using mdio bus)
  
  Signed-off-by: Lucas Stach l.st...@pengutronix.de
  ---
  Please apply this as a fix for 4.2
  ---
   drivers/net/ethernet/freescale/fec_main.c | 1 +
   1 file changed, 1 insertion(+)
  
  diff --git a/drivers/net/ethernet/freescale/fec_main.c 
  b/drivers/net/ethernet/freescale/fec_main.c
  index 32e3807c650e..271bb5862346 100644
  --- a/drivers/net/ethernet/freescale/fec_main.c
  +++ b/drivers/net/ethernet/freescale/fec_main.c
  @@ -3433,6 +3433,7 @@ fec_probe(struct platform_device *pdev)
   
  pm_runtime_set_autosuspend_delay(pdev-dev, FEC_MDIO_PM_TIMEOUT);
  pm_runtime_use_autosuspend(pdev-dev);
  +   pm_runtime_get_noresume(pdev-dev);
  pm_runtime_set_active(pdev-dev);
  pm_runtime_enable(pdev-dev);
 
 This might work, but is it the correct fix?
 
 Documentation/power/runtime_pm.txt says:
 
 534 In addition to that, the initial runtime PM status of all devices is
 535 'suspended', but it need not reflect the actual physical state of the 
 device.
 536 Thus, if the device is initially active (i.e. it is able to process I/O), 
 its
 537 runtime PM status must be changed to 'active', with the help of
 538 pm_runtime_set_active(), before pm_runtime_enable() is called for the 
 device.
 
 At the point we call the pm_runtime_ functions above, all the clocks
 are ticking. So according to the documentation pm_runtime_set_active()
 is the right thing to do. But it makes no mention of have to call
 pm_runtime_get_noresume(). I would of expected pm_runtime_set_active()
 to set the count to the correct value.
 
It is the correct fix.

pm_runtime_enable() is the transition point between whatever state the
device was in and the runtime PM managed state. pm_runtime_set_active()
informs the RPM framework about the current device state.
pm_runtime_get_noresume() tells the RPM framework what state we want the
device to be in after the transition to RPM managed state.

We expect the device to stay on while we go through the probe() routine,
so we need to get the runtime PM refcount, otherwise it would be fine
for RPM to turn the device off immediately after calling
pm_runtime_enable(). We drop the refcount when leaving the probe
routine.

Regards,
Lucas
-- 
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] net: fec: fix initial runtime PM refcount

[Lucas Stach]

The clocks are initially active and thus the device is marked active.
This still keeps the PM refcount at 0, the pm_runtime_put_autosuspend()
call at the end of probe then leaves us with an invalid refcount of -1,
which in turn leads to the device staying in suspended state even though
netdev open had been called.

Fix this by initializing the refcount to be coherent with the initial
device status.

Fixes:
8fff755e9f8 (net: fec: Ensure clocks are enabled while using mdio bus)

Signed-off-by: Lucas Stach l.st...@pengutronix.de
---
Please apply this as a fix for 4.2
---
 drivers/net/ethernet/freescale/fec_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index 32e3807c650e..271bb5862346 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3433,6 +3433,7 @@ fec_probe(struct platform_device *pdev)
 
pm_runtime_set_autosuspend_delay(pdev-dev, FEC_MDIO_PM_TIMEOUT);
pm_runtime_use_autosuspend(pdev-dev);
+   pm_runtime_get_noresume(pdev-dev);
pm_runtime_set_active(pdev-dev);
pm_runtime_enable(pdev-dev);
 
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Patch v2 resend 1/2] net: fec: use managed DMA API functions to allocate BD ring

[Lucas Stach]

So it gets freed when the device is going away.
This fixes a DMA memory leak on driver probe() fail and driver
remove().

Signed-off-by: Lucas Stach l.st...@pengutronix.de
---
v2: Fix indentation of second line to fix alignment with opening bracket.
---
 drivers/net/ethernet/freescale/fec_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index 349365d85b92..a7f1bdf718f8 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3142,8 +3142,8 @@ static int fec_enet_init(struct net_device *ndev)
fep-bufdesc_size;
 
/* Allocate memory for buffer descriptors. */
-   cbd_base = dma_alloc_coherent(NULL, bd_size, bd_dma,
- GFP_KERNEL);
+   cbd_base = dmam_alloc_coherent(fep-pdev-dev, bd_size, bd_dma,
+  GFP_KERNEL);
if (!cbd_base) {
return -ENOMEM;
}
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Patch v2 resend 2/2] net: fec: introduce fec_ptp_stop and use in probe fail path

[Lucas Stach]

This function frees resources and cancels delayed work item that
have been initialized in fec_ptp_init().

Use this to do proper error handling if something goes wrong in
probe function after fec_ptp_init has been called.

Signed-off-by: Lucas Stach l.st...@pengutronix.de
Acked-by: Fugang Duan b38...@freescale.com
---
 drivers/net/ethernet/freescale/fec.h  |  1 +
 drivers/net/ethernet/freescale/fec_main.c |  5 ++---
 drivers/net/ethernet/freescale/fec_ptp.c  | 10 ++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h 
b/drivers/net/ethernet/freescale/fec.h
index 1eee73cccdf5..99d33e2d35e6 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -562,6 +562,7 @@ struct fec_enet_private {
 };
 
 void fec_ptp_init(struct platform_device *pdev);
+void fec_ptp_stop(struct platform_device *pdev);
 void fec_ptp_start_cyclecounter(struct net_device *ndev);
 int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr);
 int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr);
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index a7f1bdf718f8..32e3807c650e 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3494,6 +3494,7 @@ failed_register:
 failed_mii_init:
 failed_irq:
 failed_init:
+   fec_ptp_stop(pdev);
if (fep-reg_phy)
regulator_disable(fep-reg_phy);
 failed_regulator:
@@ -3515,14 +3516,12 @@ fec_drv_remove(struct platform_device *pdev)
struct net_device *ndev = platform_get_drvdata(pdev);
struct fec_enet_private *fep = netdev_priv(ndev);
 
-   cancel_delayed_work_sync(fep-time_keep);
cancel_work_sync(fep-tx_timeout_work);
+   fec_ptp_stop(pdev);
unregister_netdev(ndev);
fec_enet_mii_remove(fep);
if (fep-reg_phy)
regulator_disable(fep-reg_phy);
-   if (fep-ptp_clock)
-   ptp_clock_unregister(fep-ptp_clock);
of_node_put(fep-phy_node);
free_netdev(ndev);
 
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c 
b/drivers/net/ethernet/freescale/fec_ptp.c
index a15663ad7f5e..f457a23d0bfb 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -604,6 +604,16 @@ void fec_ptp_init(struct platform_device *pdev)
schedule_delayed_work(fep-time_keep, HZ);
 }
 
+void fec_ptp_stop(struct platform_device *pdev)
+{
+   struct net_device *ndev = platform_get_drvdata(pdev);
+   struct fec_enet_private *fep = netdev_priv(ndev);
+
+   cancel_delayed_work_sync(fep-time_keep);
+   if (fep-ptp_clock)
+   ptp_clock_unregister(fep-ptp_clock);
+}
+
 /**
  * fec_ptp_check_pps_event
  * @fep: the fec_enet_private structure handle
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] net: fec: use managed DMA API functions to allocate BD ring

[Lucas Stach]

Am Mittwoch, den 22.07.2015, 01:55 + schrieb Duan Andy:
 From: Lucas Stach l.st...@pengutronix.de Sent: Tuesday, July 21, 2015 11:11 
 PM
  To: David S. Miller
  Cc: Duan Fugang-B38611; Li Frank-B20596; netdev@vger.kernel.org;
  ker...@pengutronix.de; patchwork-...@pengutronix.de
  Subject: [PATCH v2 1/2] net: fec: use managed DMA API functions to
  allocate BD ring
  
  So it gets freed when the device is going away.
  This fixes a DMA memory leak on driver probe() fail and driver remove().
  
  Signed-off-by: Lucas Stach l.st...@pengutronix.de
  ---
  v2: Fix indentation of second line to fix alignment with opening bracket.
  ---
   drivers/net/ethernet/freescale/fec_main.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/net/ethernet/freescale/fec_main.c
  b/drivers/net/ethernet/freescale/fec_main.c
  index 349365d85b92..a7f1bdf718f8 100644
  --- a/drivers/net/ethernet/freescale/fec_main.c
  +++ b/drivers/net/ethernet/freescale/fec_main.c
  @@ -3142,8 +3142,8 @@ static int fec_enet_init(struct net_device *ndev)
  fep-bufdesc_size;
  
  /* Allocate memory for buffer descriptors. */
  -   cbd_base = dma_alloc_coherent(NULL, bd_size, bd_dma,
  - GFP_KERNEL);
  +   cbd_base = dmam_alloc_coherent(fep-pdev-dev, bd_size, bd_dma,
  +  GFP_KERNEL);
  if (!cbd_base) {
  return -ENOMEM;
  }
  --
 
 Can you also replace the below position with dma_alloc_coherent() ?
 txq-tso_hdrs = dma_alloc_coherent(NULL,
 txq-tx_ring_size * TSO_HEADER_SIZE,
 txq-tso_hdrs_dma,
 GFP_KERNEL);
 
 
No, that one has an explicit free functions.
So using managed function would result in a double free in paths. We
might want to move those over to devm eventually to make the error
handling more robust, but that's clearly out of the scope of this patch.

Regards,
Lucas

-- 
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] net: fec: use managed DMA API functions to allocate BD ring

[Lucas Stach]

So it gets freed when the device is going away.
This fixes a DMA memory leak on driver probe() fail and driver
remove().

Signed-off-by: Lucas Stach l.st...@pengutronix.de
---
v2: Fix indentation of second line to fix alignment with opening bracket.
---
 drivers/net/ethernet/freescale/fec_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index 349365d85b92..a7f1bdf718f8 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3142,8 +3142,8 @@ static int fec_enet_init(struct net_device *ndev)
fep-bufdesc_size;
 
/* Allocate memory for buffer descriptors. */
-   cbd_base = dma_alloc_coherent(NULL, bd_size, bd_dma,
- GFP_KERNEL);
+   cbd_base = dmam_alloc_coherent(fep-pdev-dev, bd_size, bd_dma,
+  GFP_KERNEL);
if (!cbd_base) {
return -ENOMEM;
}
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] net: fec: introduce fec_ptp_stop and use in probe fail path

[Lucas Stach]

This function frees resources and cancels delayed work item that
have been initialized in fec_ptp_init().

Use this to do proper error handling if something goes wrong in
probe function after fec_ptp_init has been called.

Signed-off-by: Lucas Stach l.st...@pengutronix.de
---
 drivers/net/ethernet/freescale/fec.h  |  1 +
 drivers/net/ethernet/freescale/fec_main.c |  5 ++---
 drivers/net/ethernet/freescale/fec_ptp.c  | 10 ++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h 
b/drivers/net/ethernet/freescale/fec.h
index 1eee73cccdf5..99d33e2d35e6 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -562,6 +562,7 @@ struct fec_enet_private {
 };
 
 void fec_ptp_init(struct platform_device *pdev);
+void fec_ptp_stop(struct platform_device *pdev);
 void fec_ptp_start_cyclecounter(struct net_device *ndev);
 int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr);
 int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr);
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index a7f1bdf718f8..32e3807c650e 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3494,6 +3494,7 @@ failed_register:
 failed_mii_init:
 failed_irq:
 failed_init:
+   fec_ptp_stop(pdev);
if (fep-reg_phy)
regulator_disable(fep-reg_phy);
 failed_regulator:
@@ -3515,14 +3516,12 @@ fec_drv_remove(struct platform_device *pdev)
struct net_device *ndev = platform_get_drvdata(pdev);
struct fec_enet_private *fep = netdev_priv(ndev);
 
-   cancel_delayed_work_sync(fep-time_keep);
cancel_work_sync(fep-tx_timeout_work);
+   fec_ptp_stop(pdev);
unregister_netdev(ndev);
fec_enet_mii_remove(fep);
if (fep-reg_phy)
regulator_disable(fep-reg_phy);
-   if (fep-ptp_clock)
-   ptp_clock_unregister(fep-ptp_clock);
of_node_put(fep-phy_node);
free_netdev(ndev);
 
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c 
b/drivers/net/ethernet/freescale/fec_ptp.c
index a15663ad7f5e..f457a23d0bfb 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -604,6 +604,16 @@ void fec_ptp_init(struct platform_device *pdev)
schedule_delayed_work(fep-time_keep, HZ);
 }
 
+void fec_ptp_stop(struct platform_device *pdev)
+{
+   struct net_device *ndev = platform_get_drvdata(pdev);
+   struct fec_enet_private *fep = netdev_priv(ndev);
+
+   cancel_delayed_work_sync(fep-time_keep);
+   if (fep-ptp_clock)
+   ptp_clock_unregister(fep-ptp_clock);
+}
+
 /**
  * fec_ptp_check_pps_event
  * @fep: the fec_enet_private structure handle
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] net: fec: fix runtime PM when probing MII bus

[Lucas Stach]

In the case where there is no mdio bus specified in the devicetree a plain
mdiobus_register() will be called, which tries to probe the connected PHY
by doing mdio_read() on the bus.
Since 6c3e921b18ed (net: fec: Ensure clocks are enabled while using mdio bus)
this needs runtime PM to be available, but as RPM is only later set up in the
FEC probe function those calls will fail, which in turn prevents the FEC
driver to be registered successfully.

Fix this by moving the RPM setup calls before the MII bus probing.

Also move autosuspend init calls before runtime_pm_enable() so that the RPM
callbacks aren't invoked several times during the probe function.

Signed-off-by: Lucas Stach l.st...@pengutronix.de
---
The offending commit got in with v4.2-rc3, so this should be applied as
a fix for 4.2.
---
 drivers/net/ethernet/freescale/fec_main.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index 42e20e5385ac..349365d85b92 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3431,6 +3431,11 @@ fec_probe(struct platform_device *pdev)
fep-reg_phy = NULL;
}
 
+   pm_runtime_set_autosuspend_delay(pdev-dev, FEC_MDIO_PM_TIMEOUT);
+   pm_runtime_use_autosuspend(pdev-dev);
+   pm_runtime_set_active(pdev-dev);
+   pm_runtime_enable(pdev-dev);
+
fec_reset_phy(pdev);
 
if (fep-bufdesc_ex)
@@ -3465,8 +3470,6 @@ fec_probe(struct platform_device *pdev)
netif_carrier_off(ndev);
fec_enet_clk_enable(ndev, false);
pinctrl_pm_select_sleep_state(pdev-dev);
-   pm_runtime_set_active(pdev-dev);
-   pm_runtime_enable(pdev-dev);
 
ret = register_netdev(ndev);
if (ret)
@@ -3481,8 +3484,6 @@ fec_probe(struct platform_device *pdev)
fep-rx_copybreak = COPYBREAK_DEFAULT;
INIT_WORK(fep-tx_timeout_work, fec_enet_timeout_work);
 
-   pm_runtime_set_autosuspend_delay(pdev-dev, FEC_MDIO_PM_TIMEOUT);
-   pm_runtime_use_autosuspend(pdev-dev);
pm_runtime_mark_last_busy(pdev-dev);
pm_runtime_put_autosuspend(pdev-dev);
 
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: fec: fix runtime PM when probing MII bus

[Lucas Stach]

Am Montag, den 20.07.2015, 15:59 +0200 schrieb Andrew Lunn:
 On Mon, Jul 20, 2015 at 03:48:14PM +0200, Lucas Stach wrote:
  In the case where there is no mdio bus specified in the devicetree a plain
  mdiobus_register() will be called, which tries to probe the connected PHY
  by doing mdio_read() on the bus.
  Since 6c3e921b18ed (net: fec: Ensure clocks are enabled while using mdio 
  bus)
  this needs runtime PM to be available, but as RPM is only later set up in 
  the
  FEC probe function those calls will fail, which in turn prevents the FEC
  driver to be registered successfully.
  
  Fix this by moving the RPM setup calls before the MII bus probing.
  
  Also move autosuspend init calls before runtime_pm_enable() so that the RPM
  callbacks aren't invoked several times during the probe function.
  
  Signed-off-by: Lucas Stach l.st...@pengutronix.de
  ---
  The offending commit got in with v4.2-rc3, so this should be applied as
  a fix for 4.2.
 
 Hi Lucas
 
 The patch adding runtime PM has been reverted by David.
 
 Are you O.K. if i fold you fix into my patch and resubmit. I will add
 your Signed-off-by.
 
Oh, I had missed that. Thanks for the hint.

I'm ok with folding this patch into yours. Please mention that this
fixes the regression seen on i.MX6 witch caused the revert of the
original patch.

Regards,
Lucas

-- 
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] net: fec: introduce fec_ptp_stop and use in probe fail path

[Lucas Stach]

This function frees resources and cancels delayed work item that
have been initialized in fec_ptp_init().

Use this to do proper error handling if something goes wrong in
probe function after fec_ptp_init has been called.

Signed-off-by: Lucas Stach l.st...@pengutronix.de
---
 drivers/net/ethernet/freescale/fec.h  |  1 +
 drivers/net/ethernet/freescale/fec_main.c |  5 ++---
 drivers/net/ethernet/freescale/fec_ptp.c  | 10 ++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h 
b/drivers/net/ethernet/freescale/fec.h
index 1eee73cccdf5..99d33e2d35e6 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -562,6 +562,7 @@ struct fec_enet_private {
 };
 
 void fec_ptp_init(struct platform_device *pdev);
+void fec_ptp_stop(struct platform_device *pdev);
 void fec_ptp_start_cyclecounter(struct net_device *ndev);
 int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr);
 int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr);
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index b3287c6b069b..7526e4cf62fe 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3494,6 +3494,7 @@ failed_register:
 failed_mii_init:
 failed_irq:
 failed_init:
+   fec_ptp_stop(pdev);
if (fep-reg_phy)
regulator_disable(fep-reg_phy);
 failed_regulator:
@@ -3515,14 +3516,12 @@ fec_drv_remove(struct platform_device *pdev)
struct net_device *ndev = platform_get_drvdata(pdev);
struct fec_enet_private *fep = netdev_priv(ndev);
 
-   cancel_delayed_work_sync(fep-time_keep);
cancel_work_sync(fep-tx_timeout_work);
+   fec_ptp_stop(pdev);
unregister_netdev(ndev);
fec_enet_mii_remove(fep);
if (fep-reg_phy)
regulator_disable(fep-reg_phy);
-   if (fep-ptp_clock)
-   ptp_clock_unregister(fep-ptp_clock);
of_node_put(fep-phy_node);
free_netdev(ndev);
 
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c 
b/drivers/net/ethernet/freescale/fec_ptp.c
index a15663ad7f5e..f457a23d0bfb 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -604,6 +604,16 @@ void fec_ptp_init(struct platform_device *pdev)
schedule_delayed_work(fep-time_keep, HZ);
 }
 
+void fec_ptp_stop(struct platform_device *pdev)
+{
+   struct net_device *ndev = platform_get_drvdata(pdev);
+   struct fec_enet_private *fep = netdev_priv(ndev);
+
+   cancel_delayed_work_sync(fep-time_keep);
+   if (fep-ptp_clock)
+   ptp_clock_unregister(fep-ptp_clock);
+}
+
 /**
  * fec_ptp_check_pps_event
  * @fep: the fec_enet_private structure handle
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] net: fec: use managed DMA API functions to allocate BD ring

[Lucas Stach]

So it gets freed when the device is going away.
This fixes a DMA memory leak on driver probe() fail and driver
remove().

Signed-off-by: Lucas Stach l.st...@pengutronix.de
---
 drivers/net/ethernet/freescale/fec_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index 349365d85b92..b3287c6b069b 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3142,7 +3142,7 @@ static int fec_enet_init(struct net_device *ndev)
fep-bufdesc_size;
 
/* Allocate memory for buffer descriptors. */
-   cbd_base = dma_alloc_coherent(NULL, bd_size, bd_dma,
+   cbd_base = dmam_alloc_coherent(fep-pdev-dev, bd_size, bd_dma,
  GFP_KERNEL);
if (!cbd_base) {
return -ENOMEM;
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html