Re: [PATCH] usb: cdns3: include host-export,h for cdns3_host_init

2019-10-18 Thread Ben Dooks

On 18/10/2019 04:45, Pawel Laszczak wrote:

Hi


The cdns3_host_init() function is declared in host-export.h
but host.c does not include it. Add the include to have
the declaration present (and remove the declaration of
cdns3_host_exit which is now static).

Fixes the following sparse warning:

drivers/usb/cdns3/host.c:58:5: warning: symbol 'cdns3_host_init' was not 
declared. Should it be static?


It should not be static. It can be called from core.c file.
It will be static only if CONFIG_USB_CDNS3_HOST will not be defined and in
this case function will be declared in host-export.h  as static.


I know, this isn't being made static, that's a warning.


For me It doesn't look like driver issue.




Signed-off-by: Ben Dooks 
---
Cc: Greg Kroah-Hartman 
Cc: Pawel Laszczak 
Cc: Felipe Balbi 
Cc: "Ben Dooks
Cc: linux-usb@vger.kernel.org
---
drivers/usb/cdns3/host-export.h | 1 -
drivers/usb/cdns3/host.c| 1 +
2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/cdns3/host-export.h b/drivers/usb/cdns3/host-export.h
index b498a170b7e8..ae11810f8826 100644
--- a/drivers/usb/cdns3/host-export.h
+++ b/drivers/usb/cdns3/host-export.h
@@ -12,7 +12,6 @@
#ifdef CONFIG_USB_CDNS3_HOST

int cdns3_host_init(struct cdns3 *cdns);
-void cdns3_host_exit(struct cdns3 *cdns);


We can't remove this function. It is invoked from core.c file.
If you remove it from host-export.h then it will not be visible there.



#else

diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
index 2733a8f71fcd..ad788bf3fe4f 100644
--- a/drivers/usb/cdns3/host.c
+++ b/drivers/usb/cdns3/host.c
@@ -12,6 +12,7 @@
#include 
#include "core.h"
#include "drd.h"
+#include "host-export.h"


Why host must include this file. This function is implemented
In host.c and is used only in  core.c file .


The implementation should also have the declaration to ensure
that the implemented function matches the declared prototype in
the header



--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html


[PATCH] usb: mtu3: fix missing include of mtu3_dr.h

2019-10-17 Thread Ben Dooks (Codethink)
The declarations of ssusb_gadget_{init,exit} are
in the mtu3_dr.h file but the code does that implements
them does not include this. Add the include to fix the
following sparse warnigns:

drivers/usb/mtu3/mtu3_core.c:825:5: warning: symbol 'ssusb_gadget_init' was not 
declared. Should it be static?
drivers/usb/mtu3/mtu3_core.c:925:6: warning: symbol 'ssusb_gadget_exit' was not 
declared. Should it be static?

Signed-off-by: Ben Dooks 
---
Cc: Chunfeng Yun 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-media...@lists.infradead.org
---
 drivers/usb/mtu3/mtu3_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c
index c3d5c1206eec..9dd02160cca9 100644
--- a/drivers/usb/mtu3/mtu3_core.c
+++ b/drivers/usb/mtu3/mtu3_core.c
@@ -16,6 +16,7 @@
 #include 
 
 #include "mtu3.h"
+#include "mtu3_dr.h"
 #include "mtu3_debug.h"
 #include "mtu3_trace.h"
 
-- 
2.23.0



[PATCH] usb: host: oxu210hp-hcd: fix __iomem annotations

2019-10-17 Thread Ben Dooks (Codethink)
There are a number of places in the driver where it fails
to maintain __iomem on pointers used to access registers
so fixup the warnings by adding these in the appropriate
places.

Examples of the sparse warnings fixed:

drivers/usb/host/oxu210hp-hcd.c:686:9: warning: incorrect type in argument 2 
(different address spaces)
drivers/usb/host/oxu210hp-hcd.c:686:9:expected void volatile [noderef] 
 *addr
drivers/usb/host/oxu210hp-hcd.c:686:9:got void *
drivers/usb/host/oxu210hp-hcd.c:686:9: warning: incorrect type in argument 2 
(different address spaces)
drivers/usb/host/oxu210hp-hcd.c:686:9:expected void volatile [noderef] 
 *addr
drivers/usb/host/oxu210hp-hcd.c:686:9:got void *
drivers/usb/host/oxu210hp-hcd.c:686:9: warning: incorrect type in argument 2 
(different address spaces)
drivers/usb/host/oxu210hp-hcd.c:686:9:expected void volatile [noderef] 
 *addr
drivers/usb/host/oxu210hp-hcd.c:686:9:got void *
drivers/usb/host/oxu210hp-hcd.c:681:16: warning: incorrect type in argument 1 
(different address spaces)
drivers/usb/host/oxu210hp-hcd.c:681:16:expected void const volatile 
[noderef]  *addr
drivers/usb/host/oxu210hp-hcd.c:681:16:got void *

