Re: [PATCH] usb: ohci-sa1111: convert shutdown method to native device_driver

[Russell King - ARM Linux]

On Tue, Sep 26, 2017 at 11:35:32AM -0400, Alan Stern wrote:
> On Tue, 26 Sep 2017, Russell King - ARM Linux wrote:
> 
> > On Tue, Sep 26, 2017 at 10:35:23AM -0400, Alan Stern wrote:
> > > On Tue, 26 Sep 2017, Russell King wrote:
> > > > Convert the shutdown method to use the device_driver shutdown function
> > > > pointer rather than a private bus-type shutdown.  This is the only user
> > > > for SA bus types, so having the support code in the bus doesn't
> > > > make any sense.
> 
> > > I have no objection to this patch.  But it leads me to wonder why you 
> > > don't get rid of the SA bus type entirely, rather than keeping it 
> > > just for the sake of one driver?
> > 
> > I think you misunderstood the commit message.  This is the only user of
> > the shutdown method for this bus type.  This is not the only user of
> > this bus type - there are other drivers that use this bus type.
> 
> I see -- just a slight ambiguity in the description.  That's fine.
> 
> For all three of the ohci-sa patches:
> 
> Acked-by: Alan Stern <st...@rowland.harvard.edu>

Thanks, I've improved the commit message to clear up that misunderstanding.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: ohci-sa1111: convert shutdown method to native device_driver

[Russell King - ARM Linux]

On Tue, Sep 26, 2017 at 10:35:23AM -0400, Alan Stern wrote:
> On Tue, 26 Sep 2017, Russell King wrote:
> > Convert the shutdown method to use the device_driver shutdown function
> > pointer rather than a private bus-type shutdown.  This is the only user
> > for SA bus types, so having the support code in the bus doesn't
> > make any sense.
> > 
> > Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
> > ---
> >  drivers/usb/host/ohci-sa.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/host/ohci-sa.c b/drivers/usb/host/ohci-sa.c
> > index 9aa4fe1800b9..82842918cb0c 100644
> > --- a/drivers/usb/host/ohci-sa.c
> > +++ b/drivers/usb/host/ohci-sa.c
> > @@ -247,8 +247,9 @@ static int ohci_hcd_sa_remove(struct sa_dev 
> > *dev)
> > return 0;
> >  }
> >  
> > -static void ohci_hcd_sa_shutdown(struct sa_dev *dev)
> > +static void ohci_hcd_sa_shutdown(struct device *_dev)
> >  {
> > +   struct sa_dev *dev = to_sa_device(_dev);
> > struct usb_hcd *hcd = sa_get_drvdata(dev);
> >  
> > if (test_bit(HCD_FLAG_HW_ACCESSIBLE, >flags)) {
> > @@ -261,9 +262,9 @@ static struct sa_driver ohci_hcd_sa_driver = {
> > .drv = {
> > .name   = "sa-ohci",
> > .owner  = THIS_MODULE,
> > +   .shutdown = ohci_hcd_sa_shutdown,
> > },
> > .devid  = SA_DEVID_USB,
> > .probe  = ohci_hcd_sa_probe,
> > .remove = ohci_hcd_sa_remove,
> > -   .shutdown   = ohci_hcd_sa_shutdown,
> >  };
> 
> I have no objection to this patch.  But it leads me to wonder why you 
> don't get rid of the SA bus type entirely, rather than keeping it 
> just for the sake of one driver?

I think you misunderstood the commit message.  This is the only user of
the shutdown method for this bus type.  This is not the only user of
this bus type - there are other drivers that use this bus type.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: ohci-sa1111: remove special sa1111 mmio accessors

[Russell King]

Remove the special SA MMIO accessors from the ohci-sa driver
as their definition will be removed shortly.  The SA accessors are
barrierless, so use the _relaxed variants.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/usb/host/ohci-sa.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ohci-sa.c b/drivers/usb/host/ohci-sa.c
index 82842918cb0c..8758c73215d7 100644
--- a/drivers/usb/host/ohci-sa.c
+++ b/drivers/usb/host/ohci-sa.c
@@ -42,7 +42,7 @@
 #if 0
 static void dump_hci_status(struct usb_hcd *hcd, const char *label)
 {
-   unsigned long status = sa_readl(hcd->regs + USB_STATUS);
+   unsigned long status = readl_relaxed(hcd->regs + USB_STATUS);
 
printk(KERN_DEBUG "%s USB_STATUS = { %s%s%s%s%s}\n", label,
 ((status & USB_STATUS_IRQHCIRMTWKUP) ? "IRQHCIRMTWKUP " : ""),
@@ -134,7 +134,7 @@ static int sa_start_hc(struct sa_dev *dev)
 * Configure the power sense and control lines.  Place the USB
 * host controller in reset.
 */
-   sa_writel(usb_rst | USB_RESET_FORCEIFRESET | USB_RESET_FORCEHCRESET,
+   writel_relaxed(usb_rst | USB_RESET_FORCEIFRESET | 
USB_RESET_FORCEHCRESET,
  dev->mapbase + USB_RESET);
 
/*
@@ -144,7 +144,7 @@ static int sa_start_hc(struct sa_dev *dev)
ret = sa_enable_device(dev);
if (ret == 0) {
udelay(11);
-   sa_writel(usb_rst, dev->mapbase + USB_RESET);
+   writel_relaxed(usb_rst, dev->mapbase + USB_RESET);
}
 
return ret;
@@ -159,8 +159,8 @@ static void sa_stop_hc(struct sa_dev *dev)
/*
 * Put the USB host controller into reset.
 */
-   usb_rst = sa_readl(dev->mapbase + USB_RESET);
-   sa_writel(usb_rst | USB_RESET_FORCEIFRESET | USB_RESET_FORCEHCRESET,
+   usb_rst = readl_relaxed(dev->mapbase + USB_RESET);
+   writel_relaxed(usb_rst | USB_RESET_FORCEIFRESET | 
USB_RESET_FORCEHCRESET,
  dev->mapbase + USB_RESET);
 
/*
-- 
2.7.4

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

[PATCH] usb: ohci-sa1111: convert shutdown method to native device_driver

[Russell King]

Convert the shutdown method to use the device_driver shutdown function
pointer rather than a private bus-type shutdown.  This is the only user
for SA bus types, so having the support code in the bus doesn't
make any sense.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/usb/host/ohci-sa.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ohci-sa.c b/drivers/usb/host/ohci-sa.c
index 9aa4fe1800b9..82842918cb0c 100644
--- a/drivers/usb/host/ohci-sa.c
+++ b/drivers/usb/host/ohci-sa.c
@@ -247,8 +247,9 @@ static int ohci_hcd_sa_remove(struct sa_dev *dev)
return 0;
 }
 
-static void ohci_hcd_sa_shutdown(struct sa_dev *dev)
+static void ohci_hcd_sa_shutdown(struct device *_dev)
 {
+   struct sa_dev *dev = to_sa_device(_dev);
struct usb_hcd *hcd = sa_get_drvdata(dev);
 
if (test_bit(HCD_FLAG_HW_ACCESSIBLE, >flags)) {
@@ -261,9 +262,9 @@ static struct sa_driver ohci_hcd_sa_driver = {
.drv = {
.name   = "sa-ohci",
.owner  = THIS_MODULE,
+   .shutdown = ohci_hcd_sa_shutdown,
},
.devid  = SA_DEVID_USB,
.probe  = ohci_hcd_sa_probe,
.remove = ohci_hcd_sa_remove,
-   .shutdown   = ohci_hcd_sa_shutdown,
 };
-- 
2.7.4

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

[PATCH] usb: ohci-sa1111: use sa1111_get_irq() to obtain IRQ resources

[Russell King]

Use the provided sa_get_irq() to fetch the IRQ resources for the
SA OHCI driver.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/usb/host/ohci-sa.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ohci-sa.c b/drivers/usb/host/ohci-sa.c
index 3a9ea32508df..9aa4fe1800b9 100644
--- a/drivers/usb/host/ohci-sa.c
+++ b/drivers/usb/host/ohci-sa.c
@@ -178,7 +178,7 @@ static void sa_stop_hc(struct sa_dev *dev)
 static int ohci_hcd_sa_probe(struct sa_dev *dev)
 {
struct usb_hcd *hcd;
-   int ret;
+   int ret, irq;
 
if (usb_disabled())
return -ENODEV;
@@ -196,6 +196,12 @@ static int ohci_hcd_sa_probe(struct sa_dev *dev)
hcd->rsrc_start = dev->res.start;
hcd->rsrc_len = resource_size(>res);
 
+   irq = sa_get_irq(dev, 1);
+   if (irq <= 0) {
+   ret = irq ? : -ENXIO;
+   goto err1;
+   }
+
if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
dev_dbg(>dev, "request_mem_region failed\n");
ret = -EBUSY;
@@ -208,7 +214,7 @@ static int ohci_hcd_sa_probe(struct sa_dev *dev)
if (ret)
goto err2;
 
-   ret = usb_add_hcd(hcd, dev->irq[1], 0);
+   ret = usb_add_hcd(hcd, irq, 0);
if (ret == 0) {
device_wakeup_enable(hcd->self.controller);
return ret;
-- 
2.7.4

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

[PATCH net-next 7/7] net: phy: clean up mmd_phy_indirect()

[Russell King]

Make mmd_phy_indirect() use the same terminology as the rest of the
code, making clear what each address is - phy address, devad, and
register number.

While here, remove the "inline" from this static function, leaving
it to the compiler to decide whether to inline this function, and
get rid of unnecessary parens.

Reviewed-by: Andrew Lunn <and...@lunn.ch>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/phy/phy-core.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 80795ccd3fab..357a4d0d7641 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -9,17 +9,17 @@
 #include 
 #include 
 
-static inline void mmd_phy_indirect(struct mii_bus *bus, int prtad, int devad,
-   int addr)
+static void mmd_phy_indirect(struct mii_bus *bus, int phy_addr, int devad,
+u16 regnum)
 {
/* Write the desired MMD Devad */
-   bus->write(bus, addr, MII_MMD_CTRL, devad);
+   bus->write(bus, phy_addr, MII_MMD_CTRL, devad);
 
/* Write the desired MMD register address */
-   bus->write(bus, addr, MII_MMD_DATA, prtad);
+   bus->write(bus, phy_addr, MII_MMD_DATA, regnum);
 
/* Select the Function : DATA with no post increment */
-   bus->write(bus, addr, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
+   bus->write(bus, phy_addr, MII_MMD_CTRL, devad | MII_MMD_CTRL_NOINCR);
 }
 
 /**
@@ -49,7 +49,7 @@ int phy_read_mmd(struct phy_device *phydev, int devad, u32 
regnum)
int phy_addr = phydev->mdio.addr;
 
mutex_lock(>mdio_lock);
-   mmd_phy_indirect(bus, regnum, devad, phy_addr);
+   mmd_phy_indirect(bus, phy_addr, devad, regnum);
 
/* Read the content of the MMD's selected register */
val = bus->read(bus, phy_addr, MII_MMD_DATA);
@@ -88,7 +88,7 @@ int phy_write_mmd(struct phy_device *phydev, int devad, u32 
regnum, u16 val)
int phy_addr = phydev->mdio.addr;
 
mutex_lock(>mdio_lock);
-   mmd_phy_indirect(bus, regnum, devad, phy_addr);
+   mmd_phy_indirect(bus, phy_addr, devad, regnum);
 
/* Write the data into MMD's selected register */
bus->write(bus, phy_addr, MII_MMD_DATA, val);
-- 
2.7.4

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

[PATCH net-next 6/7] net: phy: remove the indirect MMD read/write methods

[Russell King]

Remove the indirect MMD read/write methods which are now no longer
necessary.

Reviewed-by: Andrew Lunn <and...@lunn.ch>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/phy/phy-core.c | 119 +
 include/linux/phy.h|  42 
 2 files changed, 34 insertions(+), 127 deletions(-)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index d791100afab2..80795ccd3fab 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -23,102 +23,41 @@ static inline void mmd_phy_indirect(struct mii_bus *bus, 
int prtad, int devad,
 }
 
 /**
- * phy_read_mmd_indirect - reads data from the MMD registers
- * @phydev: The PHY device bus
- * @prtad: MMD Address
- * @devad: MMD DEVAD
- *
- * Description: it reads data from the MMD registers (clause 22 to access to
- * clause 45) of the specified phy address.
- * To read these register we have:
- * 1) Write reg 13 // DEVAD
- * 2) Write reg 14 // MMD Address
- * 3) Write reg 13 // MMD Data Command for MMD DEVAD
- * 3) Read  reg 14 // Read MMD data
- */
-int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
-{
-   struct phy_driver *phydrv = phydev->drv;
-   int addr = phydev->mdio.addr;
-   int value = -1;
-
-   if (!phydrv->read_mmd_indirect) {
-   struct mii_bus *bus = phydev->mdio.bus;
-
-   mutex_lock(>mdio_lock);
-   mmd_phy_indirect(bus, prtad, devad, addr);
-
-   /* Read the content of the MMD's selected register */
-   value = bus->read(bus, addr, MII_MMD_DATA);
-   mutex_unlock(>mdio_lock);
-   } else {
-   value = phydrv->read_mmd_indirect(phydev, prtad, devad, addr);
-   }
-   return value;
-}
-EXPORT_SYMBOL(phy_read_mmd_indirect);
-
-/**
  * phy_read_mmd - Convenience function for reading a register
  * from an MMD on a given PHY.
  * @phydev: The phy_device struct
- * @devad: The MMD to read from
- * @regnum: The register on the MMD to read
+ * @devad: The MMD to read from (0..31)
+ * @regnum: The register on the MMD to read (0..65535)
  *
  * Same rules as for phy_read();
  */
 int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
 {
+   int val;
+
if (regnum > (u16)~0 || devad > 32)
return -EINVAL;
 
-   if (phydev->drv->read_mmd)
-   return phydev->drv->read_mmd(phydev, devad, regnum);
-
-   if (phydev->is_c45) {
+   if (phydev->drv->read_mmd) {
+   val = phydev->drv->read_mmd(phydev, devad, regnum);
+   } else if (phydev->is_c45) {
u32 addr = MII_ADDR_C45 | (devad << 16) | (regnum & 0x);
-   return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, addr);
-   }
 
-   return phy_read_mmd_indirect(phydev, regnum, devad);
-}
-EXPORT_SYMBOL(phy_read_mmd);
-
-/**
- * phy_write_mmd_indirect - writes data to the MMD registers
- * @phydev: The PHY device
- * @prtad: MMD Address
- * @devad: MMD DEVAD
- * @data: data to write in the MMD register
- *
- * Description: Write data from the MMD registers of the specified
- * phy address.
- * To write these register we have:
- * 1) Write reg 13 // DEVAD
- * 2) Write reg 14 // MMD Address
- * 3) Write reg 13 // MMD Data Command for MMD DEVAD
- * 3) Write reg 14 // Write MMD data
- */
-void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
-  int devad, u32 data)
-{
-   struct phy_driver *phydrv = phydev->drv;
-   int addr = phydev->mdio.addr;
-
-   if (!phydrv->write_mmd_indirect) {
+   val = mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, addr);
+   } else {
struct mii_bus *bus = phydev->mdio.bus;
+   int phy_addr = phydev->mdio.addr;
 
mutex_lock(>mdio_lock);
-   mmd_phy_indirect(bus, prtad, devad, addr);
+   mmd_phy_indirect(bus, regnum, devad, phy_addr);
 
-   /* Write the data into MMD's selected register */
-   bus->write(bus, addr, MII_MMD_DATA, data);
+   /* Read the content of the MMD's selected register */
+   val = bus->read(bus, phy_addr, MII_MMD_DATA);
mutex_unlock(>mdio_lock);
-   } else {
-   phydrv->write_mmd_indirect(phydev, prtad, devad, addr, data);
}
+   return val;
 }
-EXPORT_SYMBOL(phy_write_mmd_indirect);
+EXPORT_SYMBOL(phy_read_mmd);
 
 /**
  * phy_write_mmd - Convenience function for writing a register
@@ -132,21 +71,31 @@ EXPORT_SYMBOL(phy_write_mmd_indirect);
  */
 int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
 {
+   int ret;
+
if (regnum > (u16)~0 || devad > 32)
 

[PATCH net-next 4/7] net: phy: switch remaining users to phy_(read|write)_mmd()

[Russell King]

Switch everyone over to using phy_read_mmd() and phy_write_mmd() now
that they are able to handle both Clause 22 indirect addressing and
Clause 45 direct addressing methods to the MMD registers.

Reviewed-by: Andrew Lunn <and...@lunn.ch>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/phy/bcm-phy-lib.c | 12 
 drivers/net/phy/dp83867.c | 25 +++--
 drivers/net/phy/intel-xway.c  | 26 +-
 drivers/net/phy/microchip.c   |  5 ++---
 drivers/net/phy/phy.c | 25 ++---
 drivers/net/phy/phy_device.c  |  4 ++--
 6 files changed, 42 insertions(+), 55 deletions(-)

diff --git a/drivers/net/phy/bcm-phy-lib.c b/drivers/net/phy/bcm-phy-lib.c
index 9656dbeb5de5..171010eb4d9c 100644
--- a/drivers/net/phy/bcm-phy-lib.c
+++ b/drivers/net/phy/bcm-phy-lib.c
@@ -201,8 +201,7 @@ int bcm_phy_set_eee(struct phy_device *phydev, bool enable)
int val;
 
/* Enable EEE at PHY level */
-   val = phy_read_mmd_indirect(phydev, BRCM_CL45VEN_EEE_CONTROL,
-   MDIO_MMD_AN);
+   val = phy_read_mmd(phydev, MDIO_MMD_AN, BRCM_CL45VEN_EEE_CONTROL);
if (val < 0)
return val;
 
@@ -211,12 +210,10 @@ int bcm_phy_set_eee(struct phy_device *phydev, bool 
enable)
else
val &= ~(LPI_FEATURE_EN | LPI_FEATURE_EN_DIG1000X);
 
-   phy_write_mmd_indirect(phydev, BRCM_CL45VEN_EEE_CONTROL,
-  MDIO_MMD_AN, (u32)val);
+   phy_write_mmd(phydev, MDIO_MMD_AN, BRCM_CL45VEN_EEE_CONTROL, (u32)val);
 
/* Advertise EEE */
-   val = phy_read_mmd_indirect(phydev, BCM_CL45VEN_EEE_ADV,
-   MDIO_MMD_AN);
+   val = phy_read_mmd(phydev, MDIO_MMD_AN, BCM_CL45VEN_EEE_ADV);
if (val < 0)
return val;
 
@@ -225,8 +222,7 @@ int bcm_phy_set_eee(struct phy_device *phydev, bool enable)
else
val &= ~(MDIO_EEE_100TX | MDIO_EEE_1000T);
 
-   phy_write_mmd_indirect(phydev, BCM_CL45VEN_EEE_ADV,
-  MDIO_MMD_AN, (u32)val);
+   phy_write_mmd(phydev, MDIO_MMD_AN, BCM_CL45VEN_EEE_ADV, (u32)val);
 
return 0;
 }
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 19865530e0b1..b57f20e552ba 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -133,14 +133,14 @@ static int dp83867_config_port_mirroring(struct 
phy_device *phydev)
(struct dp83867_private *)phydev->priv;
u16 val;
 
-   val = phy_read_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR);
+   val = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_CFG4);
 
if (dp83867->port_mirroring == DP83867_PORT_MIRROING_EN)
val |= DP83867_CFG4_PORT_MIRROR_EN;
else
val &= ~DP83867_CFG4_PORT_MIRROR_EN;
 
-   phy_write_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR, val);
+   phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_CFG4, val);
 
return 0;
 }
@@ -231,8 +231,7 @@ static int dp83867_config_init(struct phy_device *phydev)
 * register's bit 11 (marked as RESERVED).
 */
 
-   bs = phy_read_mmd_indirect(phydev, DP83867_STRAP_STS1,
-  DP83867_DEVADDR);
+   bs = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_STRAP_STS1);
if (bs & DP83867_STRAP_STS1_RESERVED)
val &= ~DP83867_PHYCR_RESERVED_MASK;
 
@@ -243,8 +242,7 @@ static int dp83867_config_init(struct phy_device *phydev)
 
if ((phydev->interface >= PHY_INTERFACE_MODE_RGMII_ID) &&
(phydev->interface <= PHY_INTERFACE_MODE_RGMII_RXID)) {
-   val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
-   DP83867_DEVADDR);
+   val = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIICTL);
 
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
val |= (DP83867_RGMII_TX_CLK_DELAY_EN | 
DP83867_RGMII_RX_CLK_DELAY_EN);
@@ -255,25 +253,24 @@ static int dp83867_config_init(struct phy_device *phydev)
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
val |= DP83867_RGMII_RX_CLK_DELAY_EN;
 
-   phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
-  DP83867_DEVADDR, val);
+   phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIICTL, val);
 
delay = (dp83867->rx_id_delay |
(dp83867->tx_id_delay << 
DP83867_RGMII_TX_CLK_DELAY_SHIFT));
 
-   phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
-  DP83867_D

[PATCH net-next 1/7] net: phy: move phy MMD accessors to phy-core.c

[Russell King]

Move the phy_(read|write)__mmd() helpers out of line, they will become
our main MMD accessor functions, and so will be a little more complex.
This complexity doesn't belong in an inline function.  Also move the
_indirect variants as well to keep like functionality together.

Reviewed-by: Andrew Lunn <and...@lunn.ch>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/phy/Makefile   |   2 +-
 drivers/net/phy/phy-core.c | 135 +
 drivers/net/phy/phy.c  |  85 
 include/linux/phy.h|  20 +--
 4 files changed, 138 insertions(+), 104 deletions(-)
 create mode 100644 drivers/net/phy/phy-core.c

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 407b0b601ea8..82d915614646 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -1,7 +1,7 @@
 # Makefile for Linux PHY drivers and MDIO bus drivers
 
 libphy-y   := phy.o phy_device.o mdio_bus.o mdio_device.o \
