Re: [PATCH] usb: cdns3: include host-export,h for cdns3_host_init
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
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
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
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
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
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
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)
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
-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
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
-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
-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
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
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
--- 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
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.
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
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
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
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()
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
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
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
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'
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'
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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