Re: [PATCH v2] usb: gadget: at91_udc: move prepare clk into process context
Adding USB and Atmel Maintainers in Cc.
On Thu, 7 Aug 2014 09:52:31 +0200
Boris BREZILLON wrote:
> Hi Ronald,
>
> On Wed, 6 Aug 2014 15:11:42 +0200
> Ronald Wahl wrote:
>
> > Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> > interrupt context. This is not allowed as it might sleep. Move clock
> > preparation into process context (at91udc_probe).
> > ---
> > drivers/usb/gadget/udc/at91_udc.c | 39
> > ++-
> > 1 file changed, 30 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/at91_udc.c
> > b/drivers/usb/gadget/udc/at91_udc.c
> > index cfd18bc..0b347a0 100644
> > --- a/drivers/usb/gadget/udc/at91_udc.c
> > +++ b/drivers/usb/gadget/udc/at91_udc.c
> > @@ -872,10 +872,10 @@ static void clk_on(struct at91_udc *udc)
> >
> > if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> > clk_set_rate(udc->uclk, 4800);
> > - clk_prepare_enable(udc->uclk);
> > + clk_enable(udc->uclk);
> > }
> > - clk_prepare_enable(udc->iclk);
> > - clk_prepare_enable(udc->fclk);
> > + clk_enable(udc->iclk);
> > + clk_enable(udc->fclk);
> > }
> >
> > static void clk_off(struct at91_udc *udc)
> > @@ -884,10 +884,10 @@ static void clk_off(struct at91_udc *udc)
> > return;
> > udc->clocked = 0;
> > udc->gadget.speed = USB_SPEED_UNKNOWN;
> > - clk_disable_unprepare(udc->fclk);
> > - clk_disable_unprepare(udc->iclk);
> > + clk_disable(udc->fclk);
> > + clk_disable(udc->iclk);
> > if (IS_ENABLED(CONFIG_COMMON_CLK))
> > - clk_disable_unprepare(udc->uclk);
> > + clk_disable(udc->uclk);
> > }
>
> As you stated prepare and unprepare should never be called in interrupt
> context.
>
> My concern here is that PLLB (which is often used as USB clock
> parent) will never be gated/disabled (because all this work is
> done in its unprepare method), and thus your power consumption will be
> higher (when entering suspend mode) than if you'd done a
> disable_unprepare call.
>
> How about leaving the clk_on/off unchanged and use a threaded irq
> instead of a normal irq ?
>
>
> Best Regards,
>
> Boris
>
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
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 v4 2/4] usb: dwc2: add compatible data for rockchip soc
This patch add compatible data for dwc2 controller found on
rk3066, rk3188 and rk3288 processors from rockchip.
Signed-off-by: Kever Yang
Acked-by: Paul Zimmerman
---
Changes in v4:
- max_transfer_size change to 65536, this should be enough
for most transfer, the hardware auto-detect will set this
to 0x7 which may make dma_alloc_coherent fail when
non-dword aligned buf from driver like usbnet happen.
Changes in v3: None
Changes in v2:
- set most parameters as driver auto-detect
drivers/usb/dwc2/platform.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index a10e7a3..832b103 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -75,6 +75,34 @@ static const struct dwc2_core_params params_bcm2835 = {
.uframe_sched = 0,
};
+static const struct dwc2_core_params params_rk3066 = {
+ .otg_cap= 2,/* non-HNP/non-SRP */
+ .otg_ver= -1,
+ .dma_enable = -1,
+ .dma_desc_enable= 0,
+ .speed = -1,
+ .enable_dynamic_fifo= 1,
+ .en_multiple_tx_fifo= -1,
+ .host_rx_fifo_size = 520, /* 520 DWORDs */
+ .host_nperio_tx_fifo_size = 128, /* 128 DWORDs */
+ .host_perio_tx_fifo_size= 256, /* 256 DWORDs */
+ .max_transfer_size = 65536,
+ .max_packet_count = -1,
+ .host_channels = -1,
+ .phy_type = -1,
+ .phy_utmi_width = -1,
+ .phy_ulpi_ddr = -1,
+ .phy_ulpi_ext_vbus = -1,
+ .i2c_enable = -1,
+ .ulpi_fs_ls = -1,
+ .host_support_fs_ls_low_power = -1,
+ .host_ls_low_power_phy_clk = -1,
+ .ts_dline = -1,
+ .reload_ctl = -1,
+ .ahbcfg = 0x7, /* INCR16 */
+ .uframe_sched = -1,
+};
+
/**
* dwc2_driver_remove() - Called when the DWC_otg core is unregistered with the
* DWC_otg driver
@@ -97,6 +125,7 @@ static int dwc2_driver_remove(struct platform_device *dev)
static const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "brcm,bcm2835-usb", .data = ¶ms_bcm2835 },
+ { .compatible = "rockchip,rk3066-usb", .data = ¶ms_rk3066 },
{ .compatible = "snps,dwc2", .data = NULL },
{},
};
--
1.9.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 v4 0/4] Patches to add support for Rockchip dwc2 controller
These patches to add support for dwc2 controller found in Rockchip processors rk3066, rk3188 and rk3288, and enable dts for rk3288 evb. Changes in v4: - max_transfer_size change to 65536, this should be enough for most transfer, the hardware auto-detect will set this to 0x7 which may make dma_alloc_coherent fail when non-dword aligned buf from driver like usbnet happen. - remove EHCI and HSIC dts patch for Doug had post it seprately. Changes in v3: - EHCI and HSIC move new for version 3. - Rebase Changes in v2: - Split out dr_mode and rk3288 bindings. - add compatible "snps,dwc2" bingding info - set most parameters as driver auto-detect - evb patch added in version 2 Kever Yang (4): Documentation: dt-bindings: add dt binding info for Rockchip dwc2 usb: dwc2: add compatible data for rockchip soc ARM: dts: add rk3288 dwc2 controller support ARM: dts: Enable USB otg and host1(dwc) on rk3288-evb Documentation/devicetree/bindings/usb/dwc2.txt | 3 +++ arch/arm/boot/dts/rk3288-evb.dtsi | 6 ++ arch/arm/boot/dts/rk3288.dtsi | 20 ++ drivers/usb/dwc2/platform.c| 29 ++ 4 files changed, 58 insertions(+) -- 1.9.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] rtl8150: Prefix macros with RTL8150 to avoid collides
ACK
On 14-08-07 08:04:21, Nick Krause wrote:
> Avoid collides in global namespaces by prefixing macros with RTL8150 as
> suggested by David Miller. Collides as follows:
>
> drivers/net/usb/rtl8150.c:30:0: warning: "RSR" redefined
> arch/xtensa/include/asm/processor.h:189:0: note: this is the location of the
> previous definition
>
> with help from kernelnewbies. Test compiled on sandybridge.
>
> Signed-off-by: Nick Krause
> Suggested-by: David Miller
> ---
> drivers/net/usb/rtl8150.c | 102
> +++---
> 1 file changed, 52 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index 6e87e57..1e1c408 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c
> @@ -21,27 +21,27 @@
> #define DRIVER_AUTHOR "Petko Manolov "
> #define DRIVER_DESC "rtl8150 based usb-ethernet driver"
>
> -#define IDR 0x0120
> -#define MAR 0x0126
> -#define CR 0x012e
> -#define TCR 0x012f
> -#define RCR 0x0130
> -#define TSR 0x0132
> -#define RSR 0x0133
> -#define CON00x0135
> -#define CON10x0136
> -#define MSR 0x0137
> -#define PHYADD 0x0138
> -#define PHYDAT 0x0139
> -#define PHYCNT 0x013b
> -#define GPPC0x013d
> -#define BMCR0x0140
> -#define BMSR0x0142
> -#define ANAR0x0144
> -#define ANLP0x0146
> -#define AER 0x0148
> -#define CSCR 0x014C /* This one has the link status */
> -#define CSCR_LINK_STATUS (1 << 3)
> +#define RTL8150_IDR 0x0120
> +#define RTL8150_MAR 0x0126
> +#define RTL8150_CR 0x012e
> +#define RTL8150_TCR 0x012f
> +#define RTL8150_RCR 0x0130
> +#define RTL8150_TSR 0x0132
> +#define RTL8150_RSR 0x0133
> +#define RTL8150_CON00x0135
> +#define RTL8150_CON10x0136
> +#define RTL8150_MSR 0x0137
> +#define RTL8150_PHYADD 0x0138
> +#define RTL8150_PHYDAT 0x0139
> +#define RTL8150_PHYCNT 0x013b
> +#define RTL8150_GPPC0x013d
> +#define RTL8150_BMCR0x0140
> +#define RTL8150_BMSR0x0142
> +#define RTL8150_ANAR0x0144
> +#define RTL8150_ANLP0x0146
> +#define RTL8150_AER 0x0148
> +#define RTL8150_CSCR 0x014C /*This one has the link status*/
> +#define RTL8150_CSCR_LINK_STATUS (1 << 3)
>
> #define IDR_EEPROM 0x1202
>
> @@ -220,14 +220,14 @@ static int read_mii_word(rtl8150_t * dev, u8 phy, __u8
> indx, u16 * reg)
> tmp = indx | PHY_READ | PHY_GO;
> i = 0;
>
> - set_registers(dev, PHYADD, sizeof(data), data);
> - set_registers(dev, PHYCNT, 1, &tmp);
> + set_registers(dev, RTL8150_PHYADD, sizeof(data), data);
> + set_registers(dev, RTL8150_PHYCNT, 1, &tmp);
> do {
> - get_registers(dev, PHYCNT, 1, data);
> + get_registers(dev, RTL8150_PHYCNT, 1, data);
> } while ((data[0] & PHY_GO) && (i++ < MII_TIMEOUT));
>
> if (i <= MII_TIMEOUT) {
> - get_registers(dev, PHYDAT, 2, data);
> + get_registers(dev, RTL8150_PHYDAT, 2, data);
> *reg = data[0] | (data[1] << 8);
> return 0;
> } else
> @@ -245,10 +245,10 @@ static int write_mii_word(rtl8150_t * dev, u8 phy, __u8
> indx, u16 reg)
> tmp = indx | PHY_WRITE | PHY_GO;
> i = 0;
>
> - set_registers(dev, PHYADD, sizeof(data), data);
> - set_registers(dev, PHYCNT, 1, &tmp);
> + set_registers(dev, RTL8150_PHYADD, sizeof(data), data);
> + set_registers(dev, RTL8150_PHYCNT, 1, &tmp);
> do {
> - get_registers(dev, PHYCNT, 1, data);
> + get_registers(dev, RTL8150_PHYCNT, 1, data);
> } while ((data[0] & PHY_GO) && (i++ < MII_TIMEOUT));
>
> if (i <= MII_TIMEOUT)
> @@ -261,7 +261,7 @@ static inline void set_ethernet_addr(rtl8150_t * dev)
> {
> u8 node_id[6];
>
> - get_registers(dev, IDR, sizeof(node_id), node_id);
> + get_registers(dev, RTL8150_IDR, sizeof(node_id), node_id);
> memcpy(dev->netdev->dev_addr, node_id, sizeof(node_id));
> }
>
> @@ -275,17 +275,17 @@ static int rtl8150_set_mac_address(struct net_device
> *netdev, void *p)
>
> memcpy(netdev->dev_addr, addr->sa_
Re: [PATCHv2 01/13] usb: dwc2: Update Kconfig to support dual-role
On 8/1/14, 4:41 PM, Dinh Nguyen wrote: > On Fri, 2014-08-01 at 20:42 +, Paul Zimmerman wrote: >>> From: linux-usb-ow...@vger.kernel.org >>> [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of dingu...@altera.com >>> Sent: Wednesday, July 30, 2014 8:21 AM >>> >>> Update DWC2 kconfig and makefile to support dual-role mode. The platform >>> file will always get compiled for the case where the controller is directly >>> connected to the CPU. So for loadable modules, only dwc2.ko is needed. >>> >>> Signed-off-by: Dinh Nguyen >>> --- >>> v2: Remove reference to dwc2_gadget >>> --- >>> drivers/usb/dwc2/Kconfig | 59 >>> + >>> drivers/usb/dwc2/Makefile | 21 >>> 2 files changed, 44 insertions(+), 36 deletions(-) >>> >>> diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig >>> index f93807b..3d69928 100644 >>> --- a/drivers/usb/dwc2/Kconfig >>> +++ b/drivers/usb/dwc2/Kconfig >>> @@ -1,40 +1,29 @@ >>> config USB_DWC2 >>> - bool "DesignWare USB2 DRD Core Support" >>> + tristate "DesignWare USB2 DRD Core Support" >>> depends on USB >>> help >>> Say Y here if your system has a Dual Role Hi-Speed USB >>> controller based on the DesignWare HSOTG IP Core. >>> >>> - For host mode, if you choose to build the driver as dynamically >>> - linked modules, the core module will be called dwc2.ko, the PCI >>> - bus interface module (if you have a PCI bus system) will be >>> - called dwc2_pci.ko, and the platform interface module (for >>> - controllers directly connected to the CPU) will be called >>> - dwc2_platform.ko. For gadget mode, there will be a single >>> - module called dwc2_gadget.ko. >>> - >>> - NOTE: The s3c-hsotg driver is now renamed to dwc2_gadget. The >>> - host and gadget drivers are still currently separate drivers. >>> - There are plans to merge the dwc2_gadget driver with the dwc2 >>> - host driver in the near future to create a dual-role driver. >>> + If you choose to build the driver as dynamically >>> + linked modules, a single dwc2.ko(regardless of mode of operation) >>> + will get built for both platform IPs and PCI. >>> >>> if USB_DWC2 >>> >>> +choice >>> + bool "DWC2 Mode Selection" >>> + default USB_DWC2_DUAL_ROLE if (USB && USB_GADGET) >>> + default USB_DWC2_HOST if (USB && !USB_GADGET) >>> + default USB_DWC2_PERIPHERAL if (!USB && USB_GADGET) >>> + >>> config USB_DWC2_HOST >>> - tristate "Host only mode" >>> + bool "Host only mode" >>> depends on USB >>> help >>> The Designware USB2.0 high-speed host controller >>> - integrated into many SoCs. >>> - >>> -config USB_DWC2_PLATFORM >>> - bool "DWC2 Platform" >>> - depends on USB_DWC2_HOST >>> - default USB_DWC2_HOST >>> - help >>> - The Designware USB2.0 platform interface module for >>> - controllers directly connected to the CPU. This is only >>> - used for host mode. >>> + integrated into many SoCs. Select this option if you want the >>> + driver to operate in Host-only mode. >>> >>> config USB_DWC2_PCI >>> bool "DWC2 PCI" >>> @@ -47,11 +36,29 @@ config USB_DWC2_PCI >>> comment "Gadget mode requires USB Gadget support to be enabled" >>> >>> config USB_DWC2_PERIPHERAL >>> - tristate "Gadget only mode" >>> + bool "Gadget only mode" >>> depends on USB_GADGET >>> help >>> The Designware USB2.0 high-speed gadget controller >>> - integrated into many SoCs. >>> + integrated into many SoCs. Select this option if you want the >>> + driver to operate in Peripheral-only mode. >>> + >>> +config USB_DWC2_DUAL_ROLE >>> + bool "Dual Role mode" >>> + depends on ((USB=y || USB=USB_DWC2) && (USB_GADGET=y)) >> >> Hi Dinh, >> >> I just noticed that for dual-role mode, you are not allowing USB_GADGET >> to be modular. Is there a reason for that? If so, please mention it in >> the commit message. It should also be explained in the help text. Or >> maybe add another comment line saying "Dual-role mode requires USB Gadget >> = y" or something like that. >> > > I think it was an oversight on my part and there's not reason why > USB_GADGET can't be modular. > I went back to look this for v3 and it appears that I need USB_GADGET=y to avoid a build error when building the new driver for Gadget or Dual-role. drivers/built-in.o: In function `dwc2_gadget_init': /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3516: undefined reference to `usb_add_gadget_udc' drivers/built-in.o: In function `s3c_hsotg_remove': /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3543: undefined reference to `usb_del_gadget_udc' /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3549: undefined reference to `usb_gadget_unregister_driver' So just like the DWC3 driver, I need to add a dependency on USB_GADGET=y. I'll add a note to the help text. Dinh -- To unsubscribe from this list: se
Re: [PATCH V2] usb: xhci_suspend is not stopping the root hub timer for the shared HCD
On 08/06/2014 09:05 PM, Al Cooper wrote: > V2 - Restart polling (which will restart the timer) for the shared > HCD in xhci_resume(). > > xhci_suspend() will stop the primary HCD's root hub timer, but leaves > the shared HCD's timer running. This change adds stopping of the > shared HCD timer. > > Signed-off-by: Al Cooper > --- > drivers/usb/host/xhci.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index b6f2117..1557d4f 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -871,6 +871,8 @@ int xhci_suspend(struct xhci_hcd *xhci) > xhci_dbg(xhci, "%s: stopping port polling.\n", __func__); > clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > del_timer_sync(&hcd->rh_timer); > + clear_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags); > + del_timer_sync(&xhci->shared_hcd->rh_timer); > > spin_lock_irq(&xhci->lock); > clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); > @@ -1075,6 +1077,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > xhci_dbg(xhci, "%s: starting port polling.\n", __func__); > set_bit(HCD_FLAG_POLL_RH, &hcd->flags); > usb_hcd_poll_rh_status(hcd); > + set_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags); > + usb_hcd_poll_rh_status(xhci->shared_hcd); > > return retval; > } > Looks good, thanks. Added to my for-usb-next branch in git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git Will send it forward once 3.17-rc1 is out -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/7] xhci: various fixes and cleanups
On 08/04/2014 07:00 PM, Hans de Goede wrote: > Hi All, > > Here is a resend of my pending xhci fixes / cleanups. > > Note the first one is a bug-fix which really should go into > 3.17-rc2 (too late for rc1 I assume) the rest can wait I think. > > Changes in v2: > -rebased on top of usb-next > -merged > "xhci: Log ep-index and comp-code on TRB DMA ptr not part of current TD" > into: > "xhci: Log extra info on "ERROR Transfer event TRB DMA ptr not part of > current TD" > > Regards, > > Hans > Looks good, thanks. I haven't got a better way to debug the trb_in_td() than patch 2/7 so I'll take it. Added first patch to my for-usb-linus branch in git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git Rest of the patches are in for-usb-next I can send them forward with the other pending xhci patches once 3.17-rc1 is out -Mathias -- 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] usb: gadget: at91_udc: move prepare clk into process context
On 07.08.2014 09:59, Boris BREZILLON wrote:
On Thu, 7 Aug 2014 09:52:31 +0200
Boris BREZILLON wrote:
On Wed, 6 Aug 2014 15:11:42 +0200
Ronald Wahl wrote:
Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
interrupt context. This is not allowed as it might sleep. Move clock
preparation into process context (at91udc_probe).
---
drivers/usb/gadget/udc/at91_udc.c | 39 ++-
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/udc/at91_udc.c
b/drivers/usb/gadget/udc/at91_udc.c
index cfd18bc..0b347a0 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -872,10 +872,10 @@ static void clk_on(struct at91_udc *udc)
if (IS_ENABLED(CONFIG_COMMON_CLK)) {
clk_set_rate(udc->uclk, 4800);
- clk_prepare_enable(udc->uclk);
+ clk_enable(udc->uclk);
}
- clk_prepare_enable(udc->iclk);
- clk_prepare_enable(udc->fclk);
+ clk_enable(udc->iclk);
+ clk_enable(udc->fclk);
}
static void clk_off(struct at91_udc *udc)
@@ -884,10 +884,10 @@ static void clk_off(struct at91_udc *udc)
return;
udc->clocked = 0;
udc->gadget.speed = USB_SPEED_UNKNOWN;
- clk_disable_unprepare(udc->fclk);
- clk_disable_unprepare(udc->iclk);
+ clk_disable(udc->fclk);
+ clk_disable(udc->iclk);
if (IS_ENABLED(CONFIG_COMMON_CLK))
- clk_disable_unprepare(udc->uclk);
+ clk_disable(udc->uclk);
}
As you stated prepare and unprepare should never be called in interrupt
context.
My concern here is that PLLB (which is often used as USB clock
parent) will never be gated/disabled (because all this work is
done in its unprepare method), and thus your power consumption will be
higher (when entering suspend mode) than if you'd done a
disable_unprepare call.
How about leaving the clk_on/off unchanged and use a threaded irq
instead of a normal irq ?
Even with threaded interrupts things are still called while interrupts
are disabled by one or more layers of spin_lock_irqsave. There are also
different code paths from where clk_on() is called. So getting this
right while saving power is probably not trivial and beyond my
knowledge/experience with the udc code. It would be nice if someone with
a deeper knowledge of the atmel udc code can get this right. My primary
intention was to get the code working again.
thx & greets,
ron
--
Ronald Wahl - ronald.w...@raritan.com - Phone +49 375271349-0 Fax -99
Raritan Deutschland GmbH, Kornmarkt 7, 08056 Zwickau, Germany
USt-IdNr. DE813094160, Steuer-Nr. 227/117/01749
Amtsgericht Chemnitz HRB 23605
Geschäftsführung: Stuart Hopper, Ralf Ploenes
--
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] usb: gadget: at91_udc: move prepare clk into process context
On Thu, 07 Aug 2014 14:43:32 +0200
Ronald Wahl wrote:
> On 07.08.2014 09:59, Boris BREZILLON wrote:
> > On Thu, 7 Aug 2014 09:52:31 +0200
> > Boris BREZILLON wrote:
> >> On Wed, 6 Aug 2014 15:11:42 +0200
> >> Ronald Wahl wrote:
> >>
> >>> Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> >>> interrupt context. This is not allowed as it might sleep. Move clock
> >>> preparation into process context (at91udc_probe).
> >>> ---
> >>> drivers/usb/gadget/udc/at91_udc.c | 39
> >>> ++-
> >>> 1 file changed, 30 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/drivers/usb/gadget/udc/at91_udc.c
> >>> b/drivers/usb/gadget/udc/at91_udc.c
> >>> index cfd18bc..0b347a0 100644
> >>> --- a/drivers/usb/gadget/udc/at91_udc.c
> >>> +++ b/drivers/usb/gadget/udc/at91_udc.c
> >>> @@ -872,10 +872,10 @@ static void clk_on(struct at91_udc *udc)
> >>>
> >>> if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> >>> clk_set_rate(udc->uclk, 4800);
> >>> - clk_prepare_enable(udc->uclk);
> >>> + clk_enable(udc->uclk);
> >>> }
> >>> - clk_prepare_enable(udc->iclk);
> >>> - clk_prepare_enable(udc->fclk);
> >>> + clk_enable(udc->iclk);
> >>> + clk_enable(udc->fclk);
> >>> }
> >>>
> >>> static void clk_off(struct at91_udc *udc)
> >>> @@ -884,10 +884,10 @@ static void clk_off(struct at91_udc *udc)
> >>> return;
> >>> udc->clocked = 0;
> >>> udc->gadget.speed = USB_SPEED_UNKNOWN;
> >>> - clk_disable_unprepare(udc->fclk);
> >>> - clk_disable_unprepare(udc->iclk);
> >>> + clk_disable(udc->fclk);
> >>> + clk_disable(udc->iclk);
> >>> if (IS_ENABLED(CONFIG_COMMON_CLK))
> >>> - clk_disable_unprepare(udc->uclk);
> >>> + clk_disable(udc->uclk);
> >>> }
> >>
> >> As you stated prepare and unprepare should never be called in interrupt
> >> context.
> >>
> >> My concern here is that PLLB (which is often used as USB clock
> >> parent) will never be gated/disabled (because all this work is
> >> done in its unprepare method), and thus your power consumption will be
> >> higher (when entering suspend mode) than if you'd done a
> >> disable_unprepare call.
> >>
> >> How about leaving the clk_on/off unchanged and use a threaded irq
> >> instead of a normal irq ?
>
> Even with threaded interrupts things are still called while interrupts
> are disabled by one or more layers of spin_lock_irqsave. There are also
> different code paths from where clk_on() is called.
You're right (I only had a quick look at it). Let's fix this as a first
step and we'll figure out how to optimize power consumption later.
BTW, clk_set_rate can sleep too (AFAIK clk_enable and clk_disable
are the only one that can be called in atomic context).
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
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/4] USB: zte_ev: remove duplicate Gobi PID
Remove dublicate Gobi PID 0x9008 which is already handled by the
qcserial driver since commit f05932c0caf4 ("USB: qcserial: Add extra
device IDs").
Fixes: 799ee9243d89 ("USB: serial: add zte_ev.c driver")
Cc:
Signed-off-by: Johan Hovold
---
drivers/usb/serial/zte_ev.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/serial/zte_ev.c b/drivers/usb/serial/zte_ev.c
index 4ecff9cba286..960f70edcfd7 100644
--- a/drivers/usb/serial/zte_ev.c
+++ b/drivers/usb/serial/zte_ev.c
@@ -275,7 +275,6 @@ static const struct usb_device_id id_table[] = {
/* MG880 */
{ USB_DEVICE(0x19d2, 0xfffd) },
{ USB_DEVICE(0x05C6, 0x3197) },
- { USB_DEVICE(0x05C6, 0x9008) },
{ },
};
MODULE_DEVICE_TABLE(usb, id_table);
--
1.8.5.5
--
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/4] USB: zte_ev: remove duplicate Qualcom PID
Remove dublicate Qualcom PID 0x3197 which is already handled by the
moto-modem driver since commit 6986a978eec7 ("USB: add new moto_modem
driver for some Morotola phones").
Fixes: 799ee9243d89 ("USB: serial: add zte_ev.c driver")
Cc:
Signed-off-by: Johan Hovold
---
drivers/usb/serial/zte_ev.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/serial/zte_ev.c b/drivers/usb/serial/zte_ev.c
index 960f70edcfd7..1a132e9e947a 100644
--- a/drivers/usb/serial/zte_ev.c
+++ b/drivers/usb/serial/zte_ev.c
@@ -274,7 +274,6 @@ static void zte_ev_usb_serial_close(struct usb_serial_port
*port)
static const struct usb_device_id id_table[] = {
/* MG880 */
{ USB_DEVICE(0x19d2, 0xfffd) },
- { USB_DEVICE(0x05C6, 0x3197) },
{ },
};
MODULE_DEVICE_TABLE(usb, id_table);
--
1.8.5.5
--
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/4] Revert "USB: option,zte_ev: move most ZTE CDMA devices to zte_ev"
This reverts commit 73228a0538a70ebc4547bd09dee8971360dc1d87 ("USB:
option,zte_ev: move most ZTE CDMA devices to zte_ev").
Move the IDs of the devices that were previously driven by the option
driver back to that driver.
As several users have reported, the zte_ev driver is causing random
disconnects as well as reconnect failures.
A closer analysis of the zte_ev setup code reveals that it consists of
standard CDC requests (SET/GET_LINE_CODING and SET_CONTROL_LINE_STATE)
but unfortunately fails to get some of those right. In particular, as
reported by Liu Lei, it fails to lower DTR/RTS on close. It also appears
that the control requests lack the interface argument.
Note that the zte_ev driver is based on code (once) distributed by ZTE
that still appears to originally have been reverse-engineered and bolted
onto the generic driver.
Since line control is already handled properly by the option driver, and
the SET/GET_LINE_CODING requests appears to be redundant (amounts to a
SET 9600 8N1), this is a first step in ultimately removing the redundant
zte_ev driver.
Note that AC2726 had already been moved back to option, and that some
IDs were in the device table of both drivers prior to the commit being
reverted.
Reported-by: Lei Liu
Cc:
Signed-off-by: Johan Hovold
---
drivers/usb/serial/option.c | 24 +---
drivers/usb/serial/zte_ev.c | 18 --
2 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 34f142be5b83..084253c02b74 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -275,8 +275,12 @@ static void option_instat_callback(struct urb *urb);
#define ZTE_PRODUCT_MF622 0x0001
#define ZTE_PRODUCT_MF628 0x0015
#define ZTE_PRODUCT_MF626 0x0031
-#define ZTE_PRODUCT_MC2718 0xffe8
#define ZTE_PRODUCT_AC2726 0xfff1
+#define ZTE_PRODUCT_CDMA_TECH 0xfffe
+#define ZTE_PRODUCT_AC8710T0x
+#define ZTE_PRODUCT_MC2718 0xffe8
+#define ZTE_PRODUCT_AD3812 0xffeb
+#define ZTE_PRODUCT_MC2716 0xffed
#define BENQ_VENDOR_ID 0x04a5
#define BENQ_PRODUCT_H10 0x4068
@@ -527,10 +531,18 @@ static const struct option_blacklist_info
zte_k3765_z_blacklist = {
.reserved = BIT(4),
};
+static const struct option_blacklist_info zte_ad3812_z_blacklist = {
+ .sendsetup = BIT(0) | BIT(1) | BIT(2),
+};
+
static const struct option_blacklist_info zte_mc2718_z_blacklist = {
.sendsetup = BIT(1) | BIT(2) | BIT(3) | BIT(4),
};
+static const struct option_blacklist_info zte_mc2716_z_blacklist = {
+ .sendsetup = BIT(1) | BIT(2) | BIT(3),
+};
+
static const struct option_blacklist_info huawei_cdc12_blacklist = {
.reserved = BIT(1) | BIT(2),
};
@@ -1070,6 +1082,7 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE_INTERFACE_CLASS(BANDRICH_VENDOR_ID, BANDRICH_PRODUCT_1012,
0xff) },
{ USB_DEVICE(KYOCERA_VENDOR_ID, KYOCERA_PRODUCT_KPC650) },
{ USB_DEVICE(KYOCERA_VENDOR_ID, KYOCERA_PRODUCT_KPC680) },
+ { USB_DEVICE(QUALCOMM_VENDOR_ID, 0x6000)}, /* ZTE AC8700 */
{ USB_DEVICE(QUALCOMM_VENDOR_ID, 0x6613)}, /* Onda H600/ZTE MF330 */
{ USB_DEVICE(QUALCOMM_VENDOR_ID, 0x0023)}, /* ONYX 3G device */
{ USB_DEVICE(QUALCOMM_VENDOR_ID, 0x9000)}, /* SIMCom SIM5218 */
@@ -1544,13 +1557,18 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff93, 0xff, 0xff,
0xff) },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff94, 0xff, 0xff,
0xff) },
- /* NOTE: most ZTE CDMA devices should be driven by zte_ev, not option */
+ { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_CDMA_TECH,
0xff, 0xff, 0xff) },
+ { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_AC2726,
0xff, 0xff, 0xff) },
+ { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_AC8710T,
0xff, 0xff, 0xff) },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_MC2718,
0xff, 0xff, 0xff),
.driver_info = (kernel_ulong_t)&zte_mc2718_z_blacklist },
+ { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_AD3812,
0xff, 0xff, 0xff),
+.driver_info = (kernel_ulong_t)&zte_ad3812_z_blacklist },
+ { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_MC2716,
0xff, 0xff, 0xff),
+.driver_info = (kernel_ulong_t)&zte_mc2716_z_blacklist },
{ USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x02, 0x01) },
{ USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x02, 0x05) },
{ USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x86, 0x10) },
- { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_AC2726,
0xff, 0xff, 0xf
[PATCH 0/4] USB: serial: remove zte_ev driver
These patches move the ZTE PIDs that were originally handled by the option driver back to that driver, remove two PIDs from zte_ev that were already claimed by other drivers, and finally removes the zte_ev driver completely (and moves the sole remaining PID over to option). The zte_ev driver is based on code (once) distributed by ZTE that still appears to originally have been reverse-engineered and bolted onto the generic driver. A closer inspections of the control requests it was sending reveals that they were not doing anything not already handled (better) by option (well, except for some apparently redundant SET/GET_LINE_CODING). There have also been reports about disconnect and reconnect issues with zte_ev, something which has already led to one PID already having been moved back. Lei Liu of ZTE has asked for further PIDs to be moved after reporting issues with the driver. I intend to queue up the first three, which are also marked for stable, for v3.17-rc2, while the final removal of the driver is held back to v3.18. That way there's plenty of time to deal with any issues that, however unlikely, may arise. Comments? Johan Johan Hovold (4): Revert "USB: option,zte_ev: move most ZTE CDMA devices to zte_ev" USB: zte_ev: remove duplicate Gobi PID USB: zte_ev: remove duplicate Qualcom PID Revert "USB: serial: add zte_ev.c driver" drivers/usb/serial/Kconfig | 8 -- drivers/usb/serial/Makefile | 1 - drivers/usb/serial/option.c | 26 +++- drivers/usb/serial/zte_ev.c | 317 4 files changed, 23 insertions(+), 329 deletions(-) delete mode 100644 drivers/usb/serial/zte_ev.c -- 1.8.5.5 -- 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/4] Revert "USB: serial: add zte_ev.c driver"
This reverts commit 799ee9243d892ad959c8e5f4549593ece59f1c80 ("USB:
serial: add zte_ev.c driver").
The zte_ev driver is based on code (once) distributed by ZTE that still
appears to originally have been reverse-engineered and bolted onto the
generic driver.
A closer analysis of the zte_ev setup code reveals that it consists of
standard CDC requests (SET/GET_LINE_CODING and SET_CONTROL_LINE_STATE)
but unfortunately fails to get some of those right. In particular, as
reported by Liu Lei, it fails to lower DTR/RTS on close. It also appears
that the control requests lack the interface argument.
Since line control is already handled properly by the option driver, and
the SET/GET_LINE_CODING requests appears to be redundant (amounts to a
SET 9600 8N1) let's remove the redundant zte_ev driver.
Also move the sole remaining ZTE MG880 PID to the generic option modem
driver.
Reported-by: Lei Liu
Signed-off-by: Johan Hovold
---
drivers/usb/serial/Kconfig | 8 --
drivers/usb/serial/Makefile | 1 -
drivers/usb/serial/option.c | 2 +
drivers/usb/serial/zte_ev.c | 297
4 files changed, 2 insertions(+), 306 deletions(-)
delete mode 100644 drivers/usb/serial/zte_ev.c
diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 3ce5c74b29e4..842f28f8a75f 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -682,14 +682,6 @@ config USB_SERIAL_WISHBONE
To compile this driver as a module, choose M here: the
module will be called wishbone-serial.
-config USB_SERIAL_ZTE
- tristate "ZTE USB serial driver"
- help
- Say Y here if you want to use a ZTE USB to serial device.
-
- To compile this driver as a module, choose M here: the
- module will be called zte.
-
config USB_SERIAL_SSU100
tristate "USB Quatech SSU-100 Single Port Serial Driver"
help
diff --git a/drivers/usb/serial/Makefile b/drivers/usb/serial/Makefile
index bfdafd349441..349d9df0895f 100644
--- a/drivers/usb/serial/Makefile
+++ b/drivers/usb/serial/Makefile
@@ -60,4 +60,3 @@ obj-$(CONFIG_USB_SERIAL_WISHBONE) +=
wishbone-serial.o
obj-$(CONFIG_USB_SERIAL_WHITEHEAT) += whiteheat.o
obj-$(CONFIG_USB_SERIAL_XIRCOM)+= keyspan_pda.o
obj-$(CONFIG_USB_SERIAL_XSENS_MT) += xsens_mt.o
-obj-$(CONFIG_USB_SERIAL_ZTE) += zte_ev.o
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 084253c02b74..ec601e1bc2a7 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -276,6 +276,7 @@ static void option_instat_callback(struct urb *urb);
#define ZTE_PRODUCT_MF628 0x0015
#define ZTE_PRODUCT_MF626 0x0031
#define ZTE_PRODUCT_AC2726 0xfff1
+#define ZTE_PRODUCT_MG880 0xfffd
#define ZTE_PRODUCT_CDMA_TECH 0xfffe
#define ZTE_PRODUCT_AC8710T0x
#define ZTE_PRODUCT_MC2718 0xffe8
@@ -1557,6 +1558,7 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff93, 0xff, 0xff,
0xff) },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff94, 0xff, 0xff,
0xff) },
+ { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_MG880, 0xff,
0xff, 0xff) },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_CDMA_TECH,
0xff, 0xff, 0xff) },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_AC2726,
0xff, 0xff, 0xff) },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_AC8710T,
0xff, 0xff, 0xff) },
diff --git a/drivers/usb/serial/zte_ev.c b/drivers/usb/serial/zte_ev.c
deleted file mode 100644
index 1a132e9e947a..
--- a/drivers/usb/serial/zte_ev.c
+++ /dev/null
@@ -1,297 +0,0 @@
-/*
- * ZTE_EV USB serial driver
- *
- * Copyright (C) 2012 Greg Kroah-Hartman
- * Copyright (C) 2012 Linux Foundation
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This driver is based on code found in a ZTE_ENV patch that modified
- * the usb-serial generic driver. Comments were left in that I think
- * show the commands used to talk to the device, but I am not sure.
- */
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-
-#define MAX_SETUP_DATA_SIZE 32
-
-static void debug_data(struct device *dev, const char *function, int len,
- const unsigned char *data, int result)
-{
- dev_dbg(dev, "result = %d\n", result);
- if (result == len)
- dev_dbg(dev, "%s - length = %d, data = %*ph\n", function,
- len, len, data);
-}
-
-static int zte_ev_usb_serial_open(struct tty_struct *tty,
- struct
Re: [PATCH v3 2/6] usb: host: ehci-st: Add EHCI support for ST STB devices
Hi Arnd,
Thanks for reviewing, see my comments below: -
> > +static int st_ehci_platform_reset(struct usb_hcd *hcd)
> > +{
> > + struct platform_device *pdev = to_platform_device(hcd->self.controller);
> > + struct usb_ehci_pdata *pdata = dev_get_platdata(&pdev->dev);
> > + struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> > + int retval;
> > +
> > + if (pdata->pre_setup) {
> > + retval = pdata->pre_setup(hcd);
> > + if (retval < 0)
> > + return retval;
> > + }
>
> What is the point in going through a platform data function pointer here?
> Can't you just open-code st_ehci_configure_bus() here?
Yes I can, I've done as you suggest in v4.
>
> > +* Use reasonable defaults so platforms don't have to provide these
> > +* with DT probing on ARM.
> > +*/
> > + if (!pdata)
> > + pdata = &ehci_platform_defaults;
>
> How would you ever get here with pdata set?
This is left over from using ohci-platform as a starting point. pdata is not
set when booting via DT and gets initialized here.
As this driver depends on OF and will never run without DT I've initialised
pdata where it gets defined in V4.
I've then also removed the check further down in probe that causes some
unnecessary
indentation where we get the phys / clocks.
>
> > + err = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
> > + if (err)
> > + return err;
>
> Remove this here, and rely on the correct mask to be set from the DT scan.
I've removed in V4.
regards,
Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/6] usb: host: usb-st-common: Add common code required by ohci-st and ehci-st
Hi Arnd,
Thanks for reviewing, see my comments below: -
> > + if (priv->rst) {
> > + ret =
> > (priv->rst);
> > + if (ret)
> > + goto err_assert_power;
> > + }
>
> I wouldn't make these optional, just call the functions
> unconditionally and fail the probe function if they are
> not available.
>
> I'm not sure if it's worth keeping these functions in a
> common file. You are adding complexity this way and I don't
> think you are even saving a significant number of code lines
> compared to just having two copies of them.
I've unabstracted these common functions back into ehci-st.c and
ohci-st,c in V4.
regards,
Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/6] usb: host: usb-st-common: Add common code required by ohci-st and ehci-st
Hi Arnd, > > > (priv->rst); > > > + if (ret) > > > + goto err_assert_power; > > > + } > > > > I wouldn't make these optional, just call the functions > > unconditionally and fail the probe function if they are > > not available. Also I've also done as you suggest and made these non optional in V4. regards, Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/6] Add EHCI and OHCI drivers for STi SoC's
Hi Arnd, Thanks for reviewing, see my comments below: - > > Changes since v2: > > - Based on Arnd Berghman feedback, split out into 2 devices / drivers > > - Base drivers oh ehci-platform.c & ohci-platform.c with required > > extensions > >to allow possible re-merge in the furture. > > Hi Peter, > > This looks much better than the first version. I have some remaining comments > for > how it could be simplified a bit more. Yes I think I've addressed these now in V4, which I will send in a moment. > > The way that you deal with the 48mhz clock seems like it should fit in well > with the generic driver, just like all the rest (once the usb-st-common > stuff is moved into the ohci/ehci drivers), so the alternative would be > to make it all generic now. Yes I tried to do it in a way which would, however I would prefer not to merge into the generic one for now because: - 1) There are some other subtle changes versus the generic drivers. For example in power_on() and power_off() I put the IP into reset and power down, which isn't done in the generic driver. Also with this IP it needs power signal asserted before reset, otherwise it can hang the chip. 2) I don't have any of the other hardware which uses ehci-platform / ohci-platform to test I haven't broken things. regards, Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.15.6 USB issue with pwc cam
On 2014-08-04 11:17, Laurent Pinchart wrote: > (CC'ing Hans de Goede, the pwc maintainer, and the linux-media mailing list) > > On Saturday 02 August 2014 15:10:01 Udo van den Heuvel wrote: >> Hello, >> >> I moved a PWC webcam to a USB3 port, and this happened: I get similar stuff when trying to use a Logitech C615 cam. See attachment for full dmesg of errors but excerpt below: [80346.835015] xhci_hcd :02:00.0: ERROR: unexpected command completion code 0x11. [80346.835027] usb 6-2: Not enough bandwidth for altsetting 11 [80346.835137] [ cut here ] [80346.835155] WARNING: CPU: 3 PID: 20594 at drivers/media/v4l2-core/videobuf2-core.c:2011 __vb2_queue_cancel+0x102/0x170 [videobuf2_core]() [80346.835158] Modules linked in: uvcvideo cdc_acm bnep bluetooth fuse edac_core cpufreq_userspace ipt_REJECT nf_conntrack_netbios_ns nf_conntrack_broadcast iptable_filter ip6t_REJECT ipt_MASQUERADE xt_tcpudp nf_conntrack_ipv6 iptable_nat nf_defrag_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat xt_conntrack nf_conntrack ip_tables ip6table_filter ip6_tables x_tables eeprom it87 hwmon_vid ext2 snd_usb_audio snd_usbmidi_lib snd_hwdep snd_rawmidi ppdev pwc videobuf2_vmalloc videobuf2_memops kvm_amd kvm v4l2_common videobuf2_core snd_hda_codec_realtek snd_hda_codec_generic videodev snd_hda_intel snd_hda_controller cp210x snd_hda_codec usbserial snd_seq snd_seq_device microcode snd_pcm parport_serial parport_pc parport snd_timer k10temp snd evdev i2c_piix4 button acpi_cpufreq nfsd auth_rpcgss oid_registry [80346.835218] nfs_acl lockd sunrpc binfmt_misc autofs4 hid_generic usbhid ohci_pci ehci_pci ehci_hcd ohci_hcd radeon sr_mod cdrom fbcon bitblit cfbfillrect softcursor cfbimgblt cfbcopyarea font i2c_algo_bit xhci_hcd backlight drm_kms_helper ttm drm fb fbdev [80346.835250] CPU: 3 PID: 20594 Comm: skype Tainted: GW 3.15.8 #6 [80346.835254] Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./F2A85X-UP4, BIOS F5a 04/30/2013 [80346.835257] 79d580f4 814f2373 [80346.835262] 81069fe1 88040ec532e8 [80346.835267] 88040ec530d8 8803f0c46f00 a041d832 88040ec530d8 [80346.835272] Call Trace: [80346.835283] [] ? dump_stack+0x4a/0x75 [80346.835289] [] ? warn_slowpath_common+0x81/0xb0 [80346.835299] [] ? __vb2_queue_cancel+0x102/0x170 [videobuf2_core] [80346.835307] [] ? vb2_internal_streamoff+0x1d/0x50 [videobuf2_core] [80346.835314] [] ? uvc_queue_enable+0x75/0xb0 [uvcvideo] [80346.835321] [] ? uvc_video_enable+0x141/0x1a0 [uvcvideo] [80346.835327] [] ? uvc_v4l2_do_ioctl+0xd6f/0x1580 [uvcvideo] [80346.835339] [] ? video_usercopy+0x1f0/0x490 [videodev] [80346.835345] [] ? uvc_v4l2_set_streamparm.isra.12+0x1c0/0x1c0 [uvcvideo] [80346.835352] [] ? preempt_count_add+0x3f/0x90 [80346.835356] [] ? _raw_spin_lock+0xe/0x30 [80346.835360] [] ? _raw_spin_unlock+0xd/0x30 [80346.835367] [] ? __pte_alloc+0xce/0x170 [80346.835376] [] ? v4l2_ioctl+0x11f/0x160 [videodev] [80346.835386] [] ? do_video_ioctl+0x246/0x1330 [videodev] [80346.835392] [] ? mmap_region+0x15a/0x5a0 [80346.835402] [] ? v4l2_compat_ioctl32+0x82/0xb8 [videodev] [80346.835408] [] ? compat_SyS_ioctl+0x132/0x1120 [80346.835414] [] ? vm_mmap_pgoff+0xe3/0x120 [80346.835421] [] ? cstar_dispatch+0x7/0x1a [80346.835424] ---[ end trace 44e3d272b6c91a71 ]--- [80346.835427] [ cut here ] What is wrong here? Kind regards, Udo [80346.835015] xhci_hcd :02:00.0: ERROR: unexpected command completion code 0x11. [80346.835027] usb 6-2: Not enough bandwidth for altsetting 11 [80346.835137] [ cut here ] [80346.835155] WARNING: CPU: 3 PID: 20594 at drivers/media/v4l2-core/videobuf2-core.c:2011 __vb2_queue_cancel+0x102/0x170 [videobuf2_core]() [80346.835158] Modules linked in: uvcvideo cdc_acm bnep bluetooth fuse edac_core cpufreq_userspace ipt_REJECT nf_conntrack_netbios_ns nf_conntrack_broadcast iptable_filter ip6t_REJECT ipt_MASQUERADE xt_tcpudp nf_conntrack_ipv6 iptable_nat nf_defrag_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat xt_conntrack nf_conntrack ip_tables ip6table_filter ip6_tables x_tables eeprom it87 hwmon_vid ext2 snd_usb_audio snd_usbmidi_lib snd_hwdep snd_rawmidi ppdev pwc videobuf2_vmalloc videobuf2_memops kvm_amd kvm v4l2_common videobuf2_core snd_hda_codec_realtek snd_hda_codec_generic videodev snd_hda_intel snd_hda_controller cp210x snd_hda_codec usbserial snd_seq snd_seq_device microcode snd_pcm parport_serial parport_pc parport snd_timer k10temp snd evdev i2c_piix4 button acpi_cpufreq nfsd auth_rpcgss oid_registry [80346.835218] nfs_acl lockd sunrpc binfmt_misc autofs4 hid_generic usbhid ohci_pci ehci_pci ehci_hcd ohci_hcd radeon sr_mod cdrom fbcon bitblit cfbfillrect softcursor cfbimgblt cfbcopyarea font i2c_algo_bit xhci_hcd backlight drm_kms_helper ttm drm fb fbdev [80346.835250] CPU: 3 PID: 20594 Comm
[PATCH v4 1/5] usb: host: ehci-st: Add EHCI support for ST STB devices
This patch adds the glue code required to ensure the on-chip EHCI
controller works on STi consumer electronics SoC's from STMicroelectronics.
It mainly manages the setting and enabling of the relevant clocks and manages
the reset / power signals to the IP block.
Signed-off-by: Peter Griffin
---
drivers/usb/host/ehci-st.c | 363 +++
drivers/usb/host/usb-st-common.h | 31
2 files changed, 394 insertions(+)
create mode 100644 drivers/usb/host/ehci-st.c
create mode 100644 drivers/usb/host/usb-st-common.h
diff --git a/drivers/usb/host/ehci-st.c b/drivers/usb/host/ehci-st.c
new file mode 100644
index 000..ac72ce3
--- /dev/null
+++ b/drivers/usb/host/ehci-st.c
@@ -0,0 +1,363 @@
+/*
+ * ST EHCI driver
+ *
+ * Copyright (C) 2014 STMicroelectronics – All Rights Reserved
+ *
+ * Author: Peter Griffin
+ *
+ * Derived from ehci-platform.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "ehci.h"
+#include "usb-st-common.h"
+
+#define DRIVER_DESC "EHCI STMicroelectronics driver"
+
+#define hcd_to_ehci_priv(h) ((struct st_platform_priv *)hcd_to_ehci(h)->priv)
+
+static const char hcd_name[] = "ehci-st";
+
+#define EHCI_CAPS_SIZE 0x10
+#define AHB2STBUS_INSREG01 (EHCI_CAPS_SIZE + 0x84)
+
+static int st_ehci_platform_reset(struct usb_hcd *hcd)
+{
+ struct platform_device *pdev = to_platform_device(hcd->self.controller);
+ struct usb_ehci_pdata *pdata = dev_get_platdata(&pdev->dev);
+ struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+ int retval;
+ u32 threshold;
+
+ /* Set EHCI packet buffer IN/OUT threshold to 128 bytes */
+ threshold = 128 | (128 << 16);
+ writel(threshold, hcd->regs + AHB2STBUS_INSREG01);
+
+ ehci->caps = hcd->regs + pdata->caps_offset;
+ retval = ehci_setup(hcd);
+ if (retval)
+ return retval;
+
+ return 0;
+}
+
+static int st_ehci_platform_power_on(struct platform_device *dev)
+{
+ struct usb_hcd *hcd = platform_get_drvdata(dev);
+ struct st_platform_priv *priv = hcd_to_ehci_priv(hcd);
+ int clk, ret;
+
+ ret = reset_control_deassert(priv->pwr);
+ if (ret)
+ return ret;
+
+ ret = reset_control_deassert(priv->rst);
+ if (ret)
+ goto err_assert_power;
+
+ /* some SoCs don't have a dedicated 48Mhz clock, but those that do
+ need the rate to be explicitly set */
+ if (priv->clk48) {
+ ret = clk_set_rate(priv->clk48, 4800);
+ if (ret)
+ goto err_assert_reset;
+ }
+
+ for (clk = 0; clk < USB_MAX_CLKS && priv->clks[clk]; clk++) {
+ ret = clk_prepare_enable(priv->clks[clk]);
+ if (ret)
+ goto err_disable_clks;
+ }
+
+ ret = phy_init(priv->phy);
+ if (ret)
+ goto err_disable_clks;
+
+ ret = phy_power_on(priv->phy);
+ if (ret)
+ goto err_exit_phy;
+
+ return 0;
+
+err_exit_phy:
+ phy_exit(priv->phy);
+err_disable_clks:
+ while (--clk >= 0)
+ clk_disable_unprepare(priv->clks[clk]);
+err_assert_reset:
+ reset_control_assert(priv->rst);
+err_assert_power:
+ reset_control_assert(priv->pwr);
+
+ return ret;
+}
+
+static void st_ehci_platform_power_off(struct platform_device *dev)
+{
+ struct usb_hcd *hcd = platform_get_drvdata(dev);
+ struct st_platform_priv *priv = hcd_to_ehci_priv(hcd);
+ int clk;
+
+ reset_control_assert(priv->pwr);
+
+ reset_control_assert(priv->rst);
+
+ phy_power_off(priv->phy);
+
+ phy_exit(priv->phy);
+
+ for (clk = USB_MAX_CLKS - 1; clk >= 0; clk--)
+ if (priv->clks[clk])
+ clk_disable_unprepare(priv->clks[clk]);
+
+}
+
+static struct hc_driver __read_mostly ehci_platform_hc_driver;
+
+static const struct ehci_driver_overrides platform_overrides __initconst = {
+ .reset =st_ehci_platform_reset,
+ .extra_priv_size = sizeof(struct st_platform_priv),
+};
+
+static struct usb_ehci_pdata ehci_platform_defaults = {
+ .power_on = st_ehci_platform_power_on,
+ .power_suspend =st_ehci_platform_power_off,
+ .power_off =st_ehci_platform_power_off,
+};
+
+static int st_ehci_platform_probe(struct platform_device *dev)
+{
+ struct usb_hcd *hcd;
+ struct resource *res_mem;
+ struct usb_ehci_pdata *pdata = &ehci_platform_defaults;
+ struct st_platform_priv *priv;
+ struct ehci_hcd *ehci;
+ int err, irq, clk = 0;
+
+ if (usb_disabled())
+
[PATCH v4 0/5] Add EHCI and OHCI drivers for STi SoC's
The series has been re-worked from v2 onwards to split out the ehci and ohci parts into their own drivers / devices like most other ARM platforms based on feedback from Arnd Bergmann (see here http://www.spinics.net/lists/linux-usb/msg24.html. The ehci-platform & ohci-platform have been used as a basis for this in case we wish to merge the drivers again in the future. Changes since v3: - Make reset / power, clocks etc non optional dt options to simplify code - Get rid of non DT boot code left over from *hci-platform drivers - Remove DMA mask code - Remove pre_setup func ptr and configure ahb2st bus convertor in ehci_platform_reset - Unabstract and remove usb-st-common.c - Have one kconfig which enables both ehci-st & ohci-st drivers Changes since v2: - Based on Arnd Berghman feedback, split out into 2 devices / drivers - Base drivers oh ehci-platform.c & ohci-platform.c with required extensions to allow possible re-merge in the furture. Changes since v1: - Correct s/OCHI/OHCI/ spelling - Improve kconfig help message - Various formating / spelling nits identified by Lee Jones - Make driver depend on OF & remove node checks in code - Use devm_ioremap_resource - Remove unnecessary header files Peter Griffin (5): usb: host: ehci-st: Add EHCI support for ST STB devices usb: host: ohci-st: Add OHCI driver support for ST STB devices usb: host: ehci-st: Add ehci-st devicetree bindings documentation usb: host: ohci-st: Add ohci-st devicetree bindings documentation MAINTAINERS: Add ehci-st.c and ohci-st.c to ARCH/STI architecture Documentation/devicetree/bindings/usb/ehci-st.txt | 39 +++ Documentation/devicetree/bindings/usb/ohci-st.txt | 37 +++ MAINTAINERS | 2 + drivers/usb/host/Kconfig | 8 + drivers/usb/host/Makefile | 1 + drivers/usb/host/ehci-st.c| 363 ++ drivers/usb/host/ohci-st.c| 337 drivers/usb/host/usb-st-common.h | 31 ++ 8 files changed, 818 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/ehci-st.txt create mode 100644 Documentation/devicetree/bindings/usb/ohci-st.txt create mode 100644 drivers/usb/host/ehci-st.c create mode 100644 drivers/usb/host/ohci-st.c create mode 100644 drivers/usb/host/usb-st-common.h -- 1.9.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 v4 4/5] usb: host: ohci-st: Add ohci-st devicetree bindings documentation
This patch documents the device tree bindings required for
the ohci on-chip controller found in ST consumer electronics SoC's.
Signed-off-by: Peter Griffin
---
Documentation/devicetree/bindings/usb/ohci-st.txt | 37 +++
1 file changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ohci-st.txt
diff --git a/Documentation/devicetree/bindings/usb/ohci-st.txt
b/Documentation/devicetree/bindings/usb/ohci-st.txt
new file mode 100644
index 000..6d83937
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ohci-st.txt
@@ -0,0 +1,37 @@
+ST USB OHCI controller
+
+Required properties:
+
+ - compatible : must be "st,st-ohci-300x"
+ - reg : physical base addresses of the controller and length
of memory mapped
+ region
+ - interrupts : one OHCI controller interrupt should be described here
+ - clocks : phandle list of usb clocks
+ - clock-names : should be "ic" for interconnect clock and "clk48"
+See: Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+ - phys: phandle for the PHY device
+ - phy-names : should be "usb"
+
+ - resets : phandle to the powerdown and reset controller for the
USB IP
+ - reset-names : should be "power" and "softreset".
+See: Documentation/devicetree/bindings/reset/st,sti-powerdown.txt
+See: Documentation/devicetree/bindings/reset/reset.txt
+
+Example:
+
+ ohci0: usb@0xfe1ffc00 {
+ compatible = "st,st-ohci-300x";
+ reg = <0xfe1ffc00 0x100>;
+ interrupts = ;
+ clocks = <&clk_s_a1_ls 0>,
+<&clockgen_b0 0>;
+ clock-names = "ic", "clk48";
+ phys = <&usb2_phy>;
+ phy-names = "usb";
+ status = "okay";
+
+ resets = <&powerdown STIH416_USB0_POWERDOWN>,
+<&softreset STIH416_USB0_SOFTRESET>;
+ reset-names = "power", "softreset";
+ };
--
1.9.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 v4 5/5] MAINTAINERS: Add ehci-st.c and ohci-st.c to ARCH/STI architecture
Signed-off-by: Peter Griffin --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index c2066f4..89aef87 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1356,6 +1356,8 @@ F:drivers/pinctrl/pinctrl-st.c F: drivers/media/rc/st_rc.c F: drivers/i2c/busses/i2c-st.c F: drivers/tty/serial/st-asc.c +F: drivers/usb/host/ehci-st.c +F: drivers/usb/host/ohci-st.c ARM/TECHNOLOGIC SYSTEMS TS7250 MACHINE SUPPORT M: Lennert Buytenhek -- 1.9.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 v2] usb: gadget: at91_udc: move prepare clk into process context
Hi,
actually this should have been patch v3 and not v2. Anyway, the major
difference is that I moved setting the clock rate into process context
as well as it may also sleep.
- ron
On 07.08.2014 17:11, Ronald Wahl wrote:
Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
interrupt context. This is not allowed as it might sleep. Move clock
preparation and setting clock rate into process context (at91udc_probe).
Signed-off-by: Ronald Wahl
---
drivers/usb/gadget/udc/at91_udc.c | 44 ---
1 file changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/gadget/udc/at91_udc.c
b/drivers/usb/gadget/udc/at91_udc.c
index cfd18bc..0d685d0 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -870,12 +870,10 @@ static void clk_on(struct at91_udc *udc)
return;
udc->clocked = 1;
- if (IS_ENABLED(CONFIG_COMMON_CLK)) {
- clk_set_rate(udc->uclk, 4800);
- clk_prepare_enable(udc->uclk);
- }
- clk_prepare_enable(udc->iclk);
- clk_prepare_enable(udc->fclk);
+ if (IS_ENABLED(CONFIG_COMMON_CLK))
+ clk_enable(udc->uclk);
+ clk_enable(udc->iclk);
+ clk_enable(udc->fclk);
}
static void clk_off(struct at91_udc *udc)
@@ -884,10 +882,10 @@ static void clk_off(struct at91_udc *udc)
return;
udc->clocked = 0;
udc->gadget.speed = USB_SPEED_UNKNOWN;
- clk_disable_unprepare(udc->fclk);
- clk_disable_unprepare(udc->iclk);
+ clk_disable(udc->fclk);
+ clk_disable(udc->iclk);
if (IS_ENABLED(CONFIG_COMMON_CLK))
- clk_disable_unprepare(udc->uclk);
+ clk_disable(udc->uclk);
}
/*
@@ -1780,14 +1778,24 @@ static int at91udc_probe(struct platform_device *pdev)
}
/* don't do anything until we have both gadget driver and VBUS */
+ if (IS_ENABLED(CONFIG_COMMON_CLK)) {
+ clk_set_rate(udc->uclk, 4800);
+ retval = clk_prepare(udc->uclk);
+ if (retval)
+ goto fail1;
+ }
+ retval = clk_prepare(udc->fclk);
+ if (retval)
+ goto fail1a;
+
retval = clk_prepare_enable(udc->iclk);
if (retval)
- goto fail1;
+ goto fail1b;
at91_udp_write(udc, AT91_UDP_TXVC, AT91_UDP_TXVC_TXVDIS);
at91_udp_write(udc, AT91_UDP_IDR, 0x);
/* Clear all pending interrupts - UDP may be used by bootloader. */
at91_udp_write(udc, AT91_UDP_ICR, 0x);
- clk_disable_unprepare(udc->iclk);
+ clk_disable(udc->iclk);
/* request UDC and maybe VBUS irqs */
udc->udp_irq = platform_get_irq(pdev, 0);
@@ -1795,7 +1803,7 @@ static int at91udc_probe(struct platform_device *pdev)
0, driver_name, udc);
if (retval < 0) {
DBG("request irq %d failed\n", udc->udp_irq);
- goto fail1;
+ goto fail1c;
}
if (gpio_is_valid(udc->board.vbus_pin)) {
retval = gpio_request(udc->board.vbus_pin, "udc_vbus");
@@ -1848,6 +1856,13 @@ fail3:
gpio_free(udc->board.vbus_pin);
fail2:
free_irq(udc->udp_irq, udc);
+fail1c:
+ clk_unprepare(udc->iclk);
+fail1b:
+ clk_unprepare(udc->fclk);
+fail1a:
+ if (IS_ENABLED(CONFIG_COMMON_CLK))
+ clk_unprepare(udc->uclk);
fail1:
if (IS_ENABLED(CONFIG_COMMON_CLK) && !IS_ERR(udc->uclk))
clk_put(udc->uclk);
@@ -1896,6 +1911,11 @@ static int __exit at91udc_remove(struct platform_device
*pdev)
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
release_mem_region(res->start, resource_size(res));
+ if (IS_ENABLED(CONFIG_COMMON_CLK))
+ clk_unprepare(udc->uclk);
+ clk_unprepare(udc->fclk);
+ clk_unprepare(udc->iclk);
+
clk_put(udc->iclk);
clk_put(udc->fclk);
if (IS_ENABLED(CONFIG_COMMON_CLK))
--
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 v2] usb: gadget: at91_udc: move prepare clk into process context
Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
interrupt context. This is not allowed as it might sleep. Move clock
preparation and setting clock rate into process context (at91udc_probe).
Signed-off-by: Ronald Wahl
---
drivers/usb/gadget/udc/at91_udc.c | 44 ---
1 file changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/gadget/udc/at91_udc.c
b/drivers/usb/gadget/udc/at91_udc.c
index cfd18bc..0d685d0 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -870,12 +870,10 @@ static void clk_on(struct at91_udc *udc)
return;
udc->clocked = 1;
- if (IS_ENABLED(CONFIG_COMMON_CLK)) {
- clk_set_rate(udc->uclk, 4800);
- clk_prepare_enable(udc->uclk);
- }
- clk_prepare_enable(udc->iclk);
- clk_prepare_enable(udc->fclk);
+ if (IS_ENABLED(CONFIG_COMMON_CLK))
+ clk_enable(udc->uclk);
+ clk_enable(udc->iclk);
+ clk_enable(udc->fclk);
}
static void clk_off(struct at91_udc *udc)
@@ -884,10 +882,10 @@ static void clk_off(struct at91_udc *udc)
return;
udc->clocked = 0;
udc->gadget.speed = USB_SPEED_UNKNOWN;
- clk_disable_unprepare(udc->fclk);
- clk_disable_unprepare(udc->iclk);
+ clk_disable(udc->fclk);
+ clk_disable(udc->iclk);
if (IS_ENABLED(CONFIG_COMMON_CLK))
- clk_disable_unprepare(udc->uclk);
+ clk_disable(udc->uclk);
}
/*
@@ -1780,14 +1778,24 @@ static int at91udc_probe(struct platform_device *pdev)
}
/* don't do anything until we have both gadget driver and VBUS */
+ if (IS_ENABLED(CONFIG_COMMON_CLK)) {
+ clk_set_rate(udc->uclk, 4800);
+ retval = clk_prepare(udc->uclk);
+ if (retval)
+ goto fail1;
+ }
+ retval = clk_prepare(udc->fclk);
+ if (retval)
+ goto fail1a;
+
retval = clk_prepare_enable(udc->iclk);
if (retval)
- goto fail1;
+ goto fail1b;
at91_udp_write(udc, AT91_UDP_TXVC, AT91_UDP_TXVC_TXVDIS);
at91_udp_write(udc, AT91_UDP_IDR, 0x);
/* Clear all pending interrupts - UDP may be used by bootloader. */
at91_udp_write(udc, AT91_UDP_ICR, 0x);
- clk_disable_unprepare(udc->iclk);
+ clk_disable(udc->iclk);
/* request UDC and maybe VBUS irqs */
udc->udp_irq = platform_get_irq(pdev, 0);
@@ -1795,7 +1803,7 @@ static int at91udc_probe(struct platform_device *pdev)
0, driver_name, udc);
if (retval < 0) {
DBG("request irq %d failed\n", udc->udp_irq);
- goto fail1;
+ goto fail1c;
}
if (gpio_is_valid(udc->board.vbus_pin)) {
retval = gpio_request(udc->board.vbus_pin, "udc_vbus");
@@ -1848,6 +1856,13 @@ fail3:
gpio_free(udc->board.vbus_pin);
fail2:
free_irq(udc->udp_irq, udc);
+fail1c:
+ clk_unprepare(udc->iclk);
+fail1b:
+ clk_unprepare(udc->fclk);
+fail1a:
+ if (IS_ENABLED(CONFIG_COMMON_CLK))
+ clk_unprepare(udc->uclk);
fail1:
if (IS_ENABLED(CONFIG_COMMON_CLK) && !IS_ERR(udc->uclk))
clk_put(udc->uclk);
@@ -1896,6 +1911,11 @@ static int __exit at91udc_remove(struct platform_device
*pdev)
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
release_mem_region(res->start, resource_size(res));
+ if (IS_ENABLED(CONFIG_COMMON_CLK))
+ clk_unprepare(udc->uclk);
+ clk_unprepare(udc->fclk);
+ clk_unprepare(udc->iclk);
+
clk_put(udc->iclk);
clk_put(udc->fclk);
if (IS_ENABLED(CONFIG_COMMON_CLK))
--
1.9.3
--
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 v4 2/5] usb: host: ohci-st: Add OHCI driver support for ST STB devices
This patch adds the glue code required to ensure the on-chip OHCI
controller works on STi consumer electronics SoC's from STMicroelectronics.
It mainly manages the setting and enabling of the relevant clocks and manages
the reset / power signals to the IP block.
Signed-off-by: Peter Griffin
---
drivers/usb/host/Kconfig | 8 ++
drivers/usb/host/Makefile | 1 +
drivers/usb/host/ohci-st.c | 337 +
3 files changed, 346 insertions(+)
create mode 100644 drivers/usb/host/ohci-st.c
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 03314f8..4b577f6 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -753,6 +753,14 @@ config USB_HCD_SSB
If unsure, say N.
+config USB_HCD_ST
+ tristate "ST USB driver for ST SoC Series"
+ depends on ARCH_STI && OF
+ select GENERIC_PHY
+ help
+ Enable support for the on-chip OHCI & EHCI controller found on
+ STMicroelectronics consumer electronics SoC's.
+
config USB_HCD_TEST_MODE
bool "HCD test mode support"
---help---
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index af89a90..df7e5ac 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -74,3 +74,4 @@ obj-$(CONFIG_USB_HCD_SSB) += ssb-hcd.o
obj-$(CONFIG_USB_FUSBH200_HCD) += fusbh200-hcd.o
obj-$(CONFIG_USB_FOTG210_HCD) += fotg210-hcd.o
obj-$(CONFIG_USB_MAX3421_HCD) += max3421-hcd.o
+obj-$(CONFIG_USB_HCD_ST) += ehci-st.o ohci-st.o
diff --git a/drivers/usb/host/ohci-st.c b/drivers/usb/host/ohci-st.c
new file mode 100644
index 000..263132a
--- /dev/null
+++ b/drivers/usb/host/ohci-st.c
@@ -0,0 +1,337 @@
+/*
+ * ST OHCI driver
+ *
+ * Copyright (C) 2014 STMicroelectronics – All Rights Reserved
+ *
+ * Author: Peter Griffin
+ *
+ * Derived from ohci-platform.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "ohci.h"
+#include "usb-st-common.h"
+
+#define DRIVER_DESC "OHCI STMicroelectronics driver"
+
+#define hcd_to_ohci_priv(h) ((struct st_platform_priv *)hcd_to_ohci(h)->priv)
+
+static const char hcd_name[] = "ohci-st";
+
+static int st_ohci_platform_power_on(struct platform_device *dev)
+{
+ struct usb_hcd *hcd = platform_get_drvdata(dev);
+ struct st_platform_priv *priv = hcd_to_ohci_priv(hcd);
+ int clk, ret;
+
+ ret = reset_control_deassert(priv->pwr);
+ if (ret)
+ return ret;
+
+ ret = reset_control_deassert(priv->rst);
+ if (ret)
+ goto err_assert_power;
+
+ /* some SoCs don't have a dedicated 48Mhz clock, but those that do
+ need the rate to be explicitly set */
+ if (priv->clk48) {
+ ret = clk_set_rate(priv->clk48, 4800);
+ if (ret)
+ goto err_assert_reset;
+ }
+
+ for (clk = 0; clk < USB_MAX_CLKS && priv->clks[clk]; clk++) {
+ ret = clk_prepare_enable(priv->clks[clk]);
+ if (ret)
+ goto err_disable_clks;
+ }
+
+ ret = phy_init(priv->phy);
+ if (ret)
+ goto err_disable_clks;
+
+ ret = phy_power_on(priv->phy);
+ if (ret)
+ goto err_exit_phy;
+
+ return 0;
+
+err_exit_phy:
+ phy_exit(priv->phy);
+err_disable_clks:
+ while (--clk >= 0)
+ clk_disable_unprepare(priv->clks[clk]);
+err_assert_reset:
+ reset_control_assert(priv->rst);
+err_assert_power:
+ reset_control_assert(priv->pwr);
+
+ return ret;
+}
+
+static void st_ohci_platform_power_off(struct platform_device *dev)
+{
+ struct usb_hcd *hcd = platform_get_drvdata(dev);
+ struct st_platform_priv *priv = hcd_to_ohci_priv(hcd);
+
+ int clk;
+
+ reset_control_assert(priv->pwr);
+
+ reset_control_assert(priv->rst);
+
+ phy_power_off(priv->phy);
+
+ phy_exit(priv->phy);
+
+ for (clk = USB_MAX_CLKS - 1; clk >= 0; clk--)
+ if (priv->clks[clk])
+ clk_disable_unprepare(priv->clks[clk]);
+}
+
+static struct hc_driver __read_mostly ohci_platform_hc_driver;
+
+static const struct ohci_driver_overrides platform_overrides __initconst = {
+ .product_desc = "ST OHCI controller",
+ .extra_priv_size = sizeof(struct st_platform_priv),
+};
+
+static struct usb_ohci_pdata ohci_platform_defaults = {
+ .power_on = st_ohci_platform_power_on,
+ .power_suspend =st_ohci_platform_power_off,
+ .power_off =st_ohci_platform_power_off,
+};
+
+static int st_ohci_platform_probe(struct platform_device *dev)
+{
+ st
[PATCH v4 3/5] usb: host: ehci-st: Add ehci-st devicetree bindings documentation
This patch documents the device tree bindings required for the
ehci on-chip controller found in ST consumer electronics SoC's.
Signed-off-by: Peter Griffin
---
Documentation/devicetree/bindings/usb/ehci-st.txt | 39 +++
1 file changed, 39 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ehci-st.txt
diff --git a/Documentation/devicetree/bindings/usb/ehci-st.txt
b/Documentation/devicetree/bindings/usb/ehci-st.txt
new file mode 100644
index 000..fb45fa5
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ehci-st.txt
@@ -0,0 +1,39 @@
+ST USB EHCI controller
+
+Required properties:
+ - compatible : must be "st,st-ehci-300x"
+ - reg : physical base addresses of the controller and length
of memory mapped
+ region
+ - interrupts : one EHCI interrupt should be described here
+ - pinctrl-names : a pinctrl state named "default" must be defined
+ - pinctrl-0 : phandle referencing pin configuration of the USB
controller
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+ - clocks : phandle list of usb clocks
+ - clock-names : should be "ic" for interconnect clock and "clk48"
+See: Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+ - phys: phandle for the PHY device
+ - phy-names : should be "usb"
+ - resets : phandle + reset specifier pairs to the powerdown and
softreset lines
+ of the USB IP
+ - reset-names : should be "power" and "softreset"
+See: Documentation/devicetree/bindings/reset/st,sti-powerdown.txt
+See: Documentation/devicetree/bindings/reset/reset.txt
+
+Example:
+
+ ehci1: usb@0xfe203e00 {
+ compatible = "st,st-ehci-300x";
+ reg = <0xfe203e00 0x100>;
+ interrupts = ;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb1>;
+ clocks = <&clk_s_a1_ls 0>;
+ phys = <&usb2_phy>;
+ phy-names = "usb";
+ status = "okay";
+
+ resets = <&powerdown STIH416_USB1_POWERDOWN>,
+<&softreset STIH416_USB1_SOFTRESET>;
+ reset-names = "power", "softreset";
+ };
--
1.9.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: [Bug 80711] [PATCH]SG_FLAG_LUN_INHIBIT is no longer implemented and there's not way to prevent the kernel from using the 2nd cdb byte for the LUN
On Wed, 6 Aug 2014, Christoph Hellwig wrote: > On Wed, Aug 06, 2014 at 04:02:22PM -0400, Alan Stern wrote: > > > I doubt either of them forces users to hack up flags for these cases. > > > > Why was this change needed in the first place? There's no explanation > > in the patch itself. > > Which chance? The one to not support SG_FLAG_LUN_INHIBIT? No, the patch that started this Bugzilla entry. Tiziano says it is needed in order to send vendor-specific commands that use the LUN bits in CDB[1]. > > > At least for windows I suspect it just never sends the LUN encoded > > > in the CDB and treats USB devices special instead of our insistance > > > on pretending they are SCSI-2. > > > > We no longer pretend that USB mass-storage devices have any particular > > SCSI level. See commit 09b6b51b0b6c. > > So the origina reported device must report SCSI-2 all by itself if he's > running a recent kernel, ok. > > > > Maybe some of the USB people have on the wire traces or access to > > > device or windows documentation on this? > > > > Most likely it varies with the version of Windows and the INQUIRY data > > returned by the device. > > > > I can obtain hardware traces for the kinds of devices and computers > > lying around here. But what sort of combinations should I test? > > I'd mostly be interested to see if it actualy encodes the LUN in the CDB > for any USB multi-LUN device. I tried connecting a Linux mass-storage gadget with two logical units to a host PC running Windows 7. The host scanned the first logical unit and completely ignored the second! Didn't even send an INQUIRY command. So the question remains unanswered... Can someone tell me if anything special is needed to make Windows recognize logical units beyond the first? Alan Stern -- 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] staging: usbip: vhci_sysfs.c: Fix a style issue
Hello. On 08/07/2014 10:22 AM, Lars R. Damerow wrote: Replace a single-value sscanf() call with a call to kstrtou32(), as recommended by checkpatch.pl. Signed-off-by: Lars R. Damerow --- drivers/staging/usbip/vhci_sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/usbip/vhci_sysfs.c b/drivers/staging/usbip/vhci_sysfs.c index 211f43f..1dfbfa0 100644 --- a/drivers/staging/usbip/vhci_sysfs.c +++ b/drivers/staging/usbip/vhci_sysfs.c @@ -114,7 +114,7 @@ static ssize_t store_detach(struct device *dev, struct device_attribute *attr, int err; __u32 rhport = 0; - if (sscanf(buf, "%u", &rhport) != 1) + if (kstrtou32(buf, 10, &rhport) != 1) Looks like you didn't bother studying what result this function returns... hint: it returns 0 or negative value. WBR, Sergei -- 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] usb: gadget: at91_udc: move prepare clk into process context
Hello. On 08/07/2014 07:11 PM, Ronald Wahl wrote: Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in Please also specify that commit's summary in parens. interrupt context. This is not allowed as it might sleep. Move clock preparation and setting clock rate into process context (at91udc_probe). Signed-off-by: Ronald Wahl WBR, Sergei -- 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] staging: usbip: vhci_sysfs.c: Fix a style issue
Oops--you're right, I didn't check. I'll correct the patch and resubmit. Sorry about that, -lars -- Lars R. Damerow l...@grandstreet.us On Thu, Aug 7, 2014, at 09:08 AM, Sergei Shtylyov wrote: > Hello. > > On 08/07/2014 10:22 AM, Lars R. Damerow wrote: > > > Replace a single-value sscanf() call with a call to kstrtou32(), as > > recommended by checkpatch.pl. > > > Signed-off-by: Lars R. Damerow > > --- > > drivers/staging/usbip/vhci_sysfs.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > diff --git a/drivers/staging/usbip/vhci_sysfs.c > > b/drivers/staging/usbip/vhci_sysfs.c > > index 211f43f..1dfbfa0 100644 > > --- a/drivers/staging/usbip/vhci_sysfs.c > > +++ b/drivers/staging/usbip/vhci_sysfs.c > > @@ -114,7 +114,7 @@ static ssize_t store_detach(struct device *dev, struct > > device_attribute *attr, > > int err; > > __u32 rhport = 0; > > > > - if (sscanf(buf, "%u", &rhport) != 1) > > + if (kstrtou32(buf, 10, &rhport) != 1) > > Looks like you didn't bother studying what result this function > returns... > hint: it returns 0 or negative value. > > WBR, Sergei > -- 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 resend] staging: usbip: vhci_sysfs.c: Fix a style issue
Replace a single-value sscanf() call with a call to kstrtou32(), as recommended by checkpatch.pl. Signed-off-by: Lars R. Damerow --- drivers/staging/usbip/vhci_sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/usbip/vhci_sysfs.c b/drivers/staging/usbip/vhci_sysfs.c index 211f43f..e8f680a 100644 --- a/drivers/staging/usbip/vhci_sysfs.c +++ b/drivers/staging/usbip/vhci_sysfs.c @@ -114,7 +114,7 @@ static ssize_t store_detach(struct device *dev, struct device_attribute *attr, int err; __u32 rhport = 0; - if (sscanf(buf, "%u", &rhport) != 1) + if (kstrtou32(buf, 10, &rhport) < 0) return -EINVAL; /* check rhport */ -- 1.9.3 -- 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 v4 2/4] usb: dwc2: add compatible data for rockchip soc
> From: Kever Yang [mailto:kever.y...@gmail.com] On Behalf Of Kever Yang > Sent: Thursday, August 07, 2014 2:35 AM > > This patch add compatible data for dwc2 controller found on > rk3066, rk3188 and rk3288 processors from rockchip. > > Signed-off-by: Kever Yang > Acked-by: Paul Zimmerman > --- > > Changes in v4: > - max_transfer_size change to 65536, this should be enough > for most transfer, the hardware auto-detect will set this > to 0x7 which may make dma_alloc_coherent fail when > non-dword aligned buf from driver like usbnet happen. Hi Kever, Did you test this change thoroughly? I have vague memories of any value above 65535 causing problems, at least on my hardware. And I see it is set to 65535 in both pci.c and platform.c. I could be wrong, but I thought I should mention it. -- Paul -- 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 resend] staging: usbip: vhci_sysfs.c: Fix a style issue
On 08/07/2014 09:38 PM, Lars R. Damerow wrote: Concerning the subject: it's not resend, it's version 2. Replace a single-value sscanf() call with a call to kstrtou32(), as recommended by checkpatch.pl. Signed-off-by: Lars R. Damerow --- You're supposed to describe the differences between patch versions here. drivers/staging/usbip/vhci_sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/usbip/vhci_sysfs.c b/drivers/staging/usbip/vhci_sysfs.c index 211f43f..e8f680a 100644 --- a/drivers/staging/usbip/vhci_sysfs.c +++ b/drivers/staging/usbip/vhci_sysfs.c @@ -114,7 +114,7 @@ static ssize_t store_detach(struct device *dev, struct device_attribute *attr, int err; __u32 rhport = 0; - if (sscanf(buf, "%u", &rhport) != 1) + if (kstrtou32(buf, 10, &rhport) < 0) return -EINVAL; It's probably better to propagate error from kstrtou32(). WBR, Sergei -- 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: Resend Re: [PATCH v6] usb:serial:pl2303: add GPIOs interface on PL2303
On Tue, Aug 05, 2014 at 03:54:08PM +0200, Johan Hovold wrote: > > > I noticed that setting direction to output and setting the gpio high has > > > no effect on the read-back value (i.e. I still read back 0) for my > > > pl2303hx (note that my device has no easily accessible gpios so I > > > haven't verified the actual state of the output pin). > > > > > > What happens on your system? Is the read-back value still 0, even when > > > the GPIO output is actually high? Should we return the cached value in > > > this case? > > > > If i set direction to output, then I could control gpio high and low > > by set 1 or 0, and the read-back value is 1 or 0 according to high and > > low(I test high and low by oscillscope) > > > > I test it with my pl2303hx with only two gpios. > > > > Could you use usbmon to see whether the traffic is right according > > to comment in struct pl2303_gpio? > > The traffic appears correct judging from the debug output (which I > trust). Output-enable is reflected in register 0x81, but the value > isn't. > > What is the lsusb -v output for your device? Bus 001 Device 005: ID 067b:2303 Prolific Technology, Inc. PL2303 Serial Port. It is strange your device doesn't work, I verify the control method by analyze usbmon output from linux host which has VirtualBox running gpio test program, but I don't have right to distribute the gpio test program I think, so I can't help you to figure out why it doesn't work for your device. > I suggest you just set the label to pl2303 until we have a valid > use-case that requires something more elaborate. Ok, but pl2303-gpio maybe a better name? Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] usbip: move usbip out of staging
On Wed, Aug 6, 2014 at 10:33 PM, Greg KH wrote: > On Thu, Aug 07, 2014 at 08:10:25AM +0300, Valentina Manea wrote: >> This is a resend of the patch series from March. >> >> After migrating userspace code to libudev, converting usbip-host >> to a device driver and various bug fixes and enhancements, USB/IP >> is fully functional and can be moved out of staging. >> >> This patch series moves it as following: >> * userspace code to tools/usb/usbip >> * kernel code to drivers/usb/usbip >> >> Besides this, a warning generated in the kernel code is solved. > > What kernel version is this against? > I rebased my tree against master in the staging tree. > And can we get a maintainer for this code? I don't want to see it sit > "ownerless" in the kernel tree. Will you be willing to do this? > Yes. But I should be briefed about that responsibilities will this involve. Thanks, Valentina -- 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: dwc2: Read GNPTXFSIZ when in forced HOST mode.
The documentation for GNPTXFSIZ says that "For host mode, this field is always valid." Since we're already switching to host mode for HPTXFSIZ, let's also read GNPTXFSIZ in host mode. On an rk3288 SoC, without this change we see this at bootup: dwc2 ff58.usb: gnptxfsiz=00100400 dwc2 ff58.usb: 128 invalid for host_nperio_tx_fifo_size. Check HW configuration. After this change we see: dwc2 ff58.usb: gnptxfsiz=04000400 Signed-off-by: Doug Anderson --- drivers/usb/dwc2/core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c index 27d2c9b..c184ed43 100644 --- a/drivers/usb/dwc2/core.c +++ b/drivers/usb/dwc2/core.c @@ -2674,23 +2674,23 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg) hwcfg2 = readl(hsotg->regs + GHWCFG2); hwcfg3 = readl(hsotg->regs + GHWCFG3); hwcfg4 = readl(hsotg->regs + GHWCFG4); - gnptxfsiz = readl(hsotg->regs + GNPTXFSIZ); grxfsiz = readl(hsotg->regs + GRXFSIZ); dev_dbg(hsotg->dev, "hwcfg1=%08x\n", hwcfg1); dev_dbg(hsotg->dev, "hwcfg2=%08x\n", hwcfg2); dev_dbg(hsotg->dev, "hwcfg3=%08x\n", hwcfg3); dev_dbg(hsotg->dev, "hwcfg4=%08x\n", hwcfg4); - dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz); dev_dbg(hsotg->dev, "grxfsiz=%08x\n", grxfsiz); - /* Force host mode to get HPTXFSIZ exact power on value */ + /* Force host mode to get HPTXFSIZ / GNPTXFSIZ exact power on value */ gusbcfg = readl(hsotg->regs + GUSBCFG); gusbcfg |= GUSBCFG_FORCEHOSTMODE; writel(gusbcfg, hsotg->regs + GUSBCFG); usleep_range(10, 15); + gnptxfsiz = readl(hsotg->regs + GNPTXFSIZ); hptxfsiz = readl(hsotg->regs + HPTXFSIZ); + dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz); dev_dbg(hsotg->dev, "hptxfsiz=%08x\n", hptxfsiz); gusbcfg = readl(hsotg->regs + GUSBCFG); gusbcfg &= ~GUSBCFG_FORCEHOSTMODE; -- 2.0.0.526.g5318336 -- 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 0/3] usbip: move usbip out of staging
On Thu, Aug 7, 2014 at 1:36 PM, Valentina Manea wrote: > On Wed, Aug 6, 2014 at 10:33 PM, Greg KH wrote: >> On Thu, Aug 07, 2014 at 08:10:25AM +0300, Valentina Manea wrote: >>> This is a resend of the patch series from March. >>> >>> After migrating userspace code to libudev, converting usbip-host >>> to a device driver and various bug fixes and enhancements, USB/IP >>> is fully functional and can be moved out of staging. >>> >>> This patch series moves it as following: >>> * userspace code to tools/usb/usbip >>> * kernel code to drivers/usb/usbip >>> >>> Besides this, a warning generated in the kernel code is solved. >> >> What kernel version is this against? >> > > I rebased my tree against master in the staging tree. > >> And can we get a maintainer for this code? I don't want to see it sit >> "ownerless" in the kernel tree. Will you be willing to do this? >> > > Yes. But I should be briefed about that responsibilities will this involve. > If you would like a back-up, I can volunteer to be co-maintainer for this driver. -- Shuah -- 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 0/3] usbip: move usbip out of staging
On Thu, Aug 07, 2014 at 12:36:06PM -0700, Valentina Manea wrote: > On Wed, Aug 6, 2014 at 10:33 PM, Greg KH wrote: > > On Thu, Aug 07, 2014 at 08:10:25AM +0300, Valentina Manea wrote: > >> This is a resend of the patch series from March. > >> > >> After migrating userspace code to libudev, converting usbip-host > >> to a device driver and various bug fixes and enhancements, USB/IP > >> is fully functional and can be moved out of staging. > >> > >> This patch series moves it as following: > >> * userspace code to tools/usb/usbip > >> * kernel code to drivers/usb/usbip > >> > >> Besides this, a warning generated in the kernel code is solved. > > > > What kernel version is this against? > > > > I rebased my tree against master in the staging tree. Great, thanks. > > And can we get a maintainer for this code? I don't want to see it sit > > "ownerless" in the kernel tree. Will you be willing to do this? > > > > Yes. But I should be briefed about that responsibilities will this involve. The top of the MAINTAINERS should have this information, look in the S: Status section about what type of category you are in. Basically I'd like you to be the one to handle patches that are sent in for the code, just by reviewing them, you don't have to send them on to me if you don't want to (some usb subsystem maintainers do do this, like for usb-serial and xhci), it's up to you. Also, any bug reports or questions about the code would come to you first, along with the rest of the linux-usb@vger developers, you aren't alone in this at all. Does that help explain things better? greg k-h -- 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: dwc2: Read GNPTXFSIZ when in forced HOST mode.
> From: Doug Anderson [mailto:diand...@chromium.org] > Sent: Thursday, August 07, 2014 12:48 PM > > The documentation for GNPTXFSIZ says that "For host mode, this field > is always valid." Since we're already switching to host mode for > HPTXFSIZ, let's also read GNPTXFSIZ in host mode. > > On an rk3288 SoC, without this change we see this at bootup: > dwc2 ff58.usb: gnptxfsiz=00100400 > dwc2 ff58.usb: 128 invalid for host_nperio_tx_fifo_size. Check HW > configuration. > > After this change we see: > dwc2 ff58.usb: gnptxfsiz=04000400 > > Signed-off-by: Doug Anderson > --- > drivers/usb/dwc2/core.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c > index 27d2c9b..c184ed43 100644 > --- a/drivers/usb/dwc2/core.c > +++ b/drivers/usb/dwc2/core.c > @@ -2674,23 +2674,23 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg) > hwcfg2 = readl(hsotg->regs + GHWCFG2); > hwcfg3 = readl(hsotg->regs + GHWCFG3); > hwcfg4 = readl(hsotg->regs + GHWCFG4); > - gnptxfsiz = readl(hsotg->regs + GNPTXFSIZ); > grxfsiz = readl(hsotg->regs + GRXFSIZ); > > dev_dbg(hsotg->dev, "hwcfg1=%08x\n", hwcfg1); > dev_dbg(hsotg->dev, "hwcfg2=%08x\n", hwcfg2); > dev_dbg(hsotg->dev, "hwcfg3=%08x\n", hwcfg3); > dev_dbg(hsotg->dev, "hwcfg4=%08x\n", hwcfg4); > - dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz); > dev_dbg(hsotg->dev, "grxfsiz=%08x\n", grxfsiz); > > - /* Force host mode to get HPTXFSIZ exact power on value */ > + /* Force host mode to get HPTXFSIZ / GNPTXFSIZ exact power on value */ > gusbcfg = readl(hsotg->regs + GUSBCFG); > gusbcfg |= GUSBCFG_FORCEHOSTMODE; > writel(gusbcfg, hsotg->regs + GUSBCFG); > usleep_range(10, 15); > > + gnptxfsiz = readl(hsotg->regs + GNPTXFSIZ); > hptxfsiz = readl(hsotg->regs + HPTXFSIZ); > + dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz); > dev_dbg(hsotg->dev, "hptxfsiz=%08x\n", hptxfsiz); > gusbcfg = readl(hsotg->regs + GUSBCFG); > gusbcfg &= ~GUSBCFG_FORCEHOSTMODE; Nice! I wonder if this is a bug in the original driver, and they actually meant to read this register instead of HPTXFSIZ? Well, it doesn't really matter I guess. Acked-by: Paul Zimmerman You may want to resend this to Greg after -rc1 is out and he reopens his usb-next tree. -- Paul -- 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] uwb: init beacon cache entry before registering uwb device
Make sure the uwb_dev->bce entry is set before calling uwb_dev_add in
uwbd_dev_onair so that usermode will only see the device after it is
properly initialized. This fixes a kernel panic that can occur if
usermode tries to access the IEs sysfs attribute of a UWB device before
the driver has had a chance to set the beacon cache entry.
Signed-off-by: Thomas Pugliese
Cc: sta...@vger.kernel.org
---
drivers/uwb/lc-dev.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/uwb/lc-dev.c b/drivers/uwb/lc-dev.c
index 80079b8..d0303f0 100644
--- a/drivers/uwb/lc-dev.c
+++ b/drivers/uwb/lc-dev.c
@@ -431,16 +431,19 @@ void uwbd_dev_onair(struct uwb_rc *rc, struct uwb_beca_e
*bce)
uwb_dev->mac_addr = *bce->mac_addr;
uwb_dev->dev_addr = bce->dev_addr;
dev_set_name(&uwb_dev->dev, "%s", macbuf);
+
+ /* plug the beacon cache */
+ bce->uwb_dev = uwb_dev;
+ uwb_dev->bce = bce;
+ uwb_bce_get(bce); /* released in uwb_dev_sys_release() */
+
result = uwb_dev_add(uwb_dev, &rc->uwb_dev.dev, rc);
if (result < 0) {
dev_err(dev, "new device %s: cannot instantiate device\n",
macbuf);
goto error_dev_add;
}
- /* plug the beacon cache */
- bce->uwb_dev = uwb_dev;
- uwb_dev->bce = bce;
- uwb_bce_get(bce); /* released in uwb_dev_sys_release() */
+
dev_info(dev, "uwb device (mac %s dev %s) connected to %s %s\n",
macbuf, devbuf, rc->uwb_dev.dev.parent->bus->name,
dev_name(rc->uwb_dev.dev.parent));
@@ -448,6 +451,8 @@ void uwbd_dev_onair(struct uwb_rc *rc, struct uwb_beca_e
*bce)
return;
error_dev_add:
+ bce->uwb_dev = NULL;
+ uwb_bce_put(bce);
kfree(uwb_dev);
return;
}
--
1.8.3.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
Re: [PATCH v4 2/4] usb: dwc2: add compatible data for rockchip soc
Paul, On Thu, Aug 7, 2014 at 11:26 AM, Paul Zimmerman wrote: >> From: Kever Yang [mailto:kever.y...@gmail.com] On Behalf Of Kever Yang >> Sent: Thursday, August 07, 2014 2:35 AM >> >> This patch add compatible data for dwc2 controller found on >> rk3066, rk3188 and rk3288 processors from rockchip. >> >> Signed-off-by: Kever Yang >> Acked-by: Paul Zimmerman >> --- >> >> Changes in v4: >> - max_transfer_size change to 65536, this should be enough >> for most transfer, the hardware auto-detect will set this >> to 0x7 which may make dma_alloc_coherent fail when >> non-dword aligned buf from driver like usbnet happen. > > Hi Kever, > > Did you test this change thoroughly? I have vague memories of any > value above 65535 causing problems, at least on my hardware. And I > see it is set to 65535 in both pci.c and platform.c. I could be > wrong, but I thought I should mention it. Certainly it is documented in the header file to have a max of 65535: * @max_transfer_size: The maximum transfer size supported, in bytes * 2047 to 65,535 * Actual maximum value is autodetected and also * the default. ...but looking at the register definition that I see, the size can be up to 19 bits. A 19-bit transfer far exceeds 65535. Do you remember what the error was? Certainly I can imagine there being errors with large calls to dma_alloc_coherent()... I know that with Kever's change I can do USB Ethernet downloads, so it is at least working to some degree. ...to me it feels like Kever should resubmit with 65535 (to match the documentation) and then work in the background to figure out what the max_transfer_size really ought to be. -Doug -- 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: [PATCHv2 01/13] usb: dwc2: Update Kconfig to support dual-role
> From: Dinh Nguyen [mailto:dinh.li...@gmail.com] > Sent: Thursday, August 07, 2014 5:12 AM > > On 8/1/14, 4:41 PM, Dinh Nguyen wrote: > > On Fri, 2014-08-01 at 20:42 +, Paul Zimmerman wrote: > >>> From: linux-usb-ow...@vger.kernel.org > >>> [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of dingu...@altera.com > >>> Sent: Wednesday, July 30, 2014 8:21 AM ... > >>> config USB_DWC2_PERIPHERAL > >>> - tristate "Gadget only mode" > >>> + bool "Gadget only mode" > >>> depends on USB_GADGET > >>> help > >>> The Designware USB2.0 high-speed gadget controller > >>> - integrated into many SoCs. > >>> + integrated into many SoCs. Select this option if you want the > >>> + driver to operate in Peripheral-only mode. > >>> + > >>> +config USB_DWC2_DUAL_ROLE > >>> + bool "Dual Role mode" > >>> + depends on ((USB=y || USB=USB_DWC2) && (USB_GADGET=y)) > >> > >> Hi Dinh, > >> > >> I just noticed that for dual-role mode, you are not allowing USB_GADGET > >> to be modular. Is there a reason for that? If so, please mention it in > >> the commit message. It should also be explained in the help text. Or > >> maybe add another comment line saying "Dual-role mode requires USB Gadget > >> = y" or something like that. > >> > > > > I think it was an oversight on my part and there's not reason why > > USB_GADGET can't be modular. > > > > I went back to look this for v3 and it appears that I need USB_GADGET=y > to avoid a build error when building the new driver for Gadget or Dual-role. > > drivers/built-in.o: In function `dwc2_gadget_init': > /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3516: > undefined reference to `usb_add_gadget_udc' > drivers/built-in.o: In function `s3c_hsotg_remove': > /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3543: > undefined reference to `usb_del_gadget_udc' > /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3549: > undefined reference to `usb_gadget_unregister_driver' I don't see why that shouldn't work. usb_add_gadget_udc is defined in udc-core.c, which gets built if CONFIG_USB_GADGET is set. What Kconfig line did you use when you got this build error? I think it should be: config USB_DWC2_DUAL_ROLE bool "Dual Role mode" depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || USB_GADGET=USB_DWC2) -- Paul
[PATCH 0/4] fix error return code
The complate semantic patch that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
// identify a function that returns a negative return value at least once.
@ok exists@
identifier f,ret,i;
expression e;
constant c;
@@
f(...) {
... when any
(
return -c@i;
|
ret = -c@i;
... when != ret = e
return ret;
|
if (ret < 0) { ... return ret; }
)
... when any
}
// identify a case where the return variable is set to a non-negative value
// and then returned in error-handling code
@r exists@
identifier ret,ok.f,fn;
expression e1,e2,e3,e4,e5,e6,x;
statement S,S1;
position p1,p2,p3;
@@
f(...) {
... when any
(
if@p1 (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\)
when != &ret
when any
(
if (<+... ret = e5 ...+>) S1
|
if (<+... &ret ...+>) S1
|
if@p2(<+...x = fn(...)...+>)
{
... when != ret = e6
when forall
return@p3 ret;
}
|
break;
|
x = fn(...)
... when != \(ret = e4\|ret++\|ret--\|ret+=e4\|ret-=e4\)
when != &ret
(
if (<+... ret = e3 ...+>) S
|
if (<+... &ret ...+>) S
|
if@p2(<+...\(x != 0\|x < 0\|x == NULL\|IS_ERR(x)\)...+>)
{
... when != ret = e2
when forall
return@p3 ret;
}
)
)
... when any
}
@printer depends on r@
position p;
identifier ok.f,pr;
constant char [] c;
@@
f(...) { <...pr@p(...,c,...)...> }
@bad0 exists@
identifier r.ret,ok.f,g != {ERR_PTR,IS_ERR};
position p != printer.p;
@@
f(...) { ... when any
g@p(...,ret,...)
... when any
}
// ignore the above if there is some path where the variable is set to
// something else
@bad depends on !bad0 exists@
position r.p1,r.p2;
statement S1,S2;
identifier r.ret;
expression e1;
@@
(
if@p1 (\(ret < 0\|ret != 0\)) S1
|
ret@p1 = 0
)
... when any
\(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\|&ret\)
... when any
if@p2(...) S2
@bad1 depends on !bad0 && !bad exists@
position r.p2;
statement S2;
identifier r.ret;
expression e1;
constant c;
@@
ret = -c
... when != \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\)
when != &ret
when any
if@p2(...) S2
// likewise ignore it if there has been an intervening return
@bad2 depends on !bad0 && !bad && !bad1 exists@
position r.p1,r.p2;
identifier r.ret;
expression e1;
statement S2;
constant c;
@@
ret@p1 = 0
... when != if (...) { ... ret = e1 ... return ret; }
when != if (...) { ... return -c; }
when any
if@p2(...) S2
@script:python depends on !bad0 && !bad && !bad1 && !bad2@
p1 << r.p1;
p2 << r.p2;
p3 << r.p3;
@@
cocci.print_main("",p1)
cocci.print_secs("",p2)
cocci.print_secs("",p3)
//
--
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: [PATCHv2 01/13] usb: dwc2: Update Kconfig to support dual-role
On Thu, Aug 7, 2014 at 3:55 PM, Paul Zimmerman wrote: >> From: Dinh Nguyen [mailto:dinh.li...@gmail.com] >> Sent: Thursday, August 07, 2014 5:12 AM >> >> On 8/1/14, 4:41 PM, Dinh Nguyen wrote: >> > On Fri, 2014-08-01 at 20:42 +, Paul Zimmerman wrote: >> >>> From: linux-usb-ow...@vger.kernel.org >> >>> [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of dingu...@altera.com >> >>> Sent: Wednesday, July 30, 2014 8:21 AM > > ... > >> >>> config USB_DWC2_PERIPHERAL >> >>> - tristate "Gadget only mode" >> >>> + bool "Gadget only mode" >> >>> depends on USB_GADGET >> >>> help >> >>> The Designware USB2.0 high-speed gadget controller >> >>> - integrated into many SoCs. >> >>> + integrated into many SoCs. Select this option if you want the >> >>> + driver to operate in Peripheral-only mode. >> >>> + >> >>> +config USB_DWC2_DUAL_ROLE >> >>> + bool "Dual Role mode" >> >>> + depends on ((USB=y || USB=USB_DWC2) && (USB_GADGET=y)) >> >> >> >> Hi Dinh, >> >> >> >> I just noticed that for dual-role mode, you are not allowing USB_GADGET >> >> to be modular. Is there a reason for that? If so, please mention it in >> >> the commit message. It should also be explained in the help text. Or >> >> maybe add another comment line saying "Dual-role mode requires USB Gadget >> >> = y" or something like that. >> >> >> > >> > I think it was an oversight on my part and there's not reason why >> > USB_GADGET can't be modular. >> > >> >> I went back to look this for v3 and it appears that I need USB_GADGET=y >> to avoid a build error when building the new driver for Gadget or Dual-role. >> >> drivers/built-in.o: In function `dwc2_gadget_init': >> /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3516: >> undefined reference to `usb_add_gadget_udc' >> drivers/built-in.o: In function `s3c_hsotg_remove': >> /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3543: >> undefined reference to `usb_del_gadget_udc' >> /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3549: >> undefined reference to `usb_gadget_unregister_driver' > > I don't see why that shouldn't work. usb_add_gadget_udc is defined in > udc-core.c, which gets built if CONFIG_USB_GADGET is set. What Kconfig > line did you use when you got this build error? I think it should be: > > config USB_DWC2_DUAL_ROLE > bool "Dual Role mode" > depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || > USB_GADGET=USB_DWC2) > Right, I think your original comment was why I had "USB_GADGET=y" as a dependency for DWC2_DUAL_ROLE. If I replace USB_GADGET=y with just USB_GADGET, and make USB_GADGET a module. This is fine if I build DWC as module, but If I build DWC2 into the kernel, then the driver cannot link those gadget functions in. Dinh -- 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/4] usb: gadget: fix error return code
From: Julia Lawall
Convert a zero return value on error to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@@
identifier ret; expression e1,e2;
@@
(
if (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
//
Signed-off-by: Julia Lawall
---
drivers/usb/gadget/udc/fusb300_udc.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/udc/fusb300_udc.c
b/drivers/usb/gadget/udc/fusb300_udc.c
index d40255f..5c5d1ad 100644
--- a/drivers/usb/gadget/udc/fusb300_udc.c
+++ b/drivers/usb/gadget/udc/fusb300_udc.c
@@ -1398,13 +1398,17 @@ static int fusb300_probe(struct platform_device *pdev)
/* initialize udc */
fusb300 = kzalloc(sizeof(struct fusb300), GFP_KERNEL);
- if (fusb300 == NULL)
+ if (fusb300 == NULL) {
+ ret = -ENOMEM;
goto clean_up;
+ }
for (i = 0; i < FUSB300_MAX_NUM_EP; i++) {
_ep[i] = kzalloc(sizeof(struct fusb300_ep), GFP_KERNEL);
- if (_ep[i] == NULL)
+ if (_ep[i] == NULL) {
+ ret = -ENOMEM;
goto clean_up;
+ }
fusb300->ep[i] = _ep[i];
}
--
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 v4 2/4] usb: dwc2: add compatible data for rockchip soc
> From: diand...@google.com [mailto:diand...@google.com] On Behalf Of Doug > Anderson > Sent: Thursday, August 07, 2014 1:53 PM > > On Thu, Aug 7, 2014 at 11:26 AM, Paul Zimmerman > wrote: > >> From: Kever Yang [mailto:kever.y...@gmail.com] On Behalf Of Kever Yang > >> Sent: Thursday, August 07, 2014 2:35 AM > >> > >> This patch add compatible data for dwc2 controller found on > >> rk3066, rk3188 and rk3288 processors from rockchip. > >> > >> Signed-off-by: Kever Yang > >> Acked-by: Paul Zimmerman > >> --- > >> > >> Changes in v4: > >> - max_transfer_size change to 65536, this should be enough > >> for most transfer, the hardware auto-detect will set this > >> to 0x7 which may make dma_alloc_coherent fail when > >> non-dword aligned buf from driver like usbnet happen. > > > > Hi Kever, > > > > Did you test this change thoroughly? I have vague memories of any > > value above 65535 causing problems, at least on my hardware. And I > > see it is set to 65535 in both pci.c and platform.c. I could be > > wrong, but I thought I should mention it. > > Certainly it is documented in the header file to have a max of 65535: > > * @max_transfer_size: The maximum transfer size supported, in bytes > * 2047 to 65,535 > * Actual maximum value is autodetected and also > * the default. > > ...but looking at the register definition that I see, the size can be > up to 19 bits. A 19-bit transfer far exceeds 65535. Do you remember > what the error was? Certainly I can imagine there being errors with > large calls to dma_alloc_coherent()... It's pretty fuzzy. I think a certain type of transfer (Isoc?) didn't work. But I may be misremembering, the problem could have been with max_packet_count > 255 instead. If you have tested it thoroughly with different types of devices then it's probably OK. -- Paul
RE: [PATCHv2 01/13] usb: dwc2: Update Kconfig to support dual-role
> From: Dinh Nguyen [mailto:dinh.li...@gmail.com] > Sent: Thursday, August 07, 2014 2:01 PM > > On Thu, Aug 7, 2014 at 3:55 PM, Paul Zimmerman > wrote: > >> From: Dinh Nguyen [mailto:dinh.li...@gmail.com] > >> Sent: Thursday, August 07, 2014 5:12 AM > >> > >> On 8/1/14, 4:41 PM, Dinh Nguyen wrote: > >> > On Fri, 2014-08-01 at 20:42 +, Paul Zimmerman wrote: > >> >>> From: linux-usb-ow...@vger.kernel.org > >> >>> [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of > dingu...@altera.com > >> >>> Sent: Wednesday, July 30, 2014 8:21 AM > > > > ... > > > >> >>> config USB_DWC2_PERIPHERAL > >> >>> - tristate "Gadget only mode" > >> >>> + bool "Gadget only mode" > >> >>> depends on USB_GADGET > >> >>> help > >> >>> The Designware USB2.0 high-speed gadget controller > >> >>> - integrated into many SoCs. > >> >>> + integrated into many SoCs. Select this option if you want the > >> >>> + driver to operate in Peripheral-only mode. > >> >>> + > >> >>> +config USB_DWC2_DUAL_ROLE > >> >>> + bool "Dual Role mode" > >> >>> + depends on ((USB=y || USB=USB_DWC2) && (USB_GADGET=y)) > >> >> > >> >> Hi Dinh, > >> >> > >> >> I just noticed that for dual-role mode, you are not allowing USB_GADGET > >> >> to be modular. Is there a reason for that? If so, please mention it in > >> >> the commit message. It should also be explained in the help text. Or > >> >> maybe add another comment line saying "Dual-role mode requires USB > >> >> Gadget > >> >> = y" or something like that. > >> >> > >> > > >> > I think it was an oversight on my part and there's not reason why > >> > USB_GADGET can't be modular. > >> > > >> > >> I went back to look this for v3 and it appears that I need USB_GADGET=y > >> to avoid a build error when building the new driver for Gadget or > >> Dual-role. > >> > >> drivers/built-in.o: In function `dwc2_gadget_init': > >> /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3516: > >> undefined reference to `usb_add_gadget_udc' > >> drivers/built-in.o: In function `s3c_hsotg_remove': > >> /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3543: > >> undefined reference to `usb_del_gadget_udc' > >> /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3549: > >> undefined reference to `usb_gadget_unregister_driver' > > > > I don't see why that shouldn't work. usb_add_gadget_udc is defined in > > udc-core.c, which gets built if CONFIG_USB_GADGET is set. What Kconfig > > line did you use when you got this build error? I think it should be: > > > > config USB_DWC2_DUAL_ROLE > > bool "Dual Role mode" > > depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || > > USB_GADGET=USB_DWC2) > > > > Right, I think your original comment was why I had "USB_GADGET=y" as a > dependency for DWC2_DUAL_ROLE. If I replace USB_GADGET=y with just > USB_GADGET, and make USB_GADGET a module. This is fine if I build DWC > as module, but If I build DWC2 into the kernel, then the driver cannot > link those gadget functions in. I don't think just USB_GADGET will work. Please try it with the line I showed above. -- Paul
Re: [PATCH 0/4] USB: serial: remove zte_ev driver
On Thu, Aug 07, 2014 at 04:00:12PM +0200, Johan Hovold wrote: > These patches move the ZTE PIDs that were originally handled by the > option driver back to that driver, remove two PIDs from zte_ev that were > already claimed by other drivers, and finally removes the zte_ev driver > completely (and moves the sole remaining PID over to option). > > The zte_ev driver is based on code (once) distributed by ZTE that still > appears to originally have been reverse-engineered and bolted onto the > generic driver. A closer inspections of the control requests it was > sending reveals that they were not doing anything not already handled > (better) by option (well, except for some apparently redundant > SET/GET_LINE_CODING). > > There have also been reports about disconnect and reconnect issues with > zte_ev, something which has already led to one PID already having been > moved back. Lei Liu of ZTE has asked for further PIDs to be moved after > reporting issues with the driver. > > I intend to queue up the first three, which are also marked for stable, > for v3.17-rc2, while the final removal of the driver is held back to > v3.18. That way there's plenty of time to deal with any issues that, > however unlikely, may arise. > > Comments? If all devices work properly without this driver, I have no objection to removing it from the kernel. So this series looks good to me, thanks. greg k-h -- 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: [PATCHv2 01/13] usb: dwc2: Update Kconfig to support dual-role
On Thu, Aug 7, 2014 at 4:04 PM, Paul Zimmerman wrote: >> From: Dinh Nguyen [mailto:dinh.li...@gmail.com] >> Sent: Thursday, August 07, 2014 2:01 PM >> >> On Thu, Aug 7, 2014 at 3:55 PM, Paul Zimmerman >> wrote: >> >> From: Dinh Nguyen [mailto:dinh.li...@gmail.com] >> >> Sent: Thursday, August 07, 2014 5:12 AM >> >> >> >> On 8/1/14, 4:41 PM, Dinh Nguyen wrote: >> >> > On Fri, 2014-08-01 at 20:42 +, Paul Zimmerman wrote: >> >> >>> From: linux-usb-ow...@vger.kernel.org >> >> >>> [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of >> dingu...@altera.com >> >> >>> Sent: Wednesday, July 30, 2014 8:21 AM >> > >> > ... >> > >> >> >>> config USB_DWC2_PERIPHERAL >> >> >>> - tristate "Gadget only mode" >> >> >>> + bool "Gadget only mode" >> >> >>> depends on USB_GADGET >> >> >>> help >> >> >>> The Designware USB2.0 high-speed gadget controller >> >> >>> - integrated into many SoCs. >> >> >>> + integrated into many SoCs. Select this option if you want the >> >> >>> + driver to operate in Peripheral-only mode. >> >> >>> + >> >> >>> +config USB_DWC2_DUAL_ROLE >> >> >>> + bool "Dual Role mode" >> >> >>> + depends on ((USB=y || USB=USB_DWC2) && (USB_GADGET=y)) >> >> >> >> >> >> Hi Dinh, >> >> >> >> >> >> I just noticed that for dual-role mode, you are not allowing USB_GADGET >> >> >> to be modular. Is there a reason for that? If so, please mention it in >> >> >> the commit message. It should also be explained in the help text. Or >> >> >> maybe add another comment line saying "Dual-role mode requires USB >> >> >> Gadget >> >> >> = y" or something like that. >> >> >> >> >> > >> >> > I think it was an oversight on my part and there's not reason why >> >> > USB_GADGET can't be modular. >> >> > >> >> >> >> I went back to look this for v3 and it appears that I need USB_GADGET=y >> >> to avoid a build error when building the new driver for Gadget or >> >> Dual-role. >> >> >> >> drivers/built-in.o: In function `dwc2_gadget_init': >> >> /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3516: >> >> undefined reference to `usb_add_gadget_udc' >> >> drivers/built-in.o: In function `s3c_hsotg_remove': >> >> /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3543: >> >> undefined reference to `usb_del_gadget_udc' >> >> /home/dinguyen/linux_dev/linux-socfpga/drivers/usb/dwc2/gadget.c:3549: >> >> undefined reference to `usb_gadget_unregister_driver' >> > >> > I don't see why that shouldn't work. usb_add_gadget_udc is defined in >> > udc-core.c, which gets built if CONFIG_USB_GADGET is set. What Kconfig >> > line did you use when you got this build error? I think it should be: >> > >> > config USB_DWC2_DUAL_ROLE >> > bool "Dual Role mode" >> > depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || >> > USB_GADGET=USB_DWC2) >> > >> >> Right, I think your original comment was why I had "USB_GADGET=y" as a >> dependency for DWC2_DUAL_ROLE. If I replace USB_GADGET=y with just >> USB_GADGET, and make USB_GADGET a module. This is fine if I build DWC >> as module, but If I build DWC2 into the kernel, then the driver cannot >> link those gadget functions in. > I don't think just USB_GADGET will work. Please try it with the line I > showed above. > Yes, your suggestion works fine. It's an additional "USB_GADGET=USB_DWC2" from what I had in v2. Thanks, Dinh -- 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/4] usb: gadget: fix error return code
Julia Lawall writes:
> diff --git a/drivers/usb/gadget/udc/fusb300_udc.c
> b/drivers/usb/gadget/udc/fusb300_udc.c
> index d40255f..5c5d1ad 100644
> --- a/drivers/usb/gadget/udc/fusb300_udc.c
> +++ b/drivers/usb/gadget/udc/fusb300_udc.c
> @@ -1398,13 +1398,17 @@ static int fusb300_probe(struct platform_device *pdev)
>
> /* initialize udc */
> fusb300 = kzalloc(sizeof(struct fusb300), GFP_KERNEL);
> - if (fusb300 == NULL)
> + if (fusb300 == NULL) {
> + ret = -ENOMEM;
> goto clean_up;
> + }
>
> for (i = 0; i < FUSB300_MAX_NUM_EP; i++) {
> _ep[i] = kzalloc(sizeof(struct fusb300_ep), GFP_KERNEL);
> - if (_ep[i] == NULL)
> + if (_ep[i] == NULL) {
> + ret = -ENOMEM;
> goto clean_up;
> + }
> fusb300->ep[i] = _ep[i];
> }
Reviewed-by: Jeff Moyer
--
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] usb: gadget: at91_udc: move prepare clk into process context
Hi,
For the next revision, please include the changelog after the --- marker
and before the diffstat. Else, this looks good to me, with the same
comment as Boris on the PLLB gating. We will take care of that later.
Regards
On 07/08/2014 at 17:23:58 +0200, Ronald Wahl wrote :
> Hi,
>
> actually this should have been patch v3 and not v2. Anyway, the
> major difference is that I moved setting the clock rate into process
> context as well as it may also sleep.
>
> - ron
>
> On 07.08.2014 17:11, Ronald Wahl wrote:
> >Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> >interrupt context. This is not allowed as it might sleep. Move clock
> >preparation and setting clock rate into process context (at91udc_probe).
> >
> >Signed-off-by: Ronald Wahl
> >---
> > drivers/usb/gadget/udc/at91_udc.c | 44
> > ---
> > 1 file changed, 32 insertions(+), 12 deletions(-)
> >
> >diff --git a/drivers/usb/gadget/udc/at91_udc.c
> >b/drivers/usb/gadget/udc/at91_udc.c
> >index cfd18bc..0d685d0 100644
> >--- a/drivers/usb/gadget/udc/at91_udc.c
> >+++ b/drivers/usb/gadget/udc/at91_udc.c
> >@@ -870,12 +870,10 @@ static void clk_on(struct at91_udc *udc)
> > return;
> > udc->clocked = 1;
> >
> >-if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> >-clk_set_rate(udc->uclk, 4800);
> >-clk_prepare_enable(udc->uclk);
> >-}
> >-clk_prepare_enable(udc->iclk);
> >-clk_prepare_enable(udc->fclk);
> >+if (IS_ENABLED(CONFIG_COMMON_CLK))
> >+clk_enable(udc->uclk);
> >+clk_enable(udc->iclk);
> >+clk_enable(udc->fclk);
> > }
> >
> > static void clk_off(struct at91_udc *udc)
> >@@ -884,10 +882,10 @@ static void clk_off(struct at91_udc *udc)
> > return;
> > udc->clocked = 0;
> > udc->gadget.speed = USB_SPEED_UNKNOWN;
> >-clk_disable_unprepare(udc->fclk);
> >-clk_disable_unprepare(udc->iclk);
> >+clk_disable(udc->fclk);
> >+clk_disable(udc->iclk);
> > if (IS_ENABLED(CONFIG_COMMON_CLK))
> >-clk_disable_unprepare(udc->uclk);
> >+clk_disable(udc->uclk);
> > }
> >
> > /*
> >@@ -1780,14 +1778,24 @@ static int at91udc_probe(struct platform_device
> >*pdev)
> > }
> >
> > /* don't do anything until we have both gadget driver and VBUS */
> >+if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> >+clk_set_rate(udc->uclk, 4800);
> >+retval = clk_prepare(udc->uclk);
> >+if (retval)
> >+goto fail1;
> >+}
> >+retval = clk_prepare(udc->fclk);
> >+if (retval)
> >+goto fail1a;
> >+
> > retval = clk_prepare_enable(udc->iclk);
> > if (retval)
> >-goto fail1;
> >+goto fail1b;
> > at91_udp_write(udc, AT91_UDP_TXVC, AT91_UDP_TXVC_TXVDIS);
> > at91_udp_write(udc, AT91_UDP_IDR, 0x);
> > /* Clear all pending interrupts - UDP may be used by bootloader. */
> > at91_udp_write(udc, AT91_UDP_ICR, 0x);
> >-clk_disable_unprepare(udc->iclk);
> >+clk_disable(udc->iclk);
> >
> > /* request UDC and maybe VBUS irqs */
> > udc->udp_irq = platform_get_irq(pdev, 0);
> >@@ -1795,7 +1803,7 @@ static int at91udc_probe(struct platform_device *pdev)
> > 0, driver_name, udc);
> > if (retval < 0) {
> > DBG("request irq %d failed\n", udc->udp_irq);
> >-goto fail1;
> >+goto fail1c;
> > }
> > if (gpio_is_valid(udc->board.vbus_pin)) {
> > retval = gpio_request(udc->board.vbus_pin, "udc_vbus");
> >@@ -1848,6 +1856,13 @@ fail3:
> > gpio_free(udc->board.vbus_pin);
> > fail2:
> > free_irq(udc->udp_irq, udc);
> >+fail1c:
> >+clk_unprepare(udc->iclk);
> >+fail1b:
> >+clk_unprepare(udc->fclk);
> >+fail1a:
> >+if (IS_ENABLED(CONFIG_COMMON_CLK))
> >+clk_unprepare(udc->uclk);
> > fail1:
> > if (IS_ENABLED(CONFIG_COMMON_CLK) && !IS_ERR(udc->uclk))
> > clk_put(udc->uclk);
> >@@ -1896,6 +1911,11 @@ static int __exit at91udc_remove(struct
> >platform_device *pdev)
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > release_mem_region(res->start, resource_size(res));
> >
> >+if (IS_ENABLED(CONFIG_COMMON_CLK))
> >+clk_unprepare(udc->uclk);
> >+clk_unprepare(udc->fclk);
> >+clk_unprepare(udc->iclk);
> >+
> > clk_put(udc->iclk);
> > clk_put(udc->fclk);
> > if (IS_ENABLED(CONFIG_COMMON_CLK))
> >
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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: dwc2: Read GNPTXFSIZ when in forced HOST mode.
Paul, On Thu, Aug 7, 2014 at 1:18 PM, Paul Zimmerman wrote: >> From: Doug Anderson [mailto:diand...@chromium.org] >> Sent: Thursday, August 07, 2014 12:48 PM >> >> The documentation for GNPTXFSIZ says that "For host mode, this field >> is always valid." Since we're already switching to host mode for >> HPTXFSIZ, let's also read GNPTXFSIZ in host mode. >> >> On an rk3288 SoC, without this change we see this at bootup: >> dwc2 ff58.usb: gnptxfsiz=00100400 >> dwc2 ff58.usb: 128 invalid for host_nperio_tx_fifo_size. Check HW >> configuration. >> >> After this change we see: >> dwc2 ff58.usb: gnptxfsiz=04000400 >> >> Signed-off-by: Doug Anderson >> --- >> drivers/usb/dwc2/core.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c >> index 27d2c9b..c184ed43 100644 >> --- a/drivers/usb/dwc2/core.c >> +++ b/drivers/usb/dwc2/core.c >> @@ -2674,23 +2674,23 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg) >> hwcfg2 = readl(hsotg->regs + GHWCFG2); >> hwcfg3 = readl(hsotg->regs + GHWCFG3); >> hwcfg4 = readl(hsotg->regs + GHWCFG4); >> - gnptxfsiz = readl(hsotg->regs + GNPTXFSIZ); >> grxfsiz = readl(hsotg->regs + GRXFSIZ); >> >> dev_dbg(hsotg->dev, "hwcfg1=%08x\n", hwcfg1); >> dev_dbg(hsotg->dev, "hwcfg2=%08x\n", hwcfg2); >> dev_dbg(hsotg->dev, "hwcfg3=%08x\n", hwcfg3); >> dev_dbg(hsotg->dev, "hwcfg4=%08x\n", hwcfg4); >> - dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz); >> dev_dbg(hsotg->dev, "grxfsiz=%08x\n", grxfsiz); >> >> - /* Force host mode to get HPTXFSIZ exact power on value */ >> + /* Force host mode to get HPTXFSIZ / GNPTXFSIZ exact power on value */ >> gusbcfg = readl(hsotg->regs + GUSBCFG); >> gusbcfg |= GUSBCFG_FORCEHOSTMODE; >> writel(gusbcfg, hsotg->regs + GUSBCFG); >> usleep_range(10, 15); >> >> + gnptxfsiz = readl(hsotg->regs + GNPTXFSIZ); >> hptxfsiz = readl(hsotg->regs + HPTXFSIZ); >> + dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz); >> dev_dbg(hsotg->dev, "hptxfsiz=%08x\n", hptxfsiz); >> gusbcfg = readl(hsotg->regs + GUSBCFG); >> gusbcfg &= ~GUSBCFG_FORCEHOSTMODE; > > Nice! I wonder if this is a bug in the original driver, and they > actually meant to read this register instead of HPTXFSIZ? Well, it > doesn't really matter I guess. > > Acked-by: Paul Zimmerman > > You may want to resend this to Greg after -rc1 is out and he reopens > his usb-next tree. Thanks! ...do you think this is a fix that needs to go in for 3.17? Is it affecting anyone there? -Doug -- 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 v4 2/4] usb: dwc2: add compatible data for rockchip soc
Paul, On 08/08/2014 02:26 AM, Paul Zimmerman wrote: From: Kever Yang [mailto:kever.y...@gmail.com] On Behalf Of Kever Yang Sent: Thursday, August 07, 2014 2:35 AM This patch add compatible data for dwc2 controller found on rk3066, rk3188 and rk3288 processors from rockchip. Signed-off-by: Kever Yang Acked-by: Paul Zimmerman --- Changes in v4: - max_transfer_size change to 65536, this should be enough for most transfer, the hardware auto-detect will set this to 0x7 which may make dma_alloc_coherent fail when non-dword aligned buf from driver like usbnet happen. Hi Kever, Did you test this change thoroughly? I have vague memories of any value above 65535 causing problems, at least on my hardware. And I see it is set to 65535 in both pci.c and platform.c. I could be wrong, but I thought I should mention it. I test it on rk3288 evb, it works find with 65536, I'm sorry for didn't mention it in my patch. The problem in my platform is if the value use hardware auto-detect, it will be 0x7, and that will cause the dma_alloc_coherent fail in hcd driver. The value less than 0x7 should be fine for hardware, but for the software, it depends on how we use it. What kind of problem did you met? Software problem or hardware problem? Maybe I should pay more attention for this value. :) -- 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: dwc2: Read GNPTXFSIZ when in forced HOST mode.
> From: diand...@google.com [mailto:diand...@google.com] On Behalf Of Doug > Anderson > Sent: Thursday, August 07, 2014 5:12 PM > > On Thu, Aug 7, 2014 at 1:18 PM, Paul Zimmerman > wrote: > >> From: Doug Anderson [mailto:diand...@chromium.org] > >> Sent: Thursday, August 07, 2014 12:48 PM > >> > >> The documentation for GNPTXFSIZ says that "For host mode, this field > >> is always valid." Since we're already switching to host mode for > >> HPTXFSIZ, let's also read GNPTXFSIZ in host mode. > >> > >> On an rk3288 SoC, without this change we see this at bootup: > >> dwc2 ff58.usb: gnptxfsiz=00100400 > >> dwc2 ff58.usb: 128 invalid for host_nperio_tx_fifo_size. Check HW > >> configuration. > >> > >> After this change we see: > >> dwc2 ff58.usb: gnptxfsiz=04000400 > >> > >> Signed-off-by: Doug Anderson > >> --- > >> drivers/usb/dwc2/core.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c > >> index 27d2c9b..c184ed43 100644 > >> --- a/drivers/usb/dwc2/core.c > >> +++ b/drivers/usb/dwc2/core.c > >> @@ -2674,23 +2674,23 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg) > >> hwcfg2 = readl(hsotg->regs + GHWCFG2); > >> hwcfg3 = readl(hsotg->regs + GHWCFG3); > >> hwcfg4 = readl(hsotg->regs + GHWCFG4); > >> - gnptxfsiz = readl(hsotg->regs + GNPTXFSIZ); > >> grxfsiz = readl(hsotg->regs + GRXFSIZ); > >> > >> dev_dbg(hsotg->dev, "hwcfg1=%08x\n", hwcfg1); > >> dev_dbg(hsotg->dev, "hwcfg2=%08x\n", hwcfg2); > >> dev_dbg(hsotg->dev, "hwcfg3=%08x\n", hwcfg3); > >> dev_dbg(hsotg->dev, "hwcfg4=%08x\n", hwcfg4); > >> - dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz); > >> dev_dbg(hsotg->dev, "grxfsiz=%08x\n", grxfsiz); > >> > >> - /* Force host mode to get HPTXFSIZ exact power on value */ > >> + /* Force host mode to get HPTXFSIZ / GNPTXFSIZ exact power on value > >> */ > >> gusbcfg = readl(hsotg->regs + GUSBCFG); > >> gusbcfg |= GUSBCFG_FORCEHOSTMODE; > >> writel(gusbcfg, hsotg->regs + GUSBCFG); > >> usleep_range(10, 15); > >> > >> + gnptxfsiz = readl(hsotg->regs + GNPTXFSIZ); > >> hptxfsiz = readl(hsotg->regs + HPTXFSIZ); > >> + dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz); > >> dev_dbg(hsotg->dev, "hptxfsiz=%08x\n", hptxfsiz); > >> gusbcfg = readl(hsotg->regs + GUSBCFG); > >> gusbcfg &= ~GUSBCFG_FORCEHOSTMODE; > > > > Nice! I wonder if this is a bug in the original driver, and they > > actually meant to read this register instead of HPTXFSIZ? Well, it > > doesn't really matter I guess. > > > > Acked-by: Paul Zimmerman > > > > You may want to resend this to Greg after -rc1 is out and he reopens > > his usb-next tree. > > Thanks! > > ...do you think this is a fix that needs to go in for 3.17? Is it > affecting anyone there? I don't think so. It only has an effect if the controller starts up in peripheral mode and then later switches to host mode, right? I think yours is the first dwc2 platform in the kernel with that capability. -- Paul
Re: [PATCH] usb: dwc2: Read GNPTXFSIZ when in forced HOST mode.
Doug: On 08/08/2014 03:48 AM, Doug Anderson wrote: The documentation for GNPTXFSIZ says that "For host mode, this field is always valid." Since we're already switching to host mode for HPTXFSIZ, let's also read GNPTXFSIZ in host mode. On an rk3288 SoC, without this change we see this at bootup: dwc2 ff58.usb: gnptxfsiz=00100400 dwc2 ff58.usb: 128 invalid for host_nperio_tx_fifo_size. Check HW configuration. After this change we see: dwc2 ff58.usb: gnptxfsiz=04000400 Yeap, that is the problem cause the log you shown in rk3288-evb and further more cause fifo setting fail. I was plan to commit this patch just the same as you did. It's great that you also find out the problem and send this patch. Signed-off-by: Doug Anderson --- drivers/usb/dwc2/core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c index 27d2c9b..c184ed43 100644 --- a/drivers/usb/dwc2/core.c +++ b/drivers/usb/dwc2/core.c @@ -2674,23 +2674,23 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg) hwcfg2 = readl(hsotg->regs + GHWCFG2); hwcfg3 = readl(hsotg->regs + GHWCFG3); hwcfg4 = readl(hsotg->regs + GHWCFG4); - gnptxfsiz = readl(hsotg->regs + GNPTXFSIZ); grxfsiz = readl(hsotg->regs + GRXFSIZ); dev_dbg(hsotg->dev, "hwcfg1=%08x\n", hwcfg1); dev_dbg(hsotg->dev, "hwcfg2=%08x\n", hwcfg2); dev_dbg(hsotg->dev, "hwcfg3=%08x\n", hwcfg3); dev_dbg(hsotg->dev, "hwcfg4=%08x\n", hwcfg4); - dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz); dev_dbg(hsotg->dev, "grxfsiz=%08x\n", grxfsiz); - /* Force host mode to get HPTXFSIZ exact power on value */ + /* Force host mode to get HPTXFSIZ / GNPTXFSIZ exact power on value */ gusbcfg = readl(hsotg->regs + GUSBCFG); gusbcfg |= GUSBCFG_FORCEHOSTMODE; writel(gusbcfg, hsotg->regs + GUSBCFG); usleep_range(10, 15); + gnptxfsiz = readl(hsotg->regs + GNPTXFSIZ); hptxfsiz = readl(hsotg->regs + HPTXFSIZ); + dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz); dev_dbg(hsotg->dev, "hptxfsiz=%08x\n", hptxfsiz); gusbcfg = readl(hsotg->regs + GUSBCFG); gusbcfg &= ~GUSBCFG_FORCEHOSTMODE; There may be a potential problem still need to fix, the grxfsiz may have being changed, the bootrom and uboot will change this value if they use the dwc2 controller. The way we get the register value here can not make sure this is the power-on value which we actually need. Let me do more test for that, and maybe we need another patch. Anyway, this patch works and reasonable. Reviewed-by: Kever Yang -- 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
driver option create ttyUSB
https://bugzilla.kernel.org/show_bug.cgi?id=81801
Bug ID: 81801
Summary: driver option create ttyUSB
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
Re: [PATCH v4 2/4] usb: dwc2: add compatible data for rockchip soc
On 08/08/2014 04:52 AM, Doug Anderson wrote: Paul, On Thu, Aug 7, 2014 at 11:26 AM, Paul Zimmerman wrote: From: Kever Yang [mailto:kever.y...@gmail.com] On Behalf Of Kever Yang Sent: Thursday, August 07, 2014 2:35 AM This patch add compatible data for dwc2 controller found on rk3066, rk3188 and rk3288 processors from rockchip. Signed-off-by: Kever Yang Acked-by: Paul Zimmerman --- Changes in v4: - max_transfer_size change to 65536, this should be enough for most transfer, the hardware auto-detect will set this to 0x7 which may make dma_alloc_coherent fail when non-dword aligned buf from driver like usbnet happen. Hi Kever, Did you test this change thoroughly? I have vague memories of any value above 65535 causing problems, at least on my hardware. And I see it is set to 65535 in both pci.c and platform.c. I could be wrong, but I thought I should mention it. Certainly it is documented in the header file to have a max of 65535: * @max_transfer_size: The maximum transfer size supported, in bytes * 2047 to 65,535 * Actual maximum value is autodetected and also * the default. Sorry for didn't check the header file, I'll change it to 65535 and resubmit. ...but looking at the register definition that I see, the size can be up to 19 bits. A 19-bit transfer far exceeds 65535. Do you remember what the error was? Certainly I can imagine there being errors with large calls to dma_alloc_coherent()... I know that with Kever's change I can do USB Ethernet downloads, so it is at least working to some degree. ...to me it feels like Kever should resubmit with 65535 (to match the documentation) and then work in the background to figure out what the max_transfer_size really ought to be. You are right. -Doug -- 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: driver option create ttyUSB
On Fri, Aug 08, 2014 at 09:24:22AM +0800, w wrote: > > > https://bugzilla.kernel.org/show_bug.cgi?id=81801 > > Bug ID: 81801 > > Summary: driver option create ttyUSB I don't understand the problem, the kernel is working properly here, right? What userspace program is keeping your device node open to prevent it from being reused? And, why not use the symlinks in /dev/serial/ they will always work properly here, right? thanks, greg k-h -- 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:Re: driver option create ttyUSB
If use symlinks it can work well.
But why program in userspace can prevent driver release the node?
The program just open the node, nothing else.
At 2014-08-08 10:08:59, "Greg KH" wrote:
>On Fri, Aug 08, 2014 at 09:24:22AM +0800, w wrote:
>>
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=81801
>>
>> Bug ID: 81801
>>
>> Summary: driver option create ttyUSB
>
>I don't understand the problem, the kernel is working properly here,
>right?
>
>What userspace program is keeping your device node open to prevent it
>from being reused?
>
>And, why not use the symlinks in /dev/serial/ they will always work
>properly here, right?
>
>thanks,
>
>greg k-h
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
Re: Re: driver option create ttyUSB
On Fri, Aug 08, 2014 at 10:56:57AM +0800, w wrote: > > If use symlinks it can work well. > > But why program in userspace can prevent driver release the node? > The program just open the node, nothing else. If it holds the node open, of course the kernel can't release it, the resource is still in use. That's the way it is supposed to be, don't you agree? thanks, greg k-h -- 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 v5 2/4] usb: dwc2: add compatible data for rockchip soc
This patch add compatible data for dwc2 controller found on
rk3066, rk3188 and rk3288 processors from rockchip.
Signed-off-by: Kever Yang
Acked-by: Paul Zimmerman
---
Changes in v5:
- max_transfer_size change to 65535 to met the requirement of
header file
Changes in v4:
- max_transfer_size change to 65536, this should be enough
for most transfer, the hardware auto-detect will set this
to 0x7 which may make dma_alloc_coherent fail when
non-dword aligned buf from driver like usbnet happen.
Changes in v3: None
Changes in v2:
- set most parameters as driver auto-detect
drivers/usb/dwc2/platform.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index a10e7a3..2f859bd 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -75,6 +75,34 @@ static const struct dwc2_core_params params_bcm2835 = {
.uframe_sched = 0,
};
+static const struct dwc2_core_params params_rk3066 = {
+ .otg_cap= 2,/* non-HNP/non-SRP */
+ .otg_ver= -1,
+ .dma_enable = -1,
+ .dma_desc_enable= 0,
+ .speed = -1,
+ .enable_dynamic_fifo= 1,
+ .en_multiple_tx_fifo= -1,
+ .host_rx_fifo_size = 520, /* 520 DWORDs */
+ .host_nperio_tx_fifo_size = 128, /* 128 DWORDs */
+ .host_perio_tx_fifo_size= 256, /* 256 DWORDs */
+ .max_transfer_size = 65535,
+ .max_packet_count = -1,
+ .host_channels = -1,
+ .phy_type = -1,
+ .phy_utmi_width = -1,
+ .phy_ulpi_ddr = -1,
+ .phy_ulpi_ext_vbus = -1,
+ .i2c_enable = -1,
+ .ulpi_fs_ls = -1,
+ .host_support_fs_ls_low_power = -1,
+ .host_ls_low_power_phy_clk = -1,
+ .ts_dline = -1,
+ .reload_ctl = -1,
+ .ahbcfg = 0x7, /* INCR16 */
+ .uframe_sched = -1,
+};
+
/**
* dwc2_driver_remove() - Called when the DWC_otg core is unregistered with the
* DWC_otg driver
@@ -97,6 +125,7 @@ static int dwc2_driver_remove(struct platform_device *dev)
static const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "brcm,bcm2835-usb", .data = ¶ms_bcm2835 },
+ { .compatible = "rockchip,rk3066-usb", .data = ¶ms_rk3066 },
{ .compatible = "snps,dwc2", .data = NULL },
{},
};
--
1.9.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 v5 0/4] Patches to add support for Rockchip dwc2 controller
These patches to add support for dwc2 controller found in Rockchip processors rk3066, rk3188 and rk3288, and enable dts for rk3288 evb. Changes in v5: - max_transfer_size change to 65535 to met the requirement of header file - change the sort order of dwc2 in rk3288.dtsi - don't enable otg port for evb Changes in v4: - max_transfer_size change to 65536, this should be enough for most transfer, the hardware auto-detect will set this to 0x7 which may make dma_alloc_coherent fail when non-dword aligned buf from driver like usbnet happen. - remove EHCI and HSIC dts patch for Doug had post it seprately. Changes in v3: - EHCI and HSIC move new for version 3. - Rebase Changes in v2: - Split out dr_mode and rk3288 bindings. - add compatible "snps,dwc2" bingding info - set most parameters as driver auto-detect - evb patch added in version 2 Kever Yang (4): Documentation: dt-bindings: add dt binding info for Rockchip dwc2 usb: dwc2: add compatible data for rockchip soc ARM: dts: add rk3288 dwc2 controller support ARM: dts: Enable USB host1(dwc) on rk3288-evb Documentation/devicetree/bindings/usb/dwc2.txt | 3 +++ arch/arm/boot/dts/rk3288-evb.dtsi | 4 arch/arm/boot/dts/rk3288.dtsi | 20 ++ drivers/usb/dwc2/platform.c| 29 ++ 4 files changed, 56 insertions(+) -- 1.9.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 resend] Revert "usb: gadget: u_ether: synchronize with transmit when stopping queue"
Thanks. where can I find this patch status? like netdev from http://patchwork.ozlabs.org/project/netdev/list/ -Roy On Tue, Aug 5, 2014 at 8:57 AM, Greg KH wrote: > On Tue, Aug 05, 2014 at 08:47:33AM +0800, Li RongQing wrote: >> On Fri, Jul 25, 2014 at 2:22 PM, wrote: >> > From: Li RongQing >> > >> > This reverts commit a9232076374334ca2bc2a448dfde96d38a54349a. >> > >> > It introduced a dead lock, and did not fix anything. >> > >> > it made netif_tx_lock() be called in IRQ context, but in softirq context, >> > the same lock is locked without disabling IRQ. In fact, the commit >> > a923207637 >> > did not fix anything, since netif_stop_queue did not free the any resource >> >> >> ping > > This is up to Felipe... -- 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:Re: Re: driver option create ttyUSB
I got a patch from git log，this patch is about the cdc-acm.
But what should i do to solve my problem?
Patch as below:
From 051522bb47797f7168a617a0752d7ddc1a2f6f24 Mon Sep 17 00:00:00 2001
From: Francesco Lavra
Date: Tue, 3 Nov 2009 10:53:07 +
Subject: [PATCH] USB: cdc_acm: Fix memory leak after hangup
Am Donnerstag, 10. September 2009 15:43:53 schrieb Dietmar Hilbrich:
> Hello,
>
> i have the following problem with the cdc-acm - driver:
>
> I'm using the driver with an "Ericsson F3507G" on a Thinkpad T400.
>
> If a disable the device (with the RFKill-Switch) while it is used by a
> programm like ppp, the driver doesn't seem to correctly clean up the tty,
> even after the program has been closed)
>
> The tty is still active (e.g. there still exists an entry in
> /sys/dev/char/166:0 if ttyACM0 was used) and if a reacticate the device,
> this device entry will be skipped and the Device-Nodes ttyACM1, ttyACM2
> and ttyACM3 will be used.
>
> This problem was introduced with the commit
> 10077d4a6674f535abdbe25cdecb1202af7948f1 (before 2.6.31-rc1) and still
> exists in 2.6.31.
>
> I was able the fix this problem with the following patch:
>
> diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> index 2bfc41e..0970d2f 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -676,6 +676,7 @@ static void acm_tty_hangup(struct tty_struct *tty)
> struct acm *acm = tty->driver_data;
> tty_port_hangup(&acm->port);
> acm_port_down(acm, 0);
> + acm_tty_unregister(acm);
> }
I have the same problem with cdc-acm (I'm using a Samsung SGH-U900): when I
unplug it from the USB port during a PPP connection, the ppp daemon gets the
hangup correctly (and closes the device), but the struct acm corresponding to
the device disconnected is not freed. Hence reconnecting the device results in
creation of /dev/ttyACM(x+1). The same happens when the system is hibernated
during a PPP connection.
This memory leak is due to the fact that when the tty is hung up,
tty_port_close_start() returns always zero, and acm_tty_close() never reaches
the point where acm_tty_unregister() is called.
Here is a fix for this.
Signed-off-by: Francesco Lavra
Acked-by: Oliver Neukum
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/class/cdc-acm.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index b72fa49..e4eca78 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -686,15 +686,21 @@ static void acm_tty_close(struct tty_struct *tty, struct
file *filp)
/* Perform the closing process and see if we need to do the hardware
shutdown */
- if (!acm || tty_port_close_start(&acm->port, tty, filp) == 0)
+ if (!acm)
+ return;
+ if (tty_port_close_start(&acm->port, tty, filp) == 0) {
+ mutex_lock(&open_mutex);
+ if (!acm->dev) {
+ tty_port_tty_set(&acm->port, NULL);
+ acm_tty_unregister(acm);
+ tty->driver_data = NULL;
+ }
+ mutex_unlock(&open_mutex);
return;
+ }
acm_port_down(acm, 0);
tty_port_close_end(&acm->port, tty);
- mutex_lock(&open_mutex);
tty_port_tty_set(&acm->port, NULL);
- if (!acm->dev)
- acm_tty_unregister(acm);
- mutex_unlock(&open_mutex);
}
static int acm_tty_write(struct tty_struct *tty,
--
1.7.9.5
At 2014-08-08 11:31:41, "Greg KH" wrote:
>On Fri, Aug 08, 2014 at 10:56:57AM +0800, w wrote:
>>
>> If use symlinks it can work well.
>>
>> But why program in userspace can prevent driver release the node?
>> The program just open the node, nothing else.
>
>If it holds the node open, of course the kernel can't release it, the
>resource is still in use. That's the way it is supposed to be, don't
>you agree?
>
>thanks,
>
>greg k-h
Re: [PATCH resend] Revert "usb: gadget: u_ether: synchronize with transmit when stopping queue"
On Fri, Aug 08, 2014 at 12:15:14PM +0800, Li RongQing wrote: > Thanks. where can I find this patch status? like netdev from > http://patchwork.ozlabs.org/project/netdev/list/ I don't think any of the usb developers use patchwork, sorry. Just relax and wait, Felipe will handle it eventually. greg k-h -- 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: Re: Re: driver option create ttyUSB
On Fri, Aug 08, 2014 at 01:42:53PM +0800, w wrote: > > I got a patch from git log，this patch is about the cdc-acm. >From 2009? > But what should i do to solve my problem? I don't understand, are you using a kernel older than 2009? What kernel version are you using? confused, greg k-h -- 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:Re: Re: Re: driver option create ttyUSB
My kernel version is 3.0.
I just want to get some useful information from this patch.
Because the patch seems solved the same problem as mine.
At 2014-08-08 01:56:52, "Greg KH" wrote:
>On Fri, Aug 08, 2014 at 01:42:53PM +0800, w wrote:
>>
>> I got a patch from git log，this patch is about the cdc-acm.
>
>From 2009?
>
>> But what should i do to solve my problem?
>
>I don't understand, are you using a kernel older than 2009?
>
>What kernel version are you using?
>
>confused,
>
>greg k-h
N嫥叉靣笡y氊b瞂千v豝�)藓{.n�+壏{焙柒炟^n噐■�侂h櫒璀�&Ⅷ 瓽珴閔�(殠娸"濟�m�飦赇z罐枈帼f＂穐殘坢