-  mdio-boardinfo.o
+  mdio-boardinfo.o phy-core.o
 libphy-$(CONFIG_SWPHY) += swphy.o
 libphy-$(CONFIG_LED_TRIGGER_PHY)   += phy_led_triggers.o
 
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
new file mode 100644
index ..b8d8276a3099
--- /dev/null
+++ b/drivers/net/phy/phy-core.c
@@ -0,0 +1,135 @@
+/*
+ * Core PHY library, taken from phy.c
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#include 
+#include 
+
+static inline void mmd_phy_indirect(struct mii_bus *bus, int prtad, int devad,
+   int addr)
+{
+   /* Write the desired MMD Devad */
+   bus->write(bus, addr, MII_MMD_CTRL, devad);
+
+   /* Write the desired MMD register address */
+   bus->write(bus, addr, MII_MMD_DATA, prtad);
+
+   /* Select the Function : DATA with no post increment */
+   bus->write(bus, addr, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
+}
+
+/**
+ * phy_read_mmd_indirect - reads data from the MMD registers
+ * @phydev: The PHY device bus
+ * @prtad: MMD Address
+ * @devad: MMD DEVAD
+ *
+ * Description: it reads data from the MMD registers (clause 22 to access to
+ * clause 45) of the specified phy address.
+ * To read these register we have:
+ * 1) Write reg 13 // DEVAD
+ * 2) Write reg 14 // MMD Address
+ * 3) Write reg 13 // MMD Data Command for MMD DEVAD
+ * 3) Read  reg 14 // Read MMD data
+ */
+int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
+{
+   struct phy_driver *phydrv = phydev->drv;
+   int addr = phydev->mdio.addr;
+   int value = -1;
+
+   if (!phydrv->read_mmd_indirect) {
+   struct mii_bus *bus = phydev->mdio.bus;
+
+   mutex_lock(>mdio_lock);
+   mmd_phy_indirect(bus, prtad, devad, addr);
+
+   /* Read the content of the MMD's selected register */
+   value = bus->read(bus, addr, MII_MMD_DATA);
+   mutex_unlock(>mdio_lock);
+   } else {
+   value = phydrv->read_mmd_indirect(phydev, prtad, devad, addr);
+   }
+   return value;
+}
+EXPORT_SYMBOL(phy_read_mmd_indirect);
+
+/**
+ * phy_read_mmd - Convenience function for reading a register
+ * from an MMD on a given PHY.
+ * @phydev: The phy_device struct
+ * @devad: The MMD to read from
+ * @regnum: The register on the MMD to read
+ *
+ * Same rules as for phy_read();
+ */
+int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
+{
+   if (!phydev->is_c45)
+   return -EOPNOTSUPP;
+
+   return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr,
+   MII_ADDR_C45 | (devad << 16) | (regnum & 0x));
+}
+EXPORT_SYMBOL(phy_read_mmd);
+
+/**
+ * phy_write_mmd_indirect - writes data to the MMD registers
+ * @phydev: The PHY device
+ * @prtad: MMD Address
+ * @devad: MMD DEVAD
+ * @data: data to write in the MMD register
+ *
+ * Description: Write data from the MMD registers of the specified
+ * phy address.
+ * To write these register we have:
+ * 1) Write reg 13 // DEVAD
+ * 2) Write reg 14 // MMD Address
+ * 3) Write reg 13 // MMD Data Command for MMD DEVAD
+ * 3) Write reg 14 // Write MMD data
+ */
+void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
+  int devad, u32 data)
+{
+   struct phy_driver *phydrv = phydev->drv;
+   int addr = phydev->mdio.addr;
+
+   if (!phydrv->write_mmd_indirect) {
+   struct mii_bus *bus = phydev->mdio.bus;
+
+   mutex_lock(>mdio_lock);

[PATCH net-next 3/7] net: lan78xx: update for phy_(read|write)_mmd_indirect() removal

[Russell King]

lan78xx appears to use phylib in a rather weird way, accessing the PHY
partly through phylib, and partly by making direct accesses to it,
including to the Clause 45 registers.  As the indirect MMD accessors are
going away, update this driver to use the plain phy_(read|write)_mmd()
accessors instead.

Reviewed-by: Andrew Lunn <and...@lunn.ch>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
Acked-by: Woojung Huh <woojung@microchip.com>
Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/usb/lan78xx.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 9889a70ff4f6..d885e0325422 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1952,10 +1952,10 @@ static int lan8835_fixup(struct phy_device *phydev)
struct lan78xx_net *dev = netdev_priv(phydev->attached_dev);
 
/* LED2/PME_N/IRQ_N/RGMII_ID pin to IRQ_N mode */
-   buf = phy_read_mmd_indirect(phydev, 0x8010, 3);
+   buf = phy_read_mmd(phydev, MDIO_MMD_PCS, 0x8010);
buf &= ~0x1800;
buf |= 0x0800;
-   phy_write_mmd_indirect(phydev, 0x8010, 3, buf);
+   phy_write_mmd(phydev, MDIO_MMD_PCS, 0x8010, buf);
 
/* RGMII MAC TXC Delay Enable */
ret = lan78xx_write_reg(dev, MAC_RGMII_ID,
@@ -1975,11 +1975,11 @@ static int ksz9031rnx_fixup(struct phy_device *phydev)
 
/* Micrel9301RNX PHY configuration */
/* RGMII Control Signal Pad Skew */
-   phy_write_mmd_indirect(phydev, 4, 2, 0x0077);
+   phy_write_mmd(phydev, MDIO_MMD_WIS, 4, 0x0077);
/* RGMII RX Data Pad Skew */
-   phy_write_mmd_indirect(phydev, 5, 2, 0x);
+   phy_write_mmd(phydev, MDIO_MMD_WIS, 5, 0x);
/* RGMII RX Clock Pad Skew */
-   phy_write_mmd_indirect(phydev, 8, 2, 0x1FF);
+   phy_write_mmd(phydev, MDIO_MMD_WIS, 8, 0x1FF);
 
dev->interface = PHY_INTERFACE_MODE_RGMII_RXID;
 
-- 
2.7.4

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

[PATCH net-next 5/7] net: phy: convert micrel to new read_mmd/write_mmd driver methods

[Russell King]

Convert micrel to the new read_mmd/write_mmd driver methods.  This
Clause 22 PHY does not support any MMD access method.

Reviewed-by: Andrew Lunn <and...@lunn.ch>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/phy/micrel.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 6742070ca676..b847184de6fc 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -637,8 +637,7 @@ static int ksz8873mll_config_aneg(struct phy_device *phydev)
  * MMD extended PHY registers.
  */
 static int
-ksz9021_rd_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
- int regnum)
+ksz9021_rd_mmd_phyreg(struct phy_device *phydev, int devad, u16 regnum)
 {
return -1;
 }
@@ -646,10 +645,10 @@ ksz9021_rd_mmd_phyreg(struct phy_device *phydev, int 
ptrad, int devnum,
 /* This routine does nothing since the Micrel ksz9021 does not support
  * standard IEEE MMD extended PHY registers.
  */
-static void
-ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
- int regnum, u32 val)
+static int
+ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int devad, u16 regnum, u16 
val)
 {
+   return -1;
 }
 
 static int kszphy_get_sset_count(struct phy_device *phydev)
@@ -962,8 +961,8 @@ static struct phy_driver ksphy_driver[] = {
.get_stats  = kszphy_get_stats,
.suspend= genphy_suspend,
.resume = genphy_resume,
-   .read_mmd_indirect = ksz9021_rd_mmd_phyreg,
-   .write_mmd_indirect = ksz9021_wr_mmd_phyreg,
+   .read_mmd   = ksz9021_rd_mmd_phyreg,
+   .write_mmd  = ksz9021_wr_mmd_phyreg,
 }, {
.phy_id = PHY_ID_KSZ9031,
.phy_id_mask= MICREL_PHY_ID_MASK,
-- 
2.7.4

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

[PATCH net-next 0/7] Clean up PHY MMD accessors

[Russell King - ARM Linux]

This series cleans up phylib's MMD accessors, so that we have a common
way of accessing the Clause 45 register set.

The current situation is far from ideal - we have phy_(read|write)_mmd()
which accesses Clause 45 registers over Clause 45 accesses, and we have
phy_(read|write)_mmd_indirect(), which accesses Clause 45 registers via
Clause 22 register 13/14.

Generic code uses the indirect methods to access standard Clause 45
features, and when we come to add Clause 45 PHY support to phylib, we
would need to make these conditional upon the PHY type, or duplicate
these functions.

An alternative solution is to merge these accessors together, and select
the appropriate access method depending upon the 802.3 clause that the
PHY conforms with.  The result is that we have a single set of
phy_(read|write)_mmd() accessors.

For cases which require special handling, we still allow PHY drivers to
override all MMD accesses - except rather than just overriding the
indirect accesses.  This keeps existing overrides working.

Combining the two also has another beneficial side effect - we get rid
of similar functions that take arguments in different orders.  The
old direct accessors took the phy structure, devad and register number,
whereas the indirect accessors took the phy structure, register number
and devad in that order.  Care must be taken when updating future
drivers that the argument order is correct, and the function name is
not merely replaced.

This patch set is against net-next.

 drivers/net/phy/Makefile  |   2 +-
 drivers/net/phy/bcm-phy-lib.c |  12 ++---
 drivers/net/phy/dp83867.c |  25 +-
 drivers/net/phy/intel-xway.c  |  26 +-
 drivers/net/phy/micrel.c  |  13 +++--
 drivers/net/phy/microchip.c   |   5 +-
 drivers/net/phy/phy-core.c| 101 ++
 drivers/net/phy/phy.c | 110 --
 drivers/net/phy/phy_device.c  |   4 +-
 drivers/net/usb/lan78xx.c |  10 ++--
 include/linux/phy.h   |  80 +-
 11 files changed, 178 insertions(+), 210 deletions(-)

RFC->v1:
- minor update patch 2 to resolve a build error that the 0-day builder
  discovered.
- reviewed-by / acked-bys added

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net-next 2/7] net: phy: make phy_(read|write)_mmd() generic MMD accessors

[Russell King]

Make phy_(read|write)_mmd() generic 802.3 clause 45 register accessors
for both Clause 22 and Clause 45 PHYs, using either the direct register
reading for Clause 45, or the indirect method for Clause 22 PHYs.
Allow this behaviour to be overriden by PHY drivers where necessary.

Reviewed-by: Andrew Lunn <and...@lunn.ch>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/phy/phy-core.c | 33 +
 include/linux/phy.h| 24 
 2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index b8d8276a3099..d791100afab2 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -69,11 +69,18 @@ EXPORT_SYMBOL(phy_read_mmd_indirect);
  */
 int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
 {
-   if (!phydev->is_c45)
-   return -EOPNOTSUPP;
+   if (regnum > (u16)~0 || devad > 32)
+   return -EINVAL;
 
-   return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr,
-   MII_ADDR_C45 | (devad << 16) | (regnum & 0x));
+   if (phydev->drv->read_mmd)
+   return phydev->drv->read_mmd(phydev, devad, regnum);
+
+   if (phydev->is_c45) {
+   u32 addr = MII_ADDR_C45 | (devad << 16) | (regnum & 0x);
+   return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, addr);
+   }
+
+   return phy_read_mmd_indirect(phydev, regnum, devad);
 }
 EXPORT_SYMBOL(phy_read_mmd);
 
@@ -125,11 +132,21 @@ EXPORT_SYMBOL(phy_write_mmd_indirect);
  */
 int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
 {
-   if (!phydev->is_c45)
-   return -EOPNOTSUPP;
+   if (regnum > (u16)~0 || devad > 32)
+   return -EINVAL;
+
+   if (phydev->drv->read_mmd)
+   return phydev->drv->write_mmd(phydev, devad, regnum, val);
+
+   if (phydev->is_c45) {
+   u32 addr = MII_ADDR_C45 | (devad << 16) | (regnum & 0x);
+
+   return mdiobus_write(phydev->mdio.bus, phydev->mdio.addr,
+addr, val);
+   }
 
-   regnum = MII_ADDR_C45 | ((devad & 0x1f) << 16) | (regnum & 0x);
+   phy_write_mmd_indirect(phydev, regnum, devad, val);
 
-   return mdiobus_write(phydev->mdio.bus, phydev->mdio.addr, regnum, val);
+   return 0;
 }
 EXPORT_SYMBOL(phy_write_mmd);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index bcb4549b41d6..b8feeffeb64c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -587,6 +587,30 @@ struct phy_driver {
 */
void (*link_change_notify)(struct phy_device *dev);
 
+   /*
+* Phy specific driver override for reading a MMD register.
+* This function is optional for PHY specific drivers.  When
+* not provided, the default MMD read function will be used
+* by phy_read_mmd(), which will use either a direct read for
+* Clause 45 PHYs or an indirect read for Clause 22 PHYs.
+*  devnum is the MMD device number within the PHY device,
+*  regnum is the register within the selected MMD device.
+*/
+   int (*read_mmd)(struct phy_device *dev, int devnum, u16 regnum);
+
+   /*
+* Phy specific driver override for writing a MMD register.
+* This function is optional for PHY specific drivers.  When
+* not provided, the default MMD write function will be used
+* by phy_write_mmd(), which will use either a direct write for
+* Clause 45 PHYs, or an indirect write for Clause 22 PHYs.
+*  devnum is the MMD device number within the PHY device,
+*  regnum is the register within the selected MMD device.
+*  val is the value to be written.
+*/
+   int (*write_mmd)(struct phy_device *dev, int devnum, u16 regnum,
+u16 val);
+
/* A function provided by a phy specific driver to override the
 * the PHY driver framework support for reading a MMD register
 * from the PHY. If not supported, return -1. This function is
-- 
2.7.4

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

Re: [PATCH RFC 0/7] phylib MMD accessor cleanups

[Russell King - ARM Linux]

On Sun, Mar 19, 2017 at 03:30:38PM -0700, Florian Fainelli wrote:
> Le 03/19/17 à 03:59, Russell King - ARM Linux a écrit :
> > This series of patches does exactly that - we merge the functionality
> > of the indirect accesses into the clause 45 accessors, and use these
> > exclusively to access MMD registers.  Internally, the new clause
> > independent MMD accessors indirect via the PHY drivers read_mmd/write_mmd
> > methods if present, otherwise fall back to using clause 45 accesses if
> > the PHY is a clause 45 phy, or clause 22 indirect accesses if the PHY
> > is clause 22.
> 
> LGTM:
> 
> Reviewed-by: Florian Fainelli <f.faine...@gmail.com>

Thanks.  When I posted this last time around (19th Jan) I mentioned
about marking the old _indirect() accessors with __deprecated - is
that still something we want to do?

I haven't tested this against net-next yet, so I don't know if there
are any new users of the indirect accessors - going down the deprecated
route would avoid breakage, but means having to submit a patch later to
actually remove them.

How would people want this handled?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 3/7] net: lan78xx: update for phy_(read|write)_mmd_indirect() removal

[Russell King]

lan78xx appears to use phylib in a rather weird way, accessing the PHY
partly through phylib, and partly by makign direct accesses to it,
including to the Clause 45 registers.  As the indirect MMD accessors are
going away, update this driver to use the plain phy_(read|write)_mmd()
accessors instead.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/usb/lan78xx.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 9889a70ff4f6..d885e0325422 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1952,10 +1952,10 @@ static int lan8835_fixup(struct phy_device *phydev)
struct lan78xx_net *dev = netdev_priv(phydev->attached_dev);
 
/* LED2/PME_N/IRQ_N/RGMII_ID pin to IRQ_N mode */
-   buf = phy_read_mmd_indirect(phydev, 0x8010, 3);
+   buf = phy_read_mmd(phydev, MDIO_MMD_PCS, 0x8010);
buf &= ~0x1800;
buf |= 0x0800;
-   phy_write_mmd_indirect(phydev, 0x8010, 3, buf);
+   phy_write_mmd(phydev, MDIO_MMD_PCS, 0x8010, buf);
 
/* RGMII MAC TXC Delay Enable */
ret = lan78xx_write_reg(dev, MAC_RGMII_ID,
@@ -1975,11 +1975,11 @@ static int ksz9031rnx_fixup(struct phy_device *phydev)
 
/* Micrel9301RNX PHY configuration */
/* RGMII Control Signal Pad Skew */
-   phy_write_mmd_indirect(phydev, 4, 2, 0x0077);
+   phy_write_mmd(phydev, MDIO_MMD_WIS, 4, 0x0077);
/* RGMII RX Data Pad Skew */
-   phy_write_mmd_indirect(phydev, 5, 2, 0x);
+   phy_write_mmd(phydev, MDIO_MMD_WIS, 5, 0x);
/* RGMII RX Clock Pad Skew */
-   phy_write_mmd_indirect(phydev, 8, 2, 0x1FF);
+   phy_write_mmd(phydev, MDIO_MMD_WIS, 8, 0x1FF);
 
dev->interface = PHY_INTERFACE_MODE_RGMII_RXID;
 
-- 
2.7.4

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

[PATCH RFC 0/7] phylib MMD accessor cleanups

[Russell King - ARM Linux]

Hi,

This series cleans up the phylib MMD accessors.  We have two accessors
at present, phy_(read|write)_mmd() and phy_(read|write)_mmd_indirect()

The _indirect methods access the MMD registers via a clause 22 phy,
whereas the non-_indirect methods acess via clause 45 accesses.

Current PHY-independent parts of phylib (such as EEE) access the MMD
registers via the _indirect methods, which is no good if you have a
clause 45 PHY that doesn't respond to clause 22 accesses.

In order to make these features available, we need to rework these
accessors such that they can access the MMD registers using a method
dependent on the clause that the PHY conforms with.

This series of patches does exactly that - we merge the functionality
of the indirect accesses into the clause 45 accessors, and use these
exclusively to access MMD registers.  Internally, the new clause
independent MMD accessors indirect via the PHY drivers read_mmd/write_mmd
methods if present, otherwise fall back to using clause 45 accesses if
the PHY is a clause 45 phy, or clause 22 indirect accesses if the PHY
is clause 22.

Note: confusingly, phy_read_mmd_indirect() vs phy_read_mmd() switches
the order of prtad and devad, which means that converting between the
two is not a simple matter of just replacing the function name.  Same
applies for the write methods.

 drivers/net/phy/Makefile  |   2 +-
 drivers/net/phy/bcm-phy-lib.c |  12 ++---
 drivers/net/phy/dp83867.c |  25 +-
 drivers/net/phy/intel-xway.c  |  26 +-
 drivers/net/phy/micrel.c  |  13 +++--
 drivers/net/phy/microchip.c   |   5 +-
 drivers/net/phy/phy-core.c| 101 ++
 drivers/net/phy/phy.c | 110 --
 drivers/net/phy/phy_device.c  |   4 +-
 drivers/net/usb/lan78xx.c |  10 ++--
 include/linux/phy.h   |  80 +-
 11 files changed, 178 insertions(+), 210 deletions(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4.10-rc3 02/13] net: cgroups: fix build errors when linux/phy*.h is removed from net/dsa.h

[Russell King]

net/core/netprio_cgroup.c:303:16: error: expected declaration specifiers or 
'...' before string constant
MODULE_LICENSE("GPL v2");
   ^~~~

Add linux/module.h to fix this.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 net/core/netprio_cgroup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index 2ec86fc552df..756637dc7a57 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -13,6 +13,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.7.4

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

[PATCH 4.10-rc3 05/13] net: bgmac: fix build errors when linux/phy*.h is removed from net/dsa.h

[Russell King]

drivers/net/ethernet/broadcom/bgmac.c:1015:17: error: dereferencing pointer to 
incomplete type 'struct mii_bus'
drivers/net/ethernet/broadcom/bgmac.c:1185:2: error: implicit declaration of 
function 'phy_start' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1198:2: error: implicit declaration of 
function 'phy_stop' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1239:9: error: implicit declaration of 
function 'phy_mii_ioctl' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1389:28: error: 
'phy_ethtool_get_link_ksettings' undeclared here (not in a function)
drivers/net/ethernet/broadcom/bgmac.c:1390:28: error: 
'phy_ethtool_set_link_ksettings' undeclared here (not in a function)
drivers/net/ethernet/broadcom/bgmac.c:1403:13: error: dereferencing pointer to 
incomplete type 'struct phy_device'
drivers/net/ethernet/broadcom/bgmac.c:1417:3: error: implicit declaration of 
function 'phy_print_status' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1424:26: error: storage size of 
'fphy_status' isn't known
drivers/net/ethernet/broadcom/bgmac.c:1424:9: error: variable 'fphy_status' has 
initializer but incomplete type
drivers/net/ethernet/broadcom/bgmac.c:1425:11: warning: excess elements in 
struct initializer
drivers/net/ethernet/broadcom/bgmac.c:1425:3: error: unknown field 'link' 
specified in initializer
drivers/net/ethernet/broadcom/bgmac.c:1426:12: note: in expansion of macro 
'SPEED_1000'
drivers/net/ethernet/broadcom/bgmac.c:1426:3: error: unknown field 'speed' 
specified in initializer
drivers/net/ethernet/broadcom/bgmac.c:1427:13: note: in expansion of macro 
'DUPLEX_FULL'
drivers/net/ethernet/broadcom/bgmac.c:1427:3: error: unknown field 'duplex' 
specified in initializer
drivers/net/ethernet/broadcom/bgmac.c:1432:12: error: implicit declaration of 
function 'fixed_phy_register' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1432:31: error: 'PHY_POLL' undeclared 
(first use in this function)
drivers/net/ethernet/broadcom/bgmac.c:1438:8: error: implicit declaration of 
function 'phy_connect_direct' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1439:6: error: 'PHY_INTERFACE_MODE_MII' 
undeclared (first use in this function)
drivers/net/ethernet/broadcom/bgmac.c:1521:2: error: implicit declaration of 
function 'phy_disconnect' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1541:15: error: expected declaration 
specifiers or '...' before string constant

Add linux/phy.h to bgmac.c

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/ethernet/broadcom/bgmac.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index 0e066dc6b8cc..58a2bd3c0458 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -12,6 +12,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "bgmac.h"
 
 static bool bgmac_wait_value(struct bgmac *bgmac, u16 reg, u32 mask,
-- 
2.7.4

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

[PATCH 4.10-rc3 03/13] net: macb: fix build errors when linux/phy*.h is removed from net/dsa.h

[Russell King]

drivers/net/ethernet/cadence/macb.h:862:33: sparse: expected ; at end of 
declaration
drivers/net/ethernet/cadence/macb.h:862:33: sparse: Expected } at end of 
struct-union-enum-specifier
drivers/net/ethernet/cadence/macb.h:862:33: sparse: got phy_interface
drivers/net/ethernet/cadence/macb.h:877:1: sparse: Expected ; at the end of 
type declaration
drivers/net/ethernet/cadence/macb.h:877:1: sparse: got }
In file included from drivers/net/ethernet/cadence/macb_pci.c:29:0:
drivers/net/ethernet/cadence/macb.h:862:2: error: unknown type name 
'phy_interface_t'
 phy_interface_t  phy_interface;
 ^~~

Add linux/phy.h to macb.h

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/ethernet/cadence/macb.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index d67adad67be1..383da8cf5f6d 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -10,6 +10,8 @@
 #ifndef _MACB_H
 #define _MACB_H
 
+#include 
+
 #define MACB_GREGS_NBR 16
 #define MACB_GREGS_VERSION 2
 #define MACB_MAX_QUEUES 8
-- 
2.7.4

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

[PATCH 4.10-rc3 07/13] net: mvneta: fix build errors when linux/phy*.h is removed from net/dsa.h

[Russell King]

drivers/net/ethernet/marvell/mvneta.c:2694:26: error: storage size of 'status' 
isn't known
drivers/net/ethernet/marvell/mvneta.c:2695:26: error: storage size of 'changed' 
isn't known
drivers/net/ethernet/marvell/mvneta.c:2695:9: error: variable 'changed' has 
initializer but incomplete type
drivers/net/ethernet/marvell/mvneta.c:2709:2: error: implicit declaration of 
function 'fixed_phy_update_state' [-Werror=implicit-function-declaration]

Add linux/phy_fixed.h to mvneta.c

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvneta.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/marvell/mvneta.c 
b/drivers/net/ethernet/marvell/mvneta.c
index e05e22705cf7..eb0eb3e62ca0 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.7.4

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

[PATCH 4.10-rc3 01/13] net: sunrpc: fix build errors when linux/phy*.h is removed from net/dsa.h

[Russell King]

Removing linux/phy.h from net/dsa.h reveals a build error in the sunrpc
code:

net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'xprt_rdma_bc_put':
net/sunrpc/xprtrdma/svc_rdma_backchannel.c:277:2: error: implicit declaration 
of function 'module_put' [-Werror=implicit-function-declaration]
net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'xprt_setup_rdma_bc':
net/sunrpc/xprtrdma/svc_rdma_backchannel.c:348:7: error: implicit declaration 
of function 'try_module_get' [-Werror=implicit-function-declaration]

Fix this by adding linux/module.h to svc_rdma_backchannel.c

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c 
b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
index 288e35c2d8f4..cb1e48e54eb1 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
@@ -4,6 +4,7 @@
  * Support for backward direction RPCs on RPC/RDMA (server-side).
  */
 
+#include 
 #include 
 #include "xprt_rdma.h"
 
-- 
2.7.4

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

[PATCH 4.10-rc3 04/13] net: lan78xx: fix build errors when linux/phy*.h is removed from net/dsa.h

[Russell King]

drivers/net/usb/lan78xx.c:394:33: sparse: expected ; at end of declaration
drivers/net/usb/lan78xx.c:394:33: sparse: Expected } at end of 
struct-union-enum-specifier
drivers/net/usb/lan78xx.c:394:33: sparse: got interface
drivers/net/usb/lan78xx.c:403:1: sparse: Expected ; at the end of type 
declaration
drivers/net/usb/lan78xx.c:403:1: sparse: got }

Add linux/phy.h to lan78xx.c

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/usb/lan78xx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 08f8703e4d54..9889a70ff4f6 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "lan78xx.h"
 
 #define DRIVER_AUTHOR  "WOOJUNG HUH <woojung@microchip.com>"
-- 
2.7.4

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

[PATCH 4.10-rc3 09/13] iscsi: fix build errors when linux/phy*.h is removed from net/dsa.h

[Russell King]

drivers/target/iscsi/iscsi_target_login.c:1135:7: error: implicit declaration 
of function 'try_module_get' [-Werror=implicit-function-declaration]

Add linux/module.h to iscsi_target_login.c.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/target/iscsi/iscsi_target_login.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/target/iscsi/iscsi_target_login.c 
b/drivers/target/iscsi/iscsi_target_login.c
index 450f51deb2a2..eab274d17b5c 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -17,6 +17,7 @@
  
**/
 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.7.4

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

[PATCH 4.10-rc3 10/13] MIPS: Octeon: Remove unnecessary MODULE_*()

[Russell King]

octeon-platform.c can not be built as a module for two reasons:

(a) the Makefile doesn't allow it:
obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o

(b) the multiple *_initcall() statements, each of which are translated
to a module_init() call when attempting a module build, become
aliases to init_module().  Having more than one alias will cause a
build error.

Hence, rather than adding a linux/module.h include, remove the redundant
MODULE_*() from this file.