Signed-off-by: Ben Dooks 
---
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
---
 drivers/usb/host/oxu210hp-hcd.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index e67242e437ed..fe09b8626329 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -676,12 +676,12 @@ static int oxu_hub_control(struct usb_hcd *hcd,
  */
 
 /* Low level read/write registers functions */
-static inline u32 oxu_readl(void *base, u32 reg)
+static inline u32 oxu_readl(void __iomem *base, u32 reg)
 {
return readl(base + reg);
 }
 
-static inline void oxu_writel(void *base, u32 reg, u32 val)
+static inline void oxu_writel(void __iomem *base, u32 reg, u32 val)
 {
writel(val, base + reg);
 }
@@ -4063,7 +4063,7 @@ static const struct hc_driver oxu_hc_driver = {
  * Module stuff
  */
 
-static void oxu_configuration(struct platform_device *pdev, void *base)
+static void oxu_configuration(struct platform_device *pdev, void __iomem *base)
 {
u32 tmp;
 
@@ -4093,7 +4093,7 @@ static void oxu_configuration(struct platform_device 
*pdev, void *base)
oxu_writel(base, OXU_CHIPIRQEN_SET, OXU_USBSPHLPWUI | OXU_USBOTGLPWUI);
 }
 
-static int oxu_verify_id(struct platform_device *pdev, void *base)
+static int oxu_verify_id(struct platform_device *pdev, void __iomem *base)
 {
u32 id;
static const char * const bo[] = {
@@ -4121,7 +4121,7 @@ static int oxu_verify_id(struct platform_device *pdev, 
void *base)
 static const struct hc_driver oxu_hc_driver;
 static struct usb_hcd *oxu_create(struct platform_device *pdev,
unsigned long memstart, unsigned long memlen,
-   void *base, int irq, int otg)
+   void __iomem *base, int irq, int otg)
 {
struct device *dev = &pdev->dev;
 
@@ -4158,7 +4158,7 @@ static struct usb_hcd *oxu_create(struct platform_device 
*pdev,
 
 static int oxu_init(struct platform_device *pdev,
unsigned long memstart, unsigned long memlen,
-   void *base, int irq)
+   void __iomem *base, int irq)
 {
struct oxu_info *info = platform_get_drvdata(pdev);
struct usb_hcd *hcd;
@@ -4207,7 +4207,7 @@ static int oxu_init(struct platform_device *pdev,
 static int oxu_drv_probe(struct platform_device *pdev)
 {
struct resource *res;
-   void *base;
+   void __iomem *base;
unsigned long memstart, memlen;
int irq, ret;
struct oxu_info *info;
-- 
2.23.0



[PATCH] usb: cdns3: include host-export,h for cdns3_host_init

2019-10-17 Thread Ben Dooks (Codethink)
The cdns3_host_init() function is declared in host-export.h
but host.c does not include it. Add the include to have
the declaration present (and remove the declaration of
cdns3_host_exit which is now static).

Fixes the following sparse warning:

drivers/usb/cdns3/host.c:58:5: warning: symbol 'cdns3_host_init' was not 
declared. Should it be static?

Signed-off-by: Ben Dooks 
---
Cc: Greg Kroah-Hartman 
Cc: Pawel Laszczak 
Cc: Felipe Balbi 
Cc: "Ben Dooks
Cc: linux-usb@vger.kernel.org
---
 drivers/usb/cdns3/host-export.h | 1 -
 drivers/usb/cdns3/host.c| 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/cdns3/host-export.h b/drivers/usb/cdns3/host-export.h
index b498a170b7e8..ae11810f8826 100644
--- a/drivers/usb/cdns3/host-export.h
+++ b/drivers/usb/cdns3/host-export.h
@@ -12,7 +12,6 @@
 #ifdef CONFIG_USB_CDNS3_HOST
 
 int cdns3_host_init(struct cdns3 *cdns);
-void cdns3_host_exit(struct cdns3 *cdns);
 
 #else
 
diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
index 2733a8f71fcd..ad788bf3fe4f 100644
--- a/drivers/usb/cdns3/host.c
+++ b/drivers/usb/cdns3/host.c
@@ -12,6 +12,7 @@
 #include 
 #include "core.h"
 #include "drd.h"
+#include "host-export.h"
 
 static int __cdns3_host_init(struct cdns3 *cdns)
 {
-- 
2.23.0



Re: [PATCH] USB: host: ehci: allow tine of highspeed nak-count

2018-11-19 Thread Ben Dooks
On Fri, Nov 16, 2018 at 10:38:18AM -0500, Alan Stern wrote:
> On Fri, 16 Nov 2018, Ben Dooks wrote:
> 
> > On 14/11/18 18:47, Alan Stern wrote:
> > > On Wed, 14 Nov 2018, Ben Dooks wrote:
> > > 
> > >> From: Ben Dooks 
> > >>
> > >> At least some systems benefit with less scheduling if the NAK count
> > >> value is set higher than the default 4. For instance a Tegra3 with
> > >> an SMSC9512 showed less interrupt load when this was changed to 14.
> > > 
> > > Interesting.  Why do you think this is?  In theory, increasing the NAK
> > > count RL value should cause a higher memory bus load and perhaps a
> > > slight rise in the interrupt load (transfers will complete a little
> > > more quickly because the controller tries harder to poll the endpoints
> > > and see if they are ready).
> > 
> > I thought the NAK counter was decremented until the transfer is given
> > up on.
> 
> That's right.  So if the RL value is higher, there will be more polling
> attempts in quick succession before the NAK counter drops to 0 and the
> controller gives up.  More polling attempts in quick succession means 
> heavier memory bus usage.
> 
> > I'm going to have to go back and get some actual figures from
> > a running system as this was originally done over a year ago with the
> > SMSC9512 (IIRC) network driver.
> > 
> > >> To allow the changing of this value, add a sysfs node to each of
> > >> the controllers to allow run-time changing.
> > >>
> > >> Signed-off-by: Ben Dooks 
> > > 
> > > The patch looks okay to me.
> > 
> > I'll look at getting some tracing from the SMSC driver to see what
> > is going on.
> 
> Okay.  Should we consider the patch to be held in suspense until then?

Yes, I'm not going to have access to any of the test hardware until the
end of the week, and will re-verify my initial notes from last year.

-- 
Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.



[PATCH] usbnet: use tasklet_init() for usbnet_bh handler

2018-11-16 Thread Ben Dooks
The tasklet initialisation would be better done by tasklet_init()
instead of assuming all the fields are in an ok state by default.

Note, this does not fix any known bug.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/usbnet.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 28d76c827e70..8f7db959d319 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1704,8 +1704,7 @@ usbnet_probe (struct usb_interface *udev, const struct 
usb_device_id *prod)
skb_queue_head_init (&dev->txq);
skb_queue_head_init (&dev->done);
skb_queue_head_init(&dev->rxq_pause);
-   dev->bh.func = (void (*)(unsigned long))usbnet_bh;
-   dev->bh.data = (unsigned long)&dev->delay;
+   tasklet_init(&dev->bh, (void (*)(unsigned long))usbnet_bh, (unsigned 
long)&dev->delay);
INIT_WORK (&dev->kevent, usbnet_deferred_kevent);
init_usb_anchor(&dev->deferred);
timer_setup(&dev->delay, usbnet_bh, 0);
-- 
2.19.1



Re: [PATCH] USB: host: ehci: allow tine of highspeed nak-count

2018-11-16 Thread Ben Dooks

On 14/11/18 18:47, Alan Stern wrote:

On Wed, 14 Nov 2018, Ben Dooks wrote:


From: Ben Dooks 

At least some systems benefit with less scheduling if the NAK count
value is set higher than the default 4. For instance a Tegra3 with
an SMSC9512 showed less interrupt load when this was changed to 14.


Interesting.  Why do you think this is?  In theory, increasing the NAK
count RL value should cause a higher memory bus load and perhaps a
slight rise in the interrupt load (transfers will complete a little
more quickly because the controller tries harder to poll the endpoints
and see if they are ready).


I thought the NAK counter was decremented until the transfer is given
up on. I'm going to have to go back and get some actual figures from
a running system as this was originally done over a year ago with the
SMSC9512 (IIRC) network driver.


To allow the changing of this value, add a sysfs node to each of
the controllers to allow run-time changing.

Signed-off-by: Ben Dooks 


The patch looks okay to me.


I'll look at getting some tracing from the SMSC driver to see what
is going on.


--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html


Re: SMSC95xx driver updates (round 1)

2018-11-16 Thread Ben Dooks

On 15/11/18 18:06, woojung@microchip.com wrote:

Hi Ben,


-Original Message-
From: netdev-ow...@vger.kernel.org  On Behalf Of 
Ben Dooks
Sent: Wednesday, November 14, 2018 6:50 AM
To: net...@vger.kernel.org
Cc: oneu...@suse.com; da...@davemloft.net; linux-usb@vger.kernel.org; linux-
ker...@vger.kernel.org; steve.glendinn...@shawell.net; 
linux-ker...@lists.codethink.co.uk
Subject: SMSC95xx driver updates (round 1)

This is a series of a few driver cleanups and some fixups of the code
for the SMSC95XX driver. There have been a few reviews, and the issues
have been fixed so this should be ready for merging.

I will work on the tx-alignment and the other bits of usbnet changes
and produce at least two more patch series for this later.


Some reason, maintainer email of unglinuxdri...@microchip.com is NOT included 
in CC.
Please add it in following patch series you are creating to have a chance to 
review by
proper reviewers.

USB SMSC95XX ETHERNET DRIVER
M:  Steve Glendinning 
M:  Microchip Linux Driver Support 
L:  net...@vger.kernel.org
S:  Maintained
F:  drivers/net/usb/smsc95xx.*


Sorry, must have missed this when rebasing from v4.18 to v4.19.

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html


[PATCH] USB: host: ehci: allow tine of highspeed nak-count

2018-11-14 Thread Ben Dooks
From: Ben Dooks 

At least some systems benefit with less scheduling if the NAK count
value is set higher than the default 4. For instance a Tegra3 with
an SMSC9512 showed less interrupt load when this was changed to 14.

To allow the changing of this value, add a sysfs node to each of
the controllers to allow run-time changing.

Signed-off-by: Ben Dooks 
---
 drivers/usb/host/ehci-hcd.c   |  1 +
 drivers/usb/host/ehci-q.c |  4 +--
 drivers/usb/host/ehci-sysfs.c | 52 +--
 drivers/usb/host/ehci.h   |  1 +
 4 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 8608ac513fb7..799262951f41 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -526,6 +526,7 @@ static int ehci_init(struct usb_hcd *hcd)
hw->hw_qtd_next = EHCI_LIST_END(ehci);
ehci->async->qh_state = QH_STATE_LINKED;
hw->hw_alt_next = QTD_NEXT(ehci, ehci->async->dummy->qtd_dma);
+   ehci->nak_tune_hs = EHCI_TUNE_RL_HS;
 
/* clear interrupt enables, set irq latency */
if (log2_irq_thresh < 0 || log2_irq_thresh > 6)
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index 327630405695..ccb754893b5a 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -898,12 +898,12 @@ qh_make (
case USB_SPEED_HIGH:/* no TT involved */
info1 |= QH_HIGH_SPEED;
if (type == PIPE_CONTROL) {
-   info1 |= (EHCI_TUNE_RL_HS << 28);
+   info1 |= ehci->nak_tune_hs << 28;
info1 |= 64 << 16;  /* usb2 fixed maxpacket */
info1 |= QH_TOGGLE_CTL; /* toggle from qtd */
info2 |= (EHCI_TUNE_MULT_HS << 30);
} else if (type == PIPE_BULK) {
-   info1 |= (EHCI_TUNE_RL_HS << 28);
+   info1 |= ehci->nak_tune_hs << 28;
/* The USB spec says that high speed bulk endpoints
 * always use 512 byte maxpacket.  But some device
 * vendors decided to ignore that, and MSFT is happy
diff --git a/drivers/usb/host/ehci-sysfs.c b/drivers/usb/host/ehci-sysfs.c
index 8f75cb7b197c..d710d35282a6 100644
--- a/drivers/usb/host/ehci-sysfs.c
+++ b/drivers/usb/host/ehci-sysfs.c
@@ -145,19 +145,66 @@ static ssize_t uframe_periodic_max_store(struct device 
*dev,
 }
 static DEVICE_ATTR_RW(uframe_periodic_max);
 
+static ssize_t nak_tune_hs_show(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct ehci_hcd *ehci;
+
+   ehci = hcd_to_ehci(dev_get_drvdata(dev));
+   return scnprintf(buf, PAGE_SIZE, "%d\n", ehci->nak_tune_hs);
+}
+
+static ssize_t nak_tune_hs_store(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf, size_t count)
+{
+   struct ehci_hcd *ehci;
+   unsignedval;
+   unsigned long   flags;
+
+   ehci = hcd_to_ehci(dev_get_drvdata(dev));
+
+   if (kstrtouint(buf, 0, &val) < 0)
+   return -EINVAL;
+
+   if (val >= 15) {
+   ehci_info(ehci, "invalid value for nak_tune_hs (%d)\n", val);
+   return -EINVAL;
+   }
+
+   spin_lock_irqsave (&ehci->lock, flags);
+   ehci->nak_tune_hs = val;
+   spin_unlock_irqrestore (&ehci->lock, flags);
+   return count;
+}
+
+static DEVICE_ATTR_RW(nak_tune_hs);
 
 static inline int create_sysfs_files(struct ehci_hcd *ehci)
 {
struct device   *controller = ehci_to_hcd(ehci)->self.controller;
int i = 0;
 
+   i = device_create_file(controller, &dev_attr_nak_tune_hs);
+   if (i)
+   goto out;
+
+   i = device_create_file(controller, &dev_attr_uframe_periodic_max);
+   if (i)
+   goto out_nak;
+
/* with integrated TT there is no companion! */
if (!ehci_is_TDI(ehci))
i = device_create_file(controller, &dev_attr_companion);
if (i)
-   goto out;
+   goto out_all;
 
-   i = device_create_file(controller, &dev_attr_uframe_periodic_max);
+   return 0;
+out_all:
+   device_remove_file(controller, &dev_attr_uframe_periodic_max);
+out_nak:
+   device_remove_file(controller, &dev_attr_nak_tune_hs);
 out:
return i;
 }
@@ -170,5 +217,6 @@ static inline void remove_sysfs_files(struct ehci_hcd *ehci)
if (!ehci_is_TDI(ehci))
device_remove_file(controller, &dev_attr_companion);
 
+   device_remove_file(controller, &dev_attr_nak_tune_hs);
device_remo

[PATCH 1/4] usbnet: smsc95xx: fix rx packet alignment

2018-11-14 Thread Ben Dooks
The smsc95xx driver already takes into account the NET_IP_ALIGN
parameter when setting up the receive packet data, which means
we do not need to worry about aligning the packets in the usbnet
driver.

Adding the EVENT_NO_IP_ALIGN means that the IPv4 header is now
passed to the ip_rcv() routine with the start on an aligned address.

Tested on Raspberry Pi B3.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/smsc95xx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 06b4d290784d..401ec9feb495 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1292,6 +1292,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct 
usb_interface *intf)
dev->net->features |= NETIF_F_RXCSUM;
 
dev->net->hw_features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM;
+   set_bit(EVENT_NO_IP_ALIGN, &dev->flags);
 
smsc95xx_init_mac_address(dev);
 
-- 
2.19.1



SMSC95xx driver updates (round 1)

2018-11-14 Thread Ben Dooks
This is a series of a few driver cleanups and some fixups of the code
for the SMSC95XX driver. There have been a few reviews, and the issues
have been fixed so this should be ready for merging.

I will work on the tx-alignment and the other bits of usbnet changes
and produce at least two more patch series for this later.




[PATCH 2/4] usbnet: smsc95xx: simplify tx_fixup code

2018-11-14 Thread Ben Dooks
The smsc95xx_tx_fixup is doing multiple calls to skb_push() to
put an 8-byte command header onto the packet. It would be easier
to do one skb_push() and then copy the data in once the push is
done.

We also make the code smaller by using proper unaligned puts for
the header. This merges in the CPU to LE32 conversion as well and
makes the whole sequence easier to understand hopefully.

Signed-off-by: Ben Dooks 
---
Since v1:
- Add alignment changes using put_unaligned_le32()
---
 drivers/net/usb/smsc95xx.c | 28 +---
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 401ec9feb495..50e97a159500 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -2006,6 +2006,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
u32 tx_cmd_a, tx_cmd_b;
+   void *ptr;
 
/* We do not advertise SG, so skbs should be already linearized */
BUG_ON(skb_shinfo(skb)->nr_frags);
@@ -2019,6 +2020,9 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
return NULL;
}
 
+   tx_cmd_b = (u32)skb->len;
+   tx_cmd_a = tx_cmd_b | TX_CMD_A_FIRST_SEG_ | TX_CMD_A_LAST_SEG_;
+
if (csum) {
if (skb->len <= 45) {
/* workaround - hardware tx checksum does not work
@@ -2032,24 +2036,18 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
csum = false;
} else {
u32 csum_preamble = smsc95xx_calc_csum_preamble(skb);
-   skb_push(skb, 4);
-   cpu_to_le32s(&csum_preamble);
-   memcpy(skb->data, &csum_preamble, 4);
+   ptr = skb_push(skb, 4);
+   put_unaligned_le32(csum_preamble, ptr);
+
+   tx_cmd_a += 4;
+   tx_cmd_b += 4;
+   tx_cmd_b |= TX_CMD_B_CSUM_ENABLE;
}
}
 
-   skb_push(skb, 4);
-   tx_cmd_b = (u32)(skb->len - 4);
-   if (csum)
-   tx_cmd_b |= TX_CMD_B_CSUM_ENABLE;
-   cpu_to_le32s(&tx_cmd_b);
-   memcpy(skb->data, &tx_cmd_b, 4);
-
-   skb_push(skb, 4);
-   tx_cmd_a = (u32)(skb->len - 8) | TX_CMD_A_FIRST_SEG_ |
-   TX_CMD_A_LAST_SEG_;
-   cpu_to_le32s(&tx_cmd_a);
-   memcpy(skb->data, &tx_cmd_a, 4);
+   ptr = skb_push(skb, 8);
+   put_unaligned_le32(tx_cmd_a, ptr);
+   put_unaligned_le32(tx_cmd_b, ptr+4);
 
return skb;
 }
-- 
2.19.1



[PATCH 3/4] usbnet: smsc95xx: fix memcpy for accessing rx-data

2018-11-14 Thread Ben Dooks
Change the RX code to use get_unaligned_le32() instead of the combo
of memcpy and cpu_to_le32s(&var).

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/smsc95xx.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 50e97a159500..8f7c473f3260 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -618,9 +618,7 @@ static void smsc95xx_status(struct usbnet *dev, struct urb 
*urb)
return;
}
 
-   memcpy(&intdata, urb->transfer_buffer, 4);
-   le32_to_cpus(&intdata);
-
+   intdata = get_unaligned_le32(urb->transfer_buffer);
netif_dbg(dev, link, dev->net, "intdata: 0x%08X\n", intdata);
 
if (intdata & INT_ENP_PHY_INT_)
@@ -1922,8 +1920,7 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct 
sk_buff *skb)
unsigned char *packet;
u16 size;
 
-   memcpy(&header, skb->data, sizeof(header));
-   le32_to_cpus(&header);
+   header = get_unaligned_le32(skb->data);
skb_pull(skb, 4 + NET_IP_ALIGN);
packet = skb->data;
 
-- 
2.19.1



[PATCH 4/4] usbnet: smsc95xx: check for csum being in last four bytes

2018-11-14 Thread Ben Dooks
The manual states that the checksum cannot lie in the last DWORD of the
transmission, so add a basic check for this and fall back to software
checksumming the packet.

This only seems to trigger for ACK packets with no options or data to
return to the other end, and the use of the tx-alignment option makes
it more likely to happen.

Signed-off-by: Ben Dooks 
---
Fixes for v2:
- Fix spelling of check at Sergei's suggestion
- Move skb->len check into smsc95xx_can_tx_checksum()
- Change name of smsc95xx_can_checksum to explicitly say it is tx-csum
---
 drivers/net/usb/smsc95xx.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 8f7c473f3260..cc78ef78cc93 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1997,6 +1997,23 @@ static u32 smsc95xx_calc_csum_preamble(struct sk_buff 
*skb)
return (high_16 << 16) | low_16;
 }
 
+/* The TX CSUM won't work if the checksum lies in the last 4 bytes of the
+ * transmission. This is fairly unlikely, only seems to trigger with some
+ * short TCP ACK packets sent.
+ *
+ * Note, this calculation should probably check for the alignment of the
+ * data as well, but a straight check for csum being in the last four bytes
+ * of the packet should be ok for now.
+*/
+static bool smsc95xx_can_tx_checksum(struct sk_buff *skb)
+{
+   unsigned int len = skb->len - skb_checksum_start_offset(skb);
+
+   if (skb->len <= 45)
+  return false;
+   return skb->csum_offset < (len - (4 + 1));
+}
+
 static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 struct sk_buff *skb, gfp_t flags)
 {
@@ -2021,7 +2038,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
tx_cmd_a = tx_cmd_b | TX_CMD_A_FIRST_SEG_ | TX_CMD_A_LAST_SEG_;
 
if (csum) {
-   if (skb->len <= 45) {
+   if (!smsc95xx_can_tx_checksum(skb)) {
/* workaround - hardware tx checksum does not work
 * properly with extremely small packets */
long csstart = skb_checksum_start_offset(skb);
-- 
2.19.1



Re: [PATCH 4/7] net: qmi_wwan: add usbnet -> priv function

2018-10-12 Thread Ben Dooks

On 12/10/18 11:44, Bjørn Mork wrote:

Ben Dooks  writes:


+static inline struct qmi_wwan_state *usbnet_to_qmi(struct usbnet *usb)
+{
+   return (void *) &usb->data;
+}



checkpatch doesn't like this, and it is also inconsistent with the
coding style elsewhere in the driver.


Thank you for pointing this out, will fix in v2.


CHECK: No space is necessary after a cast
#30: FILE: drivers/net/usb/qmi_wwan.c:81:
+   return (void *) &usb->data;

total: 0 errors, 0 warnings, 1 checks, 115 lines checked


And as for consistency:  I realize that "dev" is a very generic name,
but it's used consistendly for all struct usbnet pointers in the driver:


Ok, I'll change it.



bjorn@miraculix:/usr/local/src/git/linux$ git grep 'struct usbnet' 
drivers/net/usb/qmi_wwan.c
drivers/net/usb/qmi_wwan.c:static struct net_device *qmimux_find_dev(struct 
usbnet *dev, u8 mux_id)
drivers/net/usb/qmi_wwan.c:static bool qmimux_has_slaves(struct usbnet *dev)
drivers/net/usb/qmi_wwan.c:static int qmimux_rx_fixup(struct usbnet *dev, 
struct sk_buff *skb)
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(net);
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(to_net_dev(d));
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(to_net_dev(d));
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(to_net_dev(d));
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(to_net_dev(d));
drivers/net/usb/qmi_wwan.c:static int qmi_wwan_rx_fixup(struct usbnet *dev, 
struct sk_buff *skb)
drivers/net/usb/qmi_wwan.c:static int qmi_wwan_manage_power(struct usbnet *dev, 
int on)
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = usb_get_intfdata(intf);
drivers/net/usb/qmi_wwan.c:static int qmi_wwan_register_subdriver(struct usbnet 
*dev)
drivers/net/usb/qmi_wwan.c:static int qmi_wwan_change_dtr(struct usbnet *dev, 
bool on)
drivers/net/usb/qmi_wwan.c:static int qmi_wwan_bind(struct usbnet *dev, struct 
usb_interface *intf)
drivers/net/usb/qmi_wwan.c: BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) 
<
drivers/net/usb/qmi_wwan.c:static void qmi_wwan_unbind(struct usbnet *dev, 
struct usb_interface *intf)
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = usb_get_intfdata(intf);
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = usb_get_intfdata(intf);
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = usb_get_intfdata(intf);

The name was chosen to be consistent with the cdc_ether usage. I'd
prefer it if this function could use the "dev" name, to avoid confusing
things unnecessarly with yet another generic name like "usb". Which
isn't used anywhere else in the driver, and could just as easily refer
to a struct usb_device or struct usb_interface.


Ok, should be fairly easy to change this.


(yes, I know there are a couple of "struct usb_device *dev" cases. But
I'd rather see those renamed TBH)


I'd rather not touch that at the moment, this is already gaining complexity.


I also don't see why this function should be qmi_wwan specific. No need
to duplicate the same function in every minidriver, when you can do with
two generic helpers (maybe with more meaningful names):


I personally prefer the return type to be specifically the private
data type for the driver. It makes it a bit easier to spot when
you don't get the cast right.

The functions below are the idea I am working towards for the
usbnet driver, I was trying to avoid doing everything in one
go. Making the accessor functions means the next round of changes
can be much smaller, touching 1-2 areas per driver.


static inline void *usbnet_priv(const struct usbnet *dev)
{
 return (void *)&dev->data;
}

static inline void *usbnet_priv0(const struct usbnet * *dev)
{
return (void *)dev->data[0];
}


This is probably the end-game of the patch series, just need to
look at a couple of issues and see if there's anything better
than can be done.

I might also add a usbnet_set_privdata(dev, data) or something
like usbnet_alloc_privdata(dev, sizeof(data).

Note, there's also the fun here of the usbnet having private data
for itself, and these sub-drivers also having their own private
data... this probably needs to be named carefully.


Please fix the checkpatch warning and consider the other comments.

Personally I don't really think it makes the code any easier to read.
But if you want it, then I'm fine this. I guess I've grown too used to
seeing those casts ;-)


--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html


[PATCH 2/7] net: asix: add usbnet -> priv function

2018-10-12 Thread Ben Dooks
There are a number of places in the asix driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/asix.h |  5 +
 drivers/net/usb/asix_common.c  |  4 ++--
 drivers/net/usb/asix_devices.c | 16 
 drivers/net/usb/ax88172a.c |  2 +-
 4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index 9a4171b90947..4bbb52669ac4 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -197,6 +197,11 @@ extern const struct driver_info ax88172a_info;
 /* ASIX specific flags */
 #define FLAG_EEPROM_MAC(1UL << 0)  /* init device MAC from 
eeprom */
 
+static inline struct asix_data *usbnet_to_asix(struct usbnet *usb)
+{
+   return (struct asix_data *)&usb->data;
+}
+
 int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
  u16 size, void *data, int in_pm);
 
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index e95dd12edec4..1fd650faca5b 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -418,7 +418,7 @@ int asix_write_gpio(struct usbnet *dev, u16 value, int 
sleep, int in_pm)
 void asix_set_multicast(struct net_device *net)
 {
struct usbnet *dev = netdev_priv(net);
-   struct asix_data *data = (struct asix_data *)&dev->data;
+   struct asix_data *data = usbnet_to_asix(dev);
u16 rx_ctl = AX_DEFAULT_RX_CTL;
 
if (net->flags & IFF_PROMISC) {
@@ -751,7 +751,7 @@ void asix_get_drvinfo(struct net_device *net, struct 
ethtool_drvinfo *info)
 int asix_set_mac_address(struct net_device *net, void *p)
 {
struct usbnet *dev = netdev_priv(net);
-   struct asix_data *data = (struct asix_data *)&dev->data;
+   struct asix_data *data = usbnet_to_asix(dev);
struct sockaddr *addr = p;
 
if (netif_running(net))
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index b654f05b2ccd..8e387f06dccf 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -144,7 +144,7 @@ static const struct ethtool_ops ax88172_ethtool_ops = {
 static void ax88172_set_multicast(struct net_device *net)
 {
struct usbnet *dev = netdev_priv(net);
-   struct asix_data *data = (struct asix_data *)&dev->data;
+   struct asix_data *data = usbnet_to_asix(dev);
u8 rx_ctl = 0x8c;
 
if (net->flags & IFF_PROMISC) {
@@ -332,7 +332,7 @@ static int ax88772_link_reset(struct usbnet *dev)
 
 static int ax88772_reset(struct usbnet *dev)
 {
-   struct asix_data *data = (struct asix_data *)&dev->data;
+   struct asix_data *data = usbnet_to_asix(dev);
int ret;
 
/* Rewrite MAC address */
@@ -359,7 +359,7 @@ static int ax88772_reset(struct usbnet *dev)
 
 static int ax88772_hw_reset(struct usbnet *dev, int in_pm)
 {
-   struct asix_data *data = (struct asix_data *)&dev->data;
+   struct asix_data *data = usbnet_to_asix(dev);
int ret, embd_phy;
u16 rx_ctl;
 
@@ -454,7 +454,7 @@ static int ax88772_hw_reset(struct usbnet *dev, int in_pm)
 
 static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
 {
-   struct asix_data *data = (struct asix_data *)&dev->data;
+   struct asix_data *data = usbnet_to_asix(dev);
int ret, embd_phy;
u16 rx_ctl, phy14h, phy15h, phy16h;
u8 chipcode = 0;
@@ -795,7 +795,7 @@ static const struct ethtool_ops ax88178_ethtool_ops = {
 
 static int marvell_phy_init(struct usbnet *dev)
 {
-   struct asix_data *data = (struct asix_data *)&dev->data;
+   struct asix_data *data = usbnet_to_asix(dev);
u16 reg;
 
netdev_dbg(dev->net, "marvell_phy_init()\n");
@@ -827,7 +827,7 @@ static int marvell_phy_init(struct usbnet *dev)
 
 static int rtl8211cl_phy_init(struct usbnet *dev)
 {
-   struct asix_data *data = (struct asix_data *)&dev->data;
+   struct asix_data *data = usbnet_to_asix(dev);
 
netdev_dbg(dev->net, "rtl8211cl_phy_init()\n");
 
@@ -874,7 +874,7 @@ static int marvell_led_status(struct usbnet *dev, u16 speed)
 
 static int ax88178_reset(struct usbnet *dev)
 {
-   struct asix_data *data = (struct asix_data *)&dev->data;
+   struct asix_data *data = usbnet_to_asix(dev);
int ret;
__le16 eeprom;
u8 status;
@@ -962,7 +962,7 @@ static int ax88178_link_reset(struct usbnet *dev)
 {
u16 mode;
struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
-   struct asix_data *data = (struct asix_data *)&dev->data;
+   struct asix_data *data = usbnet_to_asix(dev);
u32 speed;
 
netdev_dbg(dev->net, "ax88178_link_reset()\n");
diff --git a/driver

[PATCH 6/7] net: huawei_cdc_ncm: add usbnet -> priv function

2018-10-12 Thread Ben Dooks
There are a number of places in the huawei driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.

Signed-off-by: Ben Dooks 
---
---
 drivers/net/usb/huawei_cdc_ncm.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
index 63f28908afda..d290b8c318be 100644
--- a/drivers/net/usb/huawei_cdc_ncm.c
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -38,9 +38,14 @@ struct huawei_cdc_ncm_state {
struct usb_interface *data;
 };
 
+static inline struct huawei_cdc_ncm_state *usbnet_to_state(struct usbnet *usb)
+{
+   return (struct huawei_cdc_ncm_state *)&usb->data;
+}
+
 static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
 {
-   struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+   struct huawei_cdc_ncm_state *drvstate = usbnet_to_state(usbnet_dev);
int rv;
 
if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) ||
@@ -72,7 +77,7 @@ static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev,
struct cdc_ncm_ctx *ctx;
struct usb_driver *subdriver = ERR_PTR(-ENODEV);
int ret = -ENODEV;
-   struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+   struct huawei_cdc_ncm_state *drvstate = usbnet_to_state(usbnet_dev);
int drvflags = 0;
 
/* altsetting should always be 1 for NCM devices - so we hard-coded
@@ -119,7 +124,7 @@ static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev,
 static void huawei_cdc_ncm_unbind(struct usbnet *usbnet_dev,
  struct usb_interface *intf)
 {
-   struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+   struct huawei_cdc_ncm_state *drvstate = usbnet_to_state(usbnet_dev);
struct cdc_ncm_ctx *ctx = drvstate->ctx;
 
if (drvstate->subdriver && drvstate->subdriver->disconnect)
@@ -134,7 +139,7 @@ static int huawei_cdc_ncm_suspend(struct usb_interface 
*intf,
 {
int ret = 0;
struct usbnet *usbnet_dev = usb_get_intfdata(intf);
-   struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+   struct huawei_cdc_ncm_state *drvstate = usbnet_to_state(usbnet_dev);
struct cdc_ncm_ctx *ctx = drvstate->ctx;
 
if (ctx == NULL) {
@@ -161,7 +166,7 @@ static int huawei_cdc_ncm_resume(struct usb_interface *intf)
 {
int ret = 0;
struct usbnet *usbnet_dev = usb_get_intfdata(intf);
-   struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+   struct huawei_cdc_ncm_state *drvstate = usbnet_to_state(usbnet_dev);
bool callsub;
struct cdc_ncm_ctx *ctx = drvstate->ctx;
 
-- 
2.19.1



[PATCH 1/7] net: smsc75xx: add usbnet -> priv function

2018-10-12 Thread Ben Dooks
There are a number of places in the smsc75xx driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/smsc75xx.c | 31 ++-
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 05553d252446..6d12fcd9b4b0 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -73,6 +73,11 @@ struct smsc75xx_priv {
u8 suspend_flags;
 };
 
+static inline struct smsc75xx_priv *usbnet_to_smsc(struct usbnet *dev)
+{
+   return (struct smsc75xx_priv *)dev->data[0];
+}
+
 struct usb_context {
struct usb_ctrlrequest req;
struct usbnet *dev;
@@ -469,7 +474,7 @@ static int smsc75xx_dataport_wait_not_busy(struct usbnet 
*dev)
 static int smsc75xx_dataport_write(struct usbnet *dev, u32 ram_select, u32 
addr,
   u32 length, u32 *buf)
 {
-   struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+   struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 dp_sel;
int i, ret;
 
@@ -553,7 +558,7 @@ static void smsc75xx_deferred_multicast_write(struct 
work_struct *param)
 static void smsc75xx_set_multicast(struct net_device *netdev)
 {
struct usbnet *dev = netdev_priv(netdev);
-   struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+   struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
unsigned long flags;
int i;
 
@@ -718,7 +723,7 @@ static void smsc75xx_ethtool_get_wol(struct net_device *net,
 struct ethtool_wolinfo *wolinfo)
 {
struct usbnet *dev = netdev_priv(net);
-   struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+   struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
 
wolinfo->supported = SUPPORTED_WAKE;
wolinfo->wolopts = pdata->wolopts;
@@ -728,7 +733,7 @@ static int smsc75xx_ethtool_set_wol(struct net_device *net,
struct ethtool_wolinfo *wolinfo)
 {
struct usbnet *dev = netdev_priv(net);
-   struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+   struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
int ret;
 
pdata->wolopts = wolinfo->wolopts & SUPPORTED_WAKE;
@@ -945,7 +950,7 @@ static int smsc75xx_set_features(struct net_device *netdev,
netdev_features_t features)
 {
struct usbnet *dev = netdev_priv(netdev);
-   struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+   struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
unsigned long flags;
int ret;
 
@@ -1051,7 +1056,7 @@ static int smsc75xx_phy_gig_workaround(struct usbnet *dev)
 
 static int smsc75xx_reset(struct usbnet *dev)
 {
-   struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+   struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 buf;
int ret = 0, timeout;
 
@@ -1515,7 +1520,7 @@ static int smsc75xx_bind(struct usbnet *dev, struct 
usb_interface *intf)
 
 static void smsc75xx_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
-   struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+   struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
if (pdata) {
netif_dbg(dev, ifdown, dev->net, "free pdata\n");
kfree(pdata);
@@ -1571,7 +1576,7 @@ static int smsc75xx_write_wuff(struct usbnet *dev, int 
filter, u32 wuf_cfg,
 
 static int smsc75xx_enter_suspend0(struct usbnet *dev)
 {
-   struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+   struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 val;
int ret;
 
@@ -1597,7 +1602,7 @@ static int smsc75xx_enter_suspend0(struct usbnet *dev)
 
 static int smsc75xx_enter_suspend1(struct usbnet *dev)
 {
-   struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+   struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 val;
int ret;
 
@@ -1633,7 +1638,7 @@ static int smsc75xx_enter_suspend1(struct usbnet *dev)
 
 static int smsc75xx_enter_suspend2(struct usbnet *dev)
 {
-   struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+   struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 val;
int ret;
 
@@ -1659,7 +1664,7 @@ static int smsc75xx_enter_suspend2(struct usbnet *dev)
 
 static int smsc75xx_enter_suspend3(struct usbnet *dev)
 {
-   struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+   struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 val;
int ret;
 
@@ -1794,7 +1799,7 @@ static int smsc75xx_autosuspend(struct usbnet *dev, u

usbnet private-data accessor functions

2018-10-12 Thread Ben Dooks
I have been looking at the usbnet drivers and the possibility of some
code cleanups. One of the things I've found is that changing the way
the drivers use the private data with the usbnet structure is often
hand-coded each time is needed.

An easy cleanp (and making it easier in the future to change the
access method) would be to add an inline conversion function to
each driver so that it is done in one place.

Future work would be to look at the usbnet.data and see if it could
be changed (such as moving to a union, allocation of the data after
the usbnet structure, or similar).




[PATCH 3/7] net: usb: asix88179_178a: add usbnet -> priv function

2018-10-12 Thread Ben Dooks
There are a number of places in the asix88179_178a driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/ax88179_178a.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 9e8ad372f419..f4b64e7e7706 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -196,6 +196,11 @@ static const struct {
{7, 0xcc, 0x4c, 0x18, 8},
 };
 
+static inline struct ax88179_data *usbnet_to_ax(struct usbnet *usb)
+{
+   return (struct ax88179_data *)usb->data;
+}
+
 static int __ax88179_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
  u16 size, void *data, int in_pm)
 {
@@ -678,7 +683,7 @@ ax88179_ethtool_set_eee(struct usbnet *dev, struct 
ethtool_eee *data)
 static int ax88179_chk_eee(struct usbnet *dev)
 {
struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
-   struct ax88179_data *priv = (struct ax88179_data *)dev->data;
+   struct ax88179_data *priv = usbnet_to_ax(dev);
 
mii_ethtool_gset(&dev->mii, &ecmd);
 
@@ -781,7 +786,7 @@ static void ax88179_enable_eee(struct usbnet *dev)
 static int ax88179_get_eee(struct net_device *net, struct ethtool_eee *edata)
 {
struct usbnet *dev = netdev_priv(net);
-   struct ax88179_data *priv = (struct ax88179_data *)dev->data;
+   struct ax88179_data *priv = usbnet_to_ax(dev);
 
edata->eee_enabled = priv->eee_enabled;
edata->eee_active = priv->eee_active;
@@ -792,7 +797,7 @@ static int ax88179_get_eee(struct net_device *net, struct 
ethtool_eee *edata)
 static int ax88179_set_eee(struct net_device *net, struct ethtool_eee *edata)
 {
struct usbnet *dev = netdev_priv(net);
-   struct ax88179_data *priv = (struct ax88179_data *)dev->data;
+   struct ax88179_data *priv = usbnet_to_ax(dev);
int ret = -EOPNOTSUPP;
 
priv->eee_enabled = edata->eee_enabled;
@@ -841,7 +846,7 @@ static const struct ethtool_ops ax88179_ethtool_ops = {
 static void ax88179_set_multicast(struct net_device *net)
 {
struct usbnet *dev = netdev_priv(net);
-   struct ax88179_data *data = (struct ax88179_data *)dev->data;
+   struct ax88179_data *data = usbnet_to_ax(dev);
u8 *m_filter = ((u8 *)dev->data) + 12;
 
data->rxctl = (AX_RX_CTL_START | AX_RX_CTL_AB | AX_RX_CTL_IPE);
@@ -1228,7 +1233,7 @@ static int ax88179_bind(struct usbnet *dev, struct 
usb_interface *intf)
u8 buf[5];
u16 *tmp16;
u8 *tmp;
-   struct ax88179_data *ax179_data = (struct ax88179_data *)dev->data;
+   struct ax88179_data *ax179_data = usbnet_to_ax(dev);
struct ethtool_eee eee_data;
 
usbnet_get_endpoints(dev, intf);
@@ -1458,7 +1463,7 @@ ax88179_tx_fixup(struct usbnet *dev, struct sk_buff *skb, 
gfp_t flags)
 
 static int ax88179_link_reset(struct usbnet *dev)
 {
-   struct ax88179_data *ax179_data = (struct ax88179_data *)dev->data;
+   struct ax88179_data *ax179_data = usbnet_to_ax(dev);
u8 tmp[5], link_sts;
u16 mode, tmp16, delay = HZ / 10;
u32 tmp32 = 0x4000;
@@ -1533,7 +1538,7 @@ static int ax88179_reset(struct usbnet *dev)
u8 buf[5];
u16 *tmp16;
u8 *tmp;
-   struct ax88179_data *ax179_data = (struct ax88179_data *)dev->data;
+   struct ax88179_data *ax179_data = usbnet_to_ax(dev);
struct ethtool_eee eee_data;
 
tmp16 = (u16 *)buf;
-- 
2.19.1



[PATCH 4/7] net: qmi_wwan: add usbnet -> priv function

2018-10-12 Thread Ben Dooks
There are a number of places in the qmi_wwan driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/qmi_wwan.c | 31 ++-
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 533b6fb8d923..45930758a945 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -76,6 +76,11 @@ struct qmimux_priv {
u8 mux_id;
 };
 
+static inline struct qmi_wwan_state *usbnet_to_qmi(struct usbnet *usb)
+{
+   return (void *) &usb->data;
+}
+
 static int qmimux_open(struct net_device *dev)
 {
struct qmimux_priv *priv = netdev_priv(dev);
@@ -253,7 +258,7 @@ static void qmimux_unregister_device(struct net_device *dev)
 static void qmi_wwan_netdev_setup(struct net_device *net)
 {
struct usbnet *dev = netdev_priv(net);
-   struct qmi_wwan_state *info = (void *)&dev->data;
+   struct qmi_wwan_state *info = usbnet_to_qmi(dev);
 
if (info->flags & QMI_WWAN_FLAG_RAWIP) {
net->header_ops  = NULL;  /* No header */
@@ -276,7 +281,7 @@ static void qmi_wwan_netdev_setup(struct net_device *net)
 static ssize_t raw_ip_show(struct device *d, struct device_attribute *attr, 
char *buf)
 {
struct usbnet *dev = netdev_priv(to_net_dev(d));
-   struct qmi_wwan_state *info = (void *)&dev->data;
+   struct qmi_wwan_state *info = usbnet_to_qmi(dev);
 
return sprintf(buf, "%c\n", info->flags & QMI_WWAN_FLAG_RAWIP ? 'Y' : 
'N');
 }
@@ -284,7 +289,7 @@ static ssize_t raw_ip_show(struct device *d, struct 
device_attribute *attr, char
 static ssize_t raw_ip_store(struct device *d,  struct device_attribute *attr, 
const char *buf, size_t len)
 {
struct usbnet *dev = netdev_priv(to_net_dev(d));
-   struct qmi_wwan_state *info = (void *)&dev->data;
+   struct qmi_wwan_state *info = usbnet_to_qmi(dev);
bool enable;
int ret;
 
@@ -346,7 +351,7 @@ static ssize_t add_mux_show(struct device *d, struct 
device_attribute *attr, cha
 static ssize_t add_mux_store(struct device *d,  struct device_attribute *attr, 
const char *buf, size_t len)
 {
struct usbnet *dev = netdev_priv(to_net_dev(d));
-   struct qmi_wwan_state *info = (void *)&dev->data;
+   struct qmi_wwan_state *info = usbnet_to_qmi(dev);
u8 mux_id;
int ret;
 
@@ -391,7 +396,7 @@ static ssize_t del_mux_show(struct device *d, struct 
device_attribute *attr, cha
 static ssize_t del_mux_store(struct device *d,  struct device_attribute *attr, 
const char *buf, size_t len)
 {
struct usbnet *dev = netdev_priv(to_net_dev(d));
-   struct qmi_wwan_state *info = (void *)&dev->data;
+   struct qmi_wwan_state *info = usbnet_to_qmi(dev);
struct net_device *del_dev;
u8 mux_id;
int ret = 0;
@@ -468,7 +473,7 @@ static const u8 buggy_fw_addr[ETH_ALEN] = {0x00, 0xa0, 
0xc6, 0x00, 0x00, 0x00};
  */
 static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
-   struct qmi_wwan_state *info = (void *)&dev->data;
+   struct qmi_wwan_state *info = usbnet_to_qmi(dev);
bool rawip = info->flags & QMI_WWAN_FLAG_RAWIP;
__be16 proto;
 
@@ -555,7 +560,7 @@ static const struct net_device_ops qmi_wwan_netdev_ops = {
  */
 static int qmi_wwan_manage_power(struct usbnet *dev, int on)
 {
-   struct qmi_wwan_state *info = (void *)&dev->data;
+   struct qmi_wwan_state *info = usbnet_to_qmi(dev);
int rv;
 
dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__,
@@ -589,7 +594,7 @@ static int qmi_wwan_register_subdriver(struct usbnet *dev)
 {
int rv;
struct usb_driver *subdriver = NULL;
-   struct qmi_wwan_state *info = (void *)&dev->data;
+   struct qmi_wwan_state *info = usbnet_to_qmi(dev);
 
/* collect bulk endpoints */
rv = usbnet_get_endpoints(dev, info->data);
@@ -651,7 +656,7 @@ static int qmi_wwan_bind(struct usbnet *dev, struct 
usb_interface *intf)
struct usb_cdc_union_desc *cdc_union;
struct usb_cdc_ether_desc *cdc_ether;
struct usb_driver *driver = driver_of(intf);
-   struct qmi_wwan_state *info = (void *)&dev->data;
+   struct qmi_wwan_state *info = usbnet_to_qmi(dev);
struct usb_cdc_parsed_header hdr;
 
BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) <
@@ -746,7 +751,7 @@ static int qmi_wwan_bind(struct usbnet *dev, struct 
usb_interface *intf)
 
 static void qmi_wwan_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
-   struct qmi_wwan_state *info = (void *)&dev->data;
+   struct qmi_wwan_state *info = usbnet_to_qmi(dev);
  

[PATCH 7/7] net: usb: sr9800: add usbnet -> priv function

2018-10-12 Thread Ben Dooks
There are a number of places in the sr8900 driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/sr9800.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/sr9800.c b/drivers/net/usb/sr9800.c
index 9277a0f228df..2093ecfff5a5 100644
--- a/drivers/net/usb/sr9800.c
+++ b/drivers/net/usb/sr9800.c
@@ -25,6 +25,11 @@
 
 #include "sr9800.h"
 
+static inline struct sr_data *usbnet_to_sr(struct usbnet *usb)
+{
+   return (struct sr_data *)&usb->data;
+}
+
 static int sr_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
u16 size, void *data)
 {
@@ -296,7 +301,7 @@ static int sr_write_gpio(struct usbnet *dev, u16 value, int 
sleep)
 static void sr_set_multicast(struct net_device *net)
 {
struct usbnet *dev = netdev_priv(net);
-   struct sr_data *data = (struct sr_data *)&dev->data;
+   struct sr_data *data = usbnet_to_sr(dev);
u16 rx_ctl = SR_DEFAULT_RX_CTL;
 
if (net->flags & IFF_PROMISC) {
@@ -436,7 +441,7 @@ sr_set_wol(struct net_device *net, struct ethtool_wolinfo 
*wolinfo)
 static int sr_get_eeprom_len(struct net_device *net)
 {
struct usbnet *dev = netdev_priv(net);
-   struct sr_data *data = (struct sr_data *)&dev->data;
+   struct sr_data *data = usbnet_to_sr(dev);
 
return data->eeprom_len;
 }
@@ -493,7 +498,7 @@ static int sr_ioctl(struct net_device *net, struct ifreq 
*rq, int cmd)
 static int sr_set_mac_address(struct net_device *net, void *p)
 {
struct usbnet *dev = netdev_priv(net);
-   struct sr_data *data = (struct sr_data *)&dev->data;
+   struct sr_data *data = usbnet_to_sr(dev);
struct sockaddr *addr = p;
 
if (netif_running(net))
@@ -595,7 +600,7 @@ static int sr9800_set_default_mode(struct usbnet *dev)
 
 static int sr9800_reset(struct usbnet *dev)
 {
-   struct sr_data *data = (struct sr_data *)&dev->data;
+   struct sr_data *data = usbnet_to_sr(dev);
int ret, embd_phy;
u16 rx_ctl;
 
@@ -726,7 +731,7 @@ static int sr9800_phy_powerup(struct usbnet *dev)
 
 static int sr9800_bind(struct usbnet *dev, struct usb_interface *intf)
 {
-   struct sr_data *data = (struct sr_data *)&dev->data;
+   struct sr_data *data = usbnet_to_sr(dev);
u16 led01_mux, led23_mux;
int ret, embd_phy;
u32 phyid;
-- 
2.19.1



[PATCH 5/7] net: cdc_ether: add usbnet -> priv function

2018-10-12 Thread Ben Dooks
There are a number of places in the cdc drivers where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/cdc_ether.c  |  8 ++---
 drivers/net/usb/cdc_mbim.c   | 23 --
 drivers/net/usb/cdc_ncm.c| 61 +++-
 drivers/net/usb/rndis_host.c |  6 ++--
 include/linux/usb/usbnet.h   |  5 +++
 5 files changed, 59 insertions(+), 44 deletions(-)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 5c42cf81a08b..7fee0ebc1943 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -77,7 +77,7 @@ static const u8 mbm_guid[16] = {
 
 static void usbnet_cdc_update_filter(struct usbnet *dev)
 {
-   struct cdc_state*info = (void *) &dev->data;
+   struct cdc_state*info = usbnet_to_cdc(dev);
struct usb_interface*intf = info->control;
struct net_device   *net = dev->net;
 
@@ -115,7 +115,7 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct 
usb_interface *intf)
u8  *buf = intf->cur_altsetting->extra;
int len = intf->cur_altsetting->extralen;
struct usb_interface_descriptor *d;
-   struct cdc_state*info = (void *) &dev->data;
+   struct cdc_state*info = usbnet_to_cdc(dev);
int status;
int rndis;
boolandroid_rndis_quirk = false;
@@ -353,7 +353,7 @@ EXPORT_SYMBOL_GPL(usbnet_ether_cdc_bind);
 
 void usbnet_cdc_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
-   struct cdc_state*info = (void *) &dev->data;
+   struct cdc_state*info = usbnet_to_cdc(dev);
struct usb_driver   *driver = driver_of(intf);
 
/* combined interface - nothing  to do */
@@ -438,7 +438,7 @@ EXPORT_SYMBOL_GPL(usbnet_cdc_status);
 int usbnet_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 {
int status;
-   struct cdc_state*info = (void *) &dev->data;
+   struct cdc_state*info = usbnet_to_cdc(dev);
 
BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data)
< sizeof(struct cdc_state)));
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 0362acd5cdca..aec8f8eb21a7 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -36,6 +36,11 @@ struct cdc_mbim_state {
unsigned long flags;
 };
 
+static inline struct cdc_mbim_state *usbnet_to_mbim(struct usbnet *usb)
+{
+   return (void *)&usb->data;
+}
+
 /* flags for the cdc_mbim_state.flags field */
 enum cdc_mbim_flags {
FLAG_IPS0_VLAN = 1 << 0,/* IP session 0 is tagged  */
@@ -44,7 +49,7 @@ enum cdc_mbim_flags {
 /* using a counter to merge subdriver requests with our own into a combined 
state */
 static int cdc_mbim_manage_power(struct usbnet *dev, int on)
 {
-   struct cdc_mbim_state *info = (void *)&dev->data;
+   struct cdc_mbim_state *info = usbnet_to_mbim(dev);
int rv = 0;
 
dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__, 
atomic_read(&info->pmcount), on);
@@ -73,7 +78,7 @@ static int cdc_mbim_wdm_manage_power(struct usb_interface 
*intf, int status)
 static int cdc_mbim_rx_add_vid(struct net_device *netdev, __be16 proto, u16 
vid)
 {
struct usbnet *dev = netdev_priv(netdev);
-   struct cdc_mbim_state *info = (void *)&dev->data;
+   struct cdc_mbim_state *info = usbnet_to_mbim(dev);
 
/* creation of this VLAN is a request to tag IP session 0 */
if (vid == MBIM_IPS0_VID)
@@ -87,7 +92,7 @@ static int cdc_mbim_rx_add_vid(struct net_device *netdev, 
__be16 proto, u16 vid)
 static int cdc_mbim_rx_kill_vid(struct net_device *netdev, __be16 proto, u16 
vid)
 {
struct usbnet *dev = netdev_priv(netdev);
-   struct cdc_mbim_state *info = (void *)&dev->data;
+   struct cdc_mbim_state *info = usbnet_to_mbim(dev);
 
/* this is a request for an untagged IP session 0 */
if (vid == MBIM_IPS0_VID)
@@ -144,7 +149,7 @@ static int cdc_mbim_bind(struct usbnet *dev, struct 
usb_interface *intf)
struct usb_driver *subdriver = ERR_PTR(-ENODEV);
int ret = -ENODEV;
u8 data_altsetting = 1;
-   struct cdc_mbim_state *info = (void *)&dev->data;
+   struct cdc_mbim_state *info = usbnet_to_mbim(dev);
 
/* should we change control altsetting on a NCM/MBIM function? */
if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) {
@@ -195,7 +200,7 @@ static int cdc_mbim_bind(struct

SMSC95XX updates for packet alignment and turbo mode (V2)

2018-10-12 Thread Ben Dooks
This is the new seris of SMSC95XX patches to deal with the issues we
found when working on this driver for a client. The new series has been
tested on an Raspberry Pi3 B.

Changes since v1:
 - Change memcpy to use {get,put}_unaligned_le32() calls
 - Merge tx fixups
 - Added rx_turbo attribute





[PATCH 3/8] usbnet: smsc95xx: simplify tx_fixup code

2018-10-12 Thread Ben Dooks
The smsc95xx_tx_fixup is doing multiple calls to skb_push() to
put an 8-byte command header onto the packet. It would be easier
to do one skb_push() and then copy the data in once the push is
done.

We also make the code smaller by using proper unaligned puts for
the header. This merges in the CPU to LE32 conversion as well and
makes the whole sequence easier to understand hopefully.

Signed-off-by: Ben Dooks 
---
Since v1:
- Add alignment changes using put_unaligned_le32()
---
 drivers/net/usb/smsc95xx.c | 28 +---
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index cb19aea139d3..0c083d1b7f34 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -2006,6 +2006,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
u32 tx_cmd_a, tx_cmd_b;
+   void *ptr;
 
/* We do not advertise SG, so skbs should be already linearized */
BUG_ON(skb_shinfo(skb)->nr_frags);
@@ -2019,6 +2020,9 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
return NULL;
}
 
+   tx_cmd_b = (u32)skb->len;
+   tx_cmd_a = tx_cmd_b | TX_CMD_A_FIRST_SEG_ | TX_CMD_A_LAST_SEG_;
+
if (csum) {
if (skb->len <= 45) {
/* workaround - hardware tx checksum does not work
@@ -2032,24 +2036,18 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
csum = false;
} else {
u32 csum_preamble = smsc95xx_calc_csum_preamble(skb);
-   skb_push(skb, 4);
-   cpu_to_le32s(&csum_preamble);
-   memcpy(skb->data, &csum_preamble, 4);
+   ptr = skb_push(skb, 4);
+   put_unaligned_le32(csum_preamble, ptr);
+
+   tx_cmd_a += 4;
+   tx_cmd_b += 4;
+   tx_cmd_b |= TX_CMD_B_CSUM_ENABLE;
}
}
 
-   skb_push(skb, 4);
-   tx_cmd_b = (u32)(skb->len - 4);
-   if (csum)
-   tx_cmd_b |= TX_CMD_B_CSUM_ENABLE;
-   cpu_to_le32s(&tx_cmd_b);
-   memcpy(skb->data, &tx_cmd_b, 4);
-
-   skb_push(skb, 4);
-   tx_cmd_a = (u32)(skb->len - 8) | TX_CMD_A_FIRST_SEG_ |
-   TX_CMD_A_LAST_SEG_;
-   cpu_to_le32s(&tx_cmd_a);
-   memcpy(skb->data, &tx_cmd_a, 4);
+   ptr = skb_push(skb, 8);
+   put_unaligned_le32(tx_cmd_a, ptr);
+   put_unaligned_le32(tx_cmd_b, ptr+4);
 
return skb;
 }
-- 
2.19.1



[PATCH 5/8] usbnet: smsc95xx: align tx-buffer to word

2018-10-12 Thread Ben Dooks
The tegra EHCI driver requires alignment of the buffer, so try and
make this better by pushing the buffer start back to an word
aligned address. At the worst this makes memcpy() easier as
it is word aligned, at best it makes sure the usb can directly
map the buffer.

Signed-off-by: Ben Dooks 
---
Changes since v1:
- Removed the module parameter
- Reworked the tx code to ensure retry if alignment changed
- Explicitly mention the EHCI in the tegra
- Deal with new simpler tx code
---
 drivers/net/usb/Kconfig| 12 
 drivers/net/usb/smsc95xx.c | 22 +++---
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index c3ebc43a6582..1af6fb0cadb1 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -364,6 +364,18 @@ config USB_NET_SMSC95XX_TURBO
  soft-irq load, thus making it useful to default the option
  off for these.
 
+config USB_NET_SMSC95XX_TXALIGN
+   bool "Add bytes to align transmit buffers"
+   depends on USB_NET_SMSC95XX
+   default n
+   help
+ This option makes the tx buffers 32 bit aligned which might
+ help with systems that want tx data aligned to a 32 bit
+ boundary.
+
+ Using this option will mean there may be up to 3 bytes of
+ data per packet sent.
+
 config USB_NET_GL620A
tristate "GeneSys GL620USB-A based cables"
depends on USB_USBNET
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 19e71fe15f6c..8ce190da8be0 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -2017,28 +2017,43 @@ static bool smsc95xx_can_tx_checksum(struct sk_buff 
*skb)
return skb->csum_offset < (len - (4 + 1));
 }
 
+static inline u32 tx_align(struct sk_buff *skb)
+{
+#ifdef CONFIG_USB_NET_SMSC95XX_TXALIGN
+   return ((u32)skb->data) & 3;
+#else
+   return 0;
+#endif
+}
+
 static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 struct sk_buff *skb, gfp_t flags)
 {
bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
u32 tx_cmd_a, tx_cmd_b;
+   u32 align;
void *ptr;
 
/* We do not advertise SG, so skbs should be already linearized */
BUG_ON(skb_shinfo(skb)->nr_frags);
 
+retry_align:
+   align = tx_align(skb);
+
/* Make writable and expand header space by overhead if required */
-   if (skb_cow_head(skb, overhead)) {
+   if (skb_cow_head(skb, overhead + align)) {
/* Must deallocate here as returning NULL to indicate error
 * means the skb won't be deallocated in the caller.
 */
dev_kfree_skb_any(skb);
return NULL;
-   }
+   } else if (tx_align(skb) != align)
+   goto retry_align;
 
tx_cmd_b = (u32)skb->len;
tx_cmd_a = tx_cmd_b | TX_CMD_A_FIRST_SEG_ | TX_CMD_A_LAST_SEG_;
+   tx_cmd_a |= align << 16;
 
if (csum) {
if (!smsc95xx_can_tx_checksum(skb)) {
@@ -2062,7 +2077,8 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
}
}
 
-   ptr = skb_push(skb, 8);
+   /* TX command format is in section 5.4 of SMSC95XX datasbook */
+   ptr = skb_push(skb, 8 + align);
put_unaligned_le32(tx_cmd_a, ptr);
put_unaligned_le32(tx_cmd_b, ptr+4);
 
-- 
2.19.1



[PATCH 6/8] usbnet: smsc95xx: fix memcpy for accessing rx-data

2018-10-12 Thread Ben Dooks
Change the RX code to use get_unaligned_le32() instead of the combo
of memcpy and cpu_to_le32s(&var).

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/smsc95xx.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 8ce190da8be0..03c3c02b569c 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -618,9 +618,7 @@ static void smsc95xx_status(struct usbnet *dev, struct urb 
*urb)
return;
}
 
-   memcpy(&intdata, urb->transfer_buffer, 4);
-   le32_to_cpus(&intdata);
-
+   intdata = get_unaligned_le32(urb->transfer_buffer);
netif_dbg(dev, link, dev->net, "intdata: 0x%08X\n", intdata);
 
if (intdata & INT_ENP_PHY_INT_)
@@ -1922,8 +1920,7 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct 
sk_buff *skb)
unsigned char *packet;
u16 size;
 
-   memcpy(&header, skb->data, sizeof(header));
-   le32_to_cpus(&header);
+   header = get_unaligned_le32(skb->data);
skb_pull(skb, 4 + NET_IP_ALIGN);
packet = skb->data;
 
-- 
2.19.1



[PATCH 8/8] usbnet: smsc95xx: add rx_turbo attribute

2018-10-12 Thread Ben Dooks
Add attribute for the rx_turbo mode to allow run-time configuration of
this feature.

Note, this requires a restart of the device to take effect.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/smsc95xx.c | 41 +-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 1eb0795ec90f..330c3cf5d6f6 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -73,6 +73,7 @@ struct smsc95xx_priv {
u8 features;
u8 suspend_flags;
u8 mdix_ctrl;
+   bool rx_turbo;
bool link_ok;
struct delayed_work carrier_check;
struct usbnet *dev;
@@ -1103,7 +1104,7 @@ static int smsc95xx_reset(struct usbnet *dev)
  "Read Value from HW_CFG after writing HW_CFG_BIR_: 0x%08x\n",
  read_buf);
 
-   if (!turbo_mode) {
+   if (!pdata->rx_turbo) {
burst_cap = 0;
dev->rx_urb_size = MAX_SINGLE_PACKET_SIZE;
} else if (dev->udev->speed == USB_SPEED_HIGH) {
@@ -1259,6 +1260,37 @@ static const struct net_device_ops smsc95xx_netdev_ops = 
{
.ndo_set_features   = smsc95xx_set_features,
 };
 
+static inline struct smsc95xx_priv *dev_to_priv(struct device *dev)
+{
+   struct usb_interface *ui = container_of(dev, struct usb_interface, dev);
+   return usbnet_to_smsc(usb_get_intfdata(ui));
+}
+
+/* Currently rx_turbo will not take effect until next close/open */
+static ssize_t rx_turbo_show(struct device *adev,
+struct device_attribute *attr,
+char *buf)
+{
+   struct smsc95xx_priv *priv = dev_to_priv(adev);
+   return snprintf(buf, PAGE_SIZE, "%d", priv->rx_turbo);
+}
+
+static ssize_t rx_turbo_store(struct device *adev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+   struct smsc95xx_priv *priv = dev_to_priv(adev);
+   bool res;
+
+   if (kstrtobool(buf, &res))
+   return -EINVAL;
+
+   priv->rx_turbo = res;
+   return count;
+}
+
+static DEVICE_ATTR_RW(rx_turbo);
+
 static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 {
struct smsc95xx_priv *pdata = NULL;
@@ -1280,6 +1312,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct 
usb_interface *intf)
if (!pdata)
return -ENOMEM;
 
+   pdata->rx_turbo = turbo_mode;
spin_lock_init(&pdata->mac_cr_lock);
 
/* LAN95xx devices do not alter the computed checksum of 0 to 0x.
@@ -1328,6 +1361,11 @@ static int smsc95xx_bind(struct usbnet *dev, struct 
usb_interface *intf)
INIT_DELAYED_WORK(&pdata->carrier_check, check_carrier);
schedule_delayed_work(&pdata->carrier_check, CARRIER_CHECK_DELAY);
 
+   ret = device_create_file(&dev->udev->dev, &dev_attr_rx_turbo);
+   if (ret)
+   netdev_warn(dev->net,
+   "failed to create rx_turbo attribute: %d\n", ret);
+
return 0;
 }
 
@@ -1336,6 +1374,7 @@ static void smsc95xx_unbind(struct usbnet *dev, struct 
usb_interface *intf)
struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
 
if (pdata) {
+   device_remove_file(&dev->udev->dev, &dev_attr_rx_turbo);
cancel_delayed_work(&pdata->carrier_check);
netif_dbg(dev, ifdown, dev->net, "free pdata\n");
kfree(pdata);
-- 
2.19.1



[PATCH 2/8] usbnet: smsc95xx: add kconfig for turbo mode

2018-10-12 Thread Ben Dooks
Add a configuration option for the default state of turbo mode
on the smsc95xx networking driver. Some systems it is better
to default this to off as it causes significant increases in
soft-irq load.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/Kconfig| 13 +
 drivers/net/usb/smsc95xx.c |  2 +-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 418b0904cecb..c3ebc43a6582 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -351,6 +351,19 @@ config USB_NET_SMSC95XX
  This option adds support for SMSC LAN95XX based USB 2.0
  10/100 Ethernet adapters.
 
+config USB_NET_SMSC95XX_TURBO
+   bool "Use turbo receive mode by default"
+   depends on USB_NET_SMSC95XX
+   default y
+   help
+ This options sets the default turbo mode settings for the
+ driver's receive path. These can also be altered by the
+ turbo_mode module parameter.
+
+ There are some systems where the turbo mode causes higher
+ soft-irq load, thus making it useful to default the option
+ off for these.
+
 config USB_NET_GL620A
tristate "GeneSys GL620USB-A based cables"
depends on USB_USBNET
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 401ec9feb495..cb19aea139d3 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -78,7 +78,7 @@ struct smsc95xx_priv {
struct usbnet *dev;
 };
 
-static bool turbo_mode = true;
+static bool turbo_mode = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TURBO);
 module_param(turbo_mode, bool, 0644);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
 
-- 
2.19.1



[PATCH 1/8] usbnet: smsc95xx: fix rx packet alignment

2018-10-12 Thread Ben Dooks
The smsc95xx driver already takes into account the NET_IP_ALIGN
parameter when setting up the receive packet data, which means
we do not need to worry about aligning the packets in the usbnet
driver.

Adding the EVENT_NO_IP_ALIGN means that the IPv4 header is now
passed to the ip_rcv() routine with the start on an aligned address.

Tested on Raspberry Pi B3.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/smsc95xx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 06b4d290784d..401ec9feb495 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1292,6 +1292,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct 
usb_interface *intf)
dev->net->features |= NETIF_F_RXCSUM;
 
dev->net->hw_features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM;
+   set_bit(EVENT_NO_IP_ALIGN, &dev->flags);
 
smsc95xx_init_mac_address(dev);
 
-- 
2.19.1



[PATCH 4/8] usbnet: smsc95xx: check for csum being in last four bytes

2018-10-12 Thread Ben Dooks
The manual states that the checksum cannot lie in the last DWORD of the
transmission, so add a basic check for this and fall back to software
checksumming the packet.

This only seems to trigger for ACK packets with no options or data to
return to the other end, and the use of the tx-alignment option makes
it more likely to happen.

Signed-off-by: Ben Dooks 
---
Fixes for v2:
- Fix spelling of check at Sergei's suggestion
- Move skb->len check into smsc95xx_can_tx_checksum()
- Change name of smsc95xx_can_checksum to explicitly say it is tx-csum
---
 drivers/net/usb/smsc95xx.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 0c083d1b7f34..19e71fe15f6c 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -2000,6 +2000,23 @@ static u32 smsc95xx_calc_csum_preamble(struct sk_buff 
*skb)
return (high_16 << 16) | low_16;
 }
 
+/* The TX CSUM won't work if the checksum lies in the last 4 bytes of the
+ * transmission. This is fairly unlikely, only seems to trigger with some
+ * short TCP ACK packets sent.
+ *
+ * Note, this calculation should probably check for the alignment of the
+ * data as well, but a straight check for csum being in the last four bytes
+ * of the packet should be ok for now.
+*/
+static bool smsc95xx_can_tx_checksum(struct sk_buff *skb)
+{
+   unsigned int len = skb->len - skb_checksum_start_offset(skb);
+
+   if (skb->len <= 45)
+  return false;
+   return skb->csum_offset < (len - (4 + 1));
+}
+
 static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 struct sk_buff *skb, gfp_t flags)
 {
@@ -2024,7 +2041,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
tx_cmd_a = tx_cmd_b | TX_CMD_A_FIRST_SEG_ | TX_CMD_A_LAST_SEG_;
 
if (csum) {
-   if (skb->len <= 45) {
+   if (!smsc95xx_can_tx_checksum(skb)) {
/* workaround - hardware tx checksum does not work
 * properly with extremely small packets */
long csstart = skb_checksum_start_offset(skb);
-- 
2.19.1



[PATCH 7/8] usbnet: smsc95xx: add usbnet -> priv function

2018-10-12 Thread Ben Dooks
There are a number of places in the smsc95xx driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/smsc95xx.c | 47 +-
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 03c3c02b569c..1eb0795ec90f 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -78,6 +78,11 @@ struct smsc95xx_priv {
struct usbnet *dev;
 };
 
+static inline struct smsc95xx_priv *usbnet_to_smsc(struct usbnet *dev)
+{
+   return (struct smsc95xx_priv *)dev->data[0];
+}
+
 static bool turbo_mode = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TURBO);
 module_param(turbo_mode, bool, 0644);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
@@ -467,7 +472,7 @@ static unsigned int smsc95xx_hash(char addr[ETH_ALEN])
 static void smsc95xx_set_multicast(struct net_device *netdev)
 {
struct usbnet *dev = netdev_priv(netdev);
-   struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+   struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
unsigned long flags;
int ret;
 
@@ -562,7 +567,7 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet 
*dev, u8 duplex,
 
 static int smsc95xx_link_reset(struct usbnet *dev)
 {
-   struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+   struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
struct mii_if_info *mii = &dev->mii;
struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
unsigned long flags;
@@ -630,7 +635,7 @@ static void smsc95xx_status(struct usbnet *dev, struct urb 
*urb)
 
 static void set_carrier(struct usbnet *dev, bool link)
 {
-   struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+   struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
 
if (pdata->link_ok == link)
return;
@@ -759,7 +764,7 @@ static void smsc95xx_ethtool_get_wol(struct net_device *net,
 struct ethtool_wolinfo *wolinfo)
 {
struct usbnet *dev = netdev_priv(net);
-   struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+   struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
 
wolinfo->supported = SUPPORTED_WAKE;
wolinfo->wolopts = pdata->wolopts;
@@ -769,7 +774,7 @@ static int smsc95xx_ethtool_set_wol(struct net_device *net,
struct ethtool_wolinfo *wolinfo)
 {
struct usbnet *dev = netdev_priv(net);
-   struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+   struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
int ret;
 
pdata->wolopts = wolinfo->wolopts & SUPPORTED_WAKE;
@@ -805,7 +810,7 @@ static int get_mdix_status(struct net_device *net)
 static void set_mdix_status(struct net_device *net, __u8 mdix_ctrl)
 {
struct usbnet *dev = netdev_priv(net);
-   struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+   struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
int buf;
 
if ((pdata->chip_id == ID_REV_CHIP_ID_9500A_) ||
@@ -854,7 +859,7 @@ static int smsc95xx_get_link_ksettings(struct net_device 
*net,
   struct ethtool_link_ksettings *cmd)
 {
struct usbnet *dev = netdev_priv(net);
-   struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+   struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
int retval;
 
retval = usbnet_get_link_ksettings(net, cmd);
@@ -869,7 +874,7 @@ static int smsc95xx_set_link_ksettings(struct net_device 
*net,
   const struct ethtool_link_ksettings *cmd)
 {
struct usbnet *dev = netdev_priv(net);
-   struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+   struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
int retval;
 
if (pdata->mdix_ctrl != cmd->base.eth_tp_mdix_ctrl)
@@ -951,7 +956,7 @@ static int smsc95xx_set_mac_address(struct usbnet *dev)
 /* starts the TX path */
 static int smsc95xx_start_tx_path(struct usbnet *dev)
 {
-   struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+   struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
unsigned long flags;
int ret;
 
@@ -971,7 +976,7 @@ static int smsc95xx_start_tx_path(struct usbnet *dev)
 /* Starts the Receive path */
 static int smsc95xx_start_rx_path(struct usbnet *dev, int in_pm)
 {
-   struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+   struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
unsigned long flags;
 
spin_lock_irqsave(&pdata-

[PATCH] net: cdc_ncm: use tasklet_init() for tasklet_struct init

2018-10-11 Thread Ben Dooks
The tasklet initialisation would be better done by tasklet_init()
instead of assuming all the fields are in an ok state by default.

This does not fix any actual know bug.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/cdc_ncm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 0d722b326e1b..863f3548a439 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -784,8 +784,7 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
 
hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
ctx->tx_timer.function = &cdc_ncm_tx_timer_cb;
-   ctx->bh.data = (unsigned long)dev;
-   ctx->bh.func = cdc_ncm_txpath_bh;
+   tasklet_init(&ctx->bh, cdc_ncm_txpath_bh, (unsigned long)dev);
atomic_set(&ctx->stop, 0);
spin_lock_init(&ctx->mtx);
 
-- 
2.19.1



Re: [PATCH] usbnet: smsc95xx: simplify tx_fixup code

2018-10-06 Thread Ben Dooks




On 2018-10-05 22:24, David Miller wrote:

From: Ben Dooks 
Date: Tue,  2 Oct 2018 17:56:02 +0100


-   memcpy(skb->data, &tx_cmd_a, 4);
+   ptr = skb_push(skb, 8);
+   tx_cmd_a = cpu_to_le32(tx_cmd_a);
+   tx_cmd_b = cpu_to_le32(tx_cmd_b);
+   memcpy(ptr, &tx_cmd_a, 4);
+   memcpy(ptr+4, &tx_cmd_b, 4);


Even a memcpy() through a void pointer does not guarantee that gcc will
not emit word sized loads and stores.

You must use the get_unaligned()/put_unaligned() facilities to do this
properly.


Thanks, got a new version of the series just being tested with this.
Should it go into the original, or as a separate change?



I also agree that making a proper type and structure instead of using
a void pointer would be better.


RE: [PATCH] usbnet: smsc95xx: simplify tx_fixup code

2018-10-03 Thread Ben Dooks

On 2018-10-03 14:36, David Laight wrote:

From: Ben Dooks

Sent: 02 October 2018 17:56

The smsc95xx_tx_fixup is doing multiple calls to skb_push() to
put an 8-byte command header onto the packet. It would be easier
to do one skb_push() and then copy the data in once the push is
done.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/smsc95xx.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index cb19aea139d3..813ab93ee2c3 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -2006,6 +2006,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct 
usbnet *dev,

bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
 	int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : 
SMSC95XX_TX_OVERHEAD;

u32 tx_cmd_a, tx_cmd_b;
+   void *ptr;


It might be useful to define a structure for the header.
You might need to find the 'store unaligned 32bit word' macro though.
(Actually that will probably be better than the memcpy() which might
end up doing memory-memory copies rather than storing the register.)
Although if/when you add the tx alignment that won't be needed because 
the

header will be aligned.


Ok, might be worth doing.

I did try to do a "u32 tx_cmd[2]" but the code generated ended up 
storing
stuff onto the stack before copying into the packet. I agree that 
possibly

going to the "put_unaligned" function might be nicer too.

If we did enable tx-align all the time then we'd not have to care about 
the

alignment, but I didn't want to do that if possible as that would end up
sending up to 3 bytes extra per packet.

I am trying not too do too many changes at one time to allow roll back.


/* We do not advertise SG, so skbs should be already linearized */
BUG_ON(skb_shinfo(skb)->nr_frags);
@@ -2019,6 +2020,9 @@ static struct sk_buff *smsc95xx_tx_fixup(struct 
usbnet *dev,

return NULL;
}

+   tx_cmd_b = (u32)skb->len;
+   tx_cmd_a = tx_cmd_b | TX_CMD_A_FIRST_SEG_ | TX_CMD_A_LAST_SEG_;
+
if (csum) {
if (skb->len <= 45) {
/* workaround - hardware tx checksum does not work
@@ -2035,21 +2039,18 @@ static struct sk_buff 
*smsc95xx_tx_fixup(struct usbnet *dev,

skb_push(skb, 4);
cpu_to_le32s(&csum_preamble);


Not related, but csum_preamble = cpu_to_le32(csum_preamble) is likely 
to

generate better code (at least for some architectures).

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes,
MK1 1PT, UK
Registration No: 1397386 (Wales)


[PATCH] usbnet: smsc95xx: simplify tx_fixup code

2018-10-02 Thread Ben Dooks
The smsc95xx_tx_fixup is doing multiple calls to skb_push() to
put an 8-byte command header onto the packet. It would be easier
to do one skb_push() and then copy the data in once the push is
done.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/smsc95xx.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index cb19aea139d3..813ab93ee2c3 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -2006,6 +2006,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
u32 tx_cmd_a, tx_cmd_b;
+   void *ptr;
 
/* We do not advertise SG, so skbs should be already linearized */
BUG_ON(skb_shinfo(skb)->nr_frags);
@@ -2019,6 +2020,9 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
return NULL;
}
 
+   tx_cmd_b = (u32)skb->len;
+   tx_cmd_a = tx_cmd_b | TX_CMD_A_FIRST_SEG_ | TX_CMD_A_LAST_SEG_;
+
if (csum) {
if (skb->len <= 45) {
/* workaround - hardware tx checksum does not work
@@ -2035,21 +2039,18 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
skb_push(skb, 4);
cpu_to_le32s(&csum_preamble);
memcpy(skb->data, &csum_preamble, 4);
+
+   tx_cmd_a += 4;
+   tx_cmd_b += 4;
+   tx_cmd_b |= TX_CMD_B_CSUM_ENABLE;
}
}
 
-   skb_push(skb, 4);
-   tx_cmd_b = (u32)(skb->len - 4);
-   if (csum)
-   tx_cmd_b |= TX_CMD_B_CSUM_ENABLE;
-   cpu_to_le32s(&tx_cmd_b);
-   memcpy(skb->data, &tx_cmd_b, 4);
-
-   skb_push(skb, 4);
-   tx_cmd_a = (u32)(skb->len - 8) | TX_CMD_A_FIRST_SEG_ |
-   TX_CMD_A_LAST_SEG_;
-   cpu_to_le32s(&tx_cmd_a);
-   memcpy(skb->data, &tx_cmd_a, 4);
+   ptr = skb_push(skb, 8);
+   tx_cmd_a = cpu_to_le32(tx_cmd_a);
+   tx_cmd_b = cpu_to_le32(tx_cmd_b);
+   memcpy(ptr, &tx_cmd_a, 4);
+   memcpy(ptr+4, &tx_cmd_b, 4);
 
return skb;
 }
-- 
2.19.0



Re: [PATCH 2/4] usbnet: smsc95xx: align tx-buffer to word

2018-10-02 Thread Ben Dooks

On 02/10/18 14:19, David Laight wrote:

From: Ben Dooks

Sent: 02 October 2018 10:27

The tegra driver requires alignment of the buffer, so try and
make this better by pushing the buffer start back to an word
aligned address. At the worst this makes memcpy() easier as
it is word aligned, at best it makes sure the usb can directly
map the buffer.

Signed-off-by: Ben Dooks 
[todo - make this configurable]
---
  drivers/net/usb/Kconfig| 12 
  drivers/net/usb/smsc95xx.c | 22 --
  2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig

...

+static bool align_tx = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TXALIGN);
+module_param(align_tx, bool, 0644);
+MODULE_PARM_DESC(align_tx, "Align TX buffers to word boundaries");


DM doesn't like module parameters.


  static bool turbo_mode = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TURBO);
  module_param(turbo_mode, bool, 0644);
  MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
@@ -2005,10 +2009,18 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
u32 tx_cmd_a, tx_cmd_b;
+   u32 data_len;
+   uintptr_t align = 0;

/* We do not advertise SG, so skbs should be already linearized */
BUG_ON(skb_shinfo(skb)->nr_frags);

+   if (IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TXALIGN) && align_tx) {
+   align = (uintptr_t)skb->data & 3;
+   if (align)
+   overhead += 4 - align;


Better to calculate the pad size once:
align = (-(long)skb->data) & 3;
should do it - and you can unconditionally add it in.


+   }
+
/* Make writable and expand header space by overhead if required */
if (skb_cow_head(skb, overhead)) {
/* Must deallocate here as returning NULL to indicate error
@@ -2037,16 +2049,22 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
}
}

+   data_len = skb->len;
+   if (align)
+   skb_push(skb, 4 - align);
+
skb_push(skb, 4);


You don't want to call skb_push() twice.
IIRC really horrid things happen if the data has to be copied.
(Actually what happens to the alignment in that case??)
And there is another skb_push() below


The driver does it /multiple/ times depending on the path used.
Is it wise to try and make a separate patch to skb_push() once
and also move the tx_cmd_a and tx_cmd_b bit to a single point?


-   tx_cmd_b = (u32)(skb->len - 4);
+   tx_cmd_b = (u32)(data_len);


You don't need the cast here at all (if it was ever needed).
Actually you don't need the new 'data_len' variable.
Just set tx_cmd_b earlier.

...

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)





--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html


SMSC95XX driver updates

2018-10-02 Thread Ben Dooks
I have been doing some work with tegra3 systems which have a smsc9512
USB network device on them. A couple of issues we found with alignment
of the data (both receive and transmit) and an issue where the automatic
transmit checksum failed.




[PATCH 2/4] usbnet: smsc95xx: align tx-buffer to word

2018-10-02 Thread Ben Dooks
The tegra driver requires alignment of the buffer, so try and
make this better by pushing the buffer start back to an word
aligned address. At the worst this makes memcpy() easier as
it is word aligned, at best it makes sure the usb can directly
map the buffer.

Signed-off-by: Ben Dooks 
[todo - make this configurable]
---
 drivers/net/usb/Kconfig| 12 
 drivers/net/usb/smsc95xx.c | 22 --
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index a32f1a446ce9..35bad8bd2e2a 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -360,6 +360,18 @@ config USB_NET_SMSC95XX_TURBO
  driver's receive path. These can also be altered by the
  turbo_mode module parameter.
 
+config USB_NET_SMSC95XX_TXALIGN
+   bool "Add bytes to align transmit buffers"
+   depends on USB_NET_SMSC95XX
+   default n
+   help
+ This option makes the tx buffers 32 bit aligned which might
+ help with systems that want tx data aligned to a 32 bit
+ boundary.
+
+ Using this option will mean there may be up to 3 bytes of
+ data per packet sent.
+
 config USB_NET_GL620A
tristate "GeneSys GL620USB-A based cables"
depends on USB_USBNET
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index fe13bef9579e..d244357bf1ad 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -78,6 +78,10 @@ struct smsc95xx_priv {
struct usbnet *dev;
 };
 
+static bool align_tx = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TXALIGN);
+module_param(align_tx, bool, 0644);
+MODULE_PARM_DESC(align_tx, "Align TX buffers to word boundaries");
+
 static bool turbo_mode = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TURBO);
 module_param(turbo_mode, bool, 0644);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
@@ -2005,10 +2009,18 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
u32 tx_cmd_a, tx_cmd_b;
+   u32 data_len;
+   uintptr_t align = 0;
 
/* We do not advertise SG, so skbs should be already linearized */
BUG_ON(skb_shinfo(skb)->nr_frags);
 
+   if (IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TXALIGN) && align_tx) {
+   align = (uintptr_t)skb->data & 3;
+   if (align)
+   overhead += 4 - align;
+   }
+
/* Make writable and expand header space by overhead if required */
if (skb_cow_head(skb, overhead)) {
/* Must deallocate here as returning NULL to indicate error
@@ -2037,16 +2049,22 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
}
}
 
+   data_len = skb->len;
+   if (align)
+   skb_push(skb, 4 - align);
+
skb_push(skb, 4);
-   tx_cmd_b = (u32)(skb->len - 4);
+   tx_cmd_b = (u32)(data_len);
if (csum)
tx_cmd_b |= TX_CMD_B_CSUM_ENABLE;
cpu_to_le32s(&tx_cmd_b);
memcpy(skb->data, &tx_cmd_b, 4);
 
skb_push(skb, 4);
-   tx_cmd_a = (u32)(skb->len - 8) | TX_CMD_A_FIRST_SEG_ |
+   tx_cmd_a = (u32)(data_len) | TX_CMD_A_FIRST_SEG_ |
TX_CMD_A_LAST_SEG_;
+   if (align)
+   tx_cmd_a |= (4 - align) << 16;
cpu_to_le32s(&tx_cmd_a);
memcpy(skb->data, &tx_cmd_a, 4);
 
-- 
2.19.0



[PATCH 1/4] usbnet: smsc95xx: add kconfig for turbo mode

2018-10-02 Thread Ben Dooks
Add a configuration option for the default state of turbo mode
on the smsc95xx networking driver. Some systems it is better
to default this to off as it causes significant increases in
soft-irq load.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/Kconfig| 9 +
 drivers/net/usb/smsc95xx.c | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 418b0904cecb..a32f1a446ce9 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -351,6 +351,15 @@ config USB_NET_SMSC95XX
  This option adds support for SMSC LAN95XX based USB 2.0
  10/100 Ethernet adapters.
 
+config USB_NET_SMSC95XX_TURBO
+   bool "Use turbo receive mode by default"
+   depends on USB_NET_SMSC95XX
+   default y
+   help
+ This options sets the default turbo mode settings for the
+ driver's receive path. These can also be altered by the
+ turbo_mode module parameter.
+
 config USB_NET_GL620A
tristate "GeneSys GL620USB-A based cables"
depends on USB_USBNET
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 06b4d290784d..fe13bef9579e 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -78,7 +78,7 @@ struct smsc95xx_priv {
struct usbnet *dev;
 };
 
-static bool turbo_mode = true;
+static bool turbo_mode = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TURBO);
 module_param(turbo_mode, bool, 0644);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
 
-- 
2.19.0



[PATCH 3/4] usbnet: smsc95xx: check for csum being in last four bytes

2018-10-02 Thread Ben Dooks
The manual states that the checksum cannot lie in the last DWORD of the
transmission, so add a basic check for this and fall back to software
checksumming the packet.

This only seems to trigger for ACK packets with no options or data to
return to the other end, and the use of the tx-alignment option makes
it more likely to happen.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/smsc95xx.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index d244357bf1ad..46385a4b8b98 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -2003,6 +2003,20 @@ static u32 smsc95xx_calc_csum_preamble(struct sk_buff 
*skb)
return (high_16 << 16) | low_16;
 }
 
+/* The CSUM won't work if the checksum lies in the last 4 bytes of the
+ * transmission. This is fairly unlikely, only seems to trigger with some
+ * short TCP ACK packets sent.
+ *
+ * Note, this calculation should probably check for the alignment of the
+ * data as well, but a straight chec for csum being in the last four bytes
+ * of the packet should be ok for now.
+*/
+static bool smsc95xx_can_checksum(struct sk_buff *skb)
+{
+   unsigned int len = skb->len - skb_checksum_start_offset(skb);
+   return skb->csum_offset < (len - (4 + 1));
+}
+
 static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 struct sk_buff *skb, gfp_t flags)
 {
@@ -2031,7 +2045,8 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet 
*dev,
}
 
if (csum) {
-   if (skb->len <= 45) {
+   /* note, csum does not work if csum in last DWORD of packet */
+   if (skb->len <= 45 || !smsc95xx_can_checksum(skb)) {
/* workaround - hardware tx checksum does not work
 * properly with extremely small packets */
long csstart = skb_checksum_start_offset(skb);
-- 
2.19.0



[PATCH 4/4] usbnet: smsc95xx: fix rx packet alignment

2018-10-02 Thread Ben Dooks
The smsc95xx driver already takes into account the NET_IP_ALIGN
parameter when setting up the receive packet data, which means
we do not need to worry about aligning the packets in the usbnet
driver.

Adding the EVENT_NO_IP_ALIGN means that the IPv4 header is now
passed to the ip_rcv() routine with the start on an aligned address.

Tested on Raspberry Pi B3.

Signed-off-by: Ben Dooks 
---
 drivers/net/usb/smsc95xx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 46385a4b8b98..2b867523cd53 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1296,6 +1296,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct 
usb_interface *intf)
dev->net->features |= NETIF_F_RXCSUM;
 
dev->net->hw_features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM;
+   set_bit(EVENT_NO_IP_ALIGN, &dev->flags);
 
smsc95xx_init_mac_address(dev);
 
-- 
2.19.0



[PATCH] net: usbnet: make driver_info const

2018-10-01 Thread Ben Dooks
From: Ben Dooks 

The driver_info field that is used for describing each of the usb-net
drivers using the usbnet.c core all declare their information as const
and the usbnet.c itself does not try and modify the struct.

It is therefore a good idea to make this const in the usbnet.c structure
in case anyone tries to modify it.

Signed-off-by: Ben Dooks 
Signed-off-by: Ben Dooks 
---
 drivers/net/usb/usbnet.c   | 12 ++--
 include/linux/usb/usbnet.h |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 770aa624147f..d6b3833c292d 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -802,7 +802,7 @@ static void usbnet_terminate_urbs(struct usbnet *dev)
 int usbnet_stop (struct net_device *net)
 {
struct usbnet   *dev = netdev_priv(net);
-   struct driver_info  *info = dev->driver_info;
+   const struct driver_info *info = dev->driver_info;
int retval, pm, mpn;
 
clear_bit(EVENT_DEV_OPEN, &dev->flags);
@@ -865,7 +865,7 @@ int usbnet_open (struct net_device *net)
 {
struct usbnet   *dev = netdev_priv(net);
int retval;
-   struct driver_info  *info = dev->driver_info;
+   const struct driver_info *info = dev->driver_info;
 
if ((retval = usb_autopm_get_interface(dev->intf)) < 0) {
netif_info(dev, ifup, dev->net,
@@ -1205,7 +1205,7 @@ usbnet_deferred_kevent (struct work_struct *work)
}
 
if (test_bit (EVENT_LINK_RESET, &dev->flags)) {
-   struct driver_info  *info = dev->driver_info;
+   const struct driver_info *info = dev->driver_info;
int retval = 0;
 
clear_bit (EVENT_LINK_RESET, &dev->flags);
@@ -1353,7 +1353,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
unsigned intlength;
struct urb  *urb = NULL;
struct skb_data *entry;
-   struct driver_info  *info = dev->driver_info;
+   const struct driver_info *info = dev->driver_info;
unsigned long   flags;
int retval;
 
@@ -1646,7 +1646,7 @@ usbnet_probe (struct usb_interface *udev, const struct 
usb_device_id *prod)
struct usbnet   *dev;
struct net_device   *net;
struct usb_host_interface   *interface;
-   struct driver_info  *info;
+   const struct driver_info*info;
struct usb_device   *xdev;
int status;
const char  *name;
@@ -1662,7 +1662,7 @@ usbnet_probe (struct usb_interface *udev, const struct 
usb_device_id *prod)
}
 
name = udev->dev.driver->name;
-   info = (struct driver_info *) prod->driver_info;
+   info = (const struct driver_info *) prod->driver_info;
if (!info) {
dev_dbg (&udev->dev, "blacklisted by %s\n", name);
return -ENODEV;
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index e2ec3582e549..d8860f2d0976 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -28,7 +28,7 @@ struct usbnet {
/* housekeeping */
struct usb_device   *udev;
struct usb_interface*intf;
-   struct driver_info  *driver_info;
+   const struct driver_info *driver_info;
const char  *driver_name;
void*driver_priv;
wait_queue_head_t   wait;
-- 
2.19.0



Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

2016-09-09 Thread Ben Dooks

On 09/09/16 17:14, Martin Blumenstingl wrote:

On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman  wrote:

However, the problem with all of the solutions proposed (runtime PM ones
included) is that we're forcing a board-specific design issue (2 devices
sharing a reset line) into a driver that should not have any
board-specific assumptions in it.

For example, if this driver is used on another platform where different
PHYs have different reset lines, then one of them (the unlucky one who
is not probed first) will never get reset.  So any form of per-device
ref-counting is not a portable solution.

maybe we should also consider Ben's solution: he played with the USB
PHY on his Meson8b board. His approach was to have only one USB PHY
driver instance which exposes two PHYs.
The downside of this: the driver would have to know the offset of the
PHYs (0x0 for the first PHY, 0x20 for the second), but we could handle
the reset using runtime PM without any hacks.

I checked the USB PHY reference driver: it seems that there will be a
new USB PHY with the GXL/GXM SoCs.
So maybe we could live with the assumption that the PHYs are at
consecutive addresses.


I'm not sure yet how the reset framework is supposed to handle shared
reset lines, but that needs some investigation.  I quick glance and it
seems that reset controllers can have shared lines, so that should be
investigated.

unfortunately shared resets are not allowed to use reset_control_reset, see [0]


[0] http://lxr.free-electrons.com/source/drivers/reset/core.c#L102


If we didn't have the shared reset, we'd have one of node per phy
and not have to have two sub-nodes... I don't think any other bits
of the PHY framework are shared.

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

2016-09-08 Thread Ben Dooks

On 08/09/16 21:42, Kevin Hilman wrote:

Ben Dooks  writes:


On 08/09/16 20:52, Martin Blumenstingl wrote:

On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman  wrote:

+ phy = devm_phy_create(&pdev->dev, NULL, &phy_meson_usb2_ops);
+ if (IS_ERR(phy)) {
+ dev_err(&pdev->dev, "failed to create PHY\n");
+ return PTR_ERR(phy);
+ }
+
+ if (usb_reset_refcnt++ == 0) {
+ ret = device_reset(&pdev->dev);
+ if (ret) {
+ dev_err(&phy->dev, "Failed to reset USB PHY\n");
+ return ret;
+ }
+ }


The ref count + reset here looks like something that could/should be
handled in a runtime PM callback.

Unfortunately that doesn't work (as Jerome found out) because both
PHYs are sharing the same reset line.
So if the second PHY would call device_reset then it would also reset
the first PHY!

There's a comment above the declaration of usb_reset_refcnt which
tries to explain this:
"The PHYs are sharing a common reset line -> we are only allowed to
reset once for all PHYs."
Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
{" line to make it easier to see?



pm-runtime has refcounting in it. When one of the nodes turns on,
the pm-runtime will call your driver to say there is a user when
this first use turns up.

If all the sub-phys turn off and drop their refcount then the driver
is called to say there are no more users and you can go to sleep.


After a chat w/Martin on IRC, It turns out runtime PM wont help here.

The reason is because there are physically two PHY devices[1].  Those 2
devices will be treated independely by runtime PM, and have separate
use-counting, which means doing what I proposed would cause a reset to
happen when either device was probed.

So, I think it's OK as it is.


Surely you can do pm_runtime_get/put on the phy's parent platform
device and do it that way?

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

2016-09-08 Thread Ben Dooks

On 08/09/16 20:52, Martin Blumenstingl wrote:

On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman  wrote:

+ phy = devm_phy_create(&pdev->dev, NULL, &phy_meson_usb2_ops);
+ if (IS_ERR(phy)) {
+ dev_err(&pdev->dev, "failed to create PHY\n");
+ return PTR_ERR(phy);
+ }
+
+ if (usb_reset_refcnt++ == 0) {
+ ret = device_reset(&pdev->dev);
+ if (ret) {
+ dev_err(&phy->dev, "Failed to reset USB PHY\n");
+ return ret;
+ }
+ }


The ref count + reset here looks like something that could/should be
handled in a runtime PM callback.

Unfortunately that doesn't work (as Jerome found out) because both
PHYs are sharing the same reset line.
So if the second PHY would call device_reset then it would also reset
the first PHY!

There's a comment above the declaration of usb_reset_refcnt which
tries to explain this:
"The PHYs are sharing a common reset line -> we are only allowed to
reset once for all PHYs."
Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
{" line to make it easier to see?



pm-runtime has refcounting in it. When one of the nodes turns on,
the pm-runtime will call your driver to say there is a user when
this first use turns up.

If all the sub-phys turn off and drop their refcount then the driver
is called to say there are no more users and you can go to sleep.

So, in phy_meson_usb2_power_on() you could do:

pm_runtime_get_sync(pdev);

and in phy_meson_usb2_power_off

pm_runtime_put(pdev);

https://www.kernel.org/doc/Documentation/power/runtime_pm.txt

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

2016-09-08 Thread Ben Dooks

On 08/09/16 20:35, Kevin Hilman wrote:

Martin Blumenstingl  writes:


This is a new driver for the USB PHY found in Meson8b and GXBB SoCs.

Signed-off-by: Martin Blumenstingl 
Signed-off-by: Jerome Brunet 


I tested this on meson-gxbb-p200 with USB host and a mass storage
device.

Tested-by: Kevin Hilman 

A minor question/comment below, for you and for Kishon...

[...]


+static int phy_meson_usb2_probe(struct platform_device *pdev)
+{
+   struct phy_meson_usb2_priv *priv;
+   struct resource *res;
+   struct phy *phy;
+   struct phy_provider *phy_provider;
+   int ret;
+
+   priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+   if (!priv)
+   return -ENOMEM;
+
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   priv->regs = devm_ioremap_resource(&pdev->dev, res);
+   if (IS_ERR(priv->regs))
+   return PTR_ERR(priv->regs);
+
+   priv->clk_usb_general = devm_clk_get(&pdev->dev, "usb_general");
+   if (IS_ERR(priv->clk_usb_general))
+   return PTR_ERR(priv->clk_usb_general);
+
+   priv->clk_usb = devm_clk_get(&pdev->dev, "usb");
+   if (IS_ERR(priv->clk_usb))
+   return PTR_ERR(priv->clk_usb);
+
+   priv->dr_mode = of_usb_get_dr_mode_by_phy(pdev->dev.of_node, -1);
+   if (priv->dr_mode == USB_DR_MODE_UNKNOWN) {
+   dev_err(&pdev->dev,
+   "missing dual role configuration of the controller\n");
+   return -EINVAL;
+   }
+
+   phy = devm_phy_create(&pdev->dev, NULL, &phy_meson_usb2_ops);
+   if (IS_ERR(phy)) {
+   dev_err(&pdev->dev, "failed to create PHY\n");
+   return PTR_ERR(phy);
+   }
+
+   if (usb_reset_refcnt++ == 0) {
+   ret = device_reset(&pdev->dev);
+   if (ret) {
+   dev_err(&phy->dev, "Failed to reset USB PHY\n");
+   return ret;
+   }
+   }


The ref count + reset here looks like something that could/should be
handled in a runtime PM callback.

IOW, there should be a pm_runtime_get_sync() here, and in the
->runtime_resume() callback, the device_reset() would be called.
Runtime PM does the refcounting, and only calls ->runtime_resume() on
the 0 -> 1 transition.

This isn't a big deal for now, so I'll let Kishon decide, but using
runtime PM from the start will help enabling other PM features later.


I agree, pm_runtime would probably be the best place to handle >1
device with shared items such as reset.

The version I wrote, I simply enabled the clocks and reset the device
when probed to work around the shared reset.


--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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-at91: make at91_dt_syscon_sfr static

2016-06-27 Thread Ben Dooks
On 26/06/16 19:48, Greg Kroah-Hartman wrote:
> On Fri, Jun 17, 2016 at 04:11:55PM +0100, Ben Dooks wrote:
>> Make at91_dt_syscon_sfr() static as it is not used or declared
>> outside the drivers/usb/host/ohci-at91.c file.
>>
>> drivers/usb/host/ohci-at91.c:144:15: warning: symbol 'at91_dt_syscon_sfr' 
>> was not declared. Should it be static?
>>
>> Signed-off-by: Ben Dooks 
>> Acked-by: Nicolas Ferre 
>> Acked-by: Alexandre Belloni 
>> ---
>> Cc: Alan Stern 
>> Cc: Greg Kroah-Hartman 
>> Cc: linux-usb@vger.kernel.org
>> Cc: Alexandre Belloni 
>> Cc: Jean-Christophe Plagniol-Villard 
>> ---
>>  drivers/usb/host/ohci-at91.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Doesn't apply properly to my tree :(

I think the function has now been removed, but I'd need to check.

-- 
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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: renesas_usbhs: make usbhs_write32() static

2016-06-21 Thread Ben Dooks
The usbhs_write32 function is not used outside of the rcar3.c
file, so fix the following sparse warning by making it static:

drivers/usb/renesas_usbhs/rcar3.c:26:6: warning: symbol 'usbhs_write32' was not 
declared. Should it be static?

Signed-off-by: Ben Dooks 
---
Cc: Yoshihiro Shimoda 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Cc: linux-renesas-...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
---
 drivers/usb/renesas_usbhs/rcar3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/renesas_usbhs/rcar3.c 
b/drivers/usb/renesas_usbhs/rcar3.c
index 38b01f2..1d70add 100644
--- a/drivers/usb/renesas_usbhs/rcar3.c
+++ b/drivers/usb/renesas_usbhs/rcar3.c
@@ -23,7 +23,7 @@
 #define UGCTRL2_RESERVED_3 0x0001  /* bit[3:0] should be B'0001 */
 #define UGCTRL2_USB0SEL_OTG0x0030
 
-void usbhs_write32(struct usbhs_priv *priv, u32 reg, u32 data)
+static void usbhs_write32(struct usbhs_priv *priv, u32 reg, u32 data)
 {
iowrite32(data, priv->base + reg);
 }
-- 
2.8.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


[PATCH] usb: ohci-at91: make at91_dt_syscon_sfr static

2016-06-17 Thread Ben Dooks
Make at91_dt_syscon_sfr() static as it is not used or declared
outside the drivers/usb/host/ohci-at91.c file.

drivers/usb/host/ohci-at91.c:144:15: warning: symbol 'at91_dt_syscon_sfr' was 
not declared. Should it be static?

Signed-off-by: Ben Dooks 
---
Cc: Alan Stern 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Cc: Nicolas Ferre 
Cc: Alexandre Belloni 
Cc: Jean-Christophe Plagniol-Villard 
---
 drivers/usb/host/ohci-at91.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 54e8feb..a69b421 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -141,7 +141,7 @@ static void at91_stop_hc(struct platform_device *pdev)
 
 /*-*/
 
-struct regmap *at91_dt_syscon_sfr(void)
+static struct regmap *at91_dt_syscon_sfr(void)
 {
struct regmap *regmap;
 
-- 
2.8.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: [PATCH] USB: core: fix missing include

2016-06-16 Thread Ben Dooks
On 14/06/16 12:08, Arnd Bergmann wrote:
> On Wednesday, June 8, 2016 3:04:27 AM CEST kbuild test robot wrote:
>>>> drivers/usb/core/of.c:32:21: error: redefinition of 'usb_of_get_child_node'
>> struct device_node *usb_of_get_child_node(struct device_node *parent,
>> ^
>>In file included from drivers/usb/core/of.c:21:0:
>>include/linux/usb/of.h:36:35: note: previous definition of 
>> 'usb_of_get_child_node' was here
>> static inline struct device_node *usb_of_get_child_node
>>   ^
>>
>> vim +/usb_of_get_child_node +32 drivers/usb/core/of.c
>>
>> 69bec725 Peter Chen 2016-02-19  26   * @portnum: the port number which 
>> device is connecting
>> 69bec725 Peter Chen 2016-02-19  27   *
>> 69bec725 Peter Chen 2016-02-19  28   * Find the node from device tree 
>> according to its port number.
>> 69bec725 Peter Chen 2016-02-19  29   *
>> 69bec725 Peter Chen 2016-02-19  30   * Return: On success, a pointer to the 
>> device node, %NULL on failure.
>> 69bec725 Peter Chen 2016-02-19  31   */
>> 69bec725 Peter Chen 2016-02-19 @32  struct device_node 
>> *usb_of_get_child_node(struct device_node *parent,
>> 69bec725 Peter Chen 2016-02-19  33  int 
>> portnum)
>> 69bec725 Peter Chen 2016-02-19  34  {
>> 69bec725 Peter Chen 2016-02-19  35  struct device_node *node;
>>
>>
> 
> I think what we want here is to make the compilation of of.o conditional on
> CONFIG_OF, so we get only one of the two definitions.

Ah, so make the of.o conditional, and also apply my patch for when
it is compiled.

Should I submit one for that, or is someone else on the case?

-- 
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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: core: fix missing include

2016-06-16 Thread Ben Dooks
On 15/06/16 03:43, Peter Chen wrote:
> On Tue, Jun 14, 2016 at 01:08:08PM +0200, Arnd Bergmann wrote:
>> On Wednesday, June 8, 2016 3:04:27 AM CEST kbuild test robot wrote:
>>>>> drivers/usb/core/of.c:32:21: error: redefinition of 
>>>>> 'usb_of_get_child_node'
>>> struct device_node *usb_of_get_child_node(struct device_node *parent,
>>> ^
>>>In file included from drivers/usb/core/of.c:21:0:
>>>include/linux/usb/of.h:36:35: note: previous definition of 
>>> 'usb_of_get_child_node' was here
>>> static inline struct device_node *usb_of_get_child_node
>>>   ^
>>>
>>> vim +/usb_of_get_child_node +32 drivers/usb/core/of.c
>>>
>>> 69bec725 Peter Chen 2016-02-19  26   * @portnum: the port number which 
>>> device is connecting
>>> 69bec725 Peter Chen 2016-02-19  27   *
>>> 69bec725 Peter Chen 2016-02-19  28   * Find the node from device tree 
>>> according to its port number.
>>> 69bec725 Peter Chen 2016-02-19  29   *
>>> 69bec725 Peter Chen 2016-02-19  30   * Return: On success, a pointer to the 
>>> device node, %NULL on failure.
>>> 69bec725 Peter Chen 2016-02-19  31   */
>>> 69bec725 Peter Chen 2016-02-19 @32  struct device_node 
>>> *usb_of_get_child_node(struct device_node *parent,
>>> 69bec725 Peter Chen 2016-02-19  33  int 
>>> portnum)
>>> 69bec725 Peter Chen 2016-02-19  34  {
>>> 69bec725 Peter Chen 2016-02-19  35  struct device_node *node;
>>>
>>>
>>
>> I think what we want here is to make the compilation of of.o conditional on
>> CONFIG_OF, so we get only one of the two definitions.
>>
>>  Arnd
> 
> Thanks, Arnd. It is the correct solution, I will send patch soon.

Aha, I didn't think of that. Thanks.


-- 
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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: core: fix missing include

2016-06-07 Thread Ben Dooks
The helper function usb_of_get_child_node() is defined in the
header  but this was not included. Fix the warning
about usb_of_get_child_node() not being declared by adding the
right include. Fixes:

drivers/usb/core/of.c:31:20: warning: symbol 'usb_of_get_child_node' was not 
declared. Should it be static?

Signed-off-by: Ben Dooks 
---
Cc: Greg Kroah-Hartman 
Cc: Arnd Bergmann 
Cc: Philipp Zabel 
Cc: Alan Stern 
Cc: Peter Chen 
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
 drivers/usb/core/of.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
index 2289700..3de4f88 100644
--- a/drivers/usb/core/of.c
+++ b/drivers/usb/core/of.c
@@ -18,6 +18,7 @@
  */
 
 #include 
+#include 
 
 /**
  * usb_of_get_child_node - Find the device node match port number
-- 
2.8.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: AM3517 usb host issue

2015-09-24 Thread Ben Dooks
On 22/05/15 14:50, Felipe Balbi wrote:
> Hi,
> 
> On Fri, May 22, 2015 at 11:04:33AM +0300, Ben Dooks wrote:
>> I am trying to get the full-speed USB host working on an custom 
>> AM3517 device with the 3.18.12 kernel. The hardware works (a 
>> 2.6.37 kernel has been used for testing).
>> 
>> Does anyone have any experience of 3.18 (or similarly recent 
>> kernel on an AM3517 system) or have any pointers as where to 
>> start debugging? The ti-linux-3.14.y does not have any patches 
>> that aren't applied to the usb on 3.18.13.
>> 
>> The cpu port 1 is connected by a TI TUSB1106 usb transceiver
>> that is directly connected to a full-speed hub (TI USB2046) hub
>> so the OHCI driver is the only one in use.
>> 
>> Note, the ohci-omap3 is loaded as a module as this is how their 
>> user application expects to be able to shut down usb when it
>> does not need it.
>> 
>> The device tree configuration for the usb host:
> 
> and what exactly doesn't work ? That old OHCI driver hasn't been 
> touched in years, it's no surprise that it stopped working :-(
> 
> Anyway, what exactly doesn't work ? No device enumerates ? Do you 
> get any IRQs by plugging a new device in ?

I just found this in my backlog, and thought I should follow up with
the issue.

It turns out that even if we're not using the EHCI unit, the 120m
functional clock needs to be enabled. This may be down to an internal
routing of port signals, or the USB front-end needing it.

I've added a local boolean fix to enable the clock, and will post it
as soon as we've had a chance to review and document.

-- 
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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] cdc-acm: Add support of ATOL FPrint fiscal printers

2015-06-02 Thread Ben Dooks
On 02/06/15 11:05, Alexey Sokolov wrote:
> ATOL FPrint fiscal printers require usb_clear_halt to be executed
> to work properly. Add quirk to fix the issue.
> 
> Signed-off-by: Alexey Sokolov 
> ---
>  drivers/usb/class/cdc-acm.c | 9 +
>  drivers/usb/class/cdc-acm.h | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> index 5c8f581..9d8a321 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -1477,6 +1477,11 @@ skip_countries:
>   goto alloc_fail8;
>   }
>  
> + if (quirks == CLEAR_HALT_CONDITIONS) {
> + usb_clear_halt(usb_dev, usb_rcvbulkpipe(usb_dev, 
> epread->bEndpointAddress));
> + usb_clear_halt(usb_dev, usb_sndbulkpipe(usb_dev, 
> epwrite->bEndpointAddress));
> + }
> +

Given quirks looks like a bitfield, it would be better if this is

if (quirks & CLEAR_HALT_CONDITIONS) {


-- 
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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: AM3517 usb host issue

2015-05-26 Thread Ben Dooks
On 22/05/15 23:13, Ben Dooks wrote:
> On 22/05/15 16:50, Felipe Balbi wrote:
>> Hi,
> 
>> On Fri, May 22, 2015 at 11:04:33AM +0300, Ben Dooks wrote:
>>> I am trying to get the full-speed USB host working on an custom
>>> AM3517 device with the 3.18.12 kernel. The hardware works (a
>>> 2.6.37 kernel has been used for testing).
>>>
>>> Does anyone have any experience of 3.18 (or similarly recent
>>> kernel on an AM3517 system) or have any pointers as where to
>>> start debugging? The ti-linux-3.14.y does not have any patches
>>> that aren't applied to the usb on 3.18.13.
>>>
>>> The cpu port 1 is connected by a TI TUSB1106 usb transceiver that
>>> is directly connected to a full-speed hub (TI USB2046) hub so the
>>> OHCI driver is the only one in use.
>>>
>>> Note, the ohci-omap3 is loaded as a module as this is how their
>>> user application expects to be able to shut down usb when it does
>>> not need it.
>>>
>>> The device tree configuration for the usb host:
> 
>> and what exactly doesn't work ? That old OHCI driver hasn't been
>> touched in years, it's no surprise that it stopped working :-(
> 
>> Anyway, what exactly doesn't work ? No device enumerates ? Do you
>> get any IRQs by plugging a new device in ?
> 
> I will check the interrupts when I get back into the office.
> 
> There is only one device (the hub) which isn't enumerating and is
> hard-wired on the board.
> 
>>>> &usbhshost { status = "okay";  /* just in case it is started
>>>> disabled */
>>>>
>>>> port1-mode = "ohci-phy-6pin-dpdm"; };
>>>>
>>>> &usbhsohci { status = "okay"; };
>>>>
>>>> &usbhsehci { status = "disabled";  /* no ehci on board */ };
>>>
>>>
>>> The usb from the logs is as follows. Some extra debugging has
>>> been added to verify the device-tree settings:
>>>
>>>> [0.00] AM3517 ES1.1 (l2cache sgx neon)
>>>>
>>>>
>>>> [0.869706] usbcore: registered new interface driver usbfs
>>>>  [0.874270] usbcore: registered new interface driver hub
>>>>  [0.878592] usbcore: registered new device driver usb
>>>>  [1.223199] usbhs_tll 48062000.usbhstll: starting TI HSUSB
>>>> TLL Controller [1.273000] usbhs_omap 48064000.usbhshost:
>>>> ports 0 [1.278291] usbhs_omap 48064000.usbhshost: port 0:
>>>> ohci-phy-6pin-dpdm [1.284476] usbhs_omap
>>>> 48064000.usbhshost: port0-mode: ohci-phy-6pin-dpdm ->5 [
>>>> 1.288689] usbhs_tll 48062000.usbhstll: omap_tll_init()
>>>>  [1.293628] usbhs_omap 48064000.usbhshost:
>>>> usbhs_runtime_resume [1.298434] usbhs_omap
>>>> 48064000.usbhshost: sysconfig 0x1009 [1.302730]
>>>> usbhs_tll 48062000.usbhstll: omap_tll_enable()
>>>>  [1.307668] usbhs_omap 48064000.usbhshost:
>>>> usbhs_runtime_suspend [1.310142] stopping usb controller
>>>>  [1.419910] usbhs_tll 48062000.usbhstll: omap_tll_disable()
>>>>  [1.423547] usbhs_omap 48064000.usbhshost: 3 ports
>>>>  [1.429065] usbhs_omap 48064000.usbhshost: starting TI
>>>> HSUSB Controller [1.433831] usbhs_omap 48064000.usbhshost:
>>>> usbhs_runtime_resume [1.438625] usbhs_omap
>>>> 48064000.usbhshost: sysconfig 0x1009 [1.442921]
>>>> usbhs_tll 48062000.usbhstll: omap_tll_enable()
>>>>  [1.448548] usbhs_omap 48064000.usbhshost:
>>>> omap_usbhs_rev1_hostconfig => [1.455034] usbhs_omap
>>>> 48064000.usbhshost: UHH setup done, uhh_hostconfig=80d [
>>>> 1.459918] usbhs_omap 48064000.usbhshost: usbhs_runtime_suspend
>>>>  [1.462337] stopping usb controller
>>>>  [1.569905] usbhs_tll 48062000.usbhstll: omap_tll_disable()
>>>>  [1.575408] usbhs_omap 48064000.usbhshost: populating usb
>>>> sub nodes
>>>>
>>>> [   77.609168] usbhs_omap 48064000.usbhshost:
>>>> usbhs_runtime_resume [   77.613927] usbhs_omap
>>>> 48064000.usbhshost: sysconfig 0x1009 [   77.618374]
>>>> usbhs_tll 48062000.usbhstll: omap_tll_enable()
>>>>  [   77.802694] usb usb1: New USB device found, idVendor=1d6b,
>>>> idProduct=0001 [   77.816003] usb usb1: New USB device strings:
>>>> Mfr=3, Product=2, SerialNumber1 [   77.827391] usb usb1:
>>>>

Re: AM3517 usb host issue

2015-05-22 Thread Ben Dooks
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 22/05/15 16:50, Felipe Balbi wrote:
> Hi,
> 
> On Fri, May 22, 2015 at 11:04:33AM +0300, Ben Dooks wrote:
>> I am trying to get the full-speed USB host working on an custom
>> AM3517 device with the 3.18.12 kernel. The hardware works (a
>> 2.6.37 kernel has been used for testing).
>> 
>> Does anyone have any experience of 3.18 (or similarly recent
>> kernel on an AM3517 system) or have any pointers as where to
>> start debugging? The ti-linux-3.14.y does not have any patches
>> that aren't applied to the usb on 3.18.13.
>> 
>> The cpu port 1 is connected by a TI TUSB1106 usb transceiver that
>> is directly connected to a full-speed hub (TI USB2046) hub so the
>> OHCI driver is the only one in use.
>> 
>> Note, the ohci-omap3 is loaded as a module as this is how their
>> user application expects to be able to shut down usb when it does
>> not need it.
>> 
>> The device tree configuration for the usb host:
> 
> and what exactly doesn't work ? That old OHCI driver hasn't been
> touched in years, it's no surprise that it stopped working :-(
> 
> Anyway, what exactly doesn't work ? No device enumerates ? Do you
> get any IRQs by plugging a new device in ?

I will check the interrupts when I get back into the office.

There is only one device (the hub) which isn't enumerating and is
hard-wired on the board.

>>> &usbhshost { status = "okay";   /* just in case it is started
>>> disabled */
>>> 
>>> port1-mode = "ohci-phy-6pin-dpdm"; };
>>> 
>>> &usbhsohci { status = "okay"; };
>>> 
>>> &usbhsehci { status = "disabled";   /* no ehci on board */ };
>> 
>> 
>> The usb from the logs is as follows. Some extra debugging has
>> been added to verify the device-tree settings:
>> 
>>> [0.00] AM3517 ES1.1 (l2cache sgx neon)
>>> 
>>> 
>>> [0.869706] usbcore: registered new interface driver usbfs
>>>  [0.874270] usbcore: registered new interface driver hub
>>>  [0.878592] usbcore: registered new device driver usb
>>>  [1.223199] usbhs_tll 48062000.usbhstll: starting TI HSUSB
>>> TLL Controller [1.273000] usbhs_omap 48064000.usbhshost:
>>> ports 0 [1.278291] usbhs_omap 48064000.usbhshost: port 0:
>>> ohci-phy-6pin-dpdm [1.284476] usbhs_omap
>>> 48064000.usbhshost: port0-mode: ohci-phy-6pin-dpdm ->5 [
>>> 1.288689] usbhs_tll 48062000.usbhstll: omap_tll_init()
>>>  [1.293628] usbhs_omap 48064000.usbhshost:
>>> usbhs_runtime_resume [1.298434] usbhs_omap
>>> 48064000.usbhshost: sysconfig 0x1009 [1.302730]
>>> usbhs_tll 48062000.usbhstll: omap_tll_enable()
>>>  [1.307668] usbhs_omap 48064000.usbhshost:
>>> usbhs_runtime_suspend [1.310142] stopping usb controller
>>>  [1.419910] usbhs_tll 48062000.usbhstll: omap_tll_disable()
>>>  [1.423547] usbhs_omap 48064000.usbhshost: 3 ports
>>>  [1.429065] usbhs_omap 48064000.usbhshost: starting TI
>>> HSUSB Controller [1.433831] usbhs_omap 48064000.usbhshost:
>>> usbhs_runtime_resume [1.438625] usbhs_omap
>>> 48064000.usbhshost: sysconfig 0x1009 [1.442921]
>>> usbhs_tll 48062000.usbhstll: omap_tll_enable()
>>>  [1.448548] usbhs_omap 48064000.usbhshost:
>>> omap_usbhs_rev1_hostconfig => [1.455034] usbhs_omap
>>> 48064000.usbhshost: UHH setup done, uhh_hostconfig=80d [
>>> 1.459918] usbhs_omap 48064000.usbhshost: usbhs_runtime_suspend
>>>  [1.462337] stopping usb controller
>>>  [1.569905] usbhs_tll 48062000.usbhstll: omap_tll_disable()
>>>  [1.575408] usbhs_omap 48064000.usbhshost: populating usb
>>> sub nodes
>>> 
>>> [   77.609168] usbhs_omap 48064000.usbhshost:
>>> usbhs_runtime_resume [   77.613927] usbhs_omap
>>> 48064000.usbhshost: sysconfig 0x1009 [   77.618374]
>>> usbhs_tll 48062000.usbhstll: omap_tll_enable()
>>>  [   77.802694] usb usb1: New USB device found, idVendor=1d6b,
>>> idProduct=0001 [   77.816003] usb usb1: New USB device strings:
>>> Mfr=3, Product=2, SerialNumber1 [   77.827391] usb usb1:
>>> Product: OHCI Host Controller [   77.838674] usb usb1:
>>> Manufacturer: Linux 3.18.13-00203-ga3c52be-dirty ohci_d [
>>> 77.849913] usb usb1: SerialNumber: 48064400.ohci
>>> 
> 
> OK, so this is roothub, what happens when a device is plugged to
> the other end ? Is VBUS charged ? We didn

AM3517 usb host issue

2015-05-22 Thread Ben Dooks
I am trying to get the full-speed USB host working on an custom AM3517
device with the 3.18.12 kernel. The hardware works (a 2.6.37 kernel has
been used for testing).

Does anyone have any experience of 3.18 (or similarly recent kernel on
an AM3517 system) or have any pointers as where to start debugging? The
ti-linux-3.14.y does not have any patches that aren't applied to the
usb on 3.18.13.

The cpu port 1 is connected by a TI TUSB1106 usb transceiver that is
directly connected to a full-speed hub (TI USB2046) hub so the OHCI
driver is the only one in use.

Note, the ohci-omap3 is loaded as a module as this is how their user
application expects to be able to shut down usb when it does not need
it.

The device tree configuration for the usb host:

> &usbhshost {
>   status = "okay";/* just in case it is started disabled */
> 
>   port1-mode = "ohci-phy-6pin-dpdm";
> };
> 
> &usbhsohci {
>   status = "okay";
> };
> 
> &usbhsehci {
>   status = "disabled";/* no ehci on board */
> };


The usb from the logs is as follows. Some extra debugging has been
added to verify the device-tree settings:

> [0.00] AM3517 ES1.1 (l2cache sgx neon)
>  
> 
> [0.869706] usbcore: registered new interface driver usbfs 
>   
> [0.874270] usbcore: registered new interface driver hub   
>   
> [0.878592] usbcore: registered new device driver usb  
>   
> [1.223199] usbhs_tll 48062000.usbhstll: starting TI HSUSB TLL Controller  
>   
> [1.273000] usbhs_omap 48064000.usbhshost: ports 0 
>   
> [1.278291] usbhs_omap 48064000.usbhshost: port 0: ohci-phy-6pin-dpdm  
>   
> [1.284476] usbhs_omap 48064000.usbhshost: port0-mode: ohci-phy-6pin-dpdm 
> ->5
> [1.288689] usbhs_tll 48062000.usbhstll: omap_tll_init()   
>   
> [1.293628] usbhs_omap 48064000.usbhshost: usbhs_runtime_resume
>   
> [1.298434] usbhs_omap 48064000.usbhshost: sysconfig 0x1009
>   
> [1.302730] usbhs_tll 48062000.usbhstll: omap_tll_enable() 
>   
> [1.307668] usbhs_omap 48064000.usbhshost: usbhs_runtime_suspend   
>   
> [1.310142] stopping usb controller
>   
> [1.419910] usbhs_tll 48062000.usbhstll: omap_tll_disable()
>   
> [1.423547] usbhs_omap 48064000.usbhshost: 3 ports 
>   
> [1.429065] usbhs_omap 48064000.usbhshost: starting TI HSUSB Controller
>   
> [1.433831] usbhs_omap 48064000.usbhshost: usbhs_runtime_resume
>   
> [1.438625] usbhs_omap 48064000.usbhshost: sysconfig 0x1009
>   
> [1.442921] usbhs_tll 48062000.usbhstll: omap_tll_enable() 
>   
> [1.448548] usbhs_omap 48064000.usbhshost: omap_usbhs_rev1_hostconfig =>   
>   
> [1.455034] usbhs_omap 48064000.usbhshost: UHH setup done, 
> uhh_hostconfig=80d
> [1.459918] usbhs_omap 48064000.usbhshost: usbhs_runtime_suspend   
>   
> [1.462337] stopping usb controller
>   
> [1.569905] usbhs_tll 48062000.usbhstll: omap_tll_disable()
>   
> [1.575408] usbhs_omap 48064000.usbhshost: populating usb sub nodes
>   
> 
> [   77.609168] usbhs_omap 48064000.usbhshost: usbhs_runtime_resume
>   
> [   77.613927] usbhs_omap 48064000.usbhshost: sysconfig 0x1009
>   
> [   77.618374] usbhs_tll 48062000.usbhstll: omap_tll_enable() 
>   
> [   77.802694] usb usb1: New USB device found, idVendor=1d6b, idProduct=0001  
>   
> [   77.816003] usb usb1: New USB device strings: Mfr=3, Product=2, 
> SerialNumber1
> [   77.827391] usb usb1: Product: OHCI Host Controller
>   
> [   77.838674] usb usb1: Manufacturer: Linux 3.18.13-00203-ga3c52be-dirty 
> ohci_d
> [   77.849913] usb usb1: SerialNumber: 48064400.ohci  
>   

-- 
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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: [RFC 5/6] usb: gadget: atmel_usba: use atmel_io.h to provide on-chip IO

2015-04-28 Thread Ben Dooks
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 28/04/15 17:30, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Apr 28, 2015 at 04:40:14PM +0100, Ben Dooks wrote:
>>>>> /* Register access macros */ -#ifdef CONFIG_AVR32 -#define 
>>>>> usba_io_readl __raw_readl -#define usba_io_writel
>>>>> __raw_writel -#define usba_io_writew  __raw_writew -#else
>>>>> -#define usba_io_readlreadl_relaxed -#define
>>>>> usba_io_writel writel_relaxed -#define usba_io_writew
>>>>> writew_relaxed -#endif +#define usba_io_readl
>>>>> atmel_oc_readl +#define usba_io_writel atmel_oc_writel
>>>>> +#define usba_io_writew   atmel_oc_writew
>>>> 
>>>> Same comment as earlier patch, it would be nice to remove
>>>> the define usba_io_{read,write}{l,w} defines in a follow-up
>>>> patch.
>>> 
>>> I'm fine with this too. Is this targetted at v4.2 ?
>> 
>> Yes, although we may move it to the soc specific include
>> directories to avoid adding more to linux/
>> 
>> I will be sorting this out next week.
> 
> I would rather not see drivers including anything from asm or
> mach-* (unless strictly necessary), otherwise it's a pain to
> build-test anything


Thanks. I think the soc/ include is available for all. I will check this
tomorrow.


- -- 
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQEcBAEBCAAGBQJVP7aBAAoJEMuhVOkVU3uzId4H/3ZsZ4Vogng63I04OpLtt5Qs
VopvttCvaLczye/wOWQKptXJuKIBmrs66BcV4ZsVi1SZjfbju9X1blzc0+6fxU9X
xE2rw3kDPZmtKm6v1GCRrE/uzhKeKgAnfp7Yf7pkdn98Lx2D9gy/s7GRN6Bkql28
pz/QIqr/H6Hp2frbubakKx/F7SFV88ZUNZXLkw8+vkn7gKGRX7ODcPFldjbjooIa
zyDdrZWiqhTHdWzQ813vNgKJfP1GiYNZuht+EhAh0ngLy0YnqnSXF0eZiOy/uF0Z
cCMlXZo1Q/2LkqVKEgywJDxGhCruT3RpPDWLk6eA9trlHF62j3WSn65nDxKtBwA=
=cqtq
-END PGP SIGNATURE-
--
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: [RFC 5/6] usb: gadget: atmel_usba: use atmel_io.h to provide on-chip IO

2015-04-28 Thread Ben Dooks
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256


>>> 
>>> /* Register access macros */ -#ifdef CONFIG_AVR32 -#define
>>> usba_io_readl   __raw_readl -#define usba_io_writel __raw_writel 
>>> -#define usba_io_writew __raw_writew -#else -#define
>>> usba_io_readl   readl_relaxed -#define usba_io_writel
>>> writel_relaxed -#define usba_io_writew  writew_relaxed -#endif 
>>> +#define usba_io_readl  atmel_oc_readl +#define usba_io_writel
>>> atmel_oc_writel +#define usba_io_writew atmel_oc_writew
>> 
>> Same comment as earlier patch, it would be nice to remove the
>> define usba_io_{read,write}{l,w} defines in a follow-up patch.
> 
> I'm fine with this too. Is this targetted at v4.2 ?

Yes, although we may move it to the soc specific include directories
to avoid adding more to linux/

I will be sorting this out next week.

- -- 
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQEcBAEBCAAGBQJVP6neAAoJEMuhVOkVU3uz6tQH/jmZ0MszsoDjiKWdTnG+etoy
WUhAEdnmaqPEfJSaBy2OPCcmj9T1O07wg1L/2rHQHBrVuR9bW0gWY/TyJQahnn3A
YG81fgfIvOTAwmxIkaF10mG1/haG1LgPQ4P6j7AaKz09Zowath0rsga17AhYuUyh
bps4Go7yHF/OpjuwB9VxSttMAbVAIHqXbuc5kufN89JAxDT6x5tV/BrqkqEoHpSV
kW7VvGb/V4Aeax1h9klfdUEBlL2czkVuYYtTnicc9lFN3T9+6XJ+SNwKLtSklEr3
SLM78DoIQQoKYWCf8vsg0FR0E9LeR1ytgZXPfOpdAoId+iZt39tBk9bMi/XOiBo=
=ceej
-END PGP SIGNATURE-
--
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


[RFC 5/6] usb: gadget: atmel_usba: use atmel_io.h to provide on-chip IO

2015-03-26 Thread Ben Dooks
Use  to provide IO accessors which work on both
AVR32 and ARM for on-chip peripherals.

Signed-off-by: Ben Dooks 
--
CC: Nicolas Ferre 
CC: Felipe Balbi 
CC: Greg Kroah-Hartman 
CC: linux-usb@vger.kernel.org
---
 drivers/usb/gadget/udc/atmel_usba_udc.c |  1 +
 drivers/usb/gadget/udc/atmel_usba_udc.h | 12 +++-
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index be2f503..6735585 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h 
b/drivers/usb/gadget/udc/atmel_usba_udc.h
index 92bd486..3d40aa3 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.h
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
@@ -191,15 +191,9 @@
 | USBA_BF(name, value))
 
 /* Register access macros */
-#ifdef CONFIG_AVR32
-#define usba_io_readl  __raw_readl
-#define usba_io_writel __raw_writel
-#define usba_io_writew __raw_writew
-#else
-#define usba_io_readl  readl_relaxed
-#define usba_io_writel writel_relaxed
-#define usba_io_writew writew_relaxed
-#endif
+#define usba_io_readl  atmel_oc_readl
+#define usba_io_writel atmel_oc_writel
+#define usba_io_writew atmel_oc_writew
 
 #define usba_readl(udc, reg)   \
usba_io_readl((udc)->regs + USBA_##reg)
-- 
2.1.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 10/13] usb: gadget: atmel_usba: use endian agnostic IO on ARM

2015-03-18 Thread Ben Dooks
Change from using the __raw IO accesors to the endian agnostic versions
of readl/writel_relaxed when not on AVR32. This fixes issues with running
big endian on ARMv7.

Signed-off-by: Ben Dooks 
--
CC: Nicolas Ferre 
CC: Felipe Balbi 
CC: Greg Kroah-Hartman 
CC: linux-usb@vger.kernel.org
---
 drivers/usb/gadget/udc/atmel_usba_udc.c |  4 ++--
 drivers/usb/gadget/udc/atmel_usba_udc.h | 22 --
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index d79cb35..be2f503 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -152,7 +152,7 @@ static int regs_dbg_open(struct inode *inode, struct file 
*file)
 
spin_lock_irq(&udc->lock);
for (i = 0; i < inode->i_size / 4; i++)
-   data[i] = __raw_readl(udc->regs + i * 4);
+   data[i] = usba_io_readl(udc->regs + i * 4);
spin_unlock_irq(&udc->lock);
 
file->private_data = data;
@@ -1249,7 +1249,7 @@ static int handle_ep0_setup(struct usba_udc *udc, struct 
usba_ep *ep,
if (crq->wLength != cpu_to_le16(sizeof(status)))
goto stall;
ep->state = DATA_STAGE_IN;
-   __raw_writew(status, ep->fifo);
+   usba_io_writew(status, ep->fifo);
usba_ep_writel(ep, SET_STA, USBA_TX_PK_RDY);
break;
}
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h 
b/drivers/usb/gadget/udc/atmel_usba_udc.h
index 497cd18..92bd486 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.h
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
@@ -191,18 +191,28 @@
 | USBA_BF(name, value))
 
 /* Register access macros */
+#ifdef CONFIG_AVR32
+#define usba_io_readl  __raw_readl
+#define usba_io_writel __raw_writel
+#define usba_io_writew __raw_writew
+#else
+#define usba_io_readl  readl_relaxed
+#define usba_io_writel writel_relaxed
+#define usba_io_writew writew_relaxed
+#endif
+
 #define usba_readl(udc, reg)   \
-   __raw_readl((udc)->regs + USBA_##reg)
+   usba_io_readl((udc)->regs + USBA_##reg)
 #define usba_writel(udc, reg, value)   \
-   __raw_writel((value), (udc)->regs + USBA_##reg)
+   usba_io_writel((value), (udc)->regs + USBA_##reg)
 #define usba_ep_readl(ep, reg) \
-   __raw_readl((ep)->ep_regs + USBA_EPT_##reg)
+   usba_io_readl((ep)->ep_regs + USBA_EPT_##reg)
 #define usba_ep_writel(ep, reg, value) \
-   __raw_writel((value), (ep)->ep_regs + USBA_EPT_##reg)
+   usba_io_writel((value), (ep)->ep_regs + USBA_EPT_##reg)
 #define usba_dma_readl(ep, reg)\
-   __raw_readl((ep)->dma_regs + USBA_DMA_##reg)
+   usba_io_readl((ep)->dma_regs + USBA_DMA_##reg)
 #define usba_dma_writel(ep, reg, value)\
-   __raw_writel((value), (ep)->dma_regs + USBA_DMA_##reg)
+   usba_io_writel((value), (ep)->dma_regs + USBA_DMA_##reg)
 
 /* Calculate base address for a given endpoint or DMA controller */
 #define USBA_EPT_BASE(x)   (0x100 + (x) * 0x20)
-- 
2.1.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 1/8] r8a66597-udc: use devm_request_and_ioremap() for registers

2014-06-17 Thread Ben Dooks
---
 drivers/usb/gadget/r8a66597-udc.c | 18 +++---
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/gadget/r8a66597-udc.c 
b/drivers/usb/gadget/r8a66597-udc.c
index aff0a67..7f3af74 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -1826,7 +1826,6 @@ static int __exit r8a66597_remove(struct platform_device 
*pdev)
 
usb_del_gadget_udc(&r8a66597->gadget);
del_timer_sync(&r8a66597->timer);
-   iounmap(r8a66597->reg);
if (r8a66597->pdata->sudmac)
iounmap(r8a66597->sudmac_reg);
free_irq(platform_get_irq(pdev, 0), r8a66597);
@@ -1877,11 +1876,9 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
unsigned long irq_trigger;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!res) {
-   ret = -ENODEV;
-   dev_err(&pdev->dev, "platform_get_resource error.\n");
-   goto clean_up;
-   }
+   reg = devm_ioremap_resource(&pdev->dev, res);
+   if (!reg)
+   return -ENODEV;
 
ires = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
irq = ires->start;
@@ -1893,13 +1890,6 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
goto clean_up;
}
 
-   reg = ioremap(res->start, resource_size(res));
-   if (reg == NULL) {
-   ret = -ENOMEM;
-   dev_err(&pdev->dev, "ioremap error.\n");
-   goto clean_up;
-   }
-
/* initialize ucd */
r8a66597 = kzalloc(sizeof(struct r8a66597), GFP_KERNEL);
if (r8a66597 == NULL) {
@@ -2008,8 +1998,6 @@ clean_up:
r8a66597->ep0_req);
kfree(r8a66597);
}
-   if (reg)
-   iounmap(reg);
 
return ret;
 }
-- 
2.0.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


linux-usb@vger.kernel.org

2014-06-17 Thread Ben Dooks
Remove usages of &pdev->dev in the driver probe function
with just dev to make the references to it easier to
write. Convert all the current users of it to use it.

Signed-off-by: Ben Dooks 
---
 drivers/usb/gadget/r8a66597-udc.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/r8a66597-udc.c 
b/drivers/usb/gadget/r8a66597-udc.c
index 7f3af74..1721e44 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -1866,6 +1866,7 @@ static int __init r8a66597_sudmac_ioremap(struct r8a66597 
*r8a66597,
 
 static int __init r8a66597_probe(struct platform_device *pdev)
 {
+   struct device *dev = &pdev->dev;
char clk_name[8];
struct resource *res, *ires;
int irq;
@@ -1886,7 +1887,7 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
 
if (irq < 0) {
ret = -ENODEV;
-   dev_err(&pdev->dev, "platform_get_irq error.\n");
+   dev_err(dev, "platform_get_irq error.\n");
goto clean_up;
}
 
@@ -1894,13 +1895,13 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
r8a66597 = kzalloc(sizeof(struct r8a66597), GFP_KERNEL);
if (r8a66597 == NULL) {
ret = -ENOMEM;
-   dev_err(&pdev->dev, "kzalloc error\n");
+   dev_err(dev, "kzalloc error\n");
goto clean_up;
}
 
spin_lock_init(&r8a66597->lock);
platform_set_drvdata(pdev, r8a66597);
-   r8a66597->pdata = dev_get_platdata(&pdev->dev);
+   r8a66597->pdata = dev_get_platdata(dev);
r8a66597->irq_sense_low = irq_trigger == IRQF_TRIGGER_LOW;
 
r8a66597->gadget.ops = &r8a66597_gadget_ops;
@@ -1914,10 +1915,9 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
 
if (r8a66597->pdata->on_chip) {
snprintf(clk_name, sizeof(clk_name), "usb%d", pdev->id);
-   r8a66597->clk = clk_get(&pdev->dev, clk_name);
+   r8a66597->clk = clk_get(dev, clk_name);
if (IS_ERR(r8a66597->clk)) {
-   dev_err(&pdev->dev, "cannot get clock \"%s\"\n",
-   clk_name);
+   dev_err(dev, "cannot get clock \"%s\"\n", clk_name);
ret = PTR_ERR(r8a66597->clk);
goto clean_up;
}
@@ -1935,7 +1935,7 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
ret = request_irq(irq, r8a66597_irq, IRQF_SHARED,
udc_name, r8a66597);
if (ret < 0) {
-   dev_err(&pdev->dev, "request_irq error (%d)\n", ret);
+   dev_err(dev, "request_irq error (%d)\n", ret);
goto clean_up2;
}
 
@@ -1973,11 +1973,11 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
}
r8a66597->ep0_req->complete = nop_completion;
 
-   ret = usb_add_gadget_udc(&pdev->dev, &r8a66597->gadget);
+   ret = usb_add_gadget_udc(dev, &r8a66597->gadget);
if (ret)
goto err_add_udc;
 
-   dev_info(&pdev->dev, "version %s\n", DRIVER_VERSION);
+   dev_info(dev, "version %s\n", DRIVER_VERSION);
return 0;
 
 err_add_udc:
-- 
2.0.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 8/8] r8a66597-udc: remove now unused clean_up and clean_up3 label.

2014-06-17 Thread Ben Dooks
With the devm additions, the clean_up and clean_up3 are now
not needed or used. Change clean_up3 and make everything use
clean_up2 and just remove clean_up.

Signed-off-by: Ben Dooks 
---
 drivers/usb/gadget/r8a66597-udc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/r8a66597-udc.c 
b/drivers/usb/gadget/r8a66597-udc.c
index 8414ba5..6ad6028 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -1954,7 +1954,7 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
GFP_KERNEL);
if (r8a66597->ep0_req == NULL) {
ret = -ENOMEM;
-   goto clean_up3;
+   goto clean_up2;
}
r8a66597->ep0_req->complete = nop_completion;
 
@@ -1967,11 +1967,10 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
 
 err_add_udc:
r8a66597_free_request(&r8a66597->ep[0].ep, r8a66597->ep0_req);
-clean_up3:
 clean_up2:
if (r8a66597->pdata->on_chip)
clk_disable_unprepare(r8a66597->clk);
-clean_up:
+
if (r8a66597->ep0_req)
r8a66597_free_request(&r8a66597->ep[0].ep, r8a66597->ep0_req);
 
-- 
2.0.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 7/8] r8a66597-udc: use devm_request_irq() to get device irq

2014-06-17 Thread Ben Dooks
Use the devm_request_irq() call to get the interrupt for the
device and have it automatically free on exit.

Signed-off-by: Ben Dooks 
---
 drivers/usb/gadget/r8a66597-udc.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/r8a66597-udc.c 
b/drivers/usb/gadget/r8a66597-udc.c
index 51eaedd..8414ba5 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -1826,7 +1826,6 @@ static int __exit r8a66597_remove(struct platform_device 
*pdev)
 
usb_del_gadget_udc(&r8a66597->gadget);
del_timer_sync(&r8a66597->timer);
-   free_irq(platform_get_irq(pdev, 0), r8a66597);
r8a66597_free_request(&r8a66597->ep[0].ep, r8a66597->ep0_req);
 
if (r8a66597->pdata->on_chip) {
@@ -1918,8 +1917,8 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
 
disable_controller(r8a66597); /* make sure controller is disabled */
 
-   ret = request_irq(irq, r8a66597_irq, IRQF_SHARED,
-   udc_name, r8a66597);
+   ret = devm_request_irq(dev, irq, r8a66597_irq, IRQF_SHARED,
+  udc_name, r8a66597);
if (ret < 0) {
dev_err(dev, "request_irq error (%d)\n", ret);
goto clean_up2;
@@ -1969,7 +1968,6 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
 err_add_udc:
r8a66597_free_request(&r8a66597->ep[0].ep, r8a66597->ep0_req);
 clean_up3:
-   free_irq(irq, r8a66597);
 clean_up2:
if (r8a66597->pdata->on_chip)
clk_disable_unprepare(r8a66597->clk);
-- 
2.0.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 6/8] r8a66597-udc: use devm_clk_get() to get clock

2014-06-17 Thread Ben Dooks
Change to using the devm_clk_get() to get the clock and
have it automatically freed on exit.

Signed-off-by: Ben Dooks 
---
 drivers/usb/gadget/r8a66597-udc.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/r8a66597-udc.c 
b/drivers/usb/gadget/r8a66597-udc.c
index 9ebe2c0..51eaedd 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -1831,7 +1831,6 @@ static int __exit r8a66597_remove(struct platform_device 
*pdev)
 
if (r8a66597->pdata->on_chip) {
clk_disable_unprepare(r8a66597->clk);
-   clk_put(r8a66597->clk);
}
 
return 0;
@@ -1903,11 +1902,10 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
 
if (r8a66597->pdata->on_chip) {
snprintf(clk_name, sizeof(clk_name), "usb%d", pdev->id);
-   r8a66597->clk = clk_get(dev, clk_name);
+   r8a66597->clk = devm_clk_get(dev, clk_name);
if (IS_ERR(r8a66597->clk)) {
dev_err(dev, "cannot get clock \"%s\"\n", clk_name);
-   ret = PTR_ERR(r8a66597->clk);
-   goto clean_up;
+   return PTR_ERR(r8a66597->clk);
}
clk_prepare_enable(r8a66597->clk);
}
@@ -1973,10 +1971,8 @@ err_add_udc:
 clean_up3:
free_irq(irq, r8a66597);
 clean_up2:
-   if (r8a66597->pdata->on_chip) {
+   if (r8a66597->pdata->on_chip)
clk_disable_unprepare(r8a66597->clk);
-   clk_put(r8a66597->clk);
-   }
 clean_up:
if (r8a66597->ep0_req)
r8a66597_free_request(&r8a66597->ep[0].ep, r8a66597->ep0_req);
-- 
2.0.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 5/8] r8a66597-udc: cleanup error path

2014-06-17 Thread Ben Dooks
With the updates for devm, the cleanup path no longer needs to
check for NULL device state, so remove it and return directly
if the irq resource missing

Signed-off-by: Ben Dooks 
---
 drivers/usb/gadget/r8a66597-udc.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/r8a66597-udc.c 
b/drivers/usb/gadget/r8a66597-udc.c
index 2662853..9ebe2c0 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -1878,9 +1878,8 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
irq_trigger = ires->flags & IRQF_TRIGGER_MASK;
 
if (irq < 0) {
-   ret = -ENODEV;
dev_err(dev, "platform_get_irq error.\n");
-   goto clean_up;
+   return -ENODEV;
}
 
/* initialize ucd */
@@ -1979,11 +1978,8 @@ clean_up2:
clk_put(r8a66597->clk);
}
 clean_up:
-   if (r8a66597) {
-   if (r8a66597->ep0_req)
-   r8a66597_free_request(&r8a66597->ep[0].ep,
-   r8a66597->ep0_req);
-   }
+   if (r8a66597->ep0_req)
+   r8a66597_free_request(&r8a66597->ep[0].ep, r8a66597->ep0_req);
 
return ret;
 }
-- 
2.0.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 4/8] r8a66597-udc: handle sudmac registers with devm_ioremap_resource()

2014-06-17 Thread Ben Dooks
Change the sudmac register handling in the devm_ioremap_resource
to use the devm variant.

Signed-off-by: Ben Dooks 
---
 drivers/usb/gadget/r8a66597-udc.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/gadget/r8a66597-udc.c 
b/drivers/usb/gadget/r8a66597-udc.c
index 48a9e0b..2662853 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -1826,8 +1826,6 @@ static int __exit r8a66597_remove(struct platform_device 
*pdev)
 
usb_del_gadget_udc(&r8a66597->gadget);
del_timer_sync(&r8a66597->timer);
-   if (r8a66597->pdata->sudmac)
-   iounmap(r8a66597->sudmac_reg);
free_irq(platform_get_irq(pdev, 0), r8a66597);
r8a66597_free_request(&r8a66597->ep[0].ep, r8a66597->ep0_req);
 
@@ -1849,15 +1847,10 @@ static int __init r8a66597_sudmac_ioremap(struct 
r8a66597 *r8a66597,
struct resource *res;
 
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sudmac");
-   if (!res) {
-   dev_err(&pdev->dev, "platform_get_resource error(sudmac).\n");
-   return -ENODEV;
-   }
-
-   r8a66597->sudmac_reg = ioremap(res->start, resource_size(res));
-   if (r8a66597->sudmac_reg == NULL) {
+   r8a66597->sudmac_reg = devm_ioremap_resource(&pdev->dev, res);
+   if (IS_ERR(r8a66597->sudmac_reg)) {
dev_err(&pdev->dev, "ioremap error(sudmac).\n");
-   return -ENOMEM;
+   return PTR_ERR(r8a66597->sudmac_reg);
}
 
return 0;
@@ -1987,8 +1980,6 @@ clean_up2:
}
 clean_up:
if (r8a66597) {
-   if (r8a66597->sudmac_reg)
-   iounmap(r8a66597->sudmac_reg);
if (r8a66597->ep0_req)
r8a66597_free_request(&r8a66597->ep[0].ep,
r8a66597->ep0_req);
-- 
2.0.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 3/8] r8a66597-udc: use devm_kzalloc() to allocate driver state

2014-06-17 Thread Ben Dooks
Update driver to use devm_kzalloc() to make tracking of resources
easier. Also remove the exit point via cleanup as there's no
cleanup necessary from this point now.

As a note, also removes the error print as the allocation calls
produce errors if they do not return memory.

Signed-off-by: Ben Dooks 
---
 drivers/usb/gadget/r8a66597-udc.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/r8a66597-udc.c 
b/drivers/usb/gadget/r8a66597-udc.c
index 1721e44..48a9e0b 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -1836,7 +1836,6 @@ static int __exit r8a66597_remove(struct platform_device 
*pdev)
clk_put(r8a66597->clk);
}
 
-   kfree(r8a66597);
return 0;
 }
 
@@ -1892,12 +1891,9 @@ static int __init r8a66597_probe(struct platform_device 
*pdev)
}
 
/* initialize ucd */
-   r8a66597 = kzalloc(sizeof(struct r8a66597), GFP_KERNEL);
-   if (r8a66597 == NULL) {
-   ret = -ENOMEM;
-   dev_err(dev, "kzalloc error\n");
-   goto clean_up;
-   }
+   r8a66597 = devm_kzalloc(dev, sizeof(struct r8a66597), GFP_KERNEL);
+   if (r8a66597 == NULL)
+   return -ENOMEM;
 
spin_lock_init(&r8a66597->lock);
platform_set_drvdata(pdev, r8a66597);
@@ -1996,7 +1992,6 @@ clean_up:
if (r8a66597->ep0_req)
r8a66597_free_request(&r8a66597->ep[0].ep,
r8a66597->ep0_req);
-   kfree(r8a66597);
}
 
return ret;
-- 
2.0.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


r8a66597-udc: use devm functions

2014-06-17 Thread Ben Dooks
Change r8a66597-udc to use devm functions and cleanup the result.

--
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 v6 1/1] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

2014-06-13 Thread Ben Dooks
On 13/06/14 15:25, Felipe Balbi wrote:
> Hi,
> 
>> + +#define FIRMWARE_NAME "r8a779x_usb3_v1.dlmem" 
>> +MODULE_FIRMWARE(FIRMWARE_NAME);

Where can we get this from, it would be nice to test this.

>> +/*** Register Offset ***/ +#define RCAR_USB3_INT_ENA0x224   /* 
>> Interrupt Enable */ +#define RCAR_USB3_DL_CTRL   0x250   /* FW 
>> Download Control & Status */ +#define RCAR_USB3_FW_DATA0 0x258
>> /* FW Data0 */ + +#define RCAR_USB3_LCLK 0xa44   /* LCLK Select 
>> */
>>  +#define RCAR_USB3_CONF10xa48   /* USB3.0 Configuration1 */ 
>> +#define RCAR_USB3_CONF2 0xa5c   /* USB3.0 Configuration2 */ 
>> +#define RCAR_USB3_CONF3 0xaa8   /* USB3.0 Configuration3 */ 
>> +#define RCAR_USB3_RX_POL0xab0   /* USB3.0 RX Polarity */
>> +#define RCAR_USB3_TX_POL0xab8   /* USB3.0 TX Polarity */ + +/***
>> Register Settings ***/ +/* Interrupt Enable */ +#define 
>> RCAR_USB3_INT_XHC_ENA0x0001 +#define RCAR_USB3_INT_PME_ENA 
>> 0x0002 +#define RCAR_USB3_INT_HSE_ENA0x0004 +#define 
>> RCAR_USB3_INT_ENA_VAL(RCAR_USB3_INT_XHC_ENA | \ + 
>> RCAR_USB3_INT_PME_ENA | RCAR_USB3_INT_HSE_ENA) + +/* FW Download 
>> Control & Status */ +#define RCAR_USB3_DL_CTRL_ENABLE0x0001
>>  +#define RCAR_USB3_DL_CTRL_FW_SUCCESS   0x0010 +#define 
>> RCAR_USB3_DL_CTRL_FW_SET_DATA0   0x0100 + +/* LCLK Select */ 
>> +#define RCAR_USB3_LCLK_ENA_VAL  0x01030001 + +/* USB3.0 
>> Configuration */ +#define RCAR_USB3_CONF1_VAL0x00030204
>> +#define RCAR_USB3_CONF2_VAL 0x00030300 +#define
>> RCAR_USB3_CONF3_VAL 0x13802007 + +/* USB3.0 Polarity */ +#define
>> RCAR_USB3_RX_POL_VAL BIT(21) +#define RCAR_USB3_TX_POL_VAL   BIT(4)
>> + +void xhci_rcar_start(struct usb_hcd *hcd) +{ +u32 temp; + +
>> if (hcd->regs != NULL) { +   /* Interrupt Enable */ +
>> temp = 
>> readl(hcd->regs + RCAR_USB3_INT_ENA); +  temp |= 
>> RCAR_USB3_INT_ENA_VAL; + writel(temp, hcd->regs + 
>> RCAR_USB3_INT_ENA); +/* LCLK Select */ + 
>> writel(RCAR_USB3_LCLK_ENA_VAL, hcd->regs + RCAR_USB3_LCLK); +
>> /* USB3.0 Configuration */ + writel(RCAR_USB3_CONF1_VAL,
>> hcd->regs + RCAR_USB3_CONF1); +  writel(RCAR_USB3_CONF2_VAL,
>> hcd->regs + RCAR_USB3_CONF2); +  writel(RCAR_USB3_CONF3_VAL,
>> hcd->regs + RCAR_USB3_CONF3); +  /* USB3.0 Polarity */ + 
>> writel(RCAR_USB3_RX_POL_VAL, hcd->regs + RCAR_USB3_RX_POL); + 
>> writel(RCAR_USB3_TX_POL_VAL, hcd->regs + RCAR_USB3_TX_POL); +} 
>> +} + +static int xhci_rcar_download_firmware(struct device *dev, 
>> void __iomem *regs) +{ + const struct firmware *fw; +int
>> retval, index, j, time; +int timeout = 1; +  u32 data, val,
>> temp; + + /* request R-Car USB3.0 firmware */ +  retval = 
>> request_firmware(&fw, FIRMWARE_NAME, dev); + if (retval) + return
>> retval; + +  /* download R-Car USB3.0 firmware */ +  temp = 
>> readl(regs + RCAR_USB3_DL_CTRL); +   temp |= 
>> RCAR_USB3_DL_CTRL_ENABLE; +  writel(temp, regs + 
>> RCAR_USB3_DL_CTRL); + +  for (index = 0; index < fw->size; index 
>> += 4) { +/* to avoid reading beyond the end of the buffer */ + 
>> for (data = 0, j = 3; j >= 0; j--) { +   if ((j + index) 
>> < 
>> fw->size) +  data |= fw->data[index + j] << (8 * j); 
>> +   } + 
>> writel(data, regs + RCAR_USB3_FW_DATA0); +   temp = readl(regs + 
>> RCAR_USB3_DL_CTRL); +temp |= RCAR_USB3_DL_CTRL_FW_SET_DATA0; 
>> + 
>> writel(temp, regs + RCAR_USB3_DL_CTRL); + +  for (time = 0; time 
>> < timeout; time++) { +   val = readl(regs + 
>> RCAR_USB3_DL_CTRL);
>> + if ((val & RCAR_USB3_DL_CTRL_FW_SET_DATA0) == 0) + 
>> break; + 
>> udelay(1); + } + if (time == timeout) { +
>> retval = 
>> -ETIMEDOUT; +break; +} + } + +   
>> temp = readl(regs + 
>> RCAR_USB3_DL_CTRL); +temp &= ~RCAR_USB3_DL_CTRL_ENABLE; + 
>> writel(temp, regs + RCAR_USB3_DL_CTRL); + +  for (time = 0; time
>> < timeout; time++) { +   val = readl(regs + RCAR_USB3_DL_CTRL); 
>> + 
>> if (val & RCAR_USB3_DL_CTRL_FW_SUCCESS) { +      retval = 0; + 
>> break; + } + udelay(1); +}

Re: [PATCH 1/2] usb: rename 'phy' field of 'struct usb_hcd' to 'transceiver'

2014-04-10 Thread Ben Dooks

On 10/04/14 12:14, David Laight wrote:

From: Ben Dooks

On 10/04/14 11:49, Sergei Shtylyov wrote:

On 10-04-2014 13:20, David Laight wrote:


  It doesn't do any pin muxing. It switches SoC internal USB
signals between
USB controllers. The pins remain the same.



Doesn't something like that already happen for the companion USB1
controllers for USB2 ports?


 Did you mean USB 1.1 and USB 2.0 controllers by USB1 and USB2?


Yes.

Why do you care which USB controller is driving the pins?


That also doesn't sound like you are changing the PHY.


 I am changing one of the PHY registers that controls USB port
(Renesas calls it channel) multiplexing.


I'd have thought that would happen if you had a single controller
that select between multiply PHY.


 No, it's not the case.


I realised that wasn't what you were doing, but at first it did seem
to be what you were doing.


There is an interesting case, the USB3 shares a PHY with a SATA
and the PCIE and SATA also share a PHY on the R8A7790.


Some of those look like pcb design decisions - so there is no dynamic
changing, just config time plumbing.
OTOH we are carrying PCIe using two SATA cables (the second carries the
clock) so I suspect some SoC system pcbs may be able to support SATA
or PCIe on the same connector).


Yes, which means we will probably want to support the case where
the USB3 is routed out of the PCIe lanes.


--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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: rename 'phy' field of 'struct usb_hcd' to 'transceiver'

2014-04-10 Thread Ben Dooks

On 10/04/14 11:49, Sergei Shtylyov wrote:

On 10-04-2014 13:20, David Laight wrote:


 It doesn't do any pin muxing. It switches SoC internal USB
signals between
USB controllers. The pins remain the same.



Doesn't something like that already happen for the companion USB1
controllers for USB2 ports?


Did you mean USB 1.1 and USB 2.0 controllers by USB1 and USB2?


That also doesn't sound like you are changing the PHY.


I am changing one of the PHY registers that controls USB port
(Renesas calls it channel) multiplexing.


I'd have thought that would happen if you had a single controller
that select between multiply PHY.


No, it's not the case.


There is an interesting case, the USB3 shares a PHY with a SATA
and the PCIE and SATA also share a PHY on the R8A7790.

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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/9] pci-rcar-gen2: add devicetree support

2014-04-04 Thread Ben Dooks

On 04/04/14 18:09, Bjorn Helgaas wrote:

On Thu, Mar 06, 2014 at 06:01:19PM +, Ben Dooks wrote:

Add OF match table for pci-rcar-gen2 driver for device tree support.

Signed-off-by: Ben Dooks 


I'm not sure what the status of this is.  I saw comments about a missing
#include, but I didn't see a fixed repost.  Ben, could you please repost
anything you have outstanding for me?


Sergei was going to re-post this series. I can have a look at
re-sending this later today if he hasn't already sorted it out.

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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 8/9] ARM: shmobile: lager.dts: add usbphy reference

2014-03-30 Thread Ben Dooks

On 30/03/14 20:28, Sergei Shtylyov wrote:

Hello.

On 03/06/2014 09:01 PM, Ben Dooks wrote:


Enable the usbphy node so that the phy driver is available.



Signed-off-by: Ben Dooks 
Reviewed-by: Ian Molton 
---
Cc: linux...@vger.kernel.org
Cc: devicet...@vger.kernel.org
Cc: Magnus Damm 
Cc: Simon Horman 
---
  arch/arm/boot/dts/r8a7790-lager.dts | 4 
  1 file changed, 4 insertions(+)



diff --git a/arch/arm/boot/dts/r8a7790-lager.dts
b/arch/arm/boot/dts/r8a7790-lager.dts
index fd6851f..63d58d6 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -246,3 +246,7 @@
  pinctrl-0 = <&usb2_pins>;
  pinctrl-names = "default";
  };
+
+&usbphy {
+status = "okay";
+};


I don't see why we need this patch. Why not make USB PHY always
enabled?


I have a current setup where we do not want the kernel to try
and probe the phy device.

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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 9/9] ARM: shmobile: lager.dts: link usb-phy to pci nodes

2014-03-30 Thread Ben Dooks

On 30/03/14 20:29, Sergei Shtylyov wrote:

Hello.

On 03/06/2014 09:01 PM, Ben Dooks wrote:


Add the necessary links to add the USB phy nodes to the PCI devices
that are behind the bridges specified.



Signed-off-by: Ben Dooks 
---
  arch/arm/boot/dts/r8a7790-lager.dts | 37
+
  1 file changed, 37 insertions(+)



diff --git a/arch/arm/boot/dts/r8a7790-lager.dts
b/arch/arm/boot/dts/r8a7790-lager.dts
index 63d58d6..62f486e 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -233,18 +233,55 @@
  &pci0 {
  pinctrl-0 = <&usb0_pins>;
  pinctrl-names = "default";
+device_type = "pci";
+
+pci@0,1 {
+reg = <0x800 0 0 0 0>;
+device_type = "pci";
+usb-phy = <&usbphy>;
+};
+
+pci@0,2 {
+reg = <0x1000 0 0 0 0>;
+device_type = "pci";
+usb-phy = <&usbphy>;
+};
  };

  &pci1 {
  status = "okay";
  pinctrl-0 = <&usb1_pins>;
  pinctrl-names = "default";
+
+pci@0,1 {
+reg = <0x800 0 0 0 0>;
+device_type = "pci";
+usb-phy = <&usbphy>;
+};
+
+pci@0,2 {
+reg = <0x1000 0 0 0 0>;
+device_type = "pci";
+usb-phy = <&usbphy>;
+};
  };

  &pci2 {
  status = "okay";
  pinctrl-0 = <&usb2_pins>;
  pinctrl-names = "default";
+
+pci@0,1 {
+reg = <0x800 0 0 0 0>;
+device_type = "pci";
+usb-phy = <&usbphy>;
+};
+
+pci@0,2 {
+reg = <0x1000 0 0 0 0>;
+    device_type = "pci";
+usb-phy = <&usbphy>;
+};
  };


I don't see why the internal PCI devices are added to the board
file, not the SoC file.


Thanks, I think they probably do belong in the .dtsi file.


--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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/9] pci-rcar-gen2: add devicetree support

2014-03-30 Thread Ben Dooks

On 30/03/14 20:26, Sergei Shtylyov wrote:

Hello.

On 03/30/2014 11:21 PM, Ben Dooks wrote:


Add OF match table for pci-rcar-gen2 driver for device tree support.

[...]



diff --git a/drivers/pci/host/pci-rcar-gen2.c
b/drivers/pci/host/pci-rcar-gen2.c
index fd3e3ab..1216784 100644
--- a/drivers/pci/host/pci-rcar-gen2.c
+++ b/drivers/pci/host/pci-rcar-gen2.c
@@ -16,6 +16,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 



Apologies all, there's a missing include of  which should
have been added here



Have you ever re-posted this patch fixed? If not, it's not going to
be merged for 3.15 I guess...



I thought I did.


I don't see such repost in 'linux-sh'.


I can have a look a reposting it.


It doesn't apply to the current renesas.git devel branch. What tree
it was against?


From my records it was against the -rc3 release of Horms' development
tree. I know Simon's tree has been closed for the board bits, but I
could re-send the rcar-pci code.

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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/9] pci-rcar-gen2: add devicetree support

2014-03-30 Thread Ben Dooks

On 30/03/14 20:10, Sergei Shtylyov wrote:

Hello.

On 03/06/2014 09:21 PM, Ben Dooks wrote:


Add OF match table for pci-rcar-gen2 driver for device tree support.

[...]


diff --git a/drivers/pci/host/pci-rcar-gen2.c
b/drivers/pci/host/pci-rcar-gen2.c
index fd3e3ab..1216784 100644
--- a/drivers/pci/host/pci-rcar-gen2.c
+++ b/drivers/pci/host/pci-rcar-gen2.c
@@ -16,6 +16,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 



Apologies all, there's a missing include of  which should
have been added here


Have you ever re-posted this patch fixed? If not, it's not going to
be merged for 3.15 I guess...


I thought I did. I can have a look a reposting it.

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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: Renesas RCar device-tree USB series

2014-03-07 Thread Ben Dooks

On 07/03/14 17:30, Sergei Shtylyov wrote:

Hello.

On 03/07/2014 05:30 PM, Ben Dooks wrote:


This is a new series covering enabling the RCar series of SoCs USB
with device-tree based booting. It has been tested on the R8A7790
Lager board.



Improvements from the previous series include:



 - mapping usb to the relevant phy by dt
 - better use of existing pci of functions



Note, there is still an issue with the second gigabyte of memory on
the Lager, which with current kernels causes the system to abort on
startup. This series will only work if the top memory area is disabled.



Thanks for these patches. I think they start looking really good.



In my mind there are two outstanding issues:


[...]


2) Per-port USB PHY driver configuration via DT
Right now each USB host controller points to the same PHY device.
Thanks for working on describing the topology! As you know, the PHY
driver itself handles several USB ports, and I'd like to use DT to
represent the mapping between which PHY port that maps to what USB
Host controller to allow proper run time configuration. Right now in
this version of the series there is no such mapping. Of course, that
depends on proper USB PHY DT bindings...



Hmm, given the shared phy is shared and referenced counted then
I don't /think/ there is much more to be done for this. If we add
more PHYs then I would assume that each one of them would


It seems you didn't finish the sentence...


I deleted the last bit by accident.

"have their own phy driver instance"



WBR, Sergei




--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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: Renesas RCar device-tree USB series

2014-03-07 Thread Ben Dooks

On 07/03/14 05:36, Magnus Damm wrote:

Hi Ben,

On Fri, Mar 7, 2014 at 3:01 AM, Ben Dooks  wrote:

This is a new series covering enabling the RCar series of SoCs USB
with device-tree based booting. It has been tested on the R8A7790
Lager board.

Improvements from the previous series include:

 - mapping usb to the relevant phy by dt
 - better use of existing pci of functions

Note, there is still an issue with the second gigabyte of memory on
the Lager, which with current kernels causes the system to abort on
startup. This series will only work if the top memory area is disabled.


Thanks for these patches. I think they start looking really good.

In my mind there are two outstanding issues:

1) Is the USB core code change acceptable or not?

The patch "[PATCH 4/9] usb: phy: check for of_node when getting phy"
looks quite fine to me, but I wonder if the USB maintainers will
accept it as-is or require some rework. I would say that the rest of
this series depends on that USB core change.


I'm not sure, but without it we need to hack the PHY driver to
turn on the PHY at start time.


2) Per-port USB PHY driver configuration via DT

Right now each USB host controller points to the same PHY device.
Thanks for working on describing the topology! As you know, the PHY
driver itself handles several USB ports, and I'd like to use DT to
represent the mapping between which PHY port that maps to what USB
Host controller to allow proper run time configuration. Right now in
this version of the series there is no such mapping. Of course, that
depends on proper USB PHY DT bindings...


Hmm, given the shared phy is shared and referenced counted then
I don't /think/ there is much more to be done for this. If we add
more PHYs then I would assume that each one of them would



--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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] phy-rcar-gen2-usb: add device tree support

2014-03-06 Thread Ben Dooks

On 06/03/14 19:39, Sergei Shtylyov wrote:

Hello.

On 03/03/2014 07:44 PM, Felipe Balbi wrote:


Add support of the device tree probing for the Renesas R-Car
generation 2 SoCs
documenting the device tree binding as necessary.



Signed-off-by: Sergei Shtylyov 



Unless someone on devicetree@vger gives me an ACK pretty soon, I'm
afraid this patch will miss v3.15.


Mark, you've commented on v1 of the patch, I hope I cleared things
up for you in my reply, how about an ACK (or at least new portion of
comments)?


And whose patches are we going to use? :-p


--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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/9] pci-rcar-gen2: add devicetree support

2014-03-06 Thread Ben Dooks

On 06/03/14 18:01, Ben Dooks wrote:

Add OF match table for pci-rcar-gen2 driver for device tree support.




diff --git a/drivers/pci/host/pci-rcar-gen2.c b/drivers/pci/host/pci-rcar-gen2.c
index fd3e3ab..1216784 100644
--- a/drivers/pci/host/pci-rcar-gen2.c
+++ b/drivers/pci/host/pci-rcar-gen2.c
@@ -16,6 +16,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 


Apologies all, there's a missing include of  which should
have been added here


--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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 3/9] phy-rcar-usb-gen2: add device tree support

2014-03-06 Thread Ben Dooks

On 06/03/14 19:16, Sergei Shtylyov wrote:

Hello.

On 03/06/2014 09:01 PM, Ben Dooks wrote:


Add support for the phy-rcar-gen2-usb driver to be probed from device
tree.



Signed-off-by: Ben Dooks 
Reviewed-by: Ian Molton 
---
Fixes from v2:
- fix missed of_match_ptr()
- fix names of channel selection booleans
- updated and merged documentation for dt entries

Fixes from v2:
- fix missing of_if patch

Fixes from v1:
- use of_property_reasd-bool()
- remove unused of_id variable

Cc: Felipe Balbi 
Cc: linux-usb@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: Magnus Damm 
Cc: Simon Horman 
Cc: devicet...@vger.kernel.org



Conflicts:
drivers/usb/phy/phy-rcar-gen2-usb.c
---
  .../bindings/usb/renesas,rcar-gen2-usb-phy.txt | 36
++
  drivers/pci/host/pci-rcar-gen2.c   |  1 +


Eh? What does this file have to do with USB PHY?


Ah, it was a fixup for a missing header that got merged
into the wrong file. Will fix that.


  drivers/usb/phy/phy-rcar-gen2-usb.c| 34
+---
  3 files changed, 66 insertions(+), 5 deletions(-)
  create mode 100644
Documentation/devicetree/bindings/usb/renesas,rcar-gen2-usb-phy.txt



diff --git
a/Documentation/devicetree/bindings/usb/renesas,rcar-gen2-usb-phy.txt
b/Documentation/devicetree/bindings/usb/renesas,rcar-gen2-usb-phy.txt
new file mode 100644
index 000..5351a30
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/renesas,rcar-gen2-usb-phy.txt
@@ -0,0 +1,36 @@
+Renesas RCar gen2 USB PHY bindings
+--
+
+Bindings for the USB PHY block used in some Renesas SoCs.
+
+Required properties:
+ - compatible:  "renesas,usb-phy-r8a7790" for the R8A7790 SoC
+"renesas,usb-phy-r8a7791" for the R8A7791 SoC
+ - reg : A single region to access device registers
+ - clocks : The reference to the clock to use for this block
+ - clock-names : The name for the clock at index 0 (must be "usbhs")
+
+Optional properties:
+
+ - renesas,usb0-device: boolean, if present USB0 is connected to HS
device
+otherwise the USB0 is connected to OHCI/EHCI host.


IIUC, the testing has shown that USBHS is dual-role controller in
that case, i.e. supports both host and device roles (the manual has the
host controller details too). Vladimir, is it so?


Currently there is no auto-detection for this, so it gets set at
start time.


+ - renesas,usb2-xhci: boolean, if present USB2 is connected to XHCI
controller
+  otherwise the USB2 is connected to OHCI/EHCI host.
+
+
+Example device node for SoC dtsi file:
+
+usbphy: usbphy@e6590100 {
+compatible = "renesas,usb-phy-r8a7790";
+clocks = <&mstp7_clks R8A7790_CLK_HSUSB>;
+clock-names = "usbhs";
+reg = < 0x0 0xe6590100 0x0 0x100>;
+status = "disabled";
+};
+
+Example board file:
+
+&usbphy {
+status = "okay";
+};


    These are usually merged into one node for the example.


Much nicer if separate.

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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 8/9] ARM: shmobile: lager.dts: add usbphy reference

2014-03-06 Thread Ben Dooks
Enable the usbphy node so that the phy driver is available.

Signed-off-by: Ben Dooks 
Reviewed-by: Ian Molton 
---
Cc: linux...@vger.kernel.org
Cc: devicet...@vger.kernel.org
Cc: Magnus Damm 
Cc: Simon Horman 
---
 arch/arm/boot/dts/r8a7790-lager.dts | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
b/arch/arm/boot/dts/r8a7790-lager.dts
index fd6851f..63d58d6 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -246,3 +246,7 @@
pinctrl-0 = <&usb2_pins>;
pinctrl-names = "default";
 };
+
+&usbphy {
+   status = "okay";
+};
-- 
1.9.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 9/9] ARM: shmobile: lager.dts: link usb-phy to pci nodes

2014-03-06 Thread Ben Dooks
Add the necessary links to add the USB phy nodes to the PCI devices
that are behind the bridges specified.

Signed-off-by: Ben Dooks 
---
 arch/arm/boot/dts/r8a7790-lager.dts | 37 +
 1 file changed, 37 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
b/arch/arm/boot/dts/r8a7790-lager.dts
index 63d58d6..62f486e 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -233,18 +233,55 @@
 &pci0 {
pinctrl-0 = <&usb0_pins>;
pinctrl-names = "default";
+   device_type = "pci";
+
+   pci@0,1 {
+   reg = <0x800 0 0 0 0>;
+   device_type = "pci";
+   usb-phy = <&usbphy>;
+   };
+
+   pci@0,2 {
+   reg = <0x1000 0 0 0 0>;
+   device_type = "pci";
+   usb-phy = <&usbphy>;
+   };
 };
 
 &pci1 {
status = "okay";
pinctrl-0 = <&usb1_pins>;
pinctrl-names = "default";
+
+   pci@0,1 {
+   reg = <0x800 0 0 0 0>;
+   device_type = "pci";
+   usb-phy = <&usbphy>;
+   };
+
+   pci@0,2 {
+   reg = <0x1000 0 0 0 0>;
+   device_type = "pci";
+   usb-phy = <&usbphy>;
+   };
 };
 
 &pci2 {
status = "okay";
pinctrl-0 = <&usb2_pins>;
pinctrl-names = "default";
+
+   pci@0,1 {
+   reg = <0x800 0 0 0 0>;
+   device_type = "pci";
+   usb-phy = <&usbphy>;
+   };
+
+   pci@0,2 {
+   reg = <0x1000 0 0 0 0>;
+   device_type = "pci";
+   usb-phy = <&usbphy>;
+   };
 };
 
 &usbphy {
-- 
1.9.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/9] pci-rcar-gen2: add devicetree support

2014-03-06 Thread Ben Dooks
Add OF match table for pci-rcar-gen2 driver for device tree support.

Signed-off-by: Ben Dooks 
---
Updates since v1:
- moved documentation into patch
- moved to using bus-range parsing
- ensured usb phy can be linked to usb devices

Cc: Bjorn Helgaas 
Cc: Simon Horman 
Cc: linux-...@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: devicet...@vger.kernel.org
---
 .../bindings/pci/renessas,pci-rcar-gen2.txt| 52 ++
 drivers/pci/host/pci-rcar-gen2.c   | 33 +-
 2 files changed, 83 insertions(+), 2 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/pci/renessas,pci-rcar-gen2.txt

diff --git a/Documentation/devicetree/bindings/pci/renessas,pci-rcar-gen2.txt 
b/Documentation/devicetree/bindings/pci/renessas,pci-rcar-gen2.txt
new file mode 100644
index 000..bd6d291
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/renessas,pci-rcar-gen2.txt
@@ -0,0 +1,52 @@
+Renesas AHB to PCI bridge
+-
+
+This is the bridge used internally to connect the USB controllers to the
+AHB. There is one bridge instance per USB port consiting of an internal
+OHCI and EHCI controller.
+
+Required properties:
+ - compatible: "renesas,pci-r8a7790" for the R8A7790 SoC
+ - reg : A list of physical regions to access the device. The first is
+ the operational registers for the OHCI/EHCI controller and the
+second region is for the bridge configuration and control registers.
+ - interrupts : interrupt for the device
+ - clocks : The reference to the device clock
+ - bus-range: The PCI bus number ranges. As this is a single bus, the range
+   should be specified as the same value twice.
+
+
+Example SoC configuration:
+
+   pci0: pci@ee09  {
+   compatible = "renesas,pci-r8a7790";
+   clocks = <&mstp7_clks R8A7790_CLK_EHCI>;
+   reg = <0x0 0xee09 0x0 0xc00>,
+ <0x0 0xee08 0x0 0x1100>;
+   interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>;
+   status = "disabled";
+
+   bus-range = <0 0>;
+   #address-cells = <3>;
+   #size-cells = <2>;
+   };
+
+Example board setup:
+
+&pci1 {
+   status = "okay";
+   pinctrl-0 = <&usb1_pins>;
+   pinctrl-names = "default";
+
+   pci@0,1 {
+   reg = <0x800 0 0 0 0>;
+   device_type = "pci";
+   usb-phy = <&usbphy>;
+   };
+
+   pci@0,2 {
+   reg = <0x1000 0 0 0 0>;
+   device_type = "pci";
+   usb-phy = <&usbphy>;
+   };
+};
diff --git a/drivers/pci/host/pci-rcar-gen2.c b/drivers/pci/host/pci-rcar-gen2.c
index fd3e3ab..1216784 100644
--- a/drivers/pci/host/pci-rcar-gen2.c
+++ b/drivers/pci/host/pci-rcar-gen2.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -98,6 +99,7 @@ struct rcar_pci_priv {
struct resource io_res;
struct resource mem_res;
struct resource *cfg_res;
+   unsigned busnr;
int irq;
unsigned long window_size;
 };
@@ -312,8 +314,8 @@ static int rcar_pci_setup(int nr, struct pci_sys_data *sys)
pci_add_resource(&sys->resources, &priv->io_res);
pci_add_resource(&sys->resources, &priv->mem_res);
 
-   /* Setup bus number based on platform device id */
-   sys->busnr = to_platform_device(priv->dev)->id;
+   /* Setup bus number based on platform device id / of bus-range */
+   sys->busnr = priv->busnr;
return 1;
 }
 
@@ -366,6 +368,23 @@ static int rcar_pci_probe(struct platform_device *pdev)
 
priv->window_size = SZ_1G;
 
+   if (pdev->dev.of_node) {
+   struct resource busnr;
+   int ret;
+
+   ret = of_pci_parse_bus_range(pdev->dev.of_node, &busnr);
+   if (ret < 0) {
+   dev_err(&pdev->dev, "failed to parse bus-range\n");
+   return ret;
+   }
+   
+   priv->busnr = busnr.start;
+   if (busnr.end != busnr.start)
+   dev_warn(&pdev->dev, "only one bus number supported\n");
+   } else {
+   priv->busnr = pdev->id;
+   }
+
hw_private[0] = priv;
memset(&hw, 0, sizeof(hw));
hw.nr_controllers = ARRAY_SIZE(hw_private);
@@ -377,11 +396,21 @@ static int rcar_pci_probe(struct platform_device *pdev)
return 0;
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id rcar_pci_of_match[] = {
+   { .compatible = "renesas,pci-r8a7790", },
+   { },
+};
+
+MODULE_DEVICE_TABLE(of, rcar_pci_of

[PATCH 5/9] ARM: shmbobile: r8a7790.dtsi: add pci0/1/2 nodes

2014-03-06 Thread Ben Dooks
Add nodes for USB PCI bridge devices.

Signed-off-by: Ben Dooks 
Reviewed-by: Ian Molton 
---
Cc: devicet...@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: horms+rene...@verge.net.au

Conflicts:
arch/arm/boot/dts/r8a7790.dtsi
---
 arch/arm/boot/dts/r8a7790.dtsi | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index a1e7c39..7325fee 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -763,4 +763,43 @@
#size-cells = <0>;
status = "disabled";
};
+
+   pci0: pci@ee09  {
+   compatible = "renesas,pci-r8a7790";
+   clocks = <&mstp7_clks R8A7790_CLK_EHCI>;
+   reg = <0x0 0xee09 0x0 0xc00>,
+ <0x0 0xee08 0x0 0x1100>;
+   interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>;
+   status = "disabled";
+
+   bus-range = <0 0>;
+   #address-cells = <3>;
+   #size-cells = <2>;
+   };
+
+   pci1: pci@ee0b  {
+   compatible = "renesas,pci-r8a7790";
+   clocks = <&mstp7_clks R8A7790_CLK_EHCI>;
+   reg = <0x0 0xee0b 0x0 0xc00>,
+ <0x0 0xee0a 0x0 0x1100>;
+   interrupts = <0 112 IRQ_TYPE_LEVEL_HIGH>;
+   status = "disabled";
+
+   bus-range = <1 1>;
+   #address-cells = <3>;
+   #size-cells = <2>;
+   };
+
+   pci2: pci@ee0d  {
+   compatible = "renesas,pci-r8a7790";
+   clocks = <&mstp7_clks R8A7790_CLK_EHCI>;
+   reg = <0x0 0xee0d 0x0 0xc00>,
+ <0x0 0xee0c 0x0 0x1100>;
+   interrupts = <0 113 IRQ_TYPE_LEVEL_HIGH>;
+   status = "disabled";
+
+   bus-range = <2 2>;
+   #address-cells = <3>;
+   #size-cells = <2>;
+   };
 };
-- 
1.9.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 3/9] phy-rcar-usb-gen2: add device tree support

2014-03-06 Thread Ben Dooks
Add support for the phy-rcar-gen2-usb driver to be probed from device tree.

Signed-off-by: Ben Dooks 
Reviewed-by: Ian Molton 
---
Fixes from v2:
- fix missed of_match_ptr()
- fix names of channel selection booleans
- updated and merged documentation for dt entries

Fixes from v2:
- fix missing of_if patch

Fixes from v1:
- use of_property_reasd-bool()
- remove unused of_id variable

Cc: Felipe Balbi 
Cc: linux-usb@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: Magnus Damm 
Cc: Simon Horman 
Cc: devicet...@vger.kernel.org

Conflicts:
drivers/usb/phy/phy-rcar-gen2-usb.c
---
 .../bindings/usb/renesas,rcar-gen2-usb-phy.txt | 36 ++
 drivers/pci/host/pci-rcar-gen2.c   |  1 +
 drivers/usb/phy/phy-rcar-gen2-usb.c| 34 +---
 3 files changed, 66 insertions(+), 5 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/usb/renesas,rcar-gen2-usb-phy.txt

diff --git 
a/Documentation/devicetree/bindings/usb/renesas,rcar-gen2-usb-phy.txt 
b/Documentation/devicetree/bindings/usb/renesas,rcar-gen2-usb-phy.txt
new file mode 100644
index 000..5351a30
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/renesas,rcar-gen2-usb-phy.txt
@@ -0,0 +1,36 @@
+Renesas RCar gen2 USB PHY bindings
+--
+
+Bindings for the USB PHY block used in some Renesas SoCs.
+
+Required properties:
+ - compatible:  "renesas,usb-phy-r8a7790" for the R8A7790 SoC
+   "renesas,usb-phy-r8a7791" for the R8A7791 SoC
+ - reg : A single region to access device registers
+ - clocks : The reference to the clock to use for this block
+ - clock-names : The name for the clock at index 0 (must be "usbhs")
+
+Optional properties:
+
+ - renesas,usb0-device: boolean, if present USB0 is connected to HS device
+   otherwise the USB0 is connected to OHCI/EHCI host.
+ - renesas,usb2-xhci: boolean, if present USB2 is connected to XHCI controller
+ otherwise the USB2 is connected to OHCI/EHCI host.
+
+
+Example device node for SoC dtsi file:
+
+   usbphy: usbphy@e6590100 {
+   compatible = "renesas,usb-phy-r8a7790";
+   clocks = <&mstp7_clks R8A7790_CLK_HSUSB>;
+   clock-names = "usbhs";
+   reg = < 0x0 0xe6590100 0x0 0x100>;
+   status = "disabled";
+   };
+
+Example board file:
+
+&usbphy {
+   status = "okay";
+};
+
diff --git a/drivers/pci/host/pci-rcar-gen2.c b/drivers/pci/host/pci-rcar-gen2.c
index 1216784..2595078 100644
--- a/drivers/pci/host/pci-rcar-gen2.c
+++ b/drivers/pci/host/pci-rcar-gen2.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c 
b/drivers/usb/phy/phy-rcar-gen2-usb.c
index 388d89f..8006c3c 100644
--- a/drivers/usb/phy/phy-rcar-gen2-usb.c
+++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -167,6 +168,15 @@ out:
spin_unlock_irqrestore(&priv->lock, flags);
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = {
+   { .compatible = "renesas,usb-phy-r8a7790", },
+   { .compatible = "renesas,usb-phy-r8a7791", },
+   { },
+};
+MODULE_DEVICE_TABLE(of, rcar_gen2_usb_phy_ofmatch);
+#endif
+
 static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
 {
struct device *dev = &pdev->dev;
@@ -178,7 +188,7 @@ static int rcar_gen2_usb_phy_probe(struct platform_device 
*pdev)
int retval;
 
pdata = dev_get_platdata(dev);
-   if (!pdata) {
+   if (!pdata && !dev->of_node) {
dev_err(dev, "No platform data\n");
return -EINVAL;
}
@@ -203,16 +213,29 @@ static int rcar_gen2_usb_phy_probe(struct platform_device 
*pdev)
spin_lock_init(&priv->lock);
priv->clk = clk;
priv->base = base;
-   priv->ugctrl2 = pdata->chan0_pci ?
-   USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS;
-   priv->ugctrl2 |= pdata->chan2_pci ?
-   USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS;
priv->phy.dev = dev;
priv->phy.label = dev_name(dev);
priv->phy.init = rcar_gen2_usb_phy_init;
priv->phy.shutdown = rcar_gen2_usb_phy_shutdown;
priv->phy.set_suspend = rcar_gen2_usb_phy_set_suspend;
 
+   if (dev->of_node) {
+   if (of_property_read_bool(dev->of_node, "renesas,usb0-device"))
+   priv->ugctrl2 = USBHS_UGCTRL2_USB0_HS;
+   else
+   priv->ugctrl2 = USBHS_UGCTRL2_USB0_PCI;
+
+   if (of_property_rea

[PATCH 7/9] ARM: shmobile: r8a7790.dtsi: add usbphy node

2014-03-06 Thread Ben Dooks
Add node for USB PHY driver to the base R8A7790 device tree include file.
It is up to the board file to enable and configure it as necessary.

Signed-off-by: Ben Dooks 
Reviewed-by: Ian Molton 
---
Cc: devicet...@vger.kernel.org
Cc: Magnus Damm 
Cc: Simon Horman 
Cc: linux...@vger.kernel.org
---
 arch/arm/boot/dts/r8a7790.dtsi | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 7325fee..4c03c46 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -802,4 +802,12 @@
#address-cells = <3>;
#size-cells = <2>;
};
+
+   usbphy: usbphy@e6590100 {
+   compatible = "renesas,usb-phy-r8a7790";
+   clocks = <&mstp7_clks R8A7790_CLK_HSUSB>;
+   clock-names = "usbhs";
+   reg = < 0x0 0xe6590100 0x0 0x100>;
+   status = "disabled";
+   };
 };
-- 
1.9.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 4/9] usb: phy: check for of_node when getting phy

2014-03-06 Thread Ben Dooks
If the PHY is specified by usb-phy handle in the device tree, then
usb_get_phy_dev() should use the device tree specified PHY. This
fixes the issue where device-tree booted Renesas SoCs fail to find
the PHY for the USB controllers.

Signed-off-by: Ben Dooks 
---
Cc: linux-usb@vger.kernel.org
Cc: Felipe Balbi 
---
 drivers/usb/phy/phy.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index 8afa813..92b3c09 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -224,6 +224,29 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8 
index)
struct usb_phy  *phy = NULL;
unsigned long   flags;
 
+   if (dev->of_node) {
+   struct device_node *node;
+
+   node = of_parse_phandle(dev->of_node, "usb-phy", index);
+   if (!node) {
+   dev_dbg(dev, "failed to get %s phandle\n",
+   dev->of_node->full_name);
+   return ERR_PTR(-ENODEV);
+   }
+
+   spin_lock_irqsave(&phy_lock, flags);
+
+   phy = __of_usb_find_phy(node);
+   if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
+   phy = ERR_PTR(-EPROBE_DEFER);
+   of_node_put(node);
+   goto err0;
+   }
+
+   get_device(phy->dev);
+   goto err0;
+   }
+
spin_lock_irqsave(&phy_lock, flags);
 
phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
-- 
1.9.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 2/9] phy-rcar-gen2-usb: always use 'dev' variable in probe() method

2014-03-06 Thread Ben Dooks
From: Sergei Shtylyov 

The probe() method has the 'dev' local variable declared and used but strangely
not in all cases where it should be...

Signed-off-by: Sergei Shtylyov 
---
Cc: Felipe Balbi 
---
 drivers/usb/phy/phy-rcar-gen2-usb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c 
b/drivers/usb/phy/phy-rcar-gen2-usb.c
index 551e0a6..388d89f 100644
--- a/drivers/usb/phy/phy-rcar-gen2-usb.c
+++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
@@ -177,15 +177,15 @@ static int rcar_gen2_usb_phy_probe(struct platform_device 
*pdev)
struct clk *clk;
int retval;
 
-   pdata = dev_get_platdata(&pdev->dev);
+   pdata = dev_get_platdata(dev);
if (!pdata) {
dev_err(dev, "No platform data\n");
return -EINVAL;
}
 
-   clk = devm_clk_get(&pdev->dev, "usbhs");
+   clk = devm_clk_get(dev, "usbhs");
if (IS_ERR(clk)) {
-   dev_err(&pdev->dev, "Can't get the clock\n");
+   dev_err(dev, "Can't get the clock\n");
return PTR_ERR(clk);
}
 
-- 
1.9.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


Renesas RCar device-tree USB series

2014-03-06 Thread Ben Dooks
This is a new series covering enabling the RCar series of SoCs USB
with device-tree based booting. It has been tested on the R8A7790
Lager board.

Improvements from the previous series include:

- mapping usb to the relevant phy by dt
- better use of existing pci of functions

Note, there is still an issue with the second gigabyte of memory on
the Lager, which with current kernels causes the system to abort on
startup. This series will only work if the top memory area is disabled.

--
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 6/9] ARM: shmobile: lager.dts: add pci 0/1/2

2014-03-06 Thread Ben Dooks
Enable pci1 and pci2 nodes for USB controllers attached to the AHB<>PCI
bridge devices. Node pci0 is added for the moment, but not enabled as it
could be switched to usb-gadget mode later.

Signed-off-by: Ben Dooks 
Reviewed-by: Ian Molton 
---
Cc: linux...@vger.kernel.org
Cc: devicet...@vger.kernel.org
Cc: Magnus Damm 
Cc: Simon Horman 
---
 arch/arm/boot/dts/r8a7790-lager.dts | 32 
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
b/arch/arm/boot/dts/r8a7790-lager.dts
index 26a9010..fd6851f 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -148,6 +148,21 @@
renesas,groups = "qspi_ctrl", "qspi_data4";
renesas,function = "qspi";
};
+
+   usb0_pins: usb0 {
+   renesas,groups = "usb0";
+   renesas,function = "usb0";
+   };
+
+   usb1_pins: usb1 {
+   renesas,groups = "usb1";
+   renesas,function = "usb1";
+   };
+
+   usb2_pins: usb2 {
+   renesas,groups = "usb2";
+   renesas,function = "usb2";
+   };
 };
 
 &mmcif1 {
@@ -214,3 +229,20 @@
cd-gpios = <&gpio3 22 GPIO_ACTIVE_LOW>;
status = "okay";
 };
+
+&pci0 {
+   pinctrl-0 = <&usb0_pins>;
+   pinctrl-names = "default";
+};
+
+&pci1 {
+   status = "okay";
+   pinctrl-0 = <&usb1_pins>;
+   pinctrl-names = "default";
+};
+
+&pci2 {
+   status = "okay";
+   pinctrl-0 = <&usb2_pins>;
+   pinctrl-names = "default";
+};
-- 
1.9.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] phy-rcar-gen2-usb: add device tree support

2014-02-27 Thread Ben Dooks

On 27/02/14 00:12, Sergei Shtylyov wrote:

Add support of the device tree probing for the Renesas R-Car generation 2 SoCs
documenting the device tree binding as necessary.


You've popped in some fixes for the driver probe in here as well.

Could you do the fixes as a patch and send those before the devicetree
code is done?


+
  static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
  {
struct device *dev = &pdev->dev;
+   struct device_node *np = dev->of_node;
struct rcar_gen2_phy_platform_data *pdata;
struct rcar_gen2_usb_phy_priv *priv;
struct resource *res;
@@ -177,13 +210,19 @@ static int rcar_gen2_usb_phy_probe(struc
struct clk *clk;
int retval;

-   pdata = dev_get_platdata(dev);
+   if (np)
+   pdata = rcar_gen2_usb_phy_parse_dt(dev);
+   else
+   pdata = dev_get_platdata(dev);
if (!pdata) {
dev_err(dev, "No platform data\n");
return -EINVAL;
}

-   clk = devm_clk_get(dev, "usbhs");
+   if (np)
+   clk = of_clk_get_by_name(np, "usbhs");
+   else
+   clk = clk_get(dev, "usbhs");


Can be removed, just add a clock-name of usbhs in the device node.


if (IS_ERR(clk)) {
dev_err(dev, "Can't get the clock\n");
return PTR_ERR(clk);
@@ -191,13 +230,16 @@ static int rcar_gen2_usb_phy_probe(struc

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(dev, res);
-   if (IS_ERR(base))
-   return PTR_ERR(base);
+   if (IS_ERR(base)) {
+   retval = PTR_ERR(base);
+   goto error;
+   }

priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv) {
dev_err(dev, "Memory allocation failed\n");
-   return -ENOMEM;
+   retval = -ENOMEM;
+   goto error;
}


Probably should be separate patch to fix probe issues.




spin_lock_init(&priv->lock);
@@ -216,12 +258,16 @@ static int rcar_gen2_usb_phy_probe(struc
retval = usb_add_phy_dev(&priv->phy);
if (retval < 0) {
dev_err(dev, "Failed to add USB phy\n");
-   return retval;
+   goto error;
}

platform_set_drvdata(pdev, priv);

return retval;
+
+error:
+   clk_put(clk);
+   return retval;
  }


Again, should have been rolled into fix patch.



--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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] phy-rcar-gen2-usb: add device tree support

2014-02-27 Thread Ben Dooks

On 27/02/14 00:12, Sergei Shtylyov wrote:

Add support of the device tree probing for the Renesas R-Car generation 2 SoCs
documenting the device tree binding as necessary.


So what happened w.r.t to my last set of patches for this?

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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] phy-rcar-usb-gen2: add device tree support

2014-01-27 Thread Ben Dooks

On 27/01/14 18:23, Sergei Shtylyov wrote:

Hello.

On 01/26/2014 08:05 PM, Ben Dooks wrote:


[snip]



+static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = {
+{ .compatible = "renesas,usb-phy-r8a7790", },
+{ .compatible = "renesas,rcar-gen2-usb-phy", },


Frankly speaking, I don't understand the need for the clearly
duplicate entries.


Thanks, will look into remove it.
Anyone else have any comments on this?

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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 5/8] phy-rcar-usb-gen2: add device tree support

2014-01-26 Thread Ben Dooks

On 26/01/14 18:03, Sergei Shtylyov wrote:

Hello.

On 01/26/2014 07:49 PM, Ben Dooks wrote:


Add support for the phy-rcar-gen2-usb driver to be probed from device
tree.



Signed-off-by: Ben Dooks 
Reviewed-by: Ian Molton 
---
Fixes from v1:
- use of_property_reasd-bool()
- remove unused of_id variable



Cc: linux-usb@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: Magnus Damm 
Cc: Simon Horman 
Cc: devicet...@vger.kernel.org
---
  drivers/usb/phy/phy-rcar-gen2-usb.c | 31
++-
  1 file changed, 26 insertions(+), 5 deletions(-)



diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c
b/drivers/usb/phy/phy-rcar-gen2-usb.c
index db3ab34..d146388 100644
--- a/drivers/usb/phy/phy-rcar-gen2-usb.c
+++ b/drivers/usb/phy/phy-rcar-gen2-usb.c

[...]

@@ -203,16 +210,29 @@ static int rcar_gen2_usb_phy_probe(struct
platform_device *pdev)

[...]

+if (of_id) {


You've removed the variable but not its use. Have you tried to
compile this patch?


Thanks, already noticed that and produced v3.

I should not be let near git-rebase when hungry.

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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] phy-rcar-usb-gen2: add device tree support

2014-01-26 Thread Ben Dooks
Add support for the phy-rcar-gen2-usb driver to be probed from device tree.

Signed-off-by: Ben Dooks 
Reviewed-by: Ian Molton 
---
Fixes from v2:
- fix missing of_if patch
(I clearly should not be let near git-rebase when hungry)

Fixes from v1:
- use of_property_reasd-bool()
- remove unused of_id variable

Cc: linux-usb@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: Magnus Damm 
Cc: Simon Horman 
Cc: devicet...@vger.kernel.org
---
 drivers/usb/phy/phy-rcar-gen2-usb.c | 31 ++-
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c 
b/drivers/usb/phy/phy-rcar-gen2-usb.c
index db3ab34..e4665b9 100644
--- a/drivers/usb/phy/phy-rcar-gen2-usb.c
+++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -167,6 +168,12 @@ out:
spin_unlock_irqrestore(&priv->lock, flags);
 }
 
+static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = {
+   { .compatible = "renesas,usb-phy-r8a7790", },
+   { .compatible = "renesas,rcar-gen2-usb-phy", },
+   { },
+};
+
 static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
 {
struct device *dev = &pdev->dev;
@@ -178,7 +185,7 @@ static int rcar_gen2_usb_phy_probe(struct platform_device 
*pdev)
int retval;
 
pdata = dev_get_platdata(&pdev->dev);
-   if (!pdata) {
+   if (!pdata && !dev->of_node) {
dev_err(dev, "No platform data\n");
return -EINVAL;
}
@@ -203,16 +210,29 @@ static int rcar_gen2_usb_phy_probe(struct platform_device 
*pdev)
spin_lock_init(&priv->lock);
priv->clk = clk;
priv->base = base;
-   priv->ugctrl2 = pdata->chan0_pci ?
-   USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS;
-   priv->ugctrl2 |= pdata->chan2_pci ?
-   USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS;
priv->phy.dev = dev;
priv->phy.label = dev_name(dev);
priv->phy.init = rcar_gen2_usb_phy_init;
priv->phy.shutdown = rcar_gen2_usb_phy_shutdown;
priv->phy.set_suspend = rcar_gen2_usb_phy_set_suspend;
 
+   if (dev->of_node) {
+   if (of_property_read_bool(dev->of_node, "renesas,usb0-hs"))
+   priv->ugctrl2 = USBHS_UGCTRL2_USB0_HS;
+   else
+   priv->ugctrl2 = USBHS_UGCTRL2_USB0_PCI;
+
+   if (of_property_read_bool(dev->of_node, "renesas,usb2-ss"))
+   priv->ugctrl2 |= USBHS_UGCTRL2_USB2_SS;
+   else
+   priv->ugctrl2 |= USBHS_UGCTRL2_USB2_PCI;
+   } else {
+   priv->ugctrl2 = pdata->chan0_pci ?
+   USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS;
+   priv->ugctrl2 |= pdata->chan2_pci ?
+   USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS;
+   }
+
retval = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2);
if (retval < 0) {
dev_err(dev, "Failed to add USB phy\n");
@@ -236,6 +256,7 @@ static int rcar_gen2_usb_phy_remove(struct platform_device 
*pdev)
 static struct platform_driver rcar_gen2_usb_phy_driver = {
.driver = {
.name = "usb_phy_rcar_gen2",
+   .of_match_table = rcar_gen2_usb_phy_ofmatch,
},
.probe = rcar_gen2_usb_phy_probe,
.remove = rcar_gen2_usb_phy_remove,
-- 
1.8.5.2

--
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 5/8] phy-rcar-usb-gen2: add device tree support

2014-01-26 Thread Ben Dooks
Add support for the phy-rcar-gen2-usb driver to be probed from device tree.

Signed-off-by: Ben Dooks 
Reviewed-by: Ian Molton 
---
Fixes from v1:
- use of_property_reasd-bool()
- remove unused of_id variable

Cc: linux-usb@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: Magnus Damm 
Cc: Simon Horman 
Cc: devicet...@vger.kernel.org
---
 drivers/usb/phy/phy-rcar-gen2-usb.c | 31 ++-
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c 
b/drivers/usb/phy/phy-rcar-gen2-usb.c
index db3ab34..d146388 100644
--- a/drivers/usb/phy/phy-rcar-gen2-usb.c
+++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -167,6 +168,12 @@ out:
spin_unlock_irqrestore(&priv->lock, flags);
 }
 
+static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = {
+   { .compatible = "renesas,usb-phy-r8a7790", },
+   { .compatible = "renesas,rcar-gen2-usb-phy", },
+   { },
+};
+
 static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
 {
struct device *dev = &pdev->dev;
@@ -178,7 +185,7 @@ static int rcar_gen2_usb_phy_probe(struct platform_device 
*pdev)
int retval;
 
pdata = dev_get_platdata(&pdev->dev);
-   if (!pdata) {
+   if (!pdata && !dev->of_node) {
dev_err(dev, "No platform data\n");
return -EINVAL;
}
@@ -203,16 +210,29 @@ static int rcar_gen2_usb_phy_probe(struct platform_device 
*pdev)
spin_lock_init(&priv->lock);
priv->clk = clk;
priv->base = base;
-   priv->ugctrl2 = pdata->chan0_pci ?
-   USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS;
-   priv->ugctrl2 |= pdata->chan2_pci ?
-   USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS;
priv->phy.dev = dev;
priv->phy.label = dev_name(dev);
priv->phy.init = rcar_gen2_usb_phy_init;
priv->phy.shutdown = rcar_gen2_usb_phy_shutdown;
priv->phy.set_suspend = rcar_gen2_usb_phy_set_suspend;
 
+   if (of_id) {
+   if (of_property_read_bool(dev->of_node, "renesas,usb0-hs"))
+   priv->ugctrl2 = USBHS_UGCTRL2_USB0_HS;
+   else
+   priv->ugctrl2 = USBHS_UGCTRL2_USB0_PCI;
+
+   if (of_property_read_bool(dev->of_node, "renesas,usb2-ss"))
+   priv->ugctrl2 |= USBHS_UGCTRL2_USB2_SS;
+   else
+   priv->ugctrl2 |= USBHS_UGCTRL2_USB2_PCI;
+   } else {
+   priv->ugctrl2 = pdata->chan0_pci ?
+   USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS;
+   priv->ugctrl2 |= pdata->chan2_pci ?
+   USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS;
+   }
+
retval = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2);
if (retval < 0) {
dev_err(dev, "Failed to add USB phy\n");
@@ -236,6 +256,7 @@ static int rcar_gen2_usb_phy_remove(struct platform_device 
*pdev)
 static struct platform_driver rcar_gen2_usb_phy_driver = {
.driver = {
.name = "usb_phy_rcar_gen2",
+   .of_match_table = rcar_gen2_usb_phy_ofmatch,
},
.probe = rcar_gen2_usb_phy_probe,
.remove = rcar_gen2_usb_phy_remove,
-- 
1.8.5.2

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


  1   2   >