Re: [PATCH RFC 1/4] phy: Add new Exynos5 USB 3.0 PHY driver

2013-11-10 Thread Tomasz Figa
Hi Vivek,

On Thursday 31 of October 2013 13:15:41 Vivek Gautam wrote:
 Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs.
 The new driver uses the generic PHY framework and will interact
 with DWC3 controller present on Exynos5 series of SoCs.
 
 Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
 ---
  .../devicetree/bindings/phy/samsung-phy.txt|   20 +
  drivers/phy/Kconfig|7 +
  drivers/phy/Makefile   |1 +
  drivers/phy/phy-exynos5-usb3.c |  562 
 
  4 files changed, 590 insertions(+), 0 deletions(-)
  create mode 100644 drivers/phy/phy-exynos5-usb3.c
 
 diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt 
 b/Documentation/devicetree/bindings/phy/samsung-phy.txt
 index c0fccaa..9b5c111 100644
 --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
 +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
 @@ -20,3 +20,23 @@ Required properties:
  - compatible : should be samsung,exynos5250-dp-video-phy;
  - reg : offset and length of the Display Port PHY register set;
  - #phy-cells : from the generic PHY bindings, must be 0;
 +
 +Samsung Exynos5 SoC seiries USB 3.0 PHY controller

typo: s/seiries/series/

 +--
 +
 +Required properties:
 +- compatible :
 + should be samsung,exynos5250-usb3phy for exynos5250 SoC
 + should be samsung,exynos5420-usb3phy for exynos5420 SoC

I'd slightly change this into something like this:

- compatible: Should be set to one of following supported values:
- samsung,exynos5250-usb3phy - for Exynos5250 SoC,
- samsung,exynos5420-usb3phy - for Exynos5420 SoC.

 +- reg : Register offset and length array
 + - first field corresponds to USB 3.0 PHY register set;
 + - second field corresponds to PHY power isolation register
 +   present in PMU;

For consistency and to make things more future-proof, you should consider
using the PMU indirectly, through the syscon interface, as done in Leela
Krishna Amudala's series[1] in case of watchdog driver.

I will tell Kamil to do the same for USB 2.0 PHY as well.