Acked-by: David Daney <david.da...@cavium.com>
Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 arch/mips/cavium-octeon/octeon-platform.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/arch/mips/cavium-octeon/octeon-platform.c 
b/arch/mips/cavium-octeon/octeon-platform.c
index 37a932d9148c..8297ce714c5e 100644
--- a/arch/mips/cavium-octeon/octeon-platform.c
+++ b/arch/mips/cavium-octeon/octeon-platform.c
@@ -1060,7 +1060,3 @@ static int __init octeon_publish_devices(void)
return of_platform_bus_probe(NULL, octeon_ids, NULL);
 }
 arch_initcall(octeon_publish_devices);
-
-MODULE_AUTHOR("David Daney <dda...@caviumnetworks.com>");
-MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("Platform driver for Octeon SOC");
-- 
2.7.4

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

[PATCH 4.10-rc3 06/13] net: fman: fix build errors when linux/phy*.h is removed from net/dsa.h

[Russell King]

drivers/net/ethernet/freescale/fman/fman_memac.c:519:21: error: dereferencing 
pointer to incomplete type 'struct fixed_phy_status'

Add linux/phy_fixed.h to fman_memac.c

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/ethernet/freescale/fman/fman_memac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c 
b/drivers/net/ethernet/freescale/fman/fman_memac.c
index 71a5ded9d1de..cd6a53eaf161 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* PCS registers */
-- 
2.7.4

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

[PATCH 4.10-rc3 08/13] net: emac: fix build errors when linux/phy*.h is removed from net/dsa.h

[Russell King]

drivers/net/ethernet/qualcomm/emac/emac-sgmii.c:58:12: error: dereferencing 
pointer to incomplete type 'struct phy_device'

Add linux/phy.h to emac-sgmii.c

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/ethernet/qualcomm/emac/emac-sgmii.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c 
b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
index bf722a9bb09d..5e31fb7e4ab8 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "emac.h"
 #include "emac-mac.h"
 #include "emac-sgmii.h"
-- 
2.7.4

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

[PATCH 4.10-rc3 11/13] net: liquidio: fix build errors when linux/phy*.h is removed from net/dsa.h

[Russell King]

 or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:40: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:40: error: type defaults to 
'int' in declaration of 'MODULE_FIRMWARE'
drivers/net/ethernet/cavium/liquidio/lio_main.c:40: error: function declaration 
isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_main.c:41: error: expected declaration 
specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:41: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:41: error: type defaults to 
'int' in declaration of 'MODULE_FIRMWARE'
drivers/net/ethernet/cavium/liquidio/lio_main.c:41: error: function declaration 
isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_main.c:42: error: expected declaration 
specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:42: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:42: error: type defaults to 
'int' in declaration of 'MODULE_FIRMWARE'
drivers/net/ethernet/cavium/liquidio/lio_main.c:42: error: function declaration 
isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_main.c:43: error: expected declaration 
specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:43: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:43: error: type defaults to 
'int' in declaration of 'MODULE_FIRMWARE'
drivers/net/ethernet/cavium/liquidio/lio_main.c:43: error: function declaration 
isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_main.c:46: error: expected ')' before 
'int'
drivers/net/ethernet/cavium/liquidio/lio_main.c:48: error: expected ')' before 
string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:53: error: expected ')' before 
'int'
drivers/net/ethernet/cavium/liquidio/lio_main.c:54: error: expected ')' before 
string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:57: error: expected ')' before 
'sizeof'
drivers/net/ethernet/cavium/liquidio/lio_main.c:58: error: expected ')' before 
string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:498: warning: data 
definitionhas no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:498: error: type defaults to 
'int' in declaration of 'MODULE_DEVICE_TABLE'
drivers/net/ethernet/cavium/liquidio/lio_main.c:498: warning: parameter names 
(without types) in function declaration
drivers/net/ethernet/cavium/liquidio/lio_main.c: In function 
'octeon_recv_vf_drv_notice':
drivers/net/ethernet/cavium/liquidio/lio_main.c:4393: error: implicit 
declaration of function 'try_module_get'
drivers/net/ethernet/cavium/liquidio/lio_main.c:4400: error: implicit 
declaration of function 'module_put'
drivers/net/ethernet/cavium/liquidio/lio_main.c: At top level:
drivers/net/ethernet/cavium/liquidio/lio_main.c:4670: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:4670: error: type defaults to 
'int' in declaration of 'module_init'
drivers/net/ethernet/cavium/liquidio/lio_main.c:4670: warning: parameter names 
(without types) in function declaration
drivers/net/ethernet/cavium/liquidio/lio_main.c:4671: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:4671: error: type defaults to 
'int' in declaration of 'module_exit'
drivers/net/ethernet/cavium/liquidio/lio_main.c:4671: warning: parameter names 
(without types) in function declaration

Add linux/module.h to both these files.

drivers/net/ethernet/cavium/liquidio/octeon_console.c:40:31: error: expected 
')' before 'int'
drivers/net/ethernet/cavium/liquidio/octeon_console.c:42:4: error: expected ')' 
before string constant

Add linux/moduleparam.h to this file.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/ethernet/cavium/liquidio/lio_main.c   | 1 +
 drivers/net/ethernet/cavium/liquidio/lio_vf_main.c| 1 +
 drivers/net/ethernet/cavium/liquidio/octeon_console.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c 
b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index 39a9665c9d00..e4dd39440687 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -15,6 +15,7 @@
  * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE, TITLE, or
  * NONINFRINGEMENT.  See the GNU General Public License for more details.
  ***/
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c 
b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index 70d96c10c673..b98d29827a0d 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drive

[PATCH 4.10-rc3 13/13] net: dsa: remove unnecessary phy*.h includes

[Russell King]

Including phy.h and phy_fixed.h into net/dsa.h causes phy*.h to be an
unnecessary dependency for quite a large amount of the kernel.  There's
very little which actually requires definitions from phy.h in net/dsa.h
- the include itself only wants the declaration of a couple of
structures and IFNAMSIZ.

Add linux/if.h for IFNAMSIZ, declarations for the structures, phy.h to
mv88e6xxx.h as it needs it for phy_interface_t, and remove both phy.h
and phy_fixed.h from net/dsa.h.

This patch reduces from around 800 files rebuilt to around 40 - even
with ccache, the time difference is noticable.

Tested-by: Vivien Didelot <vivien.dide...@savoirfairelinux.com>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 +
 include/net/dsa.h | 6 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h 
b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index af54baea47cf..3a949095068a 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifndef UINT64_MAX
 #define UINT64_MAX (u64)(~((u64)0))
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b122196d5a1f..887b2f98f9ea 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -11,15 +11,17 @@
 #ifndef __LINUX_NET_DSA_H
 #define __LINUX_NET_DSA_H
 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
 
+struct phy_device;
+struct fixed_phy_status;
+
 enum dsa_tag_protocol {
DSA_TAG_PROTO_NONE = 0,
DSA_TAG_PROTO_DSA,
-- 
2.7.4

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

[PATCH 4.10-rc3 12/13] net: ath5k: fix build errors when linux/phy*.h is removed from net/dsa.h

[Russell King]

Fix these errors reported by the 0-day builder by replacing the
linux/export.h include with linux/module.h.

In file included from include/linux/platform_device.h:14:0,
 from drivers/net/wireless/ath/ath5k/ahb.c:20:
include/linux/device.h:1463:1: warning: data definition has no type or storage 
class
 module_init(__driver##_init); \
 ^
include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
  module_driver(__platform_driver, platform_driver_register, \
  ^
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
include/linux/device.h:1463:1: error: type defaults to 'int' in declaration of 
'module_init' [-Werror=implicit-int]
 module_init(__driver##_init); \
 ^
include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
  module_driver(__platform_driver, platform_driver_register, \
  ^
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
drivers/net/wireless/ath/ath5k/ahb.c:233:1: warning: parameter names (without 
types) in function declaration
In file included from include/linux/platform_device.h:14:0,
 from drivers/net/wireless/ath/ath5k/ahb.c:20:
include/linux/device.h:1468:1: warning: data definition has no type or storage 
class
 module_exit(__driver##_exit);
 ^
include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
  module_driver(__platform_driver, platform_driver_register, \
  ^
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
include/linux/device.h:1468:1: error: type defaults to 'int' in declaration of 
'module_exit' [-Werror=implicit-int]
 module_exit(__driver##_exit);
 ^
include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
  module_driver(__platform_driver, platform_driver_register, \
  ^
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
drivers/net/wireless/ath/ath5k/ahb.c:233:1: warning: parameter names (without 
types) in function declaration
In file included from include/linux/platform_device.h:14:0,
 from drivers/net/wireless/ath/ath5k/ahb.c:20:
drivers/net/wireless/ath/ath5k/ahb.c:233:24: warning: 'ath_ahb_driver_exit' 
defined but not used [-Wunused-function]
 module_platform_driver(ath_ahb_driver);
^
include/linux/device.h:1464:20: note: in definition of macro 'module_driver'
 static void __exit __driver##_exit(void) \
^~~~
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
drivers/net/wireless/ath/ath5k/ahb.c:233:24: warning: 'ath_ahb_driver_init' 
defined but not used [-Wunused-function]
 module_platform_driver(ath_ahb_driver);
^
include/linux/device.h:1459:19: note: in definition of macro 'module_driver'
 static int __init __driver##_init(void) \
   ^~~~
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/wireless/ath/ath5k/ahb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath5k/ahb.c 
b/drivers/net/wireless/ath/ath5k/ahb.c
index 2ca88b593e4c..c0794f5988b3 100644
--- a/drivers/net/wireless/ath/ath5k/ahb.c
+++ b/drivers/net/wireless/ath/ath5k/ahb.c
@@ -16,10 +16,10 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include "ath5k.h"
 #include "debug.h"
-- 
2.7.4

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

[PATCH 4.10-rc3 00/13] net: dsa: remove unnecessary phy.h include

[Russell King - ARM Linux]

Including phy.h and phy_fixed.h into net/dsa.h causes phy*.h to be an
unnecessary dependency for quite a large amount of the kernel.  There's
very little which actually requires definitions from phy.h in net/dsa.h
- the include itself only wants the declaration of a couple of
structures and IFNAMSIZ.

Add linux/if.h for IFNAMSIZ, declarations for the structures, phy.h to
mv88e6xxx.h as it needs it for phy_interface_t, and remove both phy.h
and phy_fixed.h from net/dsa.h.

This patch reduces from around 800 files rebuilt to around 40 - even
with ccache, the time difference is noticable.

In order to make this change, several drivers need to be updated to
include necessary headers that they were picking up through this
include.  This has resulted in a much larger patch series.

I'm assuming the 0-day builder has had 24 hours with this series, and
hasn't reported any further issues with it - the last issue was two
weeks ago (before I became ill) which I fixed over the last weekend.

I'm hoping this doesn't conflict with what's already in net-next...

 arch/mips/cavium-octeon/octeon-platform.c | 4 
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 +
 drivers/net/ethernet/broadcom/bgmac.c | 2 ++
 drivers/net/ethernet/cadence/macb.h   | 2 ++
 drivers/net/ethernet/cavium/liquidio/lio_main.c   | 1 +
 drivers/net/ethernet/cavium/liquidio/lio_vf_main.c| 1 +
 drivers/net/ethernet/cavium/liquidio/octeon_console.c | 1 +
 drivers/net/ethernet/freescale/fman/fman_memac.c  | 1 +
 drivers/net/ethernet/marvell/mvneta.c | 1 +
 drivers/net/ethernet/qualcomm/emac/emac-sgmii.c   | 1 +
 drivers/net/usb/lan78xx.c | 1 +
 drivers/net/wireless/ath/ath5k/ahb.c  | 2 +-
 drivers/target/iscsi/iscsi_target_login.c | 1 +
 include/net/dsa.h | 6 --
 net/core/netprio_cgroup.c | 1 +
 net/sunrpc/xprtrdma/svc_rdma_backchannel.c| 1 +
 16 files changed, 20 insertions(+), 7 deletions(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 0/7] Clean up PHY MMD accessors

[Russell King - ARM Linux]

On Thu, Jan 19, 2017 at 11:42:47AM -0800, Florian Fainelli wrote:
> On 01/13/2017 07:20 AM, Russell King - ARM Linux wrote:
> > This series cleans up phylib's MMD accessors, so that we have a common
> > way of accessing the Clause 45 register set.
> > 
> > The current situation is far from ideal - we have phy_(read|write)_mmd()
> > which accesses Clause 45 registers over Clause 45 accesses, and we have
> > phy_(read|write)_mmd_indirect(), which accesses Clause 45 registers via
> > Clause 22 register 13/14.
> > 
> > Generic code uses the indirect methods to access standard Clause 45
> > features, and when we come to add Clause 45 PHY support to phylib, we
> > would need to make these conditional upon the PHY type, or duplicate
> > these functions.
> > 
> > An alternative solution is to merge these accessors together, and select
> > the appropriate access method depending upon the 802.3 clause that the
> > PHY conforms with.  The result is that we have a single set of
> > phy_(read|write)_mmd() accessors.
> > 
> > For cases which require special handling, we still allow PHY drivers to
> > override all MMD accesses - except rather than just overriding the
> > indirect accesses.  This keeps existing overrides working.
> > 
> > Combining the two also has another beneficial side effect - we get rid
> > of similar functions that take arguments in different orders.  The
> > old direct accessors took the phy structure, devad and register number,
> > whereas the indirect accessors took the phy structure, register number
> > and devad in that order.  Care must be taken when updating future
> > drivers that the argument order is correct, and the function name is
> > not merely replaced.
> 
> I really like that series, this is much much cleaner, do you mind
> resubmitting this without a RFC tag so David can apply it?

Hi Florian,

Thanks.

Will do in the next couple of days - I'd prefer to make one change
to the series before submitting it for real: provide compatibility
functions marked as deprecated so this doesn't create a flag day.
We can remove the deprecated _indirect functions later when we know
there's no new users.

I'd like to get the phy.h untanglement stuff into David's tree first.
I'm almost there, my allmodconfig from last night almost built fine
but for something I missed in bgmac.c, and 0-day found a MIPS file
affected by the change.  New code pushed out for 0-day and I'm
expecting tonights allmodconfig to be clear.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 0/7] Clean up PHY MMD accessors

[Russell King - ARM Linux]

This series cleans up phylib's MMD accessors, so that we have a common
way of accessing the Clause 45 register set.

The current situation is far from ideal - we have phy_(read|write)_mmd()
which accesses Clause 45 registers over Clause 45 accesses, and we have
phy_(read|write)_mmd_indirect(), which accesses Clause 45 registers via
Clause 22 register 13/14.

Generic code uses the indirect methods to access standard Clause 45
features, and when we come to add Clause 45 PHY support to phylib, we
would need to make these conditional upon the PHY type, or duplicate
these functions.

An alternative solution is to merge these accessors together, and select
the appropriate access method depending upon the 802.3 clause that the
PHY conforms with.  The result is that we have a single set of
phy_(read|write)_mmd() accessors.

For cases which require special handling, we still allow PHY drivers to
override all MMD accesses - except rather than just overriding the
indirect accesses.  This keeps existing overrides working.

Combining the two also has another beneficial side effect - we get rid
of similar functions that take arguments in different orders.  The
old direct accessors took the phy structure, devad and register number,
whereas the indirect accessors took the phy structure, register number
and devad in that order.  Care must be taken when updating future
drivers that the argument order is correct, and the function name is
not merely replaced.

This patch set is against 4.10-rc3 at present.

 drivers/net/phy/Makefile  |   3 +-
 drivers/net/phy/bcm-phy-lib.c |  12 ++---
 drivers/net/phy/dp83867.c |  18 +++
 drivers/net/phy/intel-xway.c  |  26 +-
 drivers/net/phy/micrel.c  |  13 +++--
 drivers/net/phy/microchip.c   |   5 +-
 drivers/net/phy/phy-core.c| 101 ++
 drivers/net/phy/phy.c | 110 --
 drivers/net/phy/phy_device.c  |   4 +-
 drivers/net/usb/lan78xx.c |  10 ++--
 include/linux/phy.h   |  56 +
 11 files changed, 176 insertions(+), 182 deletions(-)
 create mode 100644 drivers/net/phy/phy-core.c

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 5/7] net: phy: convert micrel to new read_mmd/write_mmd driver methods

[Russell King]

Convert micrel to the new read_mmd/write_mmd driver methods.  This
Clause 22 PHY does not support any MMD access method.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/phy/micrel.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 9a77289109b7..8d6432c23a14 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -637,8 +637,7 @@ static int ksz8873mll_config_aneg(struct phy_device *phydev)
  * MMD extended PHY registers.
  */
 static int
-ksz9021_rd_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
- int regnum)
+ksz9021_rd_mmd_phyreg(struct phy_device *phydev, int devad, u16 regnum)
 {
return -1;
 }
@@ -646,10 +645,10 @@ ksz9021_rd_mmd_phyreg(struct phy_device *phydev, int 
ptrad, int devnum,
 /* This routine does nothing since the Micrel ksz9021 does not support
  * standard IEEE MMD extended PHY registers.
  */
-static void
-ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
- int regnum, u32 val)
+static int
+ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int devad, u16 regnum, u16 
val)
 {
+   return -1;
 }
 
 static int kszphy_get_sset_count(struct phy_device *phydev)
@@ -962,8 +961,8 @@ static struct phy_driver ksphy_driver[] = {
.get_stats  = kszphy_get_stats,
.suspend= genphy_suspend,
.resume = genphy_resume,
-   .read_mmd_indirect = ksz9021_rd_mmd_phyreg,
-   .write_mmd_indirect = ksz9021_wr_mmd_phyreg,
+   .read_mmd   = ksz9021_rd_mmd_phyreg,
+   .write_mmd  = ksz9021_wr_mmd_phyreg,
 }, {
.phy_id = PHY_ID_KSZ9031,
.phy_id_mask= MICREL_PHY_ID_MASK,
-- 
2.7.4

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

[PATCH RFC 6/7] net: phy: remove the indirect MMD read/write methods

[Russell King]

Remove the indirect MMD read/write methods which are now no longer
necessary.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/phy/phy-core.c | 119 +
 include/linux/phy.h|  18 ---
 2 files changed, 35 insertions(+), 102 deletions(-)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index b50b3a64cf6a..80795ccd3fab 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -23,102 +23,41 @@ static inline void mmd_phy_indirect(struct mii_bus *bus, 
int prtad, int devad,
 }
 
 /**
- * phy_read_mmd_indirect - reads data from the MMD registers
- * @phydev: The PHY device bus
- * @prtad: MMD Address
- * @devad: MMD DEVAD
- *
- * Description: it reads data from the MMD registers (clause 22 to access to
- * clause 45) of the specified phy address.
- * To read these register we have:
- * 1) Write reg 13 // DEVAD
- * 2) Write reg 14 // MMD Address
- * 3) Write reg 13 // MMD Data Command for MMD DEVAD
- * 3) Read  reg 14 // Read MMD data
- */
-int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
-{
-   struct phy_driver *phydrv = phydev->drv;
-   int addr = phydev->mdio.addr;
-   int value = -1;
-
-   if (!phydrv->read_mmd_indirect) {
-   struct mii_bus *bus = phydev->mdio.bus;
-
-   mutex_lock(>mdio_lock);
-   mmd_phy_indirect(bus, prtad, devad, addr);
-
-   /* Read the content of the MMD's selected register */
-   value = bus->read(bus, addr, MII_MMD_DATA);
-   mutex_unlock(>mdio_lock);
-   } else {
-   value = phydrv->read_mmd_indirect(phydev, prtad, devad, addr);
-   }
-   return value;
-}
-EXPORT_SYMBOL(phy_read_mmd_indirect);
-
-/**
  * phy_read_mmd - Convenience function for reading a register
  * from an MMD on a given PHY.
  * @phydev: The phy_device struct
- * @devad: The MMD to read from
- * @regnum: The register on the MMD to read
+ * @devad: The MMD to read from (0..31)
+ * @regnum: The register on the MMD to read (0..65535)
  *
  * Same rules as for phy_read();
  */
 int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
 {
+   int val;
+
if (regnum > (u16)~0 || devad > 32)
return -EINVAL;
 
-   if (phydev->drv->read_mmd)
-   return phydev->drv->read_mmd(phydev, devad, regnum);
-
-   if (phydev->is_c45) {
+   if (phydev->drv->read_mmd) {
+   val = phydev->drv->read_mmd(phydev, devad, regnum);
+   } else if (phydev->is_c45) {
u32 addr = MII_ADDR_C45 | (devad << 16) | (regnum & 0x);
-   return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, addr);
-   }
-
-   return phy_read_mmd_indirect(phydev, regnum, devad);
-}
-EXPORT_SYMBOL(phy_read_mmd);
-
-/**
- * phy_write_mmd_indirect - writes data to the MMD registers
- * @phydev: The PHY device
- * @prtad: MMD Address
- * @devad: MMD DEVAD
- * @data: data to write in the MMD register
- *
- * Description: Write data from the MMD registers of the specified
- * phy address.
- * To write these register we have:
- * 1) Write reg 13 // DEVAD
- * 2) Write reg 14 // MMD Address
- * 3) Write reg 13 // MMD Data Command for MMD DEVAD
- * 3) Write reg 14 // Write MMD data
- */
-void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
-  int devad, u32 data)
-{
-   struct phy_driver *phydrv = phydev->drv;
-   int addr = phydev->mdio.addr;
 
-   if (!phydrv->write_mmd_indirect) {
+   val = mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, addr);
+   } else {
struct mii_bus *bus = phydev->mdio.bus;
+   int phy_addr = phydev->mdio.addr;
 
mutex_lock(>mdio_lock);
-   mmd_phy_indirect(bus, prtad, devad, addr);
+   mmd_phy_indirect(bus, regnum, devad, phy_addr);
 
-   /* Write the data into MMD's selected register */
-   bus->write(bus, addr, MII_MMD_DATA, data);
+   /* Read the content of the MMD's selected register */
+   val = bus->read(bus, phy_addr, MII_MMD_DATA);
mutex_unlock(>mdio_lock);
-   } else {
-   phydrv->write_mmd_indirect(phydev, prtad, devad, addr, data);
}
+   return val;
 }
-EXPORT_SYMBOL(phy_write_mmd_indirect);
+EXPORT_SYMBOL(phy_read_mmd);
 
 /**
  * phy_write_mmd - Convenience function for writing a register
@@ -132,19 +71,31 @@ EXPORT_SYMBOL(phy_write_mmd_indirect);
  */
 int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
 {
+   int ret;
+
if (regnum > (u16)~0 || devad > 32)
return -EINVAL;
 
-   if (phydev->drv->read_mmd)
-   return phydev->drv->write

[PATCH RFC 7/7] net: phy: clean up mmd_phy_indirect()

[Russell King]

Make mmd_phy_indirect() use the same terminology as the rest of the
code, making clear what each address is - phy address, devad, and
register number.

While here, remove the "inline" from this static function, leaving
it to the compiler to decide whether to inline this function, and
get rid of unnecessary parens.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/phy/phy-core.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 80795ccd3fab..357a4d0d7641 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -9,17 +9,17 @@
 #include 
 #include 
 
-static inline void mmd_phy_indirect(struct mii_bus *bus, int prtad, int devad,
-   int addr)
+static void mmd_phy_indirect(struct mii_bus *bus, int phy_addr, int devad,
+u16 regnum)
 {
/* Write the desired MMD Devad */
-   bus->write(bus, addr, MII_MMD_CTRL, devad);
+   bus->write(bus, phy_addr, MII_MMD_CTRL, devad);
 
/* Write the desired MMD register address */
-   bus->write(bus, addr, MII_MMD_DATA, prtad);
+   bus->write(bus, phy_addr, MII_MMD_DATA, regnum);
 
/* Select the Function : DATA with no post increment */
-   bus->write(bus, addr, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
+   bus->write(bus, phy_addr, MII_MMD_CTRL, devad | MII_MMD_CTRL_NOINCR);
 }
 
 /**
@@ -49,7 +49,7 @@ int phy_read_mmd(struct phy_device *phydev, int devad, u32 
regnum)
int phy_addr = phydev->mdio.addr;
 
mutex_lock(>mdio_lock);
-   mmd_phy_indirect(bus, regnum, devad, phy_addr);
+   mmd_phy_indirect(bus, phy_addr, devad, regnum);
 
/* Read the content of the MMD's selected register */
val = bus->read(bus, phy_addr, MII_MMD_DATA);
@@ -88,7 +88,7 @@ int phy_write_mmd(struct phy_device *phydev, int devad, u32 
regnum, u16 val)
int phy_addr = phydev->mdio.addr;
 
mutex_lock(>mdio_lock);
-   mmd_phy_indirect(bus, regnum, devad, phy_addr);
+   mmd_phy_indirect(bus, phy_addr, devad, regnum);
 
/* Write the data into MMD's selected register */
bus->write(bus, phy_addr, MII_MMD_DATA, val);
-- 
2.7.4

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

[PATCH RFC 4/7] net: phy: switch remaining users to phy_(read|write)_mmd()

[Russell King]

Switch everyone over to using phy_read_mmd() and phy_write_mmd() now
that they are able to handle both Clause 22 indirect addressing and
Clause 45 direct addressing methods to the MMD registers.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/phy/bcm-phy-lib.c | 12 
 drivers/net/phy/dp83867.c | 18 --
 drivers/net/phy/intel-xway.c  | 26 +-
 drivers/net/phy/microchip.c   |  5 ++---
 drivers/net/phy/phy.c | 25 ++---
 drivers/net/phy/phy_device.c  |  4 ++--
 6 files changed, 39 insertions(+), 51 deletions(-)

diff --git a/drivers/net/phy/bcm-phy-lib.c b/drivers/net/phy/bcm-phy-lib.c
index ab9ad689617c..90be6ee42dfa 100644
--- a/drivers/net/phy/bcm-phy-lib.c
+++ b/drivers/net/phy/bcm-phy-lib.c
@@ -201,8 +201,7 @@ int bcm_phy_set_eee(struct phy_device *phydev, bool enable)
int val;
 
/* Enable EEE at PHY level */
-   val = phy_read_mmd_indirect(phydev, BRCM_CL45VEN_EEE_CONTROL,
-   MDIO_MMD_AN);
+   val = phy_read_mmd(phydev, MDIO_MMD_AN, BRCM_CL45VEN_EEE_CONTROL);
if (val < 0)
return val;
 
@@ -211,12 +210,10 @@ int bcm_phy_set_eee(struct phy_device *phydev, bool 
enable)
else
val &= ~(LPI_FEATURE_EN | LPI_FEATURE_EN_DIG1000X);
 
-   phy_write_mmd_indirect(phydev, BRCM_CL45VEN_EEE_CONTROL,
-  MDIO_MMD_AN, (u32)val);
+   phy_write_mmd(phydev, MDIO_MMD_AN, BRCM_CL45VEN_EEE_CONTROL, (u32)val);
 
/* Advertise EEE */
-   val = phy_read_mmd_indirect(phydev, BCM_CL45VEN_EEE_ADV,
-   MDIO_MMD_AN);
+   val = phy_read_mmd(phydev, MDIO_MMD_AN, BCM_CL45VEN_EEE_ADV);
if (val < 0)
return val;
 
@@ -225,8 +222,7 @@ int bcm_phy_set_eee(struct phy_device *phydev, bool enable)
else
val &= ~(MDIO_AN_EEE_ADV_100TX | MDIO_AN_EEE_ADV_1000T);
 
-   phy_write_mmd_indirect(phydev, BCM_CL45VEN_EEE_ADV,
-  MDIO_MMD_AN, (u32)val);
+   phy_write_mmd(phydev, MDIO_MMD_AN, BCM_CL45VEN_EEE_ADV, (u32)val);
 
return 0;
 }
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 1b639242f9e2..43a5a22234f1 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -180,8 +180,7 @@ static int dp83867_config_init(struct phy_device *phydev)
 
if ((phydev->interface >= PHY_INTERFACE_MODE_RGMII_ID) &&
(phydev->interface <= PHY_INTERFACE_MODE_RGMII_RXID)) {
-   val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
-   DP83867_DEVADDR);
+   val = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIICTL);
 
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
val |= (DP83867_RGMII_TX_CLK_DELAY_EN | 
DP83867_RGMII_RX_CLK_DELAY_EN);
@@ -192,25 +191,24 @@ static int dp83867_config_init(struct phy_device *phydev)
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
val |= DP83867_RGMII_RX_CLK_DELAY_EN;
 
-   phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
-  DP83867_DEVADDR, val);
+   phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIICTL, val);
 
delay = (dp83867->rx_id_delay |
(dp83867->tx_id_delay << 
DP83867_RGMII_TX_CLK_DELAY_SHIFT));
 
-   phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
-  DP83867_DEVADDR, delay);
+   phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIIDCTL,
+ delay);
 
if (dp83867->io_impedance >= 0) {
-   val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
-   DP83867_DEVADDR);
+   val = phy_read_mmd(phydev, DP83867_DEVADDR,
+  DP83867_IO_MUX_CFG);
 
val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
val |= dp83867->io_impedance &
   DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
 
-   phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
-  DP83867_DEVADDR, val);
+   phy_write_mmd(phydev, DP83867_DEVADDR,
+ DP83867_IO_MUX_CFG, val);
}
}
 
diff --git a/drivers/net/phy/intel-xway.c b/drivers/net/phy/intel-xway.c
index b1fd7bb0e4db..55f8c52dd2f1 100644
--- a/drivers/net/phy/intel-xway.c
+++ b/drivers/net/phy/intel-xway.c
@@ -166,13 +166,13 @@ static int xway_gphy_config_init(struct phy_device 
*phydev)
/* Clear all pending interrupts */
   

[PATCH RFC 1/7] net: phy: move phy MMD accessors to phy-core.c

[Russell King]

Move the phy_(read|write)__mmd() helpers out of line, they will become
our main MMD accessor functions, and so will be a little more complex.
This complexity doesn't belong in an inline function.  Also move the
_indirect variants as well to keep like functionality together.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/phy/Makefile   |   3 +-
 drivers/net/phy/phy-core.c | 135 +
 drivers/net/phy/phy.c  |  85 
 include/linux/phy.h|  20 +--
 4 files changed, 139 insertions(+), 104 deletions(-)
 create mode 100644 drivers/net/phy/phy-core.c

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 356859ac7c18..06fa2e04ac7e 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -1,6 +1,7 @@
 # Makefile for Linux PHY drivers and MDIO bus drivers
 
-libphy-y   := phy.o phy_device.o mdio_bus.o mdio_device.o
+libphy-y   := phy.o phy_device.o mdio_bus.o mdio_device.o \
+  phy-core.o
 libphy-$(CONFIG_SWPHY) += swphy.o
 libphy-$(CONFIG_LED_TRIGGER_PHY)   += phy_led_triggers.o
 
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
new file mode 100644
index ..b8d8276a3099
--- /dev/null
+++ b/drivers/net/phy/phy-core.c
@@ -0,0 +1,135 @@
+/*
+ * Core PHY library, taken from phy.c
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#include 
+#include 
+
+static inline void mmd_phy_indirect(struct mii_bus *bus, int prtad, int devad,
+   int addr)
+{
+   /* Write the desired MMD Devad */
+   bus->write(bus, addr, MII_MMD_CTRL, devad);
+
+   /* Write the desired MMD register address */
+   bus->write(bus, addr, MII_MMD_DATA, prtad);
+
+   /* Select the Function : DATA with no post increment */
+   bus->write(bus, addr, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
+}
+
+/**
+ * phy_read_mmd_indirect - reads data from the MMD registers
+ * @phydev: The PHY device bus
+ * @prtad: MMD Address
+ * @devad: MMD DEVAD
+ *
+ * Description: it reads data from the MMD registers (clause 22 to access to
+ * clause 45) of the specified phy address.
+ * To read these register we have:
+ * 1) Write reg 13 // DEVAD
+ * 2) Write reg 14 // MMD Address
+ * 3) Write reg 13 // MMD Data Command for MMD DEVAD
+ * 3) Read  reg 14 // Read MMD data
+ */
+int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
+{
+   struct phy_driver *phydrv = phydev->drv;
+   int addr = phydev->mdio.addr;
+   int value = -1;
+
+   if (!phydrv->read_mmd_indirect) {
+   struct mii_bus *bus = phydev->mdio.bus;
+
+   mutex_lock(>mdio_lock);
+   mmd_phy_indirect(bus, prtad, devad, addr);
+
+   /* Read the content of the MMD's selected register */
+   value = bus->read(bus, addr, MII_MMD_DATA);
+   mutex_unlock(>mdio_lock);
+   } else {
+   value = phydrv->read_mmd_indirect(phydev, prtad, devad, addr);
+   }
+   return value;
+}
+EXPORT_SYMBOL(phy_read_mmd_indirect);
+
+/**
+ * phy_read_mmd - Convenience function for reading a register
+ * from an MMD on a given PHY.
+ * @phydev: The phy_device struct
+ * @devad: The MMD to read from
+ * @regnum: The register on the MMD to read
+ *
+ * Same rules as for phy_read();
+ */
+int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
+{
+   if (!phydev->is_c45)
+   return -EOPNOTSUPP;
+
+   return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr,
+   MII_ADDR_C45 | (devad << 16) | (regnum & 0x));
+}
+EXPORT_SYMBOL(phy_read_mmd);
+
+/**
+ * phy_write_mmd_indirect - writes data to the MMD registers
+ * @phydev: The PHY device
+ * @prtad: MMD Address
+ * @devad: MMD DEVAD
+ * @data: data to write in the MMD register
+ *
+ * Description: Write data from the MMD registers of the specified
+ * phy address.
+ * To write these register we have:
+ * 1) Write reg 13 // DEVAD
+ * 2) Write reg 14 // MMD Address
+ * 3) Write reg 13 // MMD Data Command for MMD DEVAD
+ * 3) Write reg 14 // Write MMD data
+ */
+void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
+  int devad, u32 data)
+{
+   struct phy_driver *phydrv = phydev->drv;
+   int addr = phydev->mdio.addr;
+
+   if (!phydrv->write_mmd_indirect) {
+   struct mii_bus *bus = phydev->mdio.bus;
+
+   mutex_lock(>mdio_lock);
+   mmd_phy_indirect(bus, prtad, devad, addr);
+
+   /* Write the 

[PATCH RFC 3/7] net: lan78xx: update for phy_(read|write)_mmd_indirect() removal

[Russell King]

lan78xx appears to use phylib in a rather weird way, accessing the PHY
partly through phylib, and partly by makign direct accesses to it,
including to the Clause 45 registers.  As the indirect MMD accessors are
going away, update this driver to use the plain phy_(read|write)_mmd()
accessors instead.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/usb/lan78xx.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 08f8703e4d54..5e222d47f212 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1951,10 +1951,10 @@ static int lan8835_fixup(struct phy_device *phydev)
struct lan78xx_net *dev = netdev_priv(phydev->attached_dev);
 
/* LED2/PME_N/IRQ_N/RGMII_ID pin to IRQ_N mode */
-   buf = phy_read_mmd_indirect(phydev, 0x8010, 3);
+   buf = phy_read_mmd(phydev, MDIO_MMD_PCS, 0x8010);
buf &= ~0x1800;
buf |= 0x0800;
-   phy_write_mmd_indirect(phydev, 0x8010, 3, buf);
+   phy_write_mmd(phydev, MDIO_MMD_PCS, 0x8010, buf);
 
/* RGMII MAC TXC Delay Enable */
ret = lan78xx_write_reg(dev, MAC_RGMII_ID,
@@ -1974,11 +1974,11 @@ static int ksz9031rnx_fixup(struct phy_device *phydev)
 
/* Micrel9301RNX PHY configuration */
/* RGMII Control Signal Pad Skew */
-   phy_write_mmd_indirect(phydev, 4, 2, 0x0077);
+   phy_write_mmd(phydev, MDIO_MMD_WIS, 4, 0x0077);
/* RGMII RX Data Pad Skew */
-   phy_write_mmd_indirect(phydev, 5, 2, 0x);
+   phy_write_mmd(phydev, MDIO_MMD_WIS, 5, 0x);
/* RGMII RX Clock Pad Skew */
-   phy_write_mmd_indirect(phydev, 8, 2, 0x1FF);
+   phy_write_mmd(phydev, MDIO_MMD_WIS, 8, 0x1FF);
 
dev->interface = PHY_INTERFACE_MODE_RGMII_RXID;
 
-- 
2.7.4

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

[PATCH RFC 2/7] net: phy: make phy_(read|write)_mmd() generic MMD accessors

[Russell King]

Make phy_(read|write)_mmd() generic 802.3 clause 45 register accessors
for both Clause 22 and Clause 45 PHYs, using either the direct register
reading for Clause 45, or the indirect method for Clause 22 PHYs.
Allow this behaviour to be overriden by PHY drivers where necessary.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/net/phy/phy-core.c | 31 +++
 include/linux/phy.h| 24 
 2 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index b8d8276a3099..b50b3a64cf6a 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -69,11 +69,18 @@ EXPORT_SYMBOL(phy_read_mmd_indirect);
  */
 int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
 {
-   if (!phydev->is_c45)
-   return -EOPNOTSUPP;
+   if (regnum > (u16)~0 || devad > 32)
+   return -EINVAL;
 
-   return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr,
-   MII_ADDR_C45 | (devad << 16) | (regnum & 0x));
+   if (phydev->drv->read_mmd)
+   return phydev->drv->read_mmd(phydev, devad, regnum);
+
+   if (phydev->is_c45) {
+   u32 addr = MII_ADDR_C45 | (devad << 16) | (regnum & 0x);
+   return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, addr);
+   }
+
+   return phy_read_mmd_indirect(phydev, regnum, devad);
 }
 EXPORT_SYMBOL(phy_read_mmd);
 
@@ -125,11 +132,19 @@ EXPORT_SYMBOL(phy_write_mmd_indirect);
  */
 int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
 {
-   if (!phydev->is_c45)
-   return -EOPNOTSUPP;
+   if (regnum > (u16)~0 || devad > 32)
+   return -EINVAL;
 
-   regnum = MII_ADDR_C45 | ((devad & 0x1f) << 16) | (regnum & 0x);
+   if (phydev->drv->read_mmd)
+   return phydev->drv->write_mmd(phydev, devad, regnum, val);
+
+   if (phydev->is_c45) {
+   u32 addr = MII_ADDR_C45 | (devad << 16) | (regnum & 0x);
+
+   return mdiobus_write(phydev->mdio.bus, phydev->mdio.addr,
+addr, val);
+   }
 
-   return mdiobus_write(phydev->mdio.bus, phydev->mdio.addr, regnum, val);
+   return phy_write_mmd_indirect(phydev, regnum, devad, val);
 }
 EXPORT_SYMBOL(phy_write_mmd);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 72acb0233b5f..54fa76efb096 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -583,6 +583,30 @@ struct phy_driver {
 */
void (*link_change_notify)(struct phy_device *dev);
 
+   /*
+* Phy specific driver override for reading a MMD register.
+* This function is optional for PHY specific drivers.  When
+* not provided, the default MMD read function will be used
+* by phy_read_mmd(), which will use either a direct read for
+* Clause 45 PHYs or an indirect read for Clause 22 PHYs.
+*  devnum is the MMD device number within the PHY device,
+*  regnum is the register within the selected MMD device.
+*/
+   int (*read_mmd)(struct phy_device *dev, int devnum, u16 regnum);
+
+   /*
+* Phy specific driver override for writing a MMD register.
+* This function is optional for PHY specific drivers.  When
+* not provided, the default MMD write function will be used
+* by phy_write_mmd(), which will use either a direct write for
+* Clause 45 PHYs, or an indirect write for Clause 22 PHYs.
+*  devnum is the MMD device number within the PHY device,
+*  regnum is the register within the selected MMD device.
+*  val is the value to be written.
+*/
+   int (*write_mmd)(struct phy_device *dev, int devnum, u16 regnum,
+u16 val);
+
/* A function provided by a phy specific driver to override the
 * the PHY driver framework support for reading a MMD register
 * from the PHY. If not supported, return -1. This function is
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] ARM: dma: fix dma_max_pfn()

[Russell King - ARM Linux]

On Wed, Sep 28, 2016 at 10:53:49AM +0300, Roger Quadros wrote:
> Hi,
> 
> On 12/09/16 14:38, Roger Quadros wrote:
> > Hi Santosh & Russell,
> > 
> > On 19/08/16 19:38, Santosh Shilimkar wrote:
> >>
> >> On 8/19/2016 12:30 AM, Roger Quadros wrote:
> >>> Hi Santosh,
> >>>
> >>
> > So I'm 99.9% convinced that the proposed change is correct.
> >
>  I will got with that then :-) and take my objection back. Just
>  saying that if there other breakages which I can't recollect now,
>  those drivers needs to be patched as well.
> 
> >>> I was able to boot the Keystone2 Edison EVM over NFS with the $subject 
> >>> patch.
> >>> Boot log is below. Do you see anything suspicious?
> >>>
> >> Logs looks ok to me. Probably do some tests where DMA and bounce buffers 
> >> etc gets tested. Running it through your internal regression
> >> suit will be good idea as well if thats possible.
> >>
> > 
> > This has been running in our internal test suite for a week on various TI
> > platforms. There haven't been any surprises.
> > 
> > Is it a good idea to at least put this in -next for a wider test audience?
> 
> Gentle reminder.

Please put it in the patch system, thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev

[Russell King - ARM Linux]

On Wed, Sep 07, 2016 at 05:29:01PM +0800, Peter Chen wrote:
> On Wed, Sep 07, 2016 at 10:52:46AM +0200, Arnd Bergmann wrote:
> > On Wednesday, September 7, 2016 3:44:28 PM CEST Peter Chen wrote:
> > > 
> > > The pre-condition of DT function at USB HCD core works is the host
> > > controller device has of_node, since it is the root node for USB tree
> > > described at DT. If the host controller device is not at DT, it needs
> > > to try to get its of_node, the chipidea driver gets it through its
> > > parent node [1]
> > 
> > > 
> > > [1] https://lkml.org/lkml/2016/8/8/119
> > > 
> > 
> > Ah, this is what I was referring to in the other mail.
> > 
> > However, the way you set the of_node might be dangerous too:
> > We should generally not have two platform_device structures with
> > the same of_node pointer, most importantly it may cause the
> > child device to be bound to the same driver as the parent
> > device since the probing is done by compatible string.
> > 
> > As you tested it successfully, it must work at the moment on your
> > machine, but it could easily break depending on deferred probing
> > or module load order.
> > 
> 
> Currently, I work around above problems by setting core device of_node
> as NULL at both probe error path and platform driver .remove routine.
> 
> I admit it is not a good way, but if we only have of_node at device's
> life periods after probe, it seems ok currently. It is hard to create
> of_node dynamically when create device, and keep some contents
> of parent's of_node, and some are not.

How about turning dwc3 into a library which can be used by a range of
platform devices?  Wouldn't that solve all the current problems, and
completely avoid the need to copy resources from one platform device
to another?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev

[Russell King - ARM Linux]

On Fri, Sep 02, 2016 at 12:43:39PM +0200, Arnd Bergmann wrote:
> On Thursday, September 1, 2016 5:14:28 PM CEST Leo Li wrote:
> > 
> > Hi Felipe and Arnd,
> > 
> > It has been a while since the last response to this discussion, but we
> > haven't reached an agreement yet!  Can we get to a conclusion on if it
> > is valid to create child platform device for abstraction purpose?  If
> > yes, can this child device do DMA by itself?
> 
> I'd say it's no problem for a driver to create child devices in order
> to represent different aspects of a device, but you should not rely on
> those devices working when used with the dma-mapping interfaces.

That's absolutely right.  Consider the USB model - only the USB host
controller can perform DMA, not the USB devices themselves.  All DMA
mappings need to be mapped using the USB host controller device struct
not the USB device struct.

The same _should_ be true everywhere else: the struct device representing
the device performing DMA must be the one used to map the transfer.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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/2] usb: ohci-sa1111: remove machine_has_neponset()

[Russell King - ARM Linux]

On Fri, Aug 26, 2016 at 02:20:50PM -0400, Alan Stern wrote:
> On Fri, 26 Aug 2016, Russell King - ARM Linux wrote:
> 
> > On Fri, Aug 26, 2016 at 01:18:25PM -0400, Alan Stern wrote:
> > > On Fri, 26 Aug 2016, Russell King wrote:
> > > 
> > > > The neponset is a daughter board for the Assabet platform, which has a
> > > > SA chip on it.  If we're initialising the SA OHCI, and we're
> > > > part of a neponset, the host platform must be an Assabet.
> > > > 
> > > > This allows us to eliminate machine_has_neponset() from this driver,
> > > > replacing it instead with machine_is_assabet(), and killing the
> > > > mach/assabet.h include.
> > > 
> > > Silly question: What happens when there's an SA OHCI controller on
> > > an Assabet platform, but contained in something other than a neponset
> > > daughterboard?
> > 
> > It's possible that there is such a thing, but unlikely - and we have
> > no code in mainline to support such a configuration.
> 
> In that case you can add:
> 
> Acked-by: Alan Stern <st...@rowland.harvard.edu>
> 
> to this and the 2/2 patch.

Thanks!

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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/2] usb: ohci-sa1111: remove machine_has_neponset()

[Russell King - ARM Linux]

On Fri, Aug 26, 2016 at 01:18:25PM -0400, Alan Stern wrote:
> On Fri, 26 Aug 2016, Russell King wrote:
> 
> > The neponset is a daughter board for the Assabet platform, which has a
> > SA chip on it.  If we're initialising the SA OHCI, and we're
> > part of a neponset, the host platform must be an Assabet.
> > 
> > This allows us to eliminate machine_has_neponset() from this driver,
> > replacing it instead with machine_is_assabet(), and killing the
> > mach/assabet.h include.
> 
> Silly question: What happens when there's an SA OHCI controller on
> an Assabet platform, but contained in something other than a neponset
> daughterboard?

It's possible that there is such a thing, but unlikely - and we have
no code in mainline to support such a configuration.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] USB: ohci-omap - avoid including mach/irqs.h

[Russell King - ARM Linux]

On Fri, Aug 26, 2016 at 01:20:53PM -0400, Alan Stern wrote:
> On Fri, 26 Aug 2016, Russell King - ARM Linux wrote:
> 
> > On Mon, Aug 22, 2016 at 11:28:21AM -0400, Alan Stern wrote:
> > > On Fri, 19 Aug 2016, Russell King wrote:
> > > 
> > > > ohci-omap doesn't need to include mach/irqs.h - nothing within this
> > > > driver needs anything from this header file.  Remove this include.
> > > > 
> > > > Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
> > > > ---
> > > >  drivers/usb/host/ohci-omap.c | 1 -
> > > >  1 file changed, 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/usb/host/ohci-omap.c b/drivers/usb/host/ohci-omap.c
> > > > index de7c68602a7e..495c1454b9e8 100644
> > > > --- a/drivers/usb/host/ohci-omap.c
> > > > +++ b/drivers/usb/host/ohci-omap.c
> > > > @@ -36,7 +36,6 @@
> > > >  #include 
> > > >  
> > > >  #include 
> > > > -#include 
> > > >  #include 
> > > 
> > > Acked-by: Alan Stern <st...@rowland.harvard.edu>
> > 
> > Thanks.  Will Greg be picking this up?
> 
> Yes, he should.  Unless you want it to go via a different tree...

I'm fine with Greg picking this (and the other two) up as none
of my (increasing) patch stack depends on these patches.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] USB: ohci-omap - avoid including mach/irqs.h

[Russell King - ARM Linux]

On Mon, Aug 22, 2016 at 11:28:21AM -0400, Alan Stern wrote:
> On Fri, 19 Aug 2016, Russell King wrote:
> 
> > ohci-omap doesn't need to include mach/irqs.h - nothing within this
> > driver needs anything from this header file.  Remove this include.
> > 
> > Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
> > ---
> >  drivers/usb/host/ohci-omap.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/usb/host/ohci-omap.c b/drivers/usb/host/ohci-omap.c
> > index de7c68602a7e..495c1454b9e8 100644
> > --- a/drivers/usb/host/ohci-omap.c
> > +++ b/drivers/usb/host/ohci-omap.c
> > @@ -36,7 +36,6 @@
> >  #include 
> >  
> >  #include 
> > -#include 
> >  #include 
> 
> Acked-by: Alan Stern <st...@rowland.harvard.edu>

Thanks.  Will Greg be picking this up?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] usb: ohci-sa1111: remove mach/hardware.h include

[Russell King]

The mach/hardware.h include doesn't seem to be necessary to build
ohci-sa, so let's remove it to kill off an unnecessary platform
specific include.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/usb/host/ohci-sa.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/ohci-sa.c b/drivers/usb/host/ohci-sa.c
index 6dc5eaed8551..3a9ea32508df 100644
--- a/drivers/usb/host/ohci-sa.c
+++ b/drivers/usb/host/ohci-sa.c
@@ -13,7 +13,6 @@
  * This file is licenced under the GPL.
  */
 
-#include 
 #include 
 #include 
 
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] usb: ohci-sa1111: remove machine_has_neponset()

[Russell King]

The neponset is a daughter board for the Assabet platform, which has a
SA chip on it.  If we're initialising the SA OHCI, and we're
part of a neponset, the host platform must be an Assabet.

This allows us to eliminate machine_has_neponset() from this driver,
replacing it instead with machine_is_assabet(), and killing the
mach/assabet.h include.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/usb/host/ohci-sa.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/ohci-sa.c b/drivers/usb/host/ohci-sa.c
index 2ac266d692a2..6dc5eaed8551 100644
--- a/drivers/usb/host/ohci-sa.c
+++ b/drivers/usb/host/ohci-sa.c
@@ -15,7 +15,6 @@
 
 #include 
 #include 
-#include 
 #include 
 
 #ifndef CONFIG_SA
@@ -127,7 +126,7 @@ static int sa_start_hc(struct sa_dev *dev)
dev_dbg(>dev, "starting SA- OHCI USB Controller\n");
 
if (machine_is_xp860() ||
-   machine_has_neponset() ||
+   machine_is_assabet() ||
machine_is_pfs168() ||
machine_is_badge4())
usb_rst = USB_RESET_PWRSENSELOW | USB_RESET_PWRCTRLLOW;
-- 
2.1.0

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

[PATCH] USB: ohci-omap - avoid including mach/irqs.h

[Russell King]

ohci-omap doesn't need to include mach/irqs.h - nothing within this
driver needs anything from this header file.  Remove this include.

Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
 drivers/usb/host/ohci-omap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/ohci-omap.c b/drivers/usb/host/ohci-omap.c
index de7c68602a7e..495c1454b9e8 100644
--- a/drivers/usb/host/ohci-omap.c
+++ b/drivers/usb/host/ohci-omap.c
@@ -36,7 +36,6 @@
 #include 
 
 #include 
-#include 
 #include 
 
 
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] ARM: dma: fix dma_max_pfn()

[Russell King - ARM Linux]

On Thu, Aug 18, 2016 at 09:55:55AM -0700, Santosh Shilimkar wrote:
> Hi Russell,
> 
> On 8/18/2016 7:24 AM, Russell King - ARM Linux wrote:
> >On Wed, Aug 17, 2016 at 03:05:17PM +0300, Roger Quadros wrote:
> >>Since commit 6ce0d2001692 ("ARM: dma: Use dma_pfn_offset for dma address 
> >>translation"),
> >>dma_to_pfn() already returns the PFN with the physical memory start offset
> >>so we don't need to add it again.
> >>
> >>This fixes USB mass storage lock-up problem on systems that can't do DMA
> >>over the entire physical memory range (e.g.) Keystone 2 systems with 4GB RAM
> >>can only do DMA over the first 2GB. [K2E-EVM].
> >>
> >>What happens there is that without this patch SCSI layer sets a wrong
> >>bounce buffer limit in scsi_calculate_bounce_limit() for the USB mass
> >>storage device. dma_max_pfn() evaluates to 0x8f and bounce_limit
> >>is set to 0x8f000 whereas maximum DMA'ble physical memory on Keystone 2
> >>is 0x87fff. This results in non DMA'ble pages being given to the
> >>USB controller and hence the lock-up.
> >>
> >>NOTE: in the above case, USB-SCSI-device's dma_pfn_offset was showing as 0.
> >>This should have really been 0x78 as on K2e, LOWMEM_START is 0x8000
> >>and HIGHMEM_START is 0x8. DMA zone is 2GB so dma_max_pfn should be
> >>0x87. The incorrect dma_pfn_offset for the USB storage device is because
> >>USB devices are not correctly inheriting the dma_pfn_offset from the
> >>USB host controller. This will be fixed by a separate patch.
> >
> >I'd like to hear from Santosh, as the author of the original change.
> >The original commit doesn't mention which platform it was intended for
> >or what the problem was, which would've been helpful.
> >
> From what I recollect, we did these changes to make the max pfn behave
> same on ARM arch as other archs. This patch was evolved as part of
> fixing the max*pfn assumption.

To me, the proposed patch _looks_ correct, because...

> >>diff --git a/arch/arm/include/asm/dma-mapping.h 
> >>b/arch/arm/include/asm/dma-mapping.h
> >>index d009f79..bf02dbd 100644
> >>--- a/arch/arm/include/asm/dma-mapping.h
> >>+++ b/arch/arm/include/asm/dma-mapping.h
> >>@@ -111,7 +111,7 @@ static inline dma_addr_t virt_to_dma(struct device 
> >>*dev, void *addr)
> >> /* The ARM override for dma_max_pfn() */
> >> static inline unsigned long dma_max_pfn(struct device *dev)
> >> {
> >>-   return PHYS_PFN_OFFSET + dma_to_pfn(dev, *dev->dma_mask);
> >>+   return dma_to_pfn(dev, *dev->dma_mask);
> >> }
> >> #define dma_max_pfn(dev) dma_max_pfn(dev)
> By doing this change I hope we don't break other drivers on Keystone so
> am not sure about the change.

dma_to_pfn() returns the page frame number referenced from physical
address zero - the default implementation of dma_to_pfn() is
bus_to_pfn(), which is __phys_to_pfn(x), which is just x >> PAGE_SHIFT.
The other thing about dma_to_pfn() is that it should return a
zero-referenced PFN number, where PFN 0 = physical address 0.

If there is some offset for keystone2, that should be taken care of
via "dev->dma_pfn_offset", and not offsetting the return value from
dma_to_pfn().

So I'm 99.9% convinced that the proposed change is correct.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] ARM: dma: fix dma_max_pfn()

[Russell King - ARM Linux]

On Wed, Aug 17, 2016 at 03:05:17PM +0300, Roger Quadros wrote:
> Since commit 6ce0d2001692 ("ARM: dma: Use dma_pfn_offset for dma address 
> translation"),
> dma_to_pfn() already returns the PFN with the physical memory start offset
> so we don't need to add it again.
> 
> This fixes USB mass storage lock-up problem on systems that can't do DMA
> over the entire physical memory range (e.g.) Keystone 2 systems with 4GB RAM
> can only do DMA over the first 2GB. [K2E-EVM].
> 
> What happens there is that without this patch SCSI layer sets a wrong
> bounce buffer limit in scsi_calculate_bounce_limit() for the USB mass
> storage device. dma_max_pfn() evaluates to 0x8f and bounce_limit
> is set to 0x8f000 whereas maximum DMA'ble physical memory on Keystone 2
> is 0x87fff. This results in non DMA'ble pages being given to the
> USB controller and hence the lock-up.
> 
> NOTE: in the above case, USB-SCSI-device's dma_pfn_offset was showing as 0.
> This should have really been 0x78 as on K2e, LOWMEM_START is 0x8000
> and HIGHMEM_START is 0x8. DMA zone is 2GB so dma_max_pfn should be
> 0x87. The incorrect dma_pfn_offset for the USB storage device is because
> USB devices are not correctly inheriting the dma_pfn_offset from the
> USB host controller. This will be fixed by a separate patch.

I'd like to hear from Santosh, as the author of the original change.
The original commit doesn't mention which platform it was intended for
or what the problem was, which would've been helpful.

> 
> Fixes: 6ce0d2001692 ("ARM: dma: Use dma_pfn_offset for dma address 
> translation")
> Cc: sta...@vger.kernel.org
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> Cc: Russell King <li...@arm.linux.org.uk>
> Cc: Arnd Bergmann <a...@arndb.de>
> Cc: Olof Johansson <o...@lixom.net>
> Cc: Catalin Marinas <catalin.mari...@arm.com>
> Cc: Linus Walleij <linus.wall...@linaro.org>
> Reported-by: Grygorii Strashko <grygorii.stras...@ti.com>
> Signed-off-by: Roger Quadros <rog...@ti.com>
> ---
>  arch/arm/include/asm/dma-mapping.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/dma-mapping.h 
> b/arch/arm/include/asm/dma-mapping.h
> index d009f79..bf02dbd 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -111,7 +111,7 @@ static inline dma_addr_t virt_to_dma(struct device *dev, 
> void *addr)
>  /* The ARM override for dma_max_pfn() */
>  static inline unsigned long dma_max_pfn(struct device *dev)
>  {
> - return PHYS_PFN_OFFSET + dma_to_pfn(dev, *dev->dma_mask);
> + return dma_to_pfn(dev, *dev->dma_mask);
>  }
>  #define dma_max_pfn(dev) dma_max_pfn(dev)
>  
> -- 
> 2.7.4
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/6] usb: chipidea: let chipidea core device of_node equal's glue layer device of_node

[Russell King - ARM Linux]

On Thu, Jul 21, 2016 at 05:20:12PM +0800, Peter Chen wrote:
> On Thu, Jul 21, 2016 at 10:14:38AM +0100, Russell King - ARM Linux wrote:
> > On Wed, Jul 20, 2016 at 05:40:28PM +0800, Peter Chen wrote:
> > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > > index 69426e6..0d05812 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -914,6 +914,16 @@ static int ci_hdrc_probe(struct platform_device 
> > > *pdev)
> > >   if (!ci)
> > >   return -ENOMEM;
> > >  
> > > + /*
> > > +  * At device tree, we have no device node for chipidea core,
> > > +  * the glue layer's node is the parent node for host and udc
> > > +  * device. But in related driver, the parent device is chipidea
> > > +  * core. So, in order to let the common driver get parent's node,
> > > +  * we let the core's device node equals glue layer's node.
> > > +  */
> > > + if (dev->parent && dev->parent->of_node)
> > > + dev->of_node = dev->parent->of_node;
> > 
> > This is a dangerous thing to do.  You're changing the dev->of_node of
> > _this_ device, which means that _this_ driver will no longer match
> > the device if you remove and reinsert the driver module, or unbind
> > and try to re-bind the device to this driver.
> > 
> 
> Thanks for commenting it.
> 
> I have tested load/unload, it does not show any problems.
> 
> The chipidea core device is created by code at runtime, not by device node.
> And we have NO device node for this chipidea core device at dts.

Okay, so we still probably have the bind/unbind problem, where "dev"
can be matched by the driver which claimed "dev->parent".  Remember,
in an OF environment, driver matching is done by the compatible
property, which is accessed via dev->of_node.

Therefore, I would suggest that you NULL dev->of_node in the error
cleanup paths and in the remove function, so you don't have an
unbound device with a duplicated (but inappropriate) dev->of_node
pointer.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/6] usb: chipidea: let chipidea core device of_node equal's glue layer device of_node

[Russell King - ARM Linux]

On Wed, Jul 20, 2016 at 05:40:28PM +0800, Peter Chen wrote:
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 69426e6..0d05812 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -914,6 +914,16 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>   if (!ci)
>   return -ENOMEM;
>  
> + /*
> +  * At device tree, we have no device node for chipidea core,
> +  * the glue layer's node is the parent node for host and udc
> +  * device. But in related driver, the parent device is chipidea
> +  * core. So, in order to let the common driver get parent's node,
> +  * we let the core's device node equals glue layer's node.
> +  */
> + if (dev->parent && dev->parent->of_node)
> + dev->of_node = dev->parent->of_node;

This is a dangerous thing to do.  You're changing the dev->of_node of
_this_ device, which means that _this_ driver will no longer match
the device if you remove and reinsert the driver module, or unbind
and try to re-bind the device to this driver.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev

[Russell King - ARM Linux]

On Thu, Apr 28, 2016 at 09:37:08AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Arnd Bergmann  writes:
> >pointer and pass that in platform_data. This is really easy, it's
> 
> Sorry but passing a struct device pointer in platform_data is
> ridiculous. Not to mention that, as I said before, we can't assume which
> device to pass to xhci_plat in the first place. It might be dwc->dev and
> it might be dwc->dev->parent.

+1.  Passing an unref-counted struct device through platform data is
totally mad, Arnd you're off your rocker if you think that's a good
idea.  What's more is that there's no way to properly refcount the
thing.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: chipidea: Configure DMA properties and ops from DT

[Russell King - ARM Linux]

On Fri, Mar 18, 2016 at 09:54:14AM +0800, Peter Chen wrote:
> Although I don't know what kinds of bugs it may have, it may be
> met before, otherwise, why most of platform drivers need to call
> dma_set_coherent_mask or dma_coerce_mask_and_coherent explicitly

See Documentation/DMA-API.txt, specifically the section starting

  Part Ic - DMA addressing limitations
  

and also Documentation/DMA-API-HOWTO.txt, the section on

  DMA addressing limitations

which provides further information.

Drivers using DMA should be using dma_set_mask_and_coherent() _or_
one of dma_set_mask() and dma_set_coherent_mask() depending on which
types of DMA they wish to perform.  Drivers should not use
dma_coerce_mask_and_coherent() except in exceptional circumstances:
that function is more a marker that they or some bus/platform code
is doing something wrong.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Russell King - ARM Linux]

On Thu, Oct 22, 2015 at 07:44:05AM -0700, Greg Kroah-Hartman wrote:
> 
> 
> On Thu, Oct 22, 2015 at 11:05:11AM +0200, Tomeu Vizoso wrote:
> > Given that downstreams are already carrying as many hacks as they
> > could think of to speed total boot up, I think this is effectively
> > telling them to go away.
> 
> No I'm not, I'm asking for real data, not hand-wavy-this-is-going-to
> solve-the-random-issue-i'm-having type patch by putting random calls in
> semi-random subsystems all over the kernel.

+1000, fully agree.

There's too much verbal diarrhoea going on in this thread and no facts.
I've been waiting for real data too, and there's not one shred of it, or
even a hint that it might appear.  So, the conclusion I'm coming to is
that there isn't any data to back up the claims made in this thread.

If it was such a problem, then in the _eight_ days that this has been
discussed so far, _someone_ would have sent some data showing the
problem.  I think the fact is, there is no data.

Someone prove me wrong.  Someone post the verifiable data showing that
there is a problem to be solved here.

Someone show what the specific failure cases are that are hampering
vendors moving forwards.  Someone show the long boot times by way of
kernel message log.  Someone show some evidence of the problems that
have been alluded to.

If no one can show some evidence, there isn't a problem here. :)

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Alternative approach to solve the deferred probe

[Russell King - ARM Linux]

On Wed, Oct 21, 2015 at 09:13:48PM +0300, Grygorii Strashko wrote:
> But I worry a bit (and that my main point) about these few additional
> rounds of deferred device probing which I have right now and which allows
> some of drivers to finish, finally, their probes successfully.
> With proposed change I'll get more messages in boot log, but some of
> them will belong to drivers which have been probed successfully and so,
> they will be not really useful.

Then you haven't properly understood my proposal.

I want to get rid of all the "X deferred its probing" messages up until
the point that we set the "please report deferred probes" flag.

That _should_ mean that all the deferred probing that goes on becomes
_totally_ silent and becomes hidden (unless you really want to see it,
in which case we can make a debug option which turns it on) up until
we're at the point where we want to enter userspace.

At that point, we then report into the kernel log which devices are
still deferring and, via appropriately placed dev_warn_deferred(),
the reasons why the devices are being deferred.

So, gone will be all the messages earlier in the log about device X
not having a GPIO/clock/whatever because the device providing the
GPIO/clock/whatever hasn't been probed.