[1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/24652

 +- clocks: Clock IDs array as required by the controller
 +- clock-names: names of clocks correseponding to IDs in the clock property;
 + Required clocks:
 + - first clock is main PHY clock (same as USB 3.0 controller IP clock)
 + - second clock is reference clock (usually crystal clock)
 + optional clock:
 + - third clock is special clock used by PHY for operation

Is this clock really optional? It looks like it's required for Exynos5420.
If so, you should instead change this to:

Additional clocks required for Exynos5420:

Also you have not specified names of the clocks, just what they are.
Please remember that those are input names, so you can imagine them as
names of clock input pins of the IP block, not SoC-level clock names.

 +- #phy-cells : from the generic PHY bindings, must be 0;

I'd also add an example of correct USB 3.0 PHY device tree node here.

 diff --git a/drivers/phy/phy-exynos5-usb3.c b/drivers/phy/phy-exynos5-usb3.c
 new file mode 100644
 index 000..b9a2674
 --- /dev/null
 +++ b/drivers/phy/phy-exynos5-usb3.c
 @@ -0,0 +1,562 @@
[snip]
 +#define EXYNOS5_DRD_PHYRESUME(0x34)
 +#define EXYNOS5_DRD_LINKPORT (0x44)
 +
 +

nit: Duplicate blank line.

 +/* Isolation, configured in the power management unit */
 +#define EXYNOS5_USB_ISOL_DRD (1  0)
 +
 +#define CLKSEL_ERROR   -1

What's this?

 +
 +#ifndef KHZ
 +#define KHZ 1000
 +#endif
 +
 +#ifndef MHZ
 +#define MHZ (KHZ * KHZ)
 +#endif

Do you really need the #ifndef's above?

 +
 +enum samsung_cpu_type {
 + TYPE_EXYNOS5250,
 + TYPE_EXYNOS5420,
 +};

Instead of using this kind of enumeration, I'd rather introduce a struct
that describes the differences between all supported types.

 +
 +enum usb3phy_state {
 + STATE_OFF,
 + STATE_ON,
 +};

Hmm, isn't it a simple boolean value - false and true?

 +
 +struct usb3phy_config {
 + enum samsung_cpu_type cpu;
 + bool has_sclk_usbphy30;
 +};

Oh, you already have such struct, so there is even a bigger reason to drop
the samsung_cpu_type enum above.

 +
 +struct usb3phy_instance {
 + char *label;
 + struct usb3phy_driver *drv;
 + struct phy *phy;
 + enum usb3phy_state state;
 + u32 clk;
 + unsigned long rate;
 +};

You seem to have just one instance in this driver. Do you really
need this struct?

 +
 +struct usb3phy_driver {
 + struct device *dev;
 + void __iomem *reg_phy;
 + void __iomem *reg_isol;
 + struct clk *clk;
 + struct clk *sclk_usbphy30;
 + struct usb3phy_instance instance;

Fields from that struct could be simply moved here.

 +};
 +
 +/*
 + * exynos5_rate_to_clk() converts the supplied clock rate to 

Re: [PATCH 2/4] dt: exynos5250: Enable support for generic USB 3.0 phy

2013-11-10 Thread Tomasz Figa
Hi Vivek,

On Thursday 31 of October 2013 13:15:42 Vivek Gautam wrote:
 Update device tree bindings for DWC3 controller and
 USB 3.0 phy present on Exynos 5250 SoC, to start using
 the phy driver based on generic phy framework.
 
 Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
 ---
  arch/arm/boot/dts/exynos5250.dtsi |   17 ++---
  1 files changed, 6 insertions(+), 11 deletions(-)
 
 diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
 b/arch/arm/boot/dts/exynos5250.dtsi
 index bbac42a..31a6595 100644
 --- a/arch/arm/boot/dts/exynos5250.dtsi
 +++ b/arch/arm/boot/dts/exynos5250.dtsi
 @@ -472,22 +472,17 @@
   compatible = synopsys,dwc3;
   reg = 0x1200 0x1;
   interrupts = 0 72 0;
 - usb-phy = usb2_phy usb3_phy;
 + phys = usb3_phy;
 + phy-names = usb3-phy;

Does the driver already support generic PHY framework?

Also it looks like originally it required two PHYs, while your patch
changes it to just one.

   };
   };
  
   usb3_phy: usbphy@1210 {
   compatible = samsung,exynos5250-usb3phy;

Hmm, this is not fully right. The new bindings should have new compatible
value. This is also a comment to patch 1/1. This is because a device tree
binding associated with specific compatible value is an ABI and should not
be changed in a way that breaks backwards compatibility.

Best regards,
Tomasz

--
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: [alsa-devel] no reset_resume for driver snd-usb-audio for logitech headset H600

2013-11-10 Thread baumber

On 2013-10-20 23:35, Takashi Iwai wrote:

At Sun, 20 Oct 2013 23:18:13 +0200,
baum...@hotmail.com wrote:


On 2013-10-09 11:16 (CEST,UTC+2), Takashi Iwai wrote:

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 9d9de8d..c591a83 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -747,6 +747,7 @@ static struct usb_driver usb_audio_driver = {
.disconnect =   usb_audio_disconnect,
.suspend =  usb_audio_suspend,
.resume =   usb_audio_resume,
+   .reset_resume = usb_audio_resume,
.id_table = usb_audio_ids,
.supports_autosuspend = 1,
   };


Shall I add the line in b/sound/usb/card.c and try it? Which kernel version 
shall I use?


It doesn't matter which kernel version is.
The error message should disappear, but I don't know whether any other
positive effect you get.  As Clemens pointed, there can be some
negative effects by this, so it cannot be taken as is.  But at least,
it'd be good to know the result...


thanks,

Takashi


I have tested the patch with kernel 3.12.0 with suspend and hibernate, and the 
problem is gone.

No messages no reset_resume for driver snd-usb-audio? or other negative 
effects/messages.

After resume, the usb_audio_device (Headset) is the master channel in KDE 
Mixer, as it should be.

= So the problem is solved for me.

Can you please add the patch to the latest kernels and LTS-kernels or whitelist 
the workaround for my device (ID 046d:0a29 Logitech, Inc. H600 [Wireless 
Headset])

Thanks, Bernhard


--
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/3] phy: Add new Exynos USB PHY driver

2013-11-10 Thread Tomasz Figa
Hi Kamil,

Please see my comments inline.

On Tuesday 05 of November 2013 17:13:19 Kamil Debski wrote:
 Add a new driver for the Exynos USB PHY. The new driver uses the generic
 PHY framework. The driver includes support for the Exynos 4x10 and 4x12
 SoC families.
 
 Signed-off-by: Kamil Debski k.deb...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  .../devicetree/bindings/phy/samsung-usbphy.txt |   52 
  drivers/phy/Kconfig|   23 +-
  drivers/phy/Makefile   |4 +
  drivers/phy/phy-exynos-usb2.c  |  234 ++
  drivers/phy/phy-exynos-usb2.h  |   87 ++
  drivers/phy/phy-exynos4210-usb2.c  |  272 
  drivers/phy/phy-exynos4212-usb2.c  |  324 
 
  7 files changed, 995 insertions(+), 1 deletion(-)
  create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt
  create mode 100644 drivers/phy/phy-exynos-usb2.c
  create mode 100644 drivers/phy/phy-exynos-usb2.h
  create mode 100644 drivers/phy/phy-exynos4210-usb2.c
  create mode 100644 drivers/phy/phy-exynos4212-usb2.c
 
 diff --git a/Documentation/devicetree/bindings/phy/samsung-usbphy.txt 
 b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
 new file mode 100644
 index 000..c8fbc70
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
 @@ -0,0 +1,52 @@
 +Samsung S5P/EXYNOS SoC series USB PHY

I would not limit this only to S5P and newer series, especially that
I'm planning to add support for S3C64xx using this framework.

Instead I would call it Samsung SoC USB 1.1/2.0 PHY.

 +-
 +
 +Required properties:
 +- compatible : should be one of the listed compatibles:
 + - samsung,exynos4210-usbphy
 + - samsung,exynos4212-usbphy
 +- reg : a list of registers used by phy driver
 + - first and obligatory is the location of phy modules registers
 + - second and also required is the location of isolation registers
 +   (isolation registers control the physical connection between the in
 +   SoC modules and outside of the SoC, this also can be called enable
 +   control in the documentation of the SoC)
 + - third is the location of the mode switch register, this only applies
 +   to SoCs that have such a feature; mode switching enables to have
 +   both host and device used the same SoC pins and is commonly used
 +   when OTG is supported

You should consider using the PMU registers indirectly, as done in Leela
Krisha Amudala's series[1] adding PMU register handling to the watchdog
driver.

[1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/24652

 +- #phy-cells : from the generic phy bindings, must be 1;
 +- clocks and clock-names:
 + - the phy clocks is required by the phy module
 + - other clocks are associated by name with their respective phys and
 +   are used to determine the value of the clock settings register

Those names should be explicitly listed.

 +
 +The second cell in the PHY specifier identifies the PHY, its  meaning is

It should say The first cell, I think?

 +compatible dependent. For the currently supported SoCs (Exynos 4210 and
 +Exynos 4212) it is as follows:
 +  0 - USB device,
 +  1 - USB host,
 +  2 - HSIC0,
 +  3 - HSIC1,
 +
 +Example:
 +
 +For Exynos 4412 (compatible with Exynos 4212):
 +
 +exynos_usbphy: exynos-usbphy@125B {

nit: Node names should be generic and the label is slightly too long, so
a better example would be:

usbphy: phy@125B {

 + compatible = samsung,exynos4212-usbphy;
 + reg = 0x125B 0x100 0x10020704 0x0c 0x1001021c 0x4;
 + clocks = clock 305, clock 2, clock 2, clock 2,
 + clock 2;
 + clock-names = phy, device, host, hsic0, hsic1;
 + status = okay;
 + #phy-cells = 1;
 +};
 +
 +Then the PHY can be used in other nodes such as:
 +
 +ehci@1258 {
 + status = okay;
 + phys = exynos_usbphy 2;
 + phy-names = hsic0;
 +};
 diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
 index a344f3d..bdf0fab 100644
 --- a/drivers/phy/Kconfig
 +++ b/drivers/phy/Kconfig
 @@ -14,7 +14,7 @@ config GENERIC_PHY
 API by which phy drivers can create PHY using the phy framework and
 phy users can obtain reference to the PHY. All the users of this
 framework should select this config.
 -
 +   

Stray white space added.

  config PHY_EXYNOS_MIPI_VIDEO
   tristate S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver
   help
 @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO
   help
 Support for Display Port PHY found on Samsung EXYNOS SoCs.
  
 +config PHY_EXYNOS_USB2

Wouldn't PHY_SAMSUNG_USB2 be better here?

 + tristate Samsung USB 2.0 PHY driver
 + help
 +   Enable this to support Samsung USB phy helper driver for 

Re: [PATCH v5 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver

2013-11-10 Thread Johan Hovold
On Fri, Nov 08, 2013 at 02:19:55PM +0100, Andrew Lunn wrote:
 Add a driver which supports the following Moxa USB to serial converters:
 *   2 ports : UPort 1250, UPort 1250I
 *   4 ports : UPort 1410, UPort 1450, UPort 1450I
 *   8 ports : UPort 1610-8, UPort 1650-8
 *  16 ports : UPort 1610-16, UPort 1650-16
 
 The UPORT devices don't directy fit the USB serial model. USB serial
 assumes a bulk in/out endpoint pair per serial port. Thus a dual port
 USB serial device is expected to have two bulk in/out pairs. The Moxa
 UPORT only has one pair for data transfer and places a header on each
 transfer over the endpoint indicating for which port the transfer
 relates to. There is a second endpoint pair for events, such as modem
 control lines changing state, setting baud rates etc. Again, a
 multiplexing header is used on these endpoints.
 
 Some ports need to have a kfifo explicitily allocated since the
 framework does not allocate one if there is no associated endpoints.
 The framework will however free it on unload of the module.
 
 All data transfers are made on port0, yet the locks are taken on PortN.
 urb-context points to PortN, even though the URB is for port0.
 
 Where possible, code from the generic driver is called. However
 mxuport_process_read_urb_data() is mostly a cut/paste of
 usb_serial_generic_process_read_urb().
 
 The driver will attempt to load firmware from userspace and compare
 the available version and the running version. If the available
 version is newer, it will be download into RAM of the device and
 started. This is optional and the driver appears to work O.K. with
 older firmware in the devices ROM.
 
 This driver is based on the MOXA driver and retains MOXAs copyright.
 
 Signed-off-by: Andrew Lunn and...@lunn.ch
 
 ---
 
 v1-v2
 
 Remove debug module parameter.
 Add missing \n to dev_dbg() strings.
 Consistent dev_dbg(%s - message in lowercase\n, __func__);
 Don't log failed allocations.
 Remove defensive checks for NULL mx_port.
 Use snprintf() instead of sprintf().
 Use port-icount and usb_serial_generic_get_icount().
 Break firmware download into smaller functions.
 Don't DMA to/from buffers on the stack.
 Use more of the generic write functions.
 Make use of port_probe() and port_remove().
 Indent the device structure.
 Minor cleanups in mxuport_close()
 __u8 - u8, __u16 - u16.
 remove mxuport_wait_until_sent() and implemented mxuport_tx_empty()
 Remove mxuport_write_room() and use the generic equivelent.
 Remove unneeded struct mx_uart_config members
 Make use of generic functions for receiving data and events.
 Name mxport consistently.
 Use usb_serial_generic_tiocmiwait()
 Let line discipline handle TCFLSH ioctl.
 Import contents of mxuport.h into mxuport.c
 Use shifts and ORs to create version number.
 Use port_number, not minor
 
 Clarify the meaning of interface in mx_uart_config.
 Remove buffer from mxuport_send_async_ctrl_urb().
 No need to multiply USB_CTRL_SET_TIMEOUT by HZ.
 Simplify buffer allocation and free.
 Swap (un)throttle to synchronous interface, remove async code.
 Pass usb_serial to mxuport_[recv|send]_ctrl_urb()
 Add missing {} on if else statement.
 Use unsigned char type, remove casts.
 Replace constants with defines.
 Add error handling when disabling HW flow control.
 Verify port is open before passing it recieved data
 Verify contents of received data  events.
 Remove broken support for alternative baud rates.
 Fix B0 and modem line handling. Remove uart_cfg structure.
 Remove hold_reason, which was set, but never used.
 s/is/if/
 Move parts of port open into port probe.
 Remove mxport-port and pass usb_serial_port instead.
 Remove mxport-flags which is no longer used.
 Allocate buffers before starting firmware download.
 Remove redundent detected debug message.
 Use devm_kzalloc() to simply memory handling.
 Remove repeated debug message when sending break.
 Implement mxuport_dtr_rts().
 Add locking around mcr_state.
 Remove unused lsr_state from mxuport_port.
 Support 485 via official IOCTL and sysfs.
 Use usleep_range() instread of mdelay().
 Simplify the comments.
 Use port0 as a template when setting up bulk out endpoints.
 Set bulk_in_size for unused endpoints to 0.
 Rebase to v3.12-rc2
 
 v3-v4
 Remove support for rs485 - No hardware to test it on.
 Fix formatting of multi line comments.
 Use HEADER_SIZE instead of hard coded 4.
 Use EIO instread of ECOMM.
 Drop use of spinlocks on  mxuport_{un}throttle().
 Remove unneeded parenthesis.
 Split mxuport_process_read_urb_event() into a number of functions.
 Check for zero bytes of data.
 Don't needlessly initialize variables.
 Fold mxuport_port_init() into mxuport_port_open()
 Correctly handle the on parameter in mxuport_dtr_rts().
 Drop mxuport_get_serial_info and the now empty ioctl handler.
 Splitup mxuport_set_termios() into smaller functions
 Make use of C_* macros for cflags.
 Terminate the firmware download on error.
 Remove some unneeded dev_dbg()'s.
 Let usb-serial 

Re: [PATCH v3 2/3] usb: ehci-s5p: Change to use phy provided by the generic phy framework

2013-11-10 Thread Tomasz Figa
Hi Kamil,

On Tuesday 05 of November 2013 17:13:20 Kamil Debski wrote:
 Change the phy provider used from the old usb phy specific to a new one
 using the generic phy framework.

I believe that until Exynos5250 also gets converted to the new PHY driver,
support for the old USB PHY API should remain in this driver.

As for the patch itself, please see my comments inline.

 
 Signed-off-by: Kamil Debski k.deb...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  drivers/usb/host/ehci-exynos.c |   34 +++---
  1 file changed, 11 insertions(+), 23 deletions(-)
 

This patch is changing a DT binding, but there is no update to the
documentation.

 diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
 index 8898c01..974001b 100644
 --- a/drivers/usb/host/ehci-exynos.c
 +++ b/drivers/usb/host/ehci-exynos.c
 @@ -19,12 +19,12 @@
  #include linux/module.h
  #include linux/of.h
  #include linux/of_gpio.h
 +#include linux/phy/phy.h
  #include linux/platform_device.h
  #include linux/usb/phy.h
  #include linux/usb/samsung_usb_phy.h
  #include linux/usb.h
  #include linux/usb/hcd.h
 -#include linux/usb/otg.h
  
  #include ehci.h
  
 @@ -44,8 +44,7 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver;
  
  struct exynos_ehci_hcd {
   struct clk *clk;
 - struct usb_phy *phy;
 - struct usb_otg *otg;
 + struct phy *phy;
  };
  
  #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd 
 *)(hcd_to_ehci(hcd)-priv)
 @@ -75,7 +74,8 @@ static int exynos_ehci_probe(struct platform_device *pdev)
   struct usb_hcd *hcd;
   struct ehci_hcd *ehci;
   struct resource *res;
 - struct usb_phy *phy;
 + struct phy *phy;
 + const char *phy_name;
   int irq;
   int err;
  
 @@ -98,12 +98,12 @@ static int exynos_ehci_probe(struct platform_device *pdev)
   return -ENOMEM;
   }
   exynos_ehci = to_exynos_ehci(hcd);
 -
   if (of_device_is_compatible(pdev-dev.of_node,
   samsung,exynos5440-ehci))
   goto skip_phy;
  
 - phy = devm_usb_get_phy(pdev-dev, USB_PHY_TYPE_USB2);
 + phy_name = of_get_property(pdev-dev.of_node, phy-names, NULL);
 + phy =  devm_phy_get(pdev-dev, phy_name);

This is definitely not the way you should parse PHY DT bindings. PHY names
are supposed to be fixed in the binding for each compatible value, which
means that you should call here devm_phy_get() with a static name.

Moreover, this driver needs one PHY per port, not just one PHY, so the
design needs to be completely changed and this patch is not really enough
to correctly support USB 2.0 on Exynos.

Best regards,
Tomasz

--
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 3/3] usb: s3c-hsotg: Use the new Exynos USB phy driver with the generic phy framework

2013-11-10 Thread Tomasz Figa
Hi Kamil,

This patch is changing a DT binding, but you haven't updated relevant
documentation. Please remember about it in next version.

On Tuesday 05 of November 2013 17:13:21 Kamil Debski wrote:
 Change the used phy driver to the new Exynos USB phy driver that uses the
 generic phy framework.
 
 Signed-off-by: Kamil Debski k.deb...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  drivers/usb/gadget/s3c-hsotg.c |   12 +++-
  1 file changed, 7 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
 index bb31262..dc7f20c 100644
 --- a/drivers/usb/gadget/s3c-hsotg.c
 +++ b/drivers/usb/gadget/s3c-hsotg.c
 @@ -31,6 +31,7 @@
  #include linux/regulator/consumer.h
  #include linux/of.h
  #include linux/of_platform.h
 +#include linux/phy/phy.h
  
  #include linux/usb/ch9.h
  #include linux/usb/gadget.h
 @@ -162,7 +163,7 @@ struct s3c_hsotg_ep {
  struct s3c_hsotg {
   struct device*dev;
   struct usb_gadget_driver *driver;
 - struct usb_phy  *phy;
 + struct phy   *phy;
   struct s3c_hsotg_plat*plat;
  
   spinlock_t  lock;
 @@ -2905,9 +2906,10 @@ static void s3c_hsotg_phy_enable(struct s3c_hsotg 
 *hsotg)
   dev_dbg(hsotg-dev, pdev 0x%p\n, pdev);
  
   if (hsotg-phy)
 - usb_phy_init(hsotg-phy);
 + phy_power_on(hsotg-phy);
   else if (hsotg-plat-phy_init)
   hsotg-plat-phy_init(pdev, hsotg-plat-phy_type);
 +

nit: Stray blank line.

Best regards,
Tomasz

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


[PATCH 2/2] check quirk to pad epout buf size when not aligned to maxpacketsize

2013-11-10 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com

Check gadget.quirk_ep_out_aligned_size to decide if buffer size requires
to be aligned to maxpacketsize of an out endpoint.  ffs_epfile_io() needs
to pad epout buffer to match above condition if quirk is found.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 I'm wondering whether the len should be aligned down rather then up.
 This would have it's own problems, but maybe better then
 a possibility of silently dropping data.

 drivers/usb/gadget/f_fs.c | 21 ++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index f875f26..ea0b8ba 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -753,8 +753,9 @@ static ssize_t ffs_epfile_io(struct file *file,
 char __user *buf, size_t len, int read)
 {
struct ffs_epfile *epfile = file-private_data;
+   struct usb_gadget *gadget = epfile-ffs-gadget;
struct ffs_ep *ep;
-   ssize_t ret;
+   ssize_t ret, data_len;
char *data;
int halt;
 
@@ -787,6 +788,13 @@ static ssize_t ffs_epfile_io(struct file *file,
 
/* Allocate  copy */
if (!halt) {
+   /*
+* Controller requires buffer size to be aligned to
+* maxpacketsize of an out endpoint.
+*/
+   data_len = read  gadget-quirk_ep_out_aligned_size ?
+   usb_ep_align_maxpacketsize(ep-ep, len) : len;
+
data = kmalloc(len, GFP_KERNEL);
if (unlikely(!data))
return -ENOMEM;
@@ -824,7 +832,7 @@ static ssize_t ffs_epfile_io(struct file *file,
req-context  = done;
req-complete = ffs_epfile_io_complete;
req-buf  = data;
-   req-length   = len;
+   req-length   = data_len;
 
ret = usb_ep_queue(ep-ep, req, GFP_ATOMIC);
 
@@ -836,9 +844,16 @@ static ssize_t ffs_epfile_io(struct file *file,
ret = -EINTR;
usb_ep_dequeue(ep-ep, req);
} else {
+   /*
+* XXX We may end up silently droping data here.
+* Since data_len (i.e. req-length) may be bigger
+* than len (after being rounded up to maxpacketsize),
+* we may end up with more data then user space has
+* space for.
+*/
ret = ep-status;
if (read  ret  0 
-   unlikely(copy_to_user(buf, data, ret)))
+   unlikely(copy_to_user(buf, data, min(ret, len
ret = -EFAULT;
}
}
-- 
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


[PATCH 1/2] usb: gadget: f_fs: remove loop from I/O function

2013-11-10 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com

When endpoint changes (due to it being disabled or alt setting changed),
mimic the action as if the change happened after the request has been
queued, instead of retrying with the new endpoint.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 drivers/usb/gadget/f_fs.c | 94 +--
 1 file changed, 41 insertions(+), 53 deletions(-)

diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 44cf775..f875f26 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -754,74 +754,61 @@ static ssize_t ffs_epfile_io(struct file *file,
 {
struct ffs_epfile *epfile = file-private_data;
struct ffs_ep *ep;
-   char *data = NULL;
ssize_t ret;
+   char *data;
int halt;

-   goto first_try;
-   do {
-   spin_unlock_irq(epfile-ffs-eps_lock);
-   mutex_unlock(epfile-mutex);
+   /* Are we still active? */
+   if (WARN_ON(epfile-ffs-state != FFS_ACTIVE)) {
+   ret = -ENODEV;
+   goto error;
+   }

-first_try:
-   /* Are we still active? */
-   if (WARN_ON(epfile-ffs-state != FFS_ACTIVE)) {
-   ret = -ENODEV;
+   /* Wait for endpoint to be enabled */
+   ep = epfile-ep;
+   if (!ep) {
+   if (file-f_flags  O_NONBLOCK) {
+   ret = -EAGAIN;
goto error;
}

-   /* Wait for endpoint to be enabled */
-   ep = epfile-ep;
-   if (!ep) {
-   if (file-f_flags  O_NONBLOCK) {
-   ret = -EAGAIN;
-   goto error;
-   }
-
-   if (wait_event_interruptible(epfile-wait,
-(ep = epfile-ep))) {
-   ret = -EINTR;
-   goto error;
-   }
-   }
-
-   /* Do we halt? */
-   halt = !read == !epfile-in;
-   if (halt  epfile-isoc) {
-   ret = -EINVAL;
+   if (wait_event_interruptible(epfile-wait, (ep = epfile-ep))) {
+   ret = -EINTR;
goto error;
}
+   }

-   /* Allocate  copy */
-   if (!halt  !data) {
-   data = kzalloc(len, GFP_KERNEL);
-   if (unlikely(!data))
-   return -ENOMEM;
+   /* Do we halt? */
+   halt = !read == !epfile-in;
+   if (halt  epfile-isoc) {
+   ret = -EINVAL;
+   goto error;
+   }

-   if (!read 
-   unlikely(__copy_from_user(data, buf, len))) {
-   ret = -EFAULT;
-   goto error;
-   }
-   }
+   /* Allocate  copy */
+   if (!halt) {
+   data = kmalloc(len, GFP_KERNEL);
+   if (unlikely(!data))
+   return -ENOMEM;

-   /* We will be using request */
-   ret = ffs_mutex_lock(epfile-mutex,
-file-f_flags  O_NONBLOCK);
-   if (unlikely(ret))
+   if (!read  unlikely(copy_from_user(data, buf, len))) {
+   ret = -EFAULT;
goto error;
+   }
+   }

-   /*
-* We're called from user space, we can use _irq rather then
-* _irqsave
-*/
-   spin_lock_irq(epfile-ffs-eps_lock);
+   /* We will be using request */
+   ret = ffs_mutex_lock(epfile-mutex, file-f_flags  O_NONBLOCK);
+   if (unlikely(ret))
+   goto error;
+   spin_lock_irq(epfile-ffs-eps_lock);

-   /*
-* While we were acquiring mutex endpoint got disabled
-* or changed?
-*/
-   } while (unlikely(epfile-ep != ep));
+   /* While we were acquiring mutex endpoint got disabled or changed. */
+   if (epfile-ep != ep) {
+   ret = -ESHUTDOWN;
+   spin_unlock_irq(epfile-ffs-eps_lock);
+   goto error_unlock;
+   }

/* Halt */
if (unlikely(halt)) {
@@ -856,6 +843,7 @@ first_try:
}
}

+error_unlock:
mutex_unlock(epfile-mutex);
 error:
kfree(data);
--
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 v5 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver

2013-11-10 Thread Andrew Lunn
Hi Johan

Thanks for the quick response.

  +static int mxuport_dtr(struct usb_serial_port *port, int on)
  +{
  +   struct mxuport_port *mxport = usb_get_serial_port_data(port);
  +   int err;
  +
  +   mutex_lock(mxport-mutex);
  +
  +   if (on)
  +   mxport-mcr_state |= UART_MCR_DTR;
  +   else
  +   mxport-mcr_state = ~UART_MCR_DTR;
  +
  +   err = mxuport_set_mcr(port, mxport-mcr_state);
  +
  +   mutex_unlock(mxport-mutex);
  +
  +   return err;
  +}
 
 You should probably use SET_DTR (and rename the function
 mxuport_set_dtr).

I considered mxuport_set_dtr(), but it does not really fit with the
usb-serial naming. tty_port_operations has dtr_rts, not set_dtr_rts.
usb_serial_driver also has dtr_rts, not set_dtr_rts.

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


Re: [PATCH v5 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver

2013-11-10 Thread Johan Hovold
On Sun, Nov 10, 2013 at 05:58:15PM +0100, Andrew Lunn wrote:
   +static int mxuport_dtr(struct usb_serial_port *port, int on)
   +{
   + struct mxuport_port *mxport = usb_get_serial_port_data(port);
   + int err;
   +
   + mutex_lock(mxport-mutex);
   +
   + if (on)
   + mxport-mcr_state |= UART_MCR_DTR;
   + else
   + mxport-mcr_state = ~UART_MCR_DTR;
   +
   + err = mxuport_set_mcr(port, mxport-mcr_state);
   +
   + mutex_unlock(mxport-mutex);
   +
   + return err;
   +}
  
  You should probably use SET_DTR (and rename the function
  mxuport_set_dtr).
 
 I considered mxuport_set_dtr(), but it does not really fit with the
 usb-serial naming. tty_port_operations has dtr_rts, not set_dtr_rts.
 usb_serial_driver also has dtr_rts, not set_dtr_rts.

You're right about that, but the tty (usb-serial) operation can remain
the oddly named one. I still prefer that the local helpers have more
descriptive names, and muport_set_dtr and mxuport_set_rts would also be
consistent with mxuport_set_mcr.

Johan
--
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: phy: remove dead code

2013-11-10 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com

---
 drivers/usb/phy/phy-am335x.c  | 2 --
 drivers/usb/phy/phy-generic.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
index 6370e50..48d41ab 100644
--- a/drivers/usb/phy/phy-am335x.c
+++ b/drivers/usb/phy/phy-am335x.c
@@ -66,8 +66,6 @@ static int am335x_phy_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, am_phy);

return 0;
-
-   return ret;
 }

 static int am335x_phy_remove(struct platform_device *pdev)
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index fce3a9e..db4fc22 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -271,8 +271,6 @@ static int usb_phy_gen_xceiv_probe(struct platform_device 
*pdev)
platform_set_drvdata(pdev, nop);

return 0;
-
-   return err;
 }

 static int usb_phy_gen_xceiv_remove(struct platform_device *pdev)
--
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


[PATCH] drivers: usb: goku: remove unused argument

2013-11-10 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com

The stop_activity function never uses driver argument (even
though it modifies it) and thus it is safe to remove it.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 drivers/usb/gadget/goku_udc.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/goku_udc.c b/drivers/usb/gadget/goku_udc.c
index f827680..7eda9fd 100644
--- a/drivers/usb/gadget/goku_udc.c
+++ b/drivers/usb/gadget/goku_udc.c
@@ -1350,16 +1350,12 @@ static int goku_udc_start(struct usb_gadget *g,
return 0;
 }
 
-static void
-stop_activity(struct goku_udc *dev, struct usb_gadget_driver *driver)
+static void stop_activity(struct goku_udc *dev)
 {
unsignedi;
 
DBG (dev, %s\n, __func__);
 
-   if (dev-gadget.speed == USB_SPEED_UNKNOWN)
-   driver = NULL;
-
/* disconnect gadget driver after quiesceing hw and the driver */
udc_reset (dev);
for (i = 0; i  4; i++)
@@ -1377,7 +1373,7 @@ static int goku_udc_stop(struct usb_gadget *g,
 
spin_lock_irqsave(dev-lock, flags);
dev-driver = NULL;
-   stop_activity(dev, driver);
+   stop_activity(dev);
spin_unlock_irqrestore(dev-lock, flags);
 
return 0;
@@ -1521,7 +1517,7 @@ rescan:
if (unlikely(stat  INT_DEVWIDE)) {
if (stat  INT_SYSERROR) {
ERROR(dev, system error\n);
-   stop_activity(dev, dev-driver);
+   stop_activity(dev);
stat = 0;
handled = 1;
// FIXME have a neater way to prevent re-enumeration
@@ -1536,7 +1532,7 @@ rescan:
} else {
DBG(dev, disconnect\n);
if (dev-gadget.speed == USB_SPEED_FULL)
-   stop_activity(dev, dev-driver);
+   stop_activity(dev);
dev-ep0state = EP0_DISCONNECT;
dev-int_enable = INT_DEVWIDE;
writel(dev-int_enable, dev-regs-int_enable);
-- 
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


[pandaboard] OTG port not working

2013-11-10 Thread Tobias Jakobi
Since suggested by gregkh, here a CC.

Greets,
Tobias

--
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: [pandaboard] OTG port not working

2013-11-10 Thread Alan Ott

On 11/10/2013 06:14 PM, Tobias Jakobi wrote:

Since suggested by gregkh, here a CC.


Typically you're going to get a better response on mailing lists if you 
give a little more information. Start with the following:


1. What kernel are you using?
2. Which gadget are you trying to use (assumed from OTG)?
3. What procedure are you using?
4. What is the expected behavior?
5. What is the acutal behavior?
6. What do you see in the logs (what can you learn from higher debugging 
levels)?


Alan.

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


Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

2013-11-10 Thread Chris Ruehl

Hi Alexander,

On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:

This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.

Signed-off-by: Alexander Shiyanshc_w...@mail.ru
---
  drivers/usb/chipidea/usbmisc_imx.c | 42 ++
  1 file changed, 42 insertions(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c
index 8a1094b..4381c5a6 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -21,6 +21,10 @@
  #define MX25_USB_PHY_CTRL_OFFSET  0x08
  #define MX25_BM_EXTERNAL_VBUS_DIVIDER BIT(23)

+#define MX27_H1_PM_BIT BIT(8)
+#define MX27_H2_PM_BIT BIT(16)
+#define MX27_OTG_PM_BITBIT(24)
+
  #define MX53_USB_OTG_PHY_CTRL_0_OFFSET0x08
  #define MX53_USB_UH2_CTRL_OFFSET  0x14
  #define MX53_USB_UH3_CTRL_OFFSET  0x18
@@ -68,6 +72,36 @@ static int usbmisc_imx25_post(struct imx_usbmisc_data *data)
return 0;
  }

+static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
+{
+   unsigned long flags;
+   u32 val;
+
+   switch (data-index) {
+   case 0:
+   val = MX27_OTG_PM_BIT;
+   break;
+   case 1:
+   val = MX27_H1_PM_BIT;
+   break;
+   case 2:
+   val = MX27_H2_PM_BIT;
+   break;
+   default:
+   return -EINVAL;
+   };
+


From my understanding this can not work, the usbmisc-base not point into the
usb control register (USB_CTRL). Reference manual 30.5.1.1 says
BASE + 0x600
you must add the offset to the readl instruction.


+   spin_lock_irqsave(usbmisc-lock, flags);
+   if (data-disable_oc)
+   val = readl(usbmisc-base) | val;


else part not needed, the registers bits are set to 0 (reset)
the function is called on start-up once only, right?!


+   else
+   val = readl(usbmisc-base)  ~val;
+   writel(val, usbmisc-base);
+   spin_unlock_irqrestore(usbmisc-lock, flags);
+
+   return 0;
+}
+
  static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
  {
void __iomem *reg = NULL;
@@ -128,6 +162,10 @@ static const struct usbmisc_ops imx25_usbmisc_ops = {
.post = usbmisc_imx25_post,
  };

+static const struct usbmisc_ops imx27_usbmisc_ops = {
+   .init = usbmisc_imx27_init,
+};
+
  static const struct usbmisc_ops imx53_usbmisc_ops = {
.init = usbmisc_imx53_init,
  };
@@ -162,6 +200,10 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = {
.data =imx25_usbmisc_ops,
},
{
+   .compatible = fsl,imx27-usbmisc,
+   .data =imx27_usbmisc_ops,
+   },
+   {
.compatible = fsl,imx53-usbmisc,
.data =imx53_usbmisc_ops,
},




Regards
Chris
--
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] usbnet: fix race condition caused spinlock bad magic issue

2013-11-10 Thread wangbiao
From: wang, biao biao.w...@intel.com
Date: Mon, 11 Nov 2013 10:23:40 +0800
Subject: [PATCH] usbnet: fix race condition caused spinlock bad magic issue

there is race between usbnet_terminate_urbs and usbnet_bh, when
unlink_wakeup used in usbnet_bh, it may be already freed and used by
other function as unlink_wakeup was a local var on stack.

btw, dev-wait should be judged again before use it as there is race
too.

Signed-off-by: wang, biao biao.w...@intel.com
---
 drivers/net/usb/usbnet.c |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 90a429b..22fc27f 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -86,6 +86,7 @@ static const char driver_name [] = usbnet;
 
 /* use ethtool to change the level for any given device */
 static int msg_level = -1;
+static wait_queue_head_t unlink_wakeup;
 module_param (msg_level, int, 0);
 MODULE_PARM_DESC (msg_level, Override default message level);
 
@@ -761,7 +762,6 @@ EXPORT_SYMBOL_GPL(usbnet_unlink_rx_urbs);
 // precondition: never called in_interrupt
 static void usbnet_terminate_urbs(struct usbnet *dev)
 {
-   DECLARE_WAIT_QUEUE_HEAD_ONSTACK(unlink_wakeup);
DECLARE_WAITQUEUE(wait, current);
int temp;
 
@@ -1448,8 +1448,10 @@ static void usbnet_bh (unsigned long param)
 
// waiting for all pending urbs to complete?
if (dev-wait) {
+   wait_queue_head_t *wait_d = dev-wait;
if ((dev-txq.qlen + dev-rxq.qlen + dev-done.qlen) == 0) {
-   wake_up (dev-wait);
+   if (wait_d)
+   wake_up(wait_d);
}
 
// or are we maybe short a few urbs?
@@ -1602,6 +1604,7 @@ usbnet_probe (struct usb_interface *udev, const struct 
usb_device_id *prod)
init_timer (dev-delay);
mutex_init (dev-phy_mutex);
mutex_init(dev-interrupt_mutex);
+   init_waitqueue_head(unlink_wakeup);
dev-interrupt_count = 0;
 
dev-net = net;
-- 
1.7.0.4



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


Re: [PATCH 2/2] check quirk to pad epout buf size when not aligned to maxpacketsize

2013-11-10 Thread David Cohen
Hi Michal,

On 11/10/2013 08:50 AM, Michal Nazarewicz wrote:
 From: Michal Nazarewicz min...@mina86.com
 
 Check gadget.quirk_ep_out_aligned_size to decide if buffer size requires
 to be aligned to maxpacketsize of an out endpoint.  ffs_epfile_io() needs
 to pad epout buffer to match above condition if quirk is found.
 
 Signed-off-by: Michal Nazarewicz min...@mina86.com
 ---
  I'm wondering whether the len should be aligned down rather then up.
  This would have it's own problems, but maybe better then
  a possibility of silently dropping data.
 
  drivers/usb/gadget/f_fs.c | 21 ++---
  1 file changed, 18 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
 index f875f26..ea0b8ba 100644
 --- a/drivers/usb/gadget/f_fs.c
 +++ b/drivers/usb/gadget/f_fs.c
 @@ -753,8 +753,9 @@ static ssize_t ffs_epfile_io(struct file *file,
char __user *buf, size_t len, int read)
  {
   struct ffs_epfile *epfile = file-private_data;
 + struct usb_gadget *gadget = epfile-ffs-gadget;
   struct ffs_ep *ep;
 - ssize_t ret;
 + ssize_t ret, data_len;
   char *data;
   int halt;
  
 @@ -787,6 +788,13 @@ static ssize_t ffs_epfile_io(struct file *file,
  
   /* Allocate  copy */
   if (!halt) {
 + /*
 +  * Controller requires buffer size to be aligned to
 +  * maxpacketsize of an out endpoint.
 +  */
 + data_len = read  gadget-quirk_ep_out_aligned_size ?
 + usb_ep_align_maxpacketsize(ep-ep, len) : len;
 +
   data = kmalloc(len, GFP_KERNEL);

Shouldn't this kmalloc() allocate 'data_len' bytes, instead of 'len'?

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


Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

2013-11-10 Thread Alexander Shiyan
 Hi Alexander,
 
 On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
  This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
 
  Signed-off-by: Alexander Shiyanshc_w...@mail.ru
  ---
drivers/usb/chipidea/usbmisc_imx.c | 42 
  ++
1 file changed, 42 insertions(+)
 
  diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
  b/drivers/usb/chipidea/usbmisc_imx.c
...
  +static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
  +{
  +   unsigned long flags;
  +   u32 val;
  +
  +   switch (data-index) {
  +   case 0:
  +   val = MX27_OTG_PM_BIT;
  +   break;
  +   case 1:
  +   val = MX27_H1_PM_BIT;
  +   break;
  +   case 2:
  +   val = MX27_H2_PM_BIT;
  +   break;
  +   default:
  +   return -EINVAL;
  +   };
  +
 
  From my understanding this can not work, the usbmisc-base not point into the
 usb control register (USB_CTRL). Reference manual 30.5.1.1 says
 BASE + 0x600
 you must add the offset to the readl instruction.

Why not work?
usbotg: usb@10024000
usbh1: usb@10024200
usbh2: usb@10024400
usbmisc: usbmisc@10024600
So, offset to USB_CTRL should already be defined by DTS.


  +   spin_lock_irqsave(usbmisc-lock, flags);
  +   if (data-disable_oc)
  +   val = readl(usbmisc-base) | val;
 
 else part not needed, the registers bits are set to 0 (reset)
 the function is called on start-up once only, right?!
 
  +   else
  +   val = readl(usbmisc-base)  ~val;
  +   writel(val, usbmisc-base);
  +   spin_unlock_irqrestore(usbmisc-lock, flags);
  +
  +   return 0;
  +}

Bit can be set/cleared wrongly by the bootloader, it is not a big
overhead to set it in proper state.

...

---


Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

2013-11-10 Thread Chris Ruehl


On Monday, November 11, 2013 12:45 PM, Alexander Shiyan wrote:

Hi Alexander,

On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:

This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.

Signed-off-by: Alexander Shiyanshc_w...@mail.ru
---
   drivers/usb/chipidea/usbmisc_imx.c | 42 
++
   1 file changed, 42 insertions(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c

...

+static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
+{
+   unsigned long flags;
+   u32 val;
+
+   switch (data-index) {
+   case 0:
+   val = MX27_OTG_PM_BIT;
+   break;
+   case 1:
+   val = MX27_H1_PM_BIT;
+   break;
+   case 2:
+   val = MX27_H2_PM_BIT;
+   break;
+   default:
+   return -EINVAL;
+   };
+


   From my understanding this can not work, the usbmisc-base not point into the
usb control register (USB_CTRL). Reference manual 30.5.1.1 says
BASE + 0x600
you must add the offset to the readl instruction.


Why not work?
usbotg: usb@10024000
usbh1: usb@10024200
usbh2: usb@10024400
usbmisc: usbmisc@10024600
So, offset to USB_CTRL should already be defined by DTS.


in the usbmisc_imx_probe() the base pointer is loaded from

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
data-base = devm_ioremap_resource(pdev-dev, res);

(and I did not see any of_ operations)

usbmisc = data;

 base is set to 0x10024000

when I look around all other functions init functions did a offset calculation.





+   spin_lock_irqsave(usbmisc-lock, flags);
+   if (data-disable_oc)
+   val = readl(usbmisc-base) | val;


else part not needed, the registers bits are set to 0 (reset)
the function is called on start-up once only, right?!


+   else
+   val = readl(usbmisc-base)   ~val;
+   writel(val, usbmisc-base);
+   spin_unlock_irqrestore(usbmisc-lock, flags);
+
+   return 0;
+}


Bit can be set/cleared wrongly by the bootloader, it is not a big
overhead to set it in proper state.


ohh, yes that true!


...


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


Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

2013-11-10 Thread Alexander Shiyan
 On Monday, November 11, 2013 12:45 PM, Alexander Shiyan wrote:
  Hi Alexander,
 
  On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
  This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
 
  Signed-off-by: Alexander Shiyanshc_w...@mail.ru
  ---
 drivers/usb/chipidea/usbmisc_imx.c | 42 
  ++
 1 file changed, 42 insertions(+)
 
  diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
  b/drivers/usb/chipidea/usbmisc_imx.c
  ...
  +static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
  +{
  + unsigned long flags;
  + u32 val;
  +
  + switch (data-index) {
  + case 0:
  + val = MX27_OTG_PM_BIT;
  + break;
  + case 1:
  + val = MX27_H1_PM_BIT;
  + break;
  + case 2:
  + val = MX27_H2_PM_BIT;
  + break;
  + default:
  + return -EINVAL;
  + };
  +
 
 From my understanding this can not work, the usbmisc-base not point 
  into the
  usb control register (USB_CTRL). Reference manual 30.5.1.1 says
  BASE + 0x600
  you must add the offset to the readl instruction.
 
  Why not work?
  usbotg: usb@10024000
  usbh1: usb@10024200
  usbh2: usb@10024400
  usbmisc: usbmisc@10024600
  So, offset to USB_CTRL should already be defined by DTS.
 
 in the usbmisc_imx_probe() the base pointer is loaded from
 
 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 data-base = devm_ioremap_resource(pdev-dev, res);
 
 (and I did not see any of_ operations)

Yes, and this is an address of usbmisc node, not otg.

 usbmisc = data;
 
   base is set to 0x10024000
 
 when I look around all other functions init functions did a offset 
 calculation.

Can you point me on this?
For example, for i.MX5 CPUs we calculate only offset to PHY_CTRL_X register
relative to basic offset 0x800, which is already defined in DTS.

...

---
N�r��yb�X��ǧv�^�)޺{.n�+{��^n�r���z���h����G���h�(�階�ݢj���m��z�ޖ���f���h���~�m�

Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

2013-11-10 Thread Chris Ruehl

On Monday, November 11, 2013 01:47 PM, Alexander Shiyan wrote:

On Monday, November 11, 2013 12:45 PM, Alexander Shiyan wrote:

Hi Alexander,

On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:

This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.

Signed-off-by: Alexander Shiyanshc_w...@mail.ru
---
drivers/usb/chipidea/usbmisc_imx.c | 42 
++
1 file changed, 42 insertions(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c

...

+static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
+{
+   unsigned long flags;
+   u32 val;
+
+   switch (data-index) {
+   case 0:
+   val = MX27_OTG_PM_BIT;
+   break;
+   case 1:
+   val = MX27_H1_PM_BIT;
+   break;
+   case 2:
+   val = MX27_H2_PM_BIT;
+   break;
+   default:
+   return -EINVAL;
+   };
+


 From my understanding this can not work, the usbmisc-base not point into 
the
usb control register (USB_CTRL). Reference manual 30.5.1.1 says
BASE + 0x600
you must add the offset to the readl instruction.


Why not work?
usbotg: usb@10024000
usbh1: usb@10024200
usbh2: usb@10024400
usbmisc: usbmisc@10024600
So, offset to USB_CTRL should already be defined by DTS.


in the usbmisc_imx_probe() the base pointer is loaded from

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
data-base = devm_ioremap_resource(pdev-dev, res);

(and I did not see any of_ operations)


Yes, and this is an address of usbmisc node, not otg.


Sorry, you are right. I misunderstood because I didn't see any of sample DTS 
files define usbmisc yet.


At this point might be good to patch the imx27.dtsi with the usb defines.

--- a/arch/arm/boot/dts/imx27.dtsi
+++ b/arch/arm/boot/dts/imx27.dtsi
@@ -30,6 +30,9 @@
spi0 = cspi1;
spi1 = cspi2;
spi2 = cspi3;
+   usb0 = usbotg;
+   usb1 = usbh1;
+   usb2 = usbh2;
};

@@ -404,6 +419,44 @@
iram = iram;
};

+   usbotg: usb@10024000 {
+   compatible = fsl,imx27-usb;
+   reg = 0x10024000 0x200;
+   interrupts = 56;
+   clocks = clks 75, clks 62;
+   clock-names = ipg, ahb;
+   dr_mode = host;
+   phy_type = ulpi;
+   status = disabled;
+   };
+
+   usbh1: usb@10024200 {
+   compatible = fsl,imx27-usb;
+   reg = 0x10024200 0x200;
+   interrupts = 54;
+   clocks = clks 75, clks 62;
+   clock-names = ipg, ahb;
+   dr_mode = host;
+   phy_type = serial;
+   status = disabled;
+   };
+
+   usbh2: usb@10024400 {
+   compatible = fsl,imx27-usb;
+   reg = 0x10024400 0x200;
+   interrupts = 55;
+   clocks = clks 75, clks 62;
+   clock-names = ipg, ahb;
+   dr_mode = host;
+   phy_type = ulpi;
+   status = disabled;
+   };
+
+   usbmisc: usbmisc@10024600 {
+   compatible = fsl,imx27-usb;
+   reg = 0x10024600 0x4;
+   };
+

Regards
Chris



usbmisc = data;

   base is set to 0x10024000

when I look around all other functions init functions did a offset calculation.


Can you point me on this?
For example, for i.MX5 CPUs we calculate only offset to PHY_CTRL_X register
relative to basic offset 0x800, which is already defined in DTS.

...



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


Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

2013-11-10 Thread Alexander Shiyan
Hello.

  On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
  This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
 
  Signed-off-by: Alexander Shiyanshc_w...@mail.ru
  ---
  drivers/usb/chipidea/usbmisc_imx.c | 42 
  ++
  1 file changed, 42 insertions(+)
...
 At this point might be good to patch the imx27.dtsi with the usb defines.
...

I have a working configuration for i.MX27 USB, but I prefer to make a few more
tests before the addition of definitions in DTS. This will be a next step.
Thanks.

---