If everything is satisfied by the time we run this last round (again,
I'm not using a three line sentence to describe exactly what I mean,
I'm sure you know by now... oops, I just did) then the kernel will
report nothing about any deferrals.  That's _got_ to be an improvement.

> 
> As result, I think, the most important thing is to identify (or create)
> some point during kernel boot when it will be possible to say that all
> built-in drivers (at least) finish their probes 100% (done or defer).
> 
> Might be do_initcalls() can be updated (smth like this):
> static void __init do_initcalls(void)
> {
>   int level;
> 
>   for (level = 0; level < ARRAY_SIZE(initcall_levels) - 1; level++)
>   do_initcall_level(level);
> 
> + wait_for_device_probe();
> + /* Now one final round, reporting any devices that remain deferred */
> + driver_deferred_probe_report = true;
> + driver_deferred_probe_trigger();
> + wait_for_device_probe();
> }
> 
> Also, in my opinion, it will be useful if this debugging feature will be
> optional.

I wonder why you want it optional... so I'm going to guess and cover
both cases I can think of below to head off another round of reply on
this point (sorry if this sucks eggs.)

I don't see it as being optional, because it's going to be cheap to run
in the case of a system which has very few or no errors - which is what
you should have for production systems, right?

Remember, only devices and drivers that are present and have been
probed once get added to the deferred probe list, not devices for
which their drivers are modules.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Alternative approach to solve the deferred probe

[Russell King - ARM Linux]

On Wed, Oct 21, 2015 at 08:36:23AM -0700, Frank Rowand wrote:
> On 10/21/2015 1:18 AM, Russell King - ARM Linux wrote:
> > On Tue, Oct 20, 2015 at 08:58:19PM -0700, Frank Rowand wrote:
> >> On 10/20/2015 8:46 AM, Russell King - ARM Linux wrote:
> 
> < snip >
> 
> >>> +
> >>>  static bool driver_deferred_probe_enable = false;
> >>> +
> >>>  /**
> >>>   * driver_deferred_probe_trigger() - Kick off re-probing deferred devices
> >>>   *
> >>> @@ -188,6 +210,13 @@ static int deferred_probe_initcall(void)
> >>>   driver_deferred_probe_trigger();
> >>
> >> Couldn't you put the "driver_deferred_probe_report = true" here?  And then
> >> not add another round of probes.
> > 
> > The idea is not to report anything for drivers that were deferred
> > during the normal bootup.  The above is part of the normal bootup,
> > and the deferred activity should not be warned about.
> 
> The above is currently the last point for probe to succeed or defer
> (until possibly, as you mentioned, module loading resolves the defer).
> If a probe defers above, it will defer again below.  The set of defers
> should be exactly the same above and below.

Why should it?  Isn't this late_initcall() the first opportunity that
deferred devices get to be re-probed from their first set of attempts
via the drivers having their initcalls called?

If what you're saying is true, what's the point of this late_initcall()?



-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Alternative approach to solve the deferred probe

[Russell King - ARM Linux]

On Tue, Oct 20, 2015 at 08:58:19PM -0700, Frank Rowand wrote:
> On 10/20/2015 8:46 AM, Russell King - ARM Linux wrote:
> > On Mon, Oct 19, 2015 at 06:21:40PM +0200, Geert Uytterhoeven wrote:
> >> Hi Russell,
> >>
> >> On Mon, Oct 19, 2015 at 5:35 PM, Russell King - ARM Linux
> >> <li...@arm.linux.org.uk> wrote:
> >>>>> What you can do is print those devices which have failed to probe at
> >>>>> late_initcall() time - possibly augmenting that with reports from
> >>>>> subsystems showing what resources are not available, but that's only
> >>>>> a guide, because of the "it might or might not be in a kernel module"
> >>>>> problem.
> >>>>
> >>>> Well, adding those reports would give you a changelog similar to the
> >>>> one in this series...
> >>>
> >>> I'm not sure about that, because what I was thinking of is adding
> >>> a flag which would be set at late_initcall() time prior to running
> >>> a final round of deferred device probing.
> >>
> >> Which round is the final round?
> >> That's the one which didn't manage to bind any new devices to drivers,
> >> which is something you only know _after_ the round has been run.
> >>
> >> So I think we need one extra round to handle this.
> >>
> >>> This flag would then be used in a deferred_warn() printk function
> >>> which would normally be silent, but when this flag is set, it would
> >>> print the reason for the deferral - and this would replace (or be
> >>> added) to the subsystems and drivers which return -EPROBE_DEFER.
> >>>
> >>> That has the effect of hiding all the deferrals up until just before
> >>> launching into userspace, which should then acomplish two things -
> >>> firstly, getting rid of the rather useless deferred messages up to
> >>> that point, and secondly printing the reason why the remaining
> >>> deferrals are happening.
> >>>
> >>> That should be a small number of new lines plus a one-line change
> >>> in subsystems and drivers.
> >>
> >> Apart from the extra round we probably can't get rid of, that sounds OK to 
> >> me.
> > 
> > Something like this.  I haven't put a lot of effort into it to change all
> > the places which return an -EPROBE_DEFER, and it also looks like we need
> > some helpers to report when we have only an device_node (or should that
> > be fwnode?)  See the commented out of_warn_deferred() in
> > drivers/gpio/gpiolib-of.c.  Adding this stuff in the subsystems searching
> > for resources should make debugging why things are getting deferred easier.
> > 
> > We could make driver_deferred_probe_report something that can be
> > deactivated again after the last deferred probe run, and provide the
> > user with a knob that they can turn it back on again.
> > 
> > I've tried this out on two of my platforms, including forcing
> > driver_deferred_probe_report to be enabled, and I get exactly one
> > deferred probe, so not a particularly good test.
> > 
> > The patch won't apply as-is to mainline for all files; it's based on my
> > tree which has some 360 additional patches (which seems to be about
> > normal for my tree now.)
> 
> I like the concept (I have been thinking along similar lines lately).
> But I think this might make the console messages more confusing than
> they are now.

If messages end up being given from the subsystem rather than the driver,
surely they become more consistent?

> The problem is that debug, warn, and error messages
> come from a somewhat random set of locations at the moment.  Some
> come from the driver probe routines and some come from the subsystems
> that the probe routines call.  So the patch is suppressing some
> messages, but not others.

The patch is not complete (read the description above).

> > +void dev_warn_deferred(struct device *dev, const char *fmt, ...)
> > +{
> > +   if (driver_deferred_probe_report) {
> > +   struct va_format vaf;
> > +   va_list ap;
> > +
> > +   va_start(ap, fmt);
> > +   vaf.fmt = fmt;
> > +   vaf.va = 
> > +
> > +   dev_warn(dev, "deferring probe: %pV", );
> > +   va_end(ap);
> > +   }
> > +}
> > +EXPORT_SYMBOL_GPL(dev_warn_deferred);
> 
> The places where dev_warn_deferred() replaces dev_dbg(), we lose the
> ability to turn on debugging and observe the driver reporting th

Re: Alternative approach to solve the deferred probe (was: [GIT PULL] On-demand device probing)

[Russell King - ARM Linux]

On Mon, Oct 19, 2015 at 06:21:40PM +0200, Geert Uytterhoeven wrote:
> Hi Russell,
> 
> On Mon, Oct 19, 2015 at 5:35 PM, Russell King - ARM Linux
> <li...@arm.linux.org.uk> wrote:
> >> > What you can do is print those devices which have failed to probe at
> >> > late_initcall() time - possibly augmenting that with reports from
> >> > subsystems showing what resources are not available, but that's only
> >> > a guide, because of the "it might or might not be in a kernel module"
> >> > problem.
> >>
> >> Well, adding those reports would give you a changelog similar to the
> >> one in this series...
> >
> > I'm not sure about that, because what I was thinking of is adding
> > a flag which would be set at late_initcall() time prior to running
> > a final round of deferred device probing.
> 
> Which round is the final round?
> That's the one which didn't manage to bind any new devices to drivers,
> which is something you only know _after_ the round has been run.
> 
> So I think we need one extra round to handle this.
> 
> > This flag would then be used in a deferred_warn() printk function
> > which would normally be silent, but when this flag is set, it would
> > print the reason for the deferral - and this would replace (or be
> > added) to the subsystems and drivers which return -EPROBE_DEFER.
> >
> > That has the effect of hiding all the deferrals up until just before
> > launching into userspace, which should then acomplish two things -
> > firstly, getting rid of the rather useless deferred messages up to
> > that point, and secondly printing the reason why the remaining
> > deferrals are happening.
> >
> > That should be a small number of new lines plus a one-line change
> > in subsystems and drivers.
> 
> Apart from the extra round we probably can't get rid of, that sounds OK to me.

Something like this.  I haven't put a lot of effort into it to change all
the places which return an -EPROBE_DEFER, and it also looks like we need
some helpers to report when we have only an device_node (or should that
be fwnode?)  See the commented out of_warn_deferred() in
drivers/gpio/gpiolib-of.c.  Adding this stuff in the subsystems searching
for resources should make debugging why things are getting deferred easier.

We could make driver_deferred_probe_report something that can be
deactivated again after the last deferred probe run, and provide the
user with a knob that they can turn it back on again.

I've tried this out on two of my platforms, including forcing
driver_deferred_probe_report to be enabled, and I get exactly one
deferred probe, so not a particularly good test.

The patch won't apply as-is to mainline for all files; it's based on my
tree which has some 360 additional patches (which seems to be about
normal for my tree now.)

 drivers/base/dd.c   | 29 +
 drivers/base/power/domain.c |  7 +--
 drivers/clk/clkdev.c|  9 -
 drivers/gpio/gpiolib-of.c   |  5 +
 drivers/gpu/drm/bridge/dw_hdmi.c|  2 +-
 drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
 drivers/gpu/drm/imx/imx-ldb.c   |  5 +++--
 drivers/gpu/drm/msm/dsi/dsi.c   |  2 +-
 drivers/gpu/drm/msm/msm_drv.c   |  3 ++-
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |  3 ++-
 drivers/of/irq.c|  5 -
 drivers/pci/host/pci-mvebu.c|  1 +
 drivers/pinctrl/core.c  |  5 +++--
 drivers/pinctrl/devicetree.c|  4 ++--
 drivers/regulator/core.c|  5 +++--
 include/linux/device.h  |  1 +
 16 files changed, 71 insertions(+), 17 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index be0eb4639128..bb12224f2901 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -129,7 +129,29 @@ void driver_deferred_probe_del(struct device *dev)
mutex_unlock(_probe_mutex);
 }
 
+static bool driver_deferred_probe_report;
+
+/**
+ * dev_warn_deferred() - report why a probe has been deferred
+ */
+void dev_warn_deferred(struct device *dev, const char *fmt, ...)
+{
+   if (driver_deferred_probe_report) {
+   struct va_format vaf;
+   va_list ap;
+
+   va_start(ap, fmt);
+   vaf.fmt = fmt;
+   vaf.va = 
+
+   dev_warn(dev, "deferring probe: %pV", );
+   va_end(ap);
+   }
+}
+EXPORT_SYMBOL_GPL(dev_warn_deferred);
+
 static bool driver_deferred_probe_enable = false;
+
 /**
  * driver_deferred_probe_trigger() - Kick off re-probing deferred devices
  *
@@ -188,6 +210,13 @@ static int deferred_probe_initcall(void)
driver_deferred_probe_trigger();
/* Sort as many dependencies as possible b

Re: [GIT PULL] On-demand device probing

[Russell King - ARM Linux]

On Mon, Oct 19, 2015 at 10:44:41AM +0100, David Woodhouse wrote:
> On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote:
> > On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
> > > On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
> > > > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
> > 
> > > > > I can't see adding calls like this all over the tree just to solve a
> > > > > bus-specific problem, you are adding of_* calls where they aren't
> > > > > needed, or wanted, at all.
> > 
> > > > This isn't bus specific, I'm not sure what makes you say that?
> > 
> > > You are making it bus-specific by putting these calls all over the tree
> > > in different bus subsystems semi-randomly for all I can determine.
> > 
> > Do you mean firmware rather than bus here?  I think that's the confusion
> > I have...
> 
> Certainly, if it literally is adding of_* calls then that would seem to
> be gratuitously firmware-specific. Nothing should be using those these
> days; any new code should be using the generic device property APIs
> (except in special cases).

I asked Linus Walleij about that with the fwnode_get_named_gpiod() stuff,
and Linus didn't seem to know how this should be used.

It doesn't help that dev->fwnode is not initialised, but dev->of_node
is.  Are we supposed to grope around in dev->of_node for the embedded
fwnode instead of using dev->fwnode?

At the moment, at least to me, fwnode looks like some kind of
experimental half-baked thing rather than a real usable solution.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Russell King - ARM Linux]

On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote:
> ... If a device is available and has
> a compatible driver, but it cannot be probed because a dependency
> isn't going to be available, that's an error and is going to cause
> real-world problems unless the device is redundant. Currently we say
> nothing because with deferred probe the probe callbacks are also part
> of the mechanism that determines the dependency order.

So what if device X depends on device Y, and we have a driver for
device Y built-in to the kernel, but the driver for device X is a
module?

I don't see this being solvable in the way you describe above - it's
going to identify X as being unable to be satisfied, and report it as
an error - but it's not an error at all.

> Having a specific switch for enabling deferred probe logging sounds
> good, but there's going to be hundreds of spurious messages about
> deferred probes that were just deferrals and only one of them is going
> to be the actual error in which a device failed to find a dependency.

Why would there be?  Sounds like something's very wrong there.

You should only get deferred probes for devices which are declared to
be present, but their resources have not yet been satisfied.  It
doesn't change anything if you have a kernel with lots of device drivers
or just the device drivers you need - the device drivers you don't need
do not contribute to the deferred probing in any way.

So, really, after boot and all appropriate modules have been loaded,
you should end up with no deferred probes.  Are you saying that you
still have "hundreds" at that point?  If you do, that sounds like
there's something very wrong.

> 3) Regarding total boot time, I don't expect this series to make much
> of a difference because though we would save a lot of matching and
> querying for resources, that's little time compared with how long we
> wait for hardware to react during probing. Async probing is more
> likely to help with drivers that take a long time to probe.

For me, on my fastest ARM board, though running serial console:

[2.293468] VFS: Mounted root (ext4 filesystem) on device 179:1.

There's a couple of delays in there, but they're not down to deferred
probing.  The biggest one is serial console startup (due to the time
it takes to write out the initial kernel messages):

[0.289962] f1012000.serial: ttyS0 at MMIO 0xf1012000 (irq = 23, base_baud = 
15625000) is a 16550A
[0.944124] console [ttyS0] enabled

and DSA switch initialisation:

[1.530655] libphy: dsa slave smi: probed
[2.034426] dsa dsa@0 lan6 (uninitialized): attached PHY at address 0 
[Generic PHY]

I'm not sure what causes that, but at a guess it's having to talk to the
DSA switch over the MDIO bus via several layers of indirect accesses.
Of course, serial console adds to the boot time significantly anyway,
especially at the "standard" kernel logging level.

> One more thing about the breakage we have seen so far is that it's
> generally caused by implicit dependencies and hunting those is
> probably the second biggest timesink of the linux embedded developer
> after failed probes.

... which is generally caused by the crappy code which the average
embedded Linux developer creates, particularly with the crappy error
messages they like creating.  For the most part, they _might_ as well
just print "Error!\n" and be done with it, for all the use they are.
When creating an error print, your average embedded Linux developer
fails to print the _reason_ why something failed, which makes debugging
it much harder.

The first thing I do when I touch code that needs this kind of debugging
is to go through and add printing of the error code.  That normally lets
me quickly narrow down what's failed.

If embedded Linux developers are struggling with this, they only have
themselves to blame.

In the case of deferred probing, what _may_ help is if we got rid of the
core code printing that driver X requested deferred probing, instead
moving the responsibility to report this onto the driver or subsystem.
Resource claiming generally has the struct device, and can use dev_warn()
to report which device is being probed, along with which resource is
not yet available.

This debug problem is solvable without needing to resort to complex
probing solutions.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Russell King - ARM Linux]

On Mon, Oct 19, 2015 at 04:10:56PM +0200, Tomeu Vizoso wrote:
> On 19 October 2015 at 15:18, Russell King - ARM Linux
> <li...@arm.linux.org.uk> wrote:
> > On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote:
> >> ... If a device is available and has
> >> a compatible driver, but it cannot be probed because a dependency
> >> isn't going to be available, that's an error and is going to cause
> >> real-world problems unless the device is redundant. Currently we say
> >> nothing because with deferred probe the probe callbacks are also part
> >> of the mechanism that determines the dependency order.
> >
> > So what if device X depends on device Y, and we have a driver for
> > device Y built-in to the kernel, but the driver for device X is a
> > module?
> >
> > I don't see this being solvable in the way you describe above - it's
> > going to identify X as being unable to be satisfied, and report it as
> > an error - but it's not an error at all.
> 
> It's going to probe Y at late_initcall, then probe X when its driver
> is registered. No deferred probes nor messages about it.
> 
> But if you meant to write the opposite case (X built-in and Y in a
> module), then I have to ask you in what situation that would make
> sense.

I did mean the opposite way around.  It may not make sense if you're
targetting a single platform, but it may make sense in a single zImage
kernel.

Consider something like a single zImage kernel that is built with
everything built-in to be able to boot and mount rootfs without
initramfs support on both platform A and platform B.  Both platforms
share some hardware (eg, an I2C GPIO expander) which is built as a
module.  It is a resource provider.  Platform B contains a driver
which is required to boot on platform A, but not platform B (so the
kernel has to have that driver built-in.)  On platform B, there is
a dependency to the I2C GPIO expander device.

> >> Having a specific switch for enabling deferred probe logging sounds
> >> good, but there's going to be hundreds of spurious messages about
> >> deferred probes that were just deferrals and only one of them is going
> >> to be the actual error in which a device failed to find a dependency.
> >
> > Why would there be?  Sounds like something's very wrong there.
> 
> Sorry about that, I have checked that only now and I "only" get 39
> deferred probe messages on exynos5250-snow.

I typically see one or two, maybe five maximum on the platforms I have
here, but normally zero.

> > So, really, after boot and all appropriate modules have been loaded,
> > you should end up with no deferred probes.  Are you saying that you
> > still have "hundreds" at that point?  If you do, that sounds like
> > there's something very wrong.
> 
> I was talking about messages if we log each -EPROBE_DEFER, not devices
> that remain to be probed. The point being that right now we don't have
> a way to know if we are deferring because the dependency will be
> around later, or if we have a problem and the dependency isn't going
> to be there at all.

What's the difference between a dependency which isn't around because
the driver is not built into the kernel but is available as a module,
and a dependency that isn't around because the module hasn't been
loaded yet?

How do you distinguish between those two scenarios?  In the former
scenario, the device will eventually come up when udev loads the
module.  In the latter case, it's a persistent failing case.

> Agreed, with the note from above on why it would be better to only
> print such a message only when the -EPROBE_DEFER is likely to be a
> problem.

... and my argument is that there's _no way_ to know for certain which
deferred probes will be a problem, and which won't.  The only way to
definitely know that is if you disable kernel modules, and require
all drivers to be built into the kernel.

What you can do is print those devices which have failed to probe at
late_initcall() time - possibly augmenting that with reports from
subsystems showing what resources are not available, but that's only
a guide, because of the "it might or might not be in a kernel module"
problem.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Russell King - ARM Linux]

On Mon, Oct 19, 2015 at 05:00:54PM +0200, Tomeu Vizoso wrote:
> On 19 October 2015 at 16:30, Russell King - ARM Linux
> <li...@arm.linux.org.uk> wrote:
> > I typically see one or two, maybe five maximum on the platforms I have
> > here, but normally zero.
> 
> Hmm, I have given a look at our lava farm and have seen 2 dozens as
> common (with multi_v7).

No, because the lava farms tend not to be public.

> > What you can do is print those devices which have failed to probe at
> > late_initcall() time - possibly augmenting that with reports from
> > subsystems showing what resources are not available, but that's only
> > a guide, because of the "it might or might not be in a kernel module"
> > problem.
> 
> Well, adding those reports would give you a changelog similar to the
> one in this series...

I'm not sure about that, because what I was thinking of is adding
a flag which would be set at late_initcall() time prior to running
a final round of deferred device probing.

This flag would then be used in a deferred_warn() printk function
which would normally be silent, but when this flag is set, it would
print the reason for the deferral - and this would replace (or be
added) to the subsystems and drivers which return -EPROBE_DEFER.

That has the effect of hiding all the deferrals up until just before
launching into userspace, which should then acomplish two things -
firstly, getting rid of the rather useless deferred messages up to
that point, and secondly printing the reason why the remaining
deferrals are happening.

That should be a small number of new lines plus a one-line change
in subsystems and drivers.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Russell King - ARM Linux]

On Mon, Oct 19, 2015 at 04:29:40PM +0100, David Woodhouse wrote:
> I don't know that there *is* a coherent plan here to address it all.
> 
> Certainly, we *will* need subsystems to have firmware-specific
> knowledge in some cases. Take GPIO as an example; ACPI *has* a way to
> describe GPIO, and properties which reference GPIO pins are intended to
> work through that — while in DT, properties which reference GPIO pins
> will have different contents. They'll be compatible at the driver
> level, in the sense that there's a call to get a given GPIO given the
> property name, but the subsystems *will* be doing different things
> behind the scenes.

It's a bit ironic that you've chosen GPIO as an example there.  The
"new" GPIO API (the gpiod_* stuff) only has a fwnode way to get the
gpio descriptor.  There's no of_* method.

I'd like to use the gpiod_* stuff, but I feel that my options are
rather limited: either use fwnode_get_named_gpiod() with
>of_node->fwnode, which seems like a hack by going underneath
the covers of how fwnode is (partially) implemented with DT, or by
using of_get_named_gpio() and the converting the gpio number to a
descriptor via gpio_to_desc().  Both feel very hacky.

If ACPI already handles GPIOs internally, then I'm left wondering
why GPIO descriptor stuff went down the fwnode route at all - it
seems rather pointless in this case, and it seems to make the use
of the gpiod* interfaces where we _do_ need to use it (DT) harder
and more hacky.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Russell King - ARM Linux]

On Mon, Oct 19, 2015 at 08:27:44PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Mon, Oct 19, 2015 at 04:43:24PM +0100, Russell King - ARM Linux wrote:
> > It's a bit ironic that you've chosen GPIO as an example there.  The
> > "new" GPIO API (the gpiod_* stuff) only has a fwnode way to get the
> > gpio descriptor.  There's no of_* method.
> 
> Without following all that fwnode discussion:
> gpiod_get et al. should work for you here, doesn't it? It just takes a
> struct device * and I'm happy with it.

What if you don't have a struct device?  I had that problem recently
when modifying the mvebu PCIe code.  The 'struct device' node doesn't
contain the GPIOs, it's the PCIe controller.  Individual ports on the
controller are described in DT as sub-nodes, and the sub-nodes can
have a GPIO for card reset purposes.  These sub-nodes don't have a
struct device.

Right now, I'm having to do this to work around this issue:

reset_gpio = of_get_named_gpio_flags(child, "reset-gpios", 0, );
if (reset_gpio == -EPROBE_DEFER) {
ret = reset_gpio;
goto err;
}

if (gpio_is_valid(reset_gpio)) {
unsigned long gpio_flags;

port->reset_name = devm_kasprintf(dev, GFP_KERNEL, "%s-reset",
  port->name);
if (!port->reset_name) {
ret = -ENOMEM;
goto err;
}

if (flags & OF_GPIO_ACTIVE_LOW) {
dev_info(dev, "%s: reset gpio is active low\n",
 of_node_full_name(child));
gpio_flags = GPIOF_ACTIVE_LOW |
 GPIOF_OUT_INIT_LOW;
} else {
gpio_flags = GPIOF_OUT_INIT_HIGH;
}

ret = devm_gpio_request_one(dev, reset_gpio, gpio_flags,
port->reset_name);
if (ret) {
if (ret == -EPROBE_DEFER)
goto err;
goto skip;
}

port->reset_gpio = gpio_to_desc(reset_gpio);
}

Not nice, is it?  Not nice to have that in lots of drivers either.

However, switching to use any of_* or fwnode_* thing also carries with
it another problem: you can't control the name appearing in the
allocation, so you end up with a bunch of GPIOs requested with a "reset"
name - meaning you lose any identification of which port the GPIO was
bound to.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Russell King - ARM Linux]

On Mon, Oct 19, 2015 at 06:21:40PM +0200, Geert Uytterhoeven wrote:
> Hi Russell,
> 
> On Mon, Oct 19, 2015 at 5:35 PM, Russell King - ARM Linux
> <li...@arm.linux.org.uk> wrote:
> >> > What you can do is print those devices which have failed to probe at
> >> > late_initcall() time - possibly augmenting that with reports from
> >> > subsystems showing what resources are not available, but that's only
> >> > a guide, because of the "it might or might not be in a kernel module"
> >> > problem.
> >>
> >> Well, adding those reports would give you a changelog similar to the
> >> one in this series...
> >
> > I'm not sure about that, because what I was thinking of is adding
> > a flag which would be set at late_initcall() time prior to running
> > a final round of deferred device probing.
> 
> Which round is the final round?

I said "a" not "the".  Maybe I should've said "one last round of deferred
probing before entering userspace".

> That's the one which didn't manage to bind any new devices to drivers,
> which is something you only know _after_ the round has been run.
> 
> So I think we need one extra round to handle this.

Yes - because the idea is that we do everything we do today without
printing anything about deferred probes.  We then set a flag which
indicates we should report defers, and then we trigger another round
of probes.

If everything probed successfully, the deferred probe list will be
empty and nothing will be seen.  Otherwise, we should end up with a
report of all the devices that weren't able to be bound to their
drivers due to -EPROBE_DEFER.

> 
> > This flag would then be used in a deferred_warn() printk function
> > which would normally be silent, but when this flag is set, it would
> > print the reason for the deferral - and this would replace (or be
> > added) to the subsystems and drivers which return -EPROBE_DEFER.
> >
> > That has the effect of hiding all the deferrals up until just before
> > launching into userspace, which should then acomplish two things -
> > firstly, getting rid of the rather useless deferred messages up to
> > that point, and secondly printing the reason why the remaining
> > deferrals are happening.
> >
> > That should be a small number of new lines plus a one-line change
> > in subsystems and drivers.
> 
> Apart from the extra round we probably can't get rid of, that sounds
> OK to me.

Yes, it means a little longer to boot, but if there's nothing pending
this _should_ only be microseconds - it should be nothing more than
setting the flag, possibly taking and releasing a lock, and checking
that the deferred probe list is empty.

Of course, if the deferred probe list isn't empty, then there'll be
more expense - but I'm willing to bet that for those developers with
serial console enabled, the kernel boot will be faster overall due
to the reduced number of characters printed during the boot.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression: USB OTG port breaks after a few hours in host mode on iMX6

[Russell King - ARM Linux]

On Thu, Oct 08, 2015 at 09:52:52AM +, Peter Chen wrote:
> I can't reproduce it for 5 hours, and will change pinmux (do the same
> thing with your platform), and do the overnight test.

There's definitely something weird going on.  Over night last night,
I left the Logitech universal receiver in the port, and this morning
it was indicating in /proc/interrupts:

283: 50  0  0  0   GPC  43 Edge  
2184000.usb

I removed it, and now I have:

283: 50  0   1716  0   GPC  43 Edge  
2184000.usb

which is increasing at a rate of 90 per minute.

Nothing in the kernel message log indicating why this may be.  It looks
like runtime PM doesn't work on this port:

/sys/bus/platform/devices/2184000.usb/power/runtime_active_time:109850496
/sys/bus/platform/devices/2184000.usb/power/runtime_status:active
/sys/bus/platform/devices/2184000.usb/power/runtime_suspended_time:0

whereas the other port (which has zero interrupts) it does:

/sys/bus/platform/devices/2184200.usb/power/runtime_active_time:16924
/sys/bus/platform/devices/2184200.usb/power/runtime_status:suspended
/sys/bus/platform/devices/2184200.usb/power/runtime_suspended_time:109861760

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regression: USB OTG port breaks after a few hours in host mode on iMX6

[Russell King - ARM Linux]

This bug is soo obscure, I'm not even sure how to start debugging it.
This never used to be a problem, but I'm not sure when it started as
USB (in general) is not something I use regularly.

In this setup, the USB OTG port is wired in host mode via pinmix
configuration of the USB OTG ID pin:

MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x13059

The problem characterised so far: after booting the kernel, USB seems
to work fine.  You can plug and unplug devices from both USB host and
USB OTG ports, and it's detected.

If you boot a kernel with nothing plugged in, leave it for maybe four
hours, then plug in a device to either port, the device will be seen
in the USB host port, but completely ignored in the USB OTG port.
Both USB OTG ports have power, confirmed last night when using a USB
microscope with built-in LED lighting: the LEDs are functional, the
device is otherwise completely ignored.

The above is basically what I know so far: I don't know when the port
fails, whether in the case of "boot, wait four hours, and it's dead"
whether it's dead from boot: when I've tried testing it immediately
after boot, it's worked.

As it takes around four hours to reproduce, and I have a massive patch
stack on top of the kernel for this platform, I'm unwilling to attempt
a git bisection to try and find when this occurred, but I know it's
been going on for a while now as I've noticed the same behaviour when
I transfer my logitech USB receiver to that port.

I had thought it was a power issue, but the USB camera last night
confirmed that it's a port driver issue.

Certainly earlier 3.x kernels did not have this behaviour.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

[Russell King - ARM Linux]

On Tue, Sep 01, 2015 at 02:54:17PM +0300, Mathias Nyman wrote:
> On 31.08.2015 21:58, Duc Dang wrote:
> >On Thu, Aug 20, 2015 at 12:38 PM, Duc Dang  wrote:
> >>The xhci platform driver needs to work on systems that
> >>either only support 64-bit DMA or only support 32-bit DMA.
> >>Attempt to set a coherent dma mask for 64-bit DMA, and
> >>attempt again with 32-bit DMA if that fails.
> >>
> >>[dhdang: regenerate the patch over 4.2-rc5 and address new comments]
> >>Signed-off-by: Mark Langsdorf 
> >>Tested-by: Mark Salter 
> >>Signed-off-by: Duc Dang 
> >>
> >>---
> >>Changes from v6:
> >> -Add WARN_ON if dma_mask is NULL
> >> -Use dma_coerce_mask_and_coherent to assign
> >> dma_mask and coherent_dma_mask
> >>
> >>Changes from v5:
> >> -Change comment
> >> -Assign dma_mask to coherent_dma_mask if dma_mask is NULL
> >> to make sure dma_set_mask_and_coherent does not fail prematurely.
> >>
> >>Changes from v4:
> >> -None
> >>
> >>Changes from v3:
> >> -Re-generate the patch over 4.2-rc5
> >> -No code change.
> >>
> >>Changes from v2:
> >> -None
> >>
> >>Changes from v1:
> >> -Consolidated to use dma_set_mask_and_coherent
> >> -Got rid of the check against sizeof(dma_addr_t)
> >>
> >>  drivers/usb/host/xhci-plat.c | 21 +
> >>  1 file changed, 13 insertions(+), 8 deletions(-)
> >>
> >>diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> >>index 890ad9d..e4c7f9d 100644
> >>--- a/drivers/usb/host/xhci-plat.c
> >>+++ b/drivers/usb/host/xhci-plat.c
> >>@@ -93,14 +93,19 @@ static int xhci_plat_probe(struct platform_device *pdev)
> >> if (irq < 0)
> >> return -ENODEV;
> >>
> >>-   /* Initialize dma_mask and coherent_dma_mask to 32-bits */
> >>-   ret = dma_set_coherent_mask(>dev, DMA_BIT_MASK(32));
> >>-   if (ret)
> >>-   return ret;
> >>-   if (!pdev->dev.dma_mask)
> >>-   pdev->dev.dma_mask = >dev.coherent_dma_mask;
> >>-   else
> >>-   dma_set_mask(>dev, DMA_BIT_MASK(32));
> >>+   /* Throw a waring if broken platform code didn't initialize 
> >>dma_mask */
> >>+   WARN_ON(!pdev->dev.dma_mask);
> >>+   /*
> >>+* Try setting dma_mask and coherent_dma_mask to 64 bits,
> >>+* then try 32 bits
> >>+*/
> >>+   ret = dma_coerce_mask_and_coherent(>dev, DMA_BIT_MASK(64));
> >>+   if (ret) {
> >>+   ret = dma_coerce_mask_and_coherent(>dev,
> >>+  DMA_BIT_MASK(32));
> >>+   if (ret)
> >>+   return ret;
> >>+   }

This isn't very good.  If dev.dma_mask is already set,
dma_coerce_mask_and_coherent() will always overwrite it.  There's also
no need to call it twice.  This, imho, is much better:

/* Try to set a 64-bit DMA mask first */
if (WARN_ON(!pdev->dev.dma_mask)) {
/* Eek, platform didn't initialise the streaming DMA mask */
ret = dma_coerce_mask_and_coherent(>dev, 
DMA_BIT_MASK(64));
} else {
ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
}

/* If that failed, fall back to a 32-bit DMA mask */
if (ret) {
ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
if (ret)
return ret;
}

since it preserves the dev.dma_mask pointer if it was properly setup

Really, drivers shouldn't be messing around with that pointer - especially
if it's already been correctly setup.  A platform may require separate
streaming and coherent masks, and we should respect that.

(The whole dma_mask being a pointer thing is a left-over from the PCI
layer which has never been cleaned up through fear of breaking something.)

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

[Russell King - ARM Linux]

On Fri, Aug 07, 2015 at 08:18:48PM -0700, Duc Dang wrote:
 diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
 index 890ad9d..5d03f8b 100644
 --- a/drivers/usb/host/xhci-plat.c
 +++ b/drivers/usb/host/xhci-plat.c
 @@ -93,14 +93,14 @@ static int xhci_plat_probe(struct platform_device *pdev)
   if (irq  0)
   return -ENODEV;
  
 - /* Initialize dma_mask and coherent_dma_mask to 32-bits */
 - ret = dma_set_coherent_mask(pdev-dev, DMA_BIT_MASK(32));
 - if (ret)
 - return ret;
 - if (!pdev-dev.dma_mask)
 - pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
 - else
 - dma_set_mask(pdev-dev, DMA_BIT_MASK(32));
 + /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
 + ret = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(64));
 + if (ret) {
 + ret = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(32));
 + if (ret)
 + return ret;
 + }

Note that dma_set_mask_and_coherent() and the original code are not
equivalent because of this:

if (!pdev-dev.dma_mask)
pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;

If we know that pdev-dev.dma_mask will always be initialised at this
point, then the above change is fine.  If not, it's introducing a
regression - dma_set_mask_and_coherent() will fail if pdev-dev.dma_mask
is NULL (depending on the architectures implementation of dma_set_mask()).

Prefixing the above change with the two lines I mention above would
ensure equivalent behaviour.  Even if we do want to get rid of this,
I'd advise to do it as a separate patch after this change, which can
be independently reverted if there's problems with its removal.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Linux-am33-list] [PATCH v2 0/7] fix build failure of mn10300

[Russell King - ARM Linux]

On Thu, Jun 18, 2015 at 10:31:07AM -0500, Jay C. Polmar wrote:
 I am on this list by mistake and 5/15/2011 we requested to be removed,
 can someone remove me from this list?

There are six mailing lists in the Cc line.  For all of the lists
present there, I can't help you, but you should be able to unsubscribe
yourself.  Look at the footer on any message you receive from the list
and it should tell you how to get more information on that subject.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb 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 0/7] fix build failure of mn10300

[Russell King - ARM Linux]

On Thu, Jun 18, 2015 at 05:47:46PM +0530, Sudip Mukherjee wrote:
 This is an attempt to fix the build failures when building mn10300 with
 allmodconfig. As I have never worked with arch files so I hope you will
 point me to right direction to correct my mistakes in this attempt.

Why has the linux-arm-kernel mailing list been copied for something
which clearly has nothing to do with ARM?  Am I missing something
in this series?

(Or is there a competition on to see who can add as many entries in
the Cc without getting caught by any filtering? :) )

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: renesas: fix extcon dependency

[Russell King - ARM Linux]

On Wed, Jan 28, 2015 at 10:47:18PM +0100, Arnd Bergmann wrote:
 The renesas usbhs driver calls extcon_get_edev_by_phandle(), which
 is defined in drivers/extcon/extcon-class.c, and that can be a
 loadable module. If the extcon-class support is disabled, usbhs
 will work correctly for all devices that do not need extcon.
 
 However, if extcon-class is a loadable module, and usbhs is
 built-in, the kernel fails to link. In order to solve that,
 we need a Kconfig dependency that allows extcon to be disabled
 but does not allow usbhs built-in if extcon is a module.
 
 Signed-off-by: Arnd Bergmann a...@arndb.de
 
 diff --git a/drivers/usb/renesas_usbhs/Kconfig 
 b/drivers/usb/renesas_usbhs/Kconfig
 index de83b9d0cd5c..0ea9040b9f10 100644
 --- a/drivers/usb/renesas_usbhs/Kconfig
 +++ b/drivers/usb/renesas_usbhs/Kconfig
 @@ -6,6 +6,7 @@ config USB_RENESAS_USBHS
   tristate 'Renesas USBHS controller'
   depends on USB_GADGET
   depends on ARCH_SHMOBILE || SUPERH || COMPILE_TEST
 + depends on EXTCON || !EXTCON # for module build if extcon is

The comment doesn't make much sense on its own (and when I read it before
reading the above description, it was down right confusing.)  Maybe a
longer comment would be a good idea?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Lockdep problem: v3.18+ (with yesterday's Linus tip - 6f51ee709e4c)

[Russell King - ARM Linux]

On Wed, Jan 07, 2015 at 10:42:14AM -0500, Alan Stern wrote:
 On Thu, 18 Dec 2014, Greg Kroah-Hartman wrote:
  That seems reasonable to me, unbinding when a reset is happening is
  going to be a rare condition, but if we get rid of it, and we try to
  queue a reset for a device that is gone, we will just fail the reset,
  right?  If all should be fine, I have no objection to removing it.
 
 Russell, can you reproduce that lockdep violation whenever you want?

I can certainly try it - I move the logitek receiver around between about
five machines depending on which I'm wanting to use.  Obviously, having
had the Christmas holidays recently, it hasn't been moved so much.

However, at the moment, I'm still not doing much in the way of kernel
work due to ongoing illness.

 If you can, does the following patch help?

I'll give it a go once I've worked out how reproducable it is, many
thanks for looking into this.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

3.18 / 3.19-rc1: ehci scheduling bug?

[Russell King - ARM Linux]

While testing an EM28xx based USB DVB stick which I've recently acquired,
I find that occasionally the driver stops responding after it returns
-EFBIG from one of its ioctls.  I've no idea whether there's a previous
kernel version which works.

Finding the EFBIG return in ehci-sched.c, I decided to make the debug
a little more verbose there, and got:

ci_hdrc ci_hdrc.0: request e7668800 would overflow (4093+63 = 4096)

So, I augmented it, which revealed:

ci_hdrc ci_hdrc.0: request ecd75000 would overflow (4090+64-1 = 4096+0)
ci_hdrc ci_hdrc.0: hs 1 xfer_flag 0x206 empty 1 new_stream 1 now 1032 next 4090 
base 1040

Here, start = 4090, period = 1, span = 64, mod = 4096, wrap = 0.

This looks fairly complex to debug - any suggestions on how to debug
this further?

Thanks.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 3.18 / 3.19-rc1: ehci scheduling bug?

[Russell King - ARM Linux]

On Wed, Dec 24, 2014 at 12:00:00PM +0100, Frank Schäfer wrote:
 See
 https://bugzilla.kernel.org/show_bug.cgi?id=72891
 
 Solved by
 http://www.spinics.net/lists/linux-usb/msg118366.html

Thanks, that seems to solve it.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Lockdep problem: v3.18+ (with yesterday's Linus tip - 6f51ee709e4c)

[Russell King - ARM Linux]

While unplugging a Logitek Keyboard/mouse micro-receiver, I got the
lockdep splat below.

However, I don't fully understand this splat - I see nothing in
flush_work() nor process_one_work() making use of intf-reset_ws -
which seems to be a USB thing.  I guess lockdep is being re-used to
validate work stuff, and lock is just plain misleading.

usb 2-1.1: USB disconnect, device number 3

=
[ INFO: possible recursive locking detected ]
3.18.0+ #1459 Not tainted
-
kworker/0:1/2758 is trying to acquire lock:
 ((intf-reset_ws)){+.+.+.}, at: [c003ba90] flush_work+0x0/0x264

but task is already holding lock:
 ((intf-reset_ws)){+.+.+.}, at: [c003ca40] process_one_work+0x130/0x4b4

other info that might help us debug this:
 Possible unsafe locking scenario:

   CPU0
   
  lock((intf-reset_ws));
  lock((intf-reset_ws));

 *** DEADLOCK ***

 May be due to missing lock nesting notation

4 locks held by kworker/0:1/2758:
 #0:  (events){.+.+.+}, at: [c003ca40] process_one_work+0x130/0x4b4
 #1:  ((intf-reset_ws)){+.+.+.}, at: [c003ca40] process_one_work+0x130/0x4b4
 #2:  (dev-mutex){..}, at: [c0438c70] 
usb_lock_device_for_reset+0x58/0xd0
 #3:  (dev-mutex){..}, at: [c038cc10] device_release_driver+0x20/0x34

stack backtrace:
CPU: 0 PID: 2758 Comm: kworker/0:1 Not tainted 3.18.0+ #1459
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Workqueue: events __usb_queue_reset_device
Backtrace:
[c0012228] (dump_backtrace) from [c00123c0] (show_stack+0x18/0x1c)
 r6:c140b5ac r5:c141ce9c r4: r3:e3b230c0
[c00123a8] (show_stack) from [c06d7f00] (dump_stack+0x7c/0x98)
[c06d7e84] (dump_stack) from [c00619a0] (__lock_acquire+0x13f4/0x1bb0)
 r4:c0bd9190 r3:e3b230c0
[c00605ac] (__lock_acquire) from [c00626c0] (lock_acquire+0xb0/0x124)
 r10: r9:c003ba90 r8: r7: r6: r5:ed810670
 r4:
[c0062610] (lock_acquire) from [c003bad4] (flush_work+0x44/0x264)
 r10:ffed r9:c0a43170 r8:ed810400 r7:ed810670 r6:0001 r5:ed810660
 r4:
[c003ba90] (flush_work) from [c003d8f0] (__cancel_work_timer+0x8c/0x124)
 r7:ffe0 r6: r5: r4:ed810660
[c003d864] (__cancel_work_timer) from [c003d9b4] 
(cancel_work_sync+0x14/0x18)
 r7:ed810420 r6:ed810420 r5:c0a43170 r4:ee357068
[c003d9a0] (cancel_work_sync) from [c04476dc] 
(usb_unbind_interface+0x90/0x280)
[c044764c] (usb_unbind_interface) from [c038cb9c] 
(__device_release_driver+078/0xcc)
 r10:ffed r9:000c r8:fff4 r7:ee357000 r6:ed810420 r5:c0a43170
 r4:ed810420
[c038cb24] (__device_release_driver) from [c038cc18] 
(device_release_driver+0x28/0x34)
 r5:ed810420 r4:ed810454
[c038cbf0] (device_release_driver) from [c044794c] 
(usb_driver_release_interface+0x80/0x84)
 r5:0001 r4:ed810400
[c04478cc] (usb_driver_release_interface) from [c0447970] 
(usb_forced_unbind_intf+0x20/0x30)
 r7:ee357000 r6:ed80c000 r5:ed80c054 r4:ed810400
[c0447950] (usb_forced_unbind_intf) from [c04479e4] 
(unbind_marked_interfaces+0x64/0x74)
 r4:0002 r3:0020
[c0447980] (unbind_marked_interfaces) from [c0447b58] 
(usb_unbind_and_rebind_marked_interfaces+0x14/0x20)
 r6:ed80c050 r5: r4:ee357000 r3:006b
[c0447b44] (usb_unbind_and_rebind_marked_interfaces) from [c043c470] 
(usb_reset_device+0x1dc/0x234)
 r4:ed81 r3:006b
[c043c294] (usb_reset_device) from [c0443f6c] 
(__usb_queue_reset_device+0x40/0x58)
 r10:eefb9b00 r9:c0a59178 r8: r7:dd505eb0 r6:ee357000 r5:ee357068
 r4:ed810260
[c0443f2c] (__usb_queue_reset_device) from [c003cad0] 
(process_one_work+0x1c0/0x4b4)
 r6:eefb6500 r5:dd6dde00 r4:ed810260 r3:c0443f2c
[c003c910] (process_one_work) from [c003ce34] (worker_thread+0x34/0x4b0)
 r10:eefb6500 r9:dd6dde00 r8:0008 r7:dd6dde18 r6:eefb6500 r5:0001
 r4:eefb6530
[c003ce00] (worker_thread) from [c0042228] (kthread+0xe0/0xfc)
 r10: r9: r8: r7:c003ce00 r6:dd6dde00 r5:
 r4:dd66e880
[c0042148] (kthread) from [c000ecc8] (ret_from_fork+0x14/0x2c)
 r7: r6: r5:c0042148 r4:dd66e880

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb 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 2/5] i2c: pxa: prepare/unprepare clocks

[Russell King - ARM Linux]

On Mon, Nov 17, 2014 at 06:07:40PM +0300, Dmitry Eremin-Solenikov wrote:
 Change clk_enable/disable() calls to clk_prepare_enable() and
 clk_disable_unprepare().
 
 Signed-off-by: Dmitry Eremin-Solenikov dbarysh...@gmail.com
 ---
  drivers/i2c/busses/i2c-pxa.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
 index be671f7..2e75375 100644
 --- a/drivers/i2c/busses/i2c-pxa.c
 +++ b/drivers/i2c/busses/i2c-pxa.c
 @@ -1297,7 +1297,7 @@ static int i2c_pxa_suspend_noirq(struct device *dev)
   struct platform_device *pdev = to_platform_device(dev);
   struct pxa_i2c *i2c = platform_get_drvdata(pdev);
  
 - clk_disable(i2c-clk);
 + clk_disable_unprepare(i2c-clk);

Since clk_unprepare() and clk_prepare() can sleep, it is unwise to call
these with IRQs disabled - the _noirq variants of these are run with
IRQs disabled.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb 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/5] serial: pxa: prepare/unprepare clocks

[Russell King - ARM Linux]

On Mon, Nov 17, 2014 at 06:07:39PM +0300, Dmitry Eremin-Solenikov wrote:
 Change clk_enable/disable() calls to clk_prepare_enable() and
 clk_disable_unprepare().

NAK.  Console writes can be called from IRQs-off regions.  clk_prepare()
can sleep.  Sleeping is not permitted with IRQs off.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb 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 3/5] usb: gadget: pxa25x_udc: prepare/unprepare clocks in pxa-ssp

[Russell King - ARM Linux]

On Tue, Nov 18, 2014 at 12:05:45AM +0400, Dmitry Eremin-Solenikov wrote:
 Hello,
 
 2014-11-17 21:44 GMT+03:00 Robert Jarzmik robert.jarz...@free.fr:
  Dmitry Eremin-Solenikov dbarysh...@gmail.com writes:
 
  Change clk_enable/disable() calls to clk_prepare_enable() and
  clk_disable_unprepare().
 
  Signed-off-by: Dmitry Eremin-Solenikov dbarysh...@gmail.com
  ---
   drivers/usb/gadget/udc/pxa25x_udc.c | 8 
   1 file changed, 4 insertions(+), 4 deletions(-)
 
  diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c 
  b/drivers/usb/gadget/udc/pxa25x_udc.c
  index 42f7eeb..e4964ee 100644
  --- a/drivers/usb/gadget/udc/pxa25x_udc.c
  +++ b/drivers/usb/gadget/udc/pxa25x_udc.c
  @@ -103,8 +103,8 @@ static const char ep0name [] = ep0;
 
   /* IXP doesn't yet support linux/clk.h */
   #define clk_get(dev,name)NULL
  -#define clk_enable(clk)  do { } while (0)
  -#define clk_disable(clk) do { } while (0)
  +#define clk_prepare_enable(clk)  do { } while (0)
  +#define clk_disable_unprepare(clk)   do { } while (0)
   #define clk_put(clk) do { } while (0)
 
   #endif
  @@ -932,7 +932,7 @@ static int pullup(struct pxa25x_udc *udc)
if (!udc-active) {
udc-active = 1;
/* Enable clock for USB device */
  - clk_enable(udc-clk);
  + clk_prepare_enable(udc-clk);
 
  Guess what, Russell's remark on i2c and serial made me cross-check.  And 
  there
  is a case where this will be called in irq context too ...
 
  See :
  - do_IRQ
- lubbock_vbus_irq()
  - pxa25x_udc_vbus_session()
- pullup()
  - clk_prepare_enable()
- CRACK
 
  Note that your patch is not really the faulty one, I think a threaded irq
  instead of the raw irq should do the trick. And it is granted on UDC api 
  that
  vbus function is called in a sleeping context (Felipe correct me if I'm
  wrong), so a patch to fix this before your current code would be fine.
 
 OK, I will take a look. It seems the correct way would be to strip this code
 away to a phy, like gpio-vbus or nop phys. Do you have access to lubbock
 (or maybe Daniel, Haojian or Russell has?)?

Robert has my Lubbock now.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RCU bug with v3.17-rc3 ?

[Russell King - ARM Linux]

On Wed, Oct 15, 2014 at 10:25:13PM +0100, Russell King - ARM Linux wrote:
 On Wed, Oct 15, 2014 at 10:23:10PM +0100, Russell King - ARM Linux wrote:
  As I said, I have a patch in progress, but it seems that there needed
  to be some discussion about exactly which compiler versions are affected.
  It seems that it's not as trivial as looking at the GCC bug entry.
 
 ... and in any case, it has been a known bug for well over a year now,
 and it seems that it doesn't affect _that_ many people.  So taking some
 extra time to get it properly correct is the _right_ thing to do.

Well, this is just great.  Pushing out the change which blacklists these
compilers takes out Olof's kernel build system...

Things are not as trivial as they seem.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RCU bug with v3.17-rc3 ?

[Russell King - ARM Linux]

On Tue, Oct 14, 2014 at 04:06:40AM +0200, Greg KH wrote:
 On Mon, Oct 13, 2014 at 12:43:07PM +0100, Russell King - ARM Linux wrote:
  I think the only viable solution here is that:
  
  1. We blacklist the bad compiler versions outright in the kernel.
 
 Yes, please do this, it's what we have done for other buggy compiler
 versions, no need to do something different here.
 
  Remember, it's the distro's choice to fix these buggy compilers, so the
  onus is on _them_ to deal with the mess they've created by doing so.
 
 I totally agree.
 
 Is someone going to send this patch, or do I have to write it myself?

As I said, I have a patch in progress, but it seems that there needed
to be some discussion about exactly which compiler versions are affected.
It seems that it's not as trivial as looking at the GCC bug entry.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RCU bug with v3.17-rc3 ?

[Russell King - ARM Linux]

On Wed, Oct 15, 2014 at 10:23:10PM +0100, Russell King - ARM Linux wrote:
 As I said, I have a patch in progress, but it seems that there needed
 to be some discussion about exactly which compiler versions are affected.
 It seems that it's not as trivial as looking at the GCC bug entry.

... and in any case, it has been a known bug for well over a year now,
and it seems that it doesn't affect _that_ many people.  So taking some
extra time to get it properly correct is the _right_ thing to do.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RCU bug with v3.17-rc3 ?

[Russell King - ARM Linux]

On Mon, Oct 13, 2014 at 09:11:34AM +, David Laight wrote:
 From: Nathan Lynch
  On 10/10/2014 11:25 AM, Russell King - ARM Linux wrote:
  
   Right, so GCC 4.8.{1,2} are totally unsuitable for kernel building (and
   it seems that this has been known about for some time.)
  
  Looking at http://gcc.gnu.org/PR58854 it seems that all 4.8.x for x  3
  are affected, as well as 4.9.0.
  
   We can blacklist these GCC versions quite easily.  We already have GCC
   3.3 blacklisted, and it's trivial to add others.  I would want to include
   some proper details about the bug, just like the other existing entries
   we already have in asm-offsets.c, where we name the functions that the
   compiler is known to break where appropriate.
  
  Before blacklisting anything, it's worth considering that simple version
  checks would break existing pre-4.8.3 compilers that have been patched
  for PR58854.  It looks like Yocto and Buildroot issued releases with
  patched 4.8.2 compilers well before the (fixed) 4.8.3 release.  I think
  the most we can reasonably do without breaking some correctly-behaving
  toolchains is to emit a warning.
 
 Is it possible to compile a small code fragment and check the generated
 code for the bug?
 Possibly predicated on the broken version number to avoid false positives.

I don't see how - it looks like it requires an interrupt to occur at an
opportune moment to provoke the function to fail.  The alternative would
be to parse the assembly generated by the compiler to determine how it
is dealing with the stack.

I think the only viable solution here is that:

1. We blacklist the bad compiler versions outright in the kernel.
2. We /consider/ a testing a preprocessor symbol which when present
   indicates that these versions are fixed and should not be blacklisted.

The argument for (2) is that /if/ distros want to patch their compilers
to fix the problem, they /also/ have the ability to patch their compilers
to make them identifyable, and that is a far more reliable solution than
trying to parse the assembly output from multiple different GCC versions.

Remember, it's the distro's choice to fix these buggy compilers, so the
onus is on _them_ to deal with the mess they've created by doing so.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RCU bug with v3.17-rc3 ?

[Russell King - ARM Linux]

On Fri, Oct 10, 2014 at 08:44:33PM -0500, Nathan Lynch wrote:
 On 10/10/2014 11:25 AM, Russell King - ARM Linux wrote:
  We can blacklist these GCC versions quite easily.  We already have GCC
  3.3 blacklisted, and it's trivial to add others.  I would want to include
  some proper details about the bug, just like the other existing entries
  we already have in asm-offsets.c, where we name the functions that the
  compiler is known to break where appropriate.
 
 Before blacklisting anything, it's worth considering that simple version
 checks would break existing pre-4.8.3 compilers that have been patched
 for PR58854.  It looks like Yocto and Buildroot issued releases with
 patched 4.8.2 compilers well before the (fixed) 4.8.3 release.  I think
 the most we can reasonably do without breaking some correctly-behaving
 toolchains is to emit a warning.

I wish that it was possible to just do the warning thing, but unfortunately
evidence is that many people ignore compiler warnings, because they see
them appearing from the kernel soo often they have become de-sensitised
to them.

This is pretty obvious from the various nightly build systems which produce
the same warnings for months without any progress on them - some of them
can be quite serious (oops-able) where printf format strings are concerned.

  for some time that GCC 4.8.1 and GCC 4.8.2 _can_ lead to filesystem
  corruption, and have sat on their backsides doing nothing about getting
  it blacklisted for something like a year.
 
 Mea culpa, although I hadn't drawn the connection to FS corruption
 reports until now.  I have known about the issue for some time, but
 figured the prevalence of the fix in downstream projects largely
 mitigated the issue.

It's the FS corruption which swings it in favour of a #error - even if
we have a bunch of compilers around with that version which have the
problem fixed, it's /far/ better to #error out.  Those people who know
definitely that they have a fixed compiler can comment out the test
after checking that they do indeed have a fixed version, or are willing
to take the risk.

What we can't do is have kernels built by people who then run into FS
corruption because of this known issue.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RCU bug with v3.17-rc3 ?

[Russell King - ARM Linux]

On Sat, Oct 11, 2014 at 11:54:32AM +0800, Peter Chen wrote:
 On Fri, Oct 10, 2014 at 08:44:33PM -0500, Nathan Lynch wrote:
  On 10/10/2014 11:25 AM, Russell King - ARM Linux wrote:
   
   Right, so GCC 4.8.{1,2} are totally unsuitable for kernel building (and
   it seems that this has been known about for some time.)
  
  Looking at http://gcc.gnu.org/PR58854 it seems that all 4.8.x for x  3
  are affected, as well as 4.9.0.
  
   We can blacklist these GCC versions quite easily.  We already have GCC
   3.3 blacklisted, and it's trivial to add others.  I would want to include
   some proper details about the bug, just like the other existing entries
   we already have in asm-offsets.c, where we name the functions that the
   compiler is known to break where appropriate.
  
  Before blacklisting anything, it's worth considering that simple version
  checks would break existing pre-4.8.3 compilers that have been patched
  for PR58854.  It looks like Yocto and Buildroot issued releases with
  patched 4.8.2 compilers well before the (fixed) 4.8.3 release.  I think
  the most we can reasonably do without breaking some correctly-behaving
  toolchains is to emit a warning.
 
 Yocto has PR58854 problem patch.
 
 http://git.yoctoproject.org/cgit/cgit.cgi/poky/tree/meta/recipes-devtools/gcc/gcc-4.8/0048-PR58854_fix_arm_apcs_epilogue.patch?h=daisy

Right, and we can provide links to these in the comments above the #error
so people have the right places to do a bit of research into whether their
compiler is safe.

It is unfortunate that they are indistinguishable from the broken versions,
but that's really a distro problem for causing that issue themselves -
especially given how serious this bug is.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RCU bug with v3.17-rc3 ?

[Russell King - ARM Linux]

On Fri, Oct 10, 2014 at 12:47:06AM +0300, Aaro Koskinen wrote:
 Hi,
 
 On Thu, Oct 09, 2014 at 10:41:01PM +0200, Rabin Vincent wrote:
What GCC version are you using?

4.8.1 and 4.8.2 are known to miscompile the ARM kernel and these
find_get_entry() crashes with 0x involved smell a lot like the
earlier reports from kernels build with those compilers:

https://lkml.org/lkml/2014/6/25/456
https://lkml.org/lkml/2014/6/30/375
https://lkml.org/lkml/2014/6/30/660
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58854
https://lkml.org/lkml/2014/5/9/330
 
 Is it possible to blacklist those GCC versions on ARM somehow as it
 seems people are still using them?
 
 This bug also ruined a file system on one of my boxes last year
 (see e.g. http://marc.info/?l=linux-arm-kernelm=139033442527244w=2).

Given that, why the fsck (pun intended) did you not shout a little louder
about getting it blacklisted.  Looking at your marc.info URL, there's
very little information there which hints at filesystem corruption, and
it's a thread of only *one* message according to marc.info.

Even _if_ I did read the message you point to above, that on its own did
not hint at filesystem corruption.

So, would you please mind passing on further details about this,
specifically which function in the ext4 code is affected, so it can
be properly written up.

Thanks.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RCU bug with v3.17-rc3 ?

[Russell King - ARM Linux]

On Fri, Oct 10, 2014 at 08:57:43AM -0500, Felipe Balbi wrote:
 On Thu, Oct 09, 2014 at 04:07:15PM -0500, Felipe Balbi wrote:
  Hi,
  
  On Thu, Oct 09, 2014 at 03:46:37PM -0500, Felipe Balbi wrote:
   On Thu, Oct 09, 2014 at 10:41:01PM +0200, Rabin Vincent wrote:
On Thu, Oct 09, 2014 at 11:26:56AM -0500, Felipe Balbi wrote:
 alright, it's pretty deterministic however. Always on the same test, 
 no
 matter which USB controller, no matter if backing store is RAM or MMC.
 
 Those two undefined instructions on the disassembly caught my 
 attention,
 perhaps I'm facing a GCC bug ?

The undefined instructions are just ARM's BUG() implementation.

But did you see the question I asked you yesterday in your other thread?
http://www.spinics.net/lists/arm-kernel/msg368634.html
   
   hmm, completely missed that, sorry. I'm using 4.8.2, will try something
   else.
  
  seems to be working fine now, thanks. I'll leave test running overnight
  just in case.
 
 yup, ran over night without any problems.

Right, so GCC 4.8.{1,2} are totally unsuitable for kernel building (and
it seems that this has been known about for some time.)

We can blacklist these GCC versions quite easily.  We already have GCC
3.3 blacklisted, and it's trivial to add others.  I would want to include
some proper details about the bug, just like the other existing entries
we already have in asm-offsets.c, where we name the functions that the
compiler is known to break where appropriate.

However, I'm rather annoyed that there are people here who have known
for some time that GCC 4.8.1 and GCC 4.8.2 _can_ lead to filesystem
corruption, and have sat on their backsides doing nothing about getting
it blacklisted for something like a year.

When people talk about the ARM community being dysfunctional... well,
this kind of irresponsible behaviour just gives them more fodder to
throw at us.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Russell King - ARM Linux]

On Thu, Jul 17, 2014 at 07:19:15PM +0800, Peter Chen wrote:
 Currently, we are designing a generic driver, we don't know what's the
 hardware architecture, we are trying to find a solution how to set
 dma mask for all possible devices which will use this driver, Antoine's
 this patch is trying to cover this feature.
 
 For example, 
 
 Marvell Berlin doesn't need to set dma mask specific.
 http://marc.info/?l=linux-arm-kernelm=140205228507045w=2
 http://www.spinics.net/lists/linux-usb/msg110598.html

I can't make sense of those messages.

What are you saying - that ci_hdrc on Berlin does not use DMA at all?
Or that it doesn't care what the DMA mask is set to?

 Xilinx zynq needs to set dma mask as 0xFFF0
 http://marc.info/?l=linux-usbm=140384129921706w=2

Why?  The DMA mask does /not/ convey the DMA alignment requirements of
the transfers.  If you need it to be aligned to 16-bytes, then that's
something which is internal to the driver.  This is no different from
devices which require 32-bit alignment or 64-bit alignment.

You can't really expect the DMA subsystem to take care of that for you.
The DMA mask is about indicating what memory ranges are acceptable for
DMA, and not the alignment.

So, in this case, your DMA mask should be DMA_BIT_MASK(32), the same as
iMX.

However, if your driver does receive memory which is not appropriately
aligned, it is the driver's responsibility, not the DMA API, to deal
with that appropriately.

So, it sounds to me like:

- The DT code should be setting the DMA mask appropriately already.

- If the driver is using streaming DMA, should call:

err = dma_set_mask(pdev-dev, DMA_BIT_MASK(32));
if (err)
handle error;

- If the driver is using coherent DMA only:

err = dma_set_coherent_mask(pdev-dev, DMA_BIT_MASK(32));
if (err)
handle error;

- If the driver uses both coherent DMA and streaming DMA:

err = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(32));
if (err)
handle error;

Where the _bus_ has restrictions on the available memory (iow, the
higher address bits), these should be dealt with inside the DMA API.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb 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 02/18] usb: host: xhci-plat: Add clocks support

[Russell King - ARM Linux]

On Fri, Apr 25, 2014 at 04:07:00PM +0200, Gregory CLEMENT wrote:
 +#if defined(CONFIG_HAVE_CLK)
 +static int try_enable_clk(struct platform_device *pdev)
 +{
 + struct clk *clk = devm_clk_get(pdev-dev, NULL);
 +
 + /* Not all platforms have a clk so it is not an error if the clock
 +does not exists. */
 + if (!IS_ERR(clk))
 + if (clk_prepare_enable(clk))
 + return  -ENODEV;
 + return 0;
 +}
 +
 +static int try_disable_clk(struct platform_device *pdev)
 +{
 + struct clk *clk = devm_clk_get(pdev-dev, NULL);
 +
 + /* Not all platforms have a clk so it is not an error if the clock
 +does not exists. */
 + if (!IS_ERR(clk))
 + clk_disable_unprepare(clk);
 +
 + return 0;
 +}

OMG.

You do realise that clk_get() ref-counts against the module which
provided the clock, so this is akin to an explicit leaking module
ref-counts.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ARM: v3.13-rc1: USB regression

[Russell King - ARM Linux]

On Mon, Nov 25, 2013 at 06:33:02PM +0200, Aaro Koskinen wrote:
 Hi,
 
 On Sun, Nov 24, 2013 at 10:43:59PM +, Russell King - ARM Linux wrote:
  On Mon, Nov 25, 2013 at 12:22:47AM +0200, Aaro Koskinen wrote:
   [   33.967324] ohci ohci: Coherent DMA mask 0x (pfn 
   0xe-0xe) covers a smaller range of system memory than the DMA 
   zone pfn 0x0-0x10
   
   I bisected this to 4dcfa60071b3d23f0181f27d8519f12e37cefbb9 (ARM: DMA-API:
   better handing of DMA masks for coherent allocations). Reverting that
   commit makes the USB work again fine.
 
 [...]
 
  Better would be:
  
  #define __arch_dma_to_pfn(dev, addr)\
  ({ unsigned long pfn = (addr)  PAGE_SHIFT;\
 if (is_lbus_device(dev)) \
  pfn += PHYS_PFN_OFFSET -\
  (OMAP1510_LB_OFFSET  PAGE_SHIFT); \
 pfn; \
  })
  
  Can you try that in arch/arm/mach-omap1/include/mach/memory.h please?
 
 Still doesn't work:
 
 [   33.878790] ohci ohci: Coherent DMA mask 0x (pfn 
 0xfffe-0xe) covers a smaller range of system memory than the DMA zone 
 pfn 0x0-0x10
 [   33.894019] ohci ohci: can't setup: -12

Well, that looks technically better, rather unfortunate that we end up
going to negative PFNs though.

Without that change, could you try this instead please:

 arch/arm/mm/dma-mapping.c |9 +++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index f0ea0134e5a3..a18cfc53f445 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -9,6 +9,7 @@
  *
  *  DMA uncached mapping support.
  */
+#include linux/bootmem.h
 #include linux/module.h
 #include linux/mm.h
 #include linux/gfp.h
@@ -162,6 +163,8 @@ static u64 get_coherent_dma_mask(struct device *dev)
u64 mask = (u64)DMA_BIT_MASK(32);
 
if (dev) {
+   unsigned long max_dma_pfn;
+
mask = dev-coherent_dma_mask;
 
/*
@@ -173,6 +176,8 @@ static u64 get_coherent_dma_mask(struct device *dev)
return 0;
}
 
+   max_dma_pfn = min(max_pfn, arm_dma_pfn_limit);
+
/*
 * If the mask allows for more memory than we can address,
 * and we actually have that much memory, then fail the
@@ -180,7 +185,7 @@ static u64 get_coherent_dma_mask(struct device *dev)
 */
if (sizeof(mask) != sizeof(dma_addr_t) 
mask  (dma_addr_t)~0 
-   dma_to_pfn(dev, ~0)  arm_dma_pfn_limit) {
+   dma_to_pfn(dev, ~0)  max_dma_pfn) {
dev_warn(dev, Coherent DMA mask %#llx is larger than 
dma_addr_t allows\n,
 mask);
dev_warn(dev, Driver did not use or check the return 
value from dma_set_coherent_mask()?\n);
@@ -192,7 +197,7 @@ static u64 get_coherent_dma_mask(struct device *dev)
 * fits within the allowable addresses which we can
 * allocate.
 */
-   if (dma_to_pfn(dev, mask)  arm_dma_pfn_limit) {
+   if (dma_to_pfn(dev, mask)  max_dma_pfn) {
dev_warn(dev, Coherent DMA mask %#llx (pfn %#lx-%#lx) 
covers a smaller range of system memory than the DMA zone pfn 0x0-%#lx\n,
 mask,
 dma_to_pfn(dev, 0), dma_to_pfn(dev, mask) + 1,

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

Re: ARM: v3.13-rc1: USB regression

[Russell King - ARM Linux]

On Mon, Nov 25, 2013 at 12:22:47AM +0200, Aaro Koskinen wrote:
 Hi,
 
 With 3.13-rc1, the USB OHCI probe fails on Amstrad E3 (ARM/OMAP1)
 as follows:
 
 [   33.814705] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
 [   33.821482] ohci-omap: OHCI OMAP driver
 [   33.925153] ohci ohci: OMAP OHCI
 [   33.929087] ohci ohci: new USB bus registered, assigned bus number 1
 [   33.967324] ohci ohci: Coherent DMA mask 0x (pfn 0xe-0xe) 
 covers a smaller range of system memory than the DMA zone pfn 0x0-0x10
 [   33.982292] ohci ohci: can't setup: -12
 [   33.987898] ohci ohci: USB bus 1 deregistered
 [   33.992984] ohci: probe of ohci failed with error -12
 
 I bisected this to 4dcfa60071b3d23f0181f27d8519f12e37cefbb9 (ARM: DMA-API:
 better handing of DMA masks for coherent allocations). Reverting that
 commit makes the USB work again fine.

This is because of this:

#define __arch_dma_to_pfn(dev, addr)\
({ dma_addr_t __dma = addr; \
   if (is_lbus_device(dev)) \
__dma += PHYS_OFFSET - OMAP1510_LB_OFFSET;  \
   __phys_to_pfn(__dma);\
})

dma_addr_t is 32-bit.  PHYS_OFFSET - OMAP1510_LB_OFFSET is 0xe000.
Consider what the result of passing 0x as addr into this is.

Better would be:

#define __arch_dma_to_pfn(dev, addr)\
({ unsigned long pfn = (addr)  PAGE_SHIFT;\
   if (is_lbus_device(dev)) \
pfn += PHYS_PFN_OFFSET -\
(OMAP1510_LB_OFFSET  PAGE_SHIFT); \
   pfn; \
})

Can you try that in arch/arm/mach-omap1/include/mach/memory.h please?
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: ohci-pxa27x: include linux/dma-mapping.h

[Russell King - ARM Linux]

On Fri, Nov 15, 2013 at 09:45:16AM +0100, Daniel Mack wrote:
 Include linux/dma-mapping.h to make the new functions available that are
 used since 22d9d8e83 (DMA-API: usb: use dma_set_coherent_mask()).
 
 Signed-off-by: Daniel Mack zon...@gmail.com
 ---
 I got the following error while building for PXA platforms from Linus'
 current git head:
 
 drivers/usb/host/ohci-pxa27x.c: In function ‘ohci_pxa_of_init’:
 drivers/usb/host/ohci-pxa27x.c:310:2: error: implicit declaration of function 
 ‘dma_coerce_mask_and_coherent’ [-Werror=implicit-function-declaration]
 drivers/usb/host/ohci-pxa27x.c:310:2: error: implicit declaration of function 
 ‘DMA_BIT_MASK’ [-Werror=implicit-function-declaration]

Please put the errors (and warnings) in the commit description; they're
useful documentation for others who may encounter the same problem.

 diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
 index e89ac4d..3963834 100644
 --- a/drivers/usb/host/ohci-pxa27x.c
 +++ b/drivers/usb/host/ohci-pxa27x.c
 @@ -29,6 +29,7 @@
  #include linux/platform_data/usb-ohci-pxa27x.h
  #include linux/platform_data/usb-pxa3xx-ulpi.h
  #include linux/platform_device.h
 +#include linux/dma-mapping.h

Please review the #include statements and see if you can spot a broad
pattern there, and then decide whether your placement follows the
established pattern, thanks. :)
--
To unsubscribe from this list: send the line unsubscribe linux-usb 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/2] chipidea: ci_hdrc_imx: Allow handling the clock for an USB phy/hub

[Russell King - ARM Linux]

On Thu, Nov 14, 2013 at 12:09:46AM -0200, Fabio Estevam wrote:
 @@ -107,10 +108,22 @@ static int ci_hdrc_imx_probe(struct platform_device 
 *pdev)
   return ret;
   }
  
 + data-clk_phy = devm_clk_get(pdev-dev, phy);
 + if (IS_ERR(data-clk_phy)) {
 + data-clk_phy = NULL;
 + } else {

Please stop using NULL as a indicator with functions which only return
failure as an error pointer.  Replace the above three lines with

if (!IS_ERR(data-clk_phy)) {

 + ret = clk_prepare_enable(data-clk_phy);
 + if (ret) {
 + dev_err(pdev-dev,
 + Failed to enable clk_phy: %d\n, ret);
 + goto err_clk;
 + }
 + }
 +
   data-phy = devm_usb_get_phy_by_phandle(pdev-dev, fsl,usbphy, 0);
   if (IS_ERR(data-phy)) {
   ret = PTR_ERR(data-phy);
 - goto err_clk;
 + goto err_clk_phy;
   }
  
   pdata.phy = data-phy;
 @@ -157,6 +170,9 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
  
  disable_device:
   ci_hdrc_remove_device(data-ci_pdev);
 +err_clk_phy:
 + if (data-clk_phy)

This test should not be necessary if you've nested the error cleanup.

 + clk_disable_unprepare(data-clk_phy);
  err_clk:
   clk_disable_unprepare(data-clk);
   return ret;
 @@ -168,6 +184,8 @@ static int ci_hdrc_imx_remove(struct platform_device 
 *pdev)
  
   pm_runtime_disable(pdev-dev);
   ci_hdrc_remove_device(data-ci_pdev);
 + if (data-clk_phy)

if (!IS_ERR(data-clk_phy))

 + clk_disable_unprepare(data-clk_phy);
   clk_disable_unprepare(data-clk);
  
   return 0;
 -- 
 1.8.1.2
 
 
 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 19/51] DMA-API: media: dt3155v4l: replace dma_set_mask()+dma_set_coherent_mask() with new helper

[Russell King - ARM Linux]

On Thu, Oct 31, 2013 at 09:46:40AM -0200, Mauro Carvalho Chehab wrote:
 Hi Russell,
 
 Em Mon, 30 Sep 2013 13:57:47 +0200
 Hans Verkuil hverk...@xs4all.nl escreveu:
 
  On 09/19/2013 11:44 PM, Russell King wrote:
   Replace the following sequence:
   
 dma_set_mask(dev, mask);
 dma_set_coherent_mask(dev, mask);
   
   with a call to the new helper dma_set_mask_and_coherent().
   
   Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
  
  Acked-by: Hans Verkuil hans.verk...@cisco.com
 
 Somehow, I lost your original post (I got unsubscribed on a few days 
 from all vger mailing lists at the end of september).
 
 I suspect that you want to sent this via your tree, right?

Yes please.

 If so:
 
 Acked-by: Mauro Carvalho Chehab m.che...@samsung.com

Added, thanks.

   - err = dma_set_mask(pdev-dev, DMA_BIT_MASK(32));
   - if (err)
   - return -ENODEV;
   - err = dma_set_coherent_mask(pdev-dev, DMA_BIT_MASK(32));
   + err = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(32));
 if (err)
 return -ENODEV;

One thing I've just noticed is that return should be return err not
return -ENODEV - are you okay for me to change that in this patch?

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

Re: [PATCH] usb/chipidea: fix oops on memory allocation failure

[Russell King - ARM Linux]

On Thu, Oct 17, 2013 at 01:50:17PM +0800, Peter Chen wrote:
 Below is my proposal fix for this problem: 
 
 diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
 index 42a0bd4..c1d05c4 100644
 --- a/drivers/usb/chipidea/host.c
 +++ b/drivers/usb/chipidea/host.c
 @@ -270,16 +270,18 @@ static void host_stop(struct ci_hdrc *ci)
  {
   struct usb_hcd *hcd = ci-hcd;
  
 - usb_remove_hcd(hcd);
 - usb_put_hcd(hcd);
 - if (ci-platdata-reg_vbus)
 - regulator_disable(ci-platdata-reg_vbus);
 + if (hcd) {
 + usb_remove_hcd(hcd);
 + usb_put_hcd(hcd);
 + if (ci-platdata-reg_vbus)
 + regulator_disable(ci-platdata-reg_vbus);
 + }
  }
  
  
  void ci_hdrc_host_destroy(struct ci_hdrc *ci)
  {
 - if (ci-role == CI_ROLE_HOST)
 + if (ci-role == CI_ROLE_HOST  ci-hcd)
   host_stop(ci);

If you're not calling host_stop() unless ci-hcd is setup, then you
don't need to check for that in host_stop() ?  Note that my oopsing
path is through the above function.

Anyway, Greg has already taken my patch from yesterday.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb/chipidea: fix oops on memory allocation failure

[Russell King - ARM Linux]

When CMA fails to initialize in v3.12-rc4, the chipidea driver oopses
the kernel while trying to remove and put the HCD which doesn't exist:

WARNING: CPU: 0 PID: 6 at /home/rmk/git/linux-rmk/arch/arm/mm/dma-mapping.c:511 
__dma_alloc+0x200/0x240()
coherent pool not initialised!
Modules linked in:
CPU: 0 PID: 6 Comm: kworker/u2:0 Tainted: GW3.12.0-rc4+ #56
Workqueue: deferwq deferred_probe_work_func
Backtrace: 
[c001218c] (dump_backtrace+0x0/0x10c) from [c0012328] (show_stack+0x18/0x1c)
 r6:c05fd9cc r5:01ff r4: r3:df86ad00
[c0012310] (show_stack+0x0/0x1c) from [c05f3a4c] (dump_stack+0x70/0x8c)
[c05f39dc] (dump_stack+0x0/0x8c) from [c00230a8] 
(warn_slowpath_common+0x6c/0x8c)
 r4:df883a60 r3:df86ad00
[c002303c] (warn_slowpath_common+0x0/0x8c) from [c002316c] 
(warn_slowpath_fmt+0x38/0x40)
 r8: r7:1000 r6:c083b808 r5: r4:df2efe80
[c0023134] (warn_slowpath_fmt+0x0/0x40) from [c00196bc] 
(__dma_alloc+0x200/0x240)
 r3: r2:c05fda00
[c00194bc] (__dma_alloc+0x0/0x240) from [c001982c] (arm_dma_alloc+0x88/0xa0)
[c00197a4] (arm_dma_alloc+0x0/0xa0) from [c03e2904] (ehci_setup+0x1f4/0x438)
[c03e2710] (ehci_setup+0x0/0x438) from [c03cbd60] (usb_add_hcd+0x18c/0x664)
[c03cbbd4] (usb_add_hcd+0x0/0x664) from [c03e89f4] (host_start+0xf0/0x180)
[c03e8904] (host_start+0x0/0x180) from [c03e7c34] (ci_hdrc_probe+0x360/0x670
)
 r6:df2ef410 r5: r4:df2c3010 r3:c03e8904
[c03e78d4] (ci_hdrc_probe+0x0/0x670) from [c0311044] 
(platform_drv_probe+0x20/0x24)
[c0311024] (platform_drv_probe+0x0/0x24) from [c030fcac] 
(driver_probe_device+0x9c/0x234)
...
---[ end trace c88ccaf3969e8422 ]---
Unable to handle kernel NULL pointer dereference at virtual address 0028
pgd = c0004000
[0028] *pgd=
Internal error: Oops: 17 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 6 Comm: kworker/u2:0 Tainted: GW3.12.0-rc4+ #56
Workqueue: deferwq deferred_probe_work_func
task: df86ad00 ti: df882000 task.ti: df882000
PC is at usb_remove_hcd+0x10/0x150
LR is at host_stop+0x1c/0x3c
pc : [c03cacec]lr : [c03e88e4]psr: 6013
sp : df883b50  ip : df883b78  fp : df883b74
r10: c11f4c54  r9 : c0836450  r8 : df30c400
r7 : fff4  r6 : df2ef410  r5 :   r4 : df2c3010
r3 :   r2 :   r1 : df86b0a0  r0 : 
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 10c53c7d  Table: 2f29404a  DAC: 0015
Process kworker/u2:0 (pid: 6, stack limit = 0xdf882240)
Stack: (0xdf883b50 to 0xdf884000)
...
Backtrace: 
[c03cacdc] (usb_remove_hcd+0x0/0x150) from [c03e88e4] (host_stop+0x1c/0x3c)
 r6:df2ef410 r5: r4:df2c3010
[c03e88c8] (host_stop+0x0/0x3c) from [c03e8aa0] 
(ci_hdrc_host_destroy+0x1c/0x20)
 r5: r4:df2c3010
[c03e8a84] (ci_hdrc_host_destroy+0x0/0x20) from [c03e7c80] 
(ci_hdrc_probe+0x3ac/0x670)
[c03e78d4] (ci_hdrc_probe+0x0/0x670) from [c0311044] 
(platform_drv_probe+0x20/0x24)
[c0311024] (platform_drv_probe+0x0/0x24) from [c030fcac] 
(driver_probe_device+0x9c/0x234)
[c030fc10] (driver_probe_device+0x0/0x234) from [c030ff28] 
(__device_attach+0x44/0x48)
...
---[ end trace c88ccaf3969e8423 ]---

Fix this so at least we can continue booting and get to a shell prompt.

Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
Tested-by: Russell King rmk+ker...@arm.linux.org.uk
---
 drivers/usb/chipidea/host.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 6f96795..64d7a6d 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -100,8 +100,10 @@ static void host_stop(struct ci_hdrc *ci)
 {
struct usb_hcd *hcd = ci-hcd;
 
-   usb_remove_hcd(hcd);
-   usb_put_hcd(hcd);
+   if (hcd) {
+   usb_remove_hcd(hcd);
+   usb_put_hcd(hcd);
+   }
if (ci-platdata-reg_vbus)
regulator_disable(ci-platdata-reg_vbus);
 }
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/11] usb: chipidea: Add power management support

[Russell King - ARM Linux]

On Tue, Oct 15, 2013 at 10:18:15AM +0800, Peter Chen wrote:
 So, the lessons for this topic are:
 
 - If one atomic variable's operation only includes one instruction like
 atomic_read and atomic_set, it is not meaningful for using atomic
 operation, we can just use bool instead of it.

The lesson here is that these are 100% equivalent as far as safety from
races is concerned:

a = atomic_read(v);a = v-counter;

atomic_set(v, b);  v-counter = b;

and in general, whenever atomic_read() gets used it's almost certainly
a sign of a bug.

Consider this (similar has been submitted):

a = atomic_read(v);
if (a != 0)
a += 1;

atomic_set(v, a);

and people have thought that somehow this is magically safe from races
because they're using atomic_t, and somehow that saves the universe.
The above is in fact no safer than:

a = *v;
if (a != 0)
a += 1;
*v = a;

The only thing that using atomic_* does is add a false sense of security
and a level of obfuscation to catch the unwary reviewer.

The reason is quite simple: a single access read in itself is atomic.
Either it has read the value, or it hasn't.  A single access store is
itself atomic.  Either the data has been written, or it hasn't.  The
issue is _always_ what you do around it.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/11] usb: chipidea: Add power management support

[Russell King - ARM Linux]

On Mon, Oct 14, 2013 at 03:55:48PM +0800, Peter Chen wrote:
 On Mon, Oct 14, 2013 at 10:04:58AM +0200, Lothar Waßmann wrote:
  Hi,
  
  Peter Chen wrote:
   This commit adds runtime and system power management support for
   chipidea core. The runtime pm support is controlled by glue
   layer, it can be enabled by flag CI_HDRC_SUPPORTS_RUNTIME_PM.
   
  [...]
   +#ifdef CONFIG_PM
   +static int ci_controller_suspend(struct device *dev)
   +{
   + struct ci_hdrc *ci = dev_get_drvdata(dev);
   +
   + dev_dbg(dev, at %s\n, __func__);
   +
   + if (atomic_read(ci-in_lpm))
   + return 0;
   +
  What does this 'atomic_read()' buy you over just testing/assinging a
  simple integer. Note that just because the function has 'atomic' in
  its name the sequence:
  atomic_read();
  ...
  atomic_set();
  does not magically become an atomic operation.
 
 I just want the read and set are atomic, not the operations
 between atomic_read and atomic_set.

There is nothing magical about atomic_read() or atomic_set():

#define atomic_read(v)  (*(volatile int *)(v)-counter)
#define atomic_set(v,i) (((v)-counter) = (i))

You might as well just read and write the variable directly if you're
going to do this.  The atomicity of atomic_t comes from the *other*
operations which can be done on an atomic_t, not from some magical
macro which starts with the name atomic_.

Lesson 1: whenever you see atomic_read() and atomic_set() in a patch
be very very very suspicious; it's 99% of times plainly wrong and a
sign of something being totally broken.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html