Re: [PATCH 2/2] input: usb: hid: Add quirk for Freescale i.MX28 ROM recovery

[Marek Vasut]

Dear Jiri Kosina,

> On Sun, 5 Aug 2012, Marek Vasut wrote:
> > > diff --git a/drivers/hid/usbhid/hid-quirks.c
> > > b/drivers/hid/usbhid/hid-quirks.c index 903eef3..6a09570 100644
> > > --- a/drivers/hid/usbhid/hid-quirks.c
> > > +++ b/drivers/hid/usbhid/hid-quirks.c
> > > @@ -101,6 +101,7 @@ static const struct hid_blacklist {
> > > 
> > >   { USB_VENDOR_ID_SIGMA_MICRO, USB_DEVICE_ID_SIGMA_MICRO_KEYBOARD,
> > > 
> > > HID_QUIRK_NO_INIT_REPORTS }, { USB_VENDOR_ID_KYE,
> > > USB_DEVICE_ID_KYE_MOUSEPEN_I608X, HID_QUIRK_MULTI_INPUT }, {
> > > USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X,
> > > HID_QUIRK_MULTI_INPUT }, +{ USB_VENDOR_ID_FREESCALE,
> > > USB_DEVICE_ID_FREESCALE_MX28,
> > > HID_QUIRK_NOGET }, { 0, 0 }
> > 
> > OT: btw. the comment mentioned the list should be sorted, yet it's not so
> > for a while. Maybe a sorting patch would be favored?
> 
> I have fixed the ordering and applied. Thanks,

Can we possibly include these changes into -stable releases too ? I'm not 
exactly sure how it works for linux-input, they might be applied already, if 
that's the case, sorry for the noise.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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] input: usb: hid: Add quirk for Freescale i.MX28 ROM recovery

[Marek Vasut]

Dear Jiri Kosina,

> On Wed, 21 Nov 2012, Marek Vasut wrote:
> > > > > diff --git a/drivers/hid/usbhid/hid-quirks.c
> > > > > b/drivers/hid/usbhid/hid-quirks.c index 903eef3..6a09570 100644
> > > > > --- a/drivers/hid/usbhid/hid-quirks.c
> > > > > +++ b/drivers/hid/usbhid/hid-quirks.c
> > > > > @@ -101,6 +101,7 @@ static const struct hid_blacklist {
> > > > > 
> > > > >   { USB_VENDOR_ID_SIGMA_MICRO, USB_DEVICE_ID_SIGMA_MICRO_KEYBOARD,
> > > > > 
> > > > > HID_QUIRK_NO_INIT_REPORTS }, { USB_VENDOR_ID_KYE,
> > > > > USB_DEVICE_ID_KYE_MOUSEPEN_I608X, HID_QUIRK_MULTI_INPUT }, {
> > > > > USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X,
> > > > > HID_QUIRK_MULTI_INPUT }, +{ USB_VENDOR_ID_FREESCALE,
> > > > > USB_DEVICE_ID_FREESCALE_MX28,
> > > > > HID_QUIRK_NOGET }, { 0, 0 }
> > > > 
> > > > OT: btw. the comment mentioned the list should be sorted, yet it's
> > > > not so for a while. Maybe a sorting patch would be favored?
> > > 
> > > I have fixed the ordering and applied. Thanks,
> > 
> > Can we possibly include these changes into -stable releases too ? I'm not
> > exactly sure how it works for linux-input, they might be applied already,
> > if that's the case, sorry for the noise.
> 
> I have just submitted it for stable, thanks for the reminder.

Thanks!

Can we push this one too?
[PATCH 1/2] input: usb: hid: Bump maximum global item tag report size to 128 
bytes

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 3/7] usb: chipidea: add otg id switch and vbus connect/disconnect detect

[Marek Vasut]

Dear Peter Chen,

[...]
> +
> +#define CI_VBUS_STABLE_TIMEOUT 500

Shall we not change this to static const int instead ?

[...]

> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -1371,6 +1371,7 @@ static int ci13xxx_vbus_session(struct usb_gadget
> *_gadget, int is_active) pm_runtime_get_sync(&_gadget->dev);
>   hw_device_reset(ci, USBMODE_CM_DC);
>   hw_device_state(ci, ci->ep0out->qh.dma);
> + dev_dbg(ci->dev, "Connected to host\n");
>   } else {
>   hw_device_state(ci, 0);
>   if (ci->platdata->notify_event)
> @@ -1378,6 +1379,7 @@ static int ci13xxx_vbus_session(struct usb_gadget
> *_gadget, int is_active) CI13XXX_CONTROLLER_STOPPED_EVENT);
>   _gadget_stop_activity(&ci->gadget);
>   pm_runtime_put_sync(&_gadget->dev);
> + dev_dbg(ci->dev, "disconnected from host\n");

Keep the capital letters at the begining of the sentence consistent -- either 
start with capital D here or fix the capital C above.

>   }
>   }
--
To unsubscribe from this list: send the line "unsubscribe 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 3/7] usb: chipidea: add otg id switch and vbus connect/disconnect detect

[Marek Vasut]

Dear Peter Chen,

> On Thu, Dec 27, 2012 at 08:21:55AM +0100, Marek Vasut wrote:
> > Dear Peter Chen,
> > 
> > [...]
> > 
> > > +
> > > +#define CI_VBUS_STABLE_TIMEOUT 500
> > 
> > Shall we not change this to static const int instead ?
> 
> Is it a must?
> I find the similar at drivers/usb/core/hub.c
[...]

No it's not. I was just pondering now that it's a local constant, it might just 
be like that.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: gadget: Repair DSTS register decoding

[Marek Vasut]

The "enumspd" field is located in register DSTS[2:1], but the current
set of macros define the possible values without the necessary offset.
This in turn causes incorrect detection of gadget link partner speed
in dwc2_hsotg_irq_enumdone() .

The rest of the macros in dwc2/hw.h always define possible values of
fields with offset, so keep this consistent, add the necessary offset
and fix the problem with speed detection.

Signed-off-by: Marek Vasut 
Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: John Youn 
---
 drivers/usb/dwc2/hw.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index 553f246..0fd5683 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -452,10 +452,10 @@
 #define DSTS_ERRATICERR(1 << 3)
 #define DSTS_ENUMSPD_MASK  (0x3 << 1)
 #define DSTS_ENUMSPD_SHIFT 1
-#define DSTS_ENUMSPD_HS0
-#define DSTS_ENUMSPD_FS1
-#define DSTS_ENUMSPD_LS2
-#define DSTS_ENUMSPD_FS48  3
+#define DSTS_ENUMSPD_HS(0 << 1)
+#define DSTS_ENUMSPD_FS(1 << 1)
+#define DSTS_ENUMSPD_LS(2 << 1)
+#define DSTS_ENUMSPD_FS48  (3 << 1)
 #define DSTS_SUSPSTS   (1 << 0)
 
 #define DIEPMSKHSOTG_REG(0x810)
-- 
2.6.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] usb: dwc2: gadget: Repair DSTS register decoding

[Marek Vasut]

On Friday, December 18, 2015 at 12:45:56 AM, John Youn wrote:
> On 12/17/2015 2:49 PM, Marek Vasut wrote:
> > The "enumspd" field is located in register DSTS[2:1], but the current
> > set of macros define the possible values without the necessary offset.
> > This in turn causes incorrect detection of gadget link partner speed
> > in dwc2_hsotg_irq_enumdone() .
> > 
> > The rest of the macros in dwc2/hw.h always define possible values of
> > fields with offset, so keep this consistent, add the necessary offset
> > and fix the problem with speed detection.
> 
> Looks like only single bits are shifted and bitfields define a SHIFT
> and MASK using unshifted values. Could you use the existing
> DSTS_ENUMSPD_SHIFT macro at the place of use. Less changes that way
> too.

Sure, I can do that.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: dwc2: gadget: Repair DSTS register decoding

[Marek Vasut]

The "enumspd" field is located in register DSTS[2:1], but the code
which checks the bitfield does not shift the value accordingly. This
in turn causes incorrect detection of gadget link partner speed in
dwc2_hsotg_irq_enumdone() .

Shift the value accordingly to fix the problem with speed detection.

Signed-off-by: Marek Vasut 
Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: John Youn 
---
 drivers/usb/dwc2/gadget.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

V2: - Modify the code itself by adding the necessary shift instead of
  modifying the header.
- Update commit message.

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0abf73c..48e47c1 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2095,7 +2095,7 @@ static void dwc2_hsotg_irq_enumdone(struct dwc2_hsotg 
*hsotg)
 */
 
/* catch both EnumSpd_FS and EnumSpd_FS48 */
-   switch (dsts & DSTS_ENUMSPD_MASK) {
+   switch ((dsts & DSTS_ENUMSPD_MASK) >> DSTS_ENUMSPD_SHIFT) {
case DSTS_ENUMSPD_FS:
case DSTS_ENUMSPD_FS48:
hsotg->gadget.speed = USB_SPEED_FULL;
-- 
2.1.4

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

Re: [RFC v5 07/15] usb: ehci: add vbus-gpio parameter

[Marek Vasut]

On Tuesday, February 09, 2016 at 09:13:53 AM, Antony Pavlov wrote:
> This patch retrieves and configures the vbus control gpio via
> the device tree.
> 
> This patch is based on a ehci-s5p.c commit fd81d59c90d38661
> ("USB: ehci-s5p: Add vbus setup function to the s5p ehci glue layer").
> 
> Signed-off-by: Antony Pavlov 
> Cc: Alan Stern 
> Cc: Greg Kroah-Hartman 
> Cc: linux-usb@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  drivers/usb/host/ehci-platform.c | 22 ++
>  1 file changed, 22 insertions(+)

I think this patch will not be needed if you switch the ar9331 to chipidea hdrc
driver. There is CI HDRC in the ar9331.

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

Re: [RFC v5 07/15] usb: ehci: add vbus-gpio parameter

[Marek Vasut]

On 02/18/2016 05:12 PM, Alan Stern wrote:
> On Tue, 9 Feb 2016, Antony Pavlov wrote:
> 
>> This patch retrieves and configures the vbus control gpio via
>> the device tree.
>>
>> This patch is based on a ehci-s5p.c commit fd81d59c90d38661
>> ("USB: ehci-s5p: Add vbus setup function to the s5p ehci glue layer").
>>
>> Signed-off-by: Antony Pavlov 
>> Cc: Alan Stern 
>> Cc: Greg Kroah-Hartman 
>> Cc: linux-usb@vger.kernel.org
>> Cc: linux-ker...@vger.kernel.org
>> ---
>>  drivers/usb/host/ehci-platform.c | 22 ++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/usb/host/ehci-platform.c 
>> b/drivers/usb/host/ehci-platform.c
>> index bd7082f2..0d95ced 100644
>> --- a/drivers/usb/host/ehci-platform.c
>> +++ b/drivers/usb/host/ehci-platform.c
>> @@ -28,6 +28,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -142,6 +143,25 @@ static struct usb_ehci_pdata ehci_platform_defaults = {
>>  .power_off =ehci_platform_power_off,
>>  };
>>  
>> +static void setup_vbus_gpio(struct device *dev)
>> +{
>> +int err;
>> +int gpio;
>> +
>> +if (!dev->of_node)
>> +return;
>> +
>> +gpio = of_get_named_gpio(dev->of_node, "vbus-gpio", 0);
>> +if (!gpio_is_valid(gpio))
>> +return;
>> +
>> +err = devm_gpio_request_one(dev, gpio,
>> +GPIOF_OUT_INIT_HIGH | GPIOF_EXPORT_DIR_FIXED,
>> +"ehci_vbus_gpio");
>> +if (err)
>> +dev_err(dev, "can't request ehci vbus gpio %d", gpio);
> 
> I don't understand this.  If you get an error here, what's the point of 
> allowing the probe to continue?  Shouldn't you return an error code so 
> the probe will fail?

The idea is I believe that if there is no vbus gpio specified, the port
might just not have vbus control, so the probe can continue. But this
patch is irrelevant anyway, since Alexey will switch to CI HDRC driver
and use standard regulator, as it should be done.


--
To unsubscribe from this list: send the line "unsubscribe 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: serial: ftdi_sio: add device ID for Microsemi/Arrow SF2PLUS Dev Kit

[Marek Vasut]

On 02/13/2017 10:29 AM, Johan Hovold wrote:
> [+CC: linux-usb]
> 
> Always make sure to CC linux-usb for USB patches.

Got it

> On Fri, Feb 10, 2017 at 05:16:12PM +0100, Marek Vasut wrote:
>> This development kit has an FT4232 on it with a custom USB VID/PID.
>> The FT4232 provides four UARTs, but only two are used. The UART 0
>> is used by the FlashPro5 programmer and UART 2 is connected to the
>> SmartFusion2 CortexM3 SoC UART port.
> 
> Don't you want to use the "jtag" quirk for this one then to prevent the
> driver from binding to interface 0? Or do you still need this interface
> to be accessible as a tty device?

I will check that next week as I don't have access to the HW this week.

>> Note that the USB VID is registered to Actel according to Linux USB
>> VID database, but that was acquired by Microsemi.
>>
>> Signed-off-by: Marek Vasut 
>> Cc: stable 
>> Cc: Johan Hovold 
>> ---
>>  drivers/usb/serial/ftdi_sio.c |  1 +
>>  drivers/usb/serial/ftdi_sio_ids.h | 10 ++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
>> index c98cf10be5af..14f0fb34f655 100644
>> --- a/drivers/usb/serial/ftdi_sio.c
>> +++ b/drivers/usb/serial/ftdi_sio.c
>> @@ -873,6 +873,7 @@ static const struct usb_device_id id_table_combined[] = {
>>  { USB_DEVICE_AND_INTERFACE_INFO(MICROCHIP_VID, MICROCHIP_USB_BOARD_PID,
>>  USB_CLASS_VENDOR_SPEC,
>>  USB_SUBCLASS_VENDOR_SPEC, 0x00) },
>> +{ USB_DEVICE(ACTEL_VID, MICROSEMI_ARROW_SF2PLUS_BOARD_PID) },
>>  { USB_DEVICE(JETI_VID, JETI_SPC1201_PID) },
>>  { USB_DEVICE(MARVELL_VID, MARVELL_SHEEVAPLUG_PID),
>>  .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
>> diff --git a/drivers/usb/serial/ftdi_sio_ids.h 
>> b/drivers/usb/serial/ftdi_sio_ids.h
>> index 168ccb03ce08..a9d538d18344 100644
>> --- a/drivers/usb/serial/ftdi_sio_ids.h
>> +++ b/drivers/usb/serial/ftdi_sio_ids.h
>> @@ -627,6 +627,16 @@
>>  #define MICROCHIP_USB_BOARD_PID 0x000A /* CDC RS-232 Emulation Demo */
>>  
>>  /*
>> + * Microsemi/Arrow SF2PLUS Dev Kit
>> + *
>> + * This board has an FT4232 on it which provides four UART ports.
>> + * UART 0 is used by the FlashPro5 programmer, UART 2 is connected
>> + * to the UART of an CortexM3 SoC-FPGA on the board.
>> + */
>> +#define ACTEL_VID   0x1514
>> +#define MICROSEMI_ARROW_SF2PLUS_BOARD_PID   0x2008
>> +
> 
> Please place this before the Olimex section to try to maintain some
> order based on VID.

So ordering of the file is based on the VIDs , got it and done.

>> +/*
>>   * RATOC REX-USB60F
>>   */
>>  #define RATOC_VENDOR_ID 0x0584
> 
> Thanks,
> Johan
> 


-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 12/12] USB: chipidea: add imx usbmisc support

[Marek Vasut]

Dear Richard Zhao,

[...]

> +static int imx6q_usbmisc_init(struct usbmisc *usbmisc, int id)
> +{
> + u32 reg;
> +
> + if (usbmisc->dis_oc) {
> + reg = readl_relaxed(usbmisc->base + id * 4);
> + writel_relaxed(reg | (1 << 7), usbmisc->base + id * 4);


1<< 7 ? This yearns for fixing with proper #defined value.

[...]

The rest of the patches looks OK, you can add my Reviewed-By:  
to 
those. This one though will need further investigation.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Does larger than 32 bits report size is supported at HID?

[Marek Vasut]

Dear Chen Peter-B29397,

> > On Thu, Mar 10, 2011 at 01:34:38AM +, Chen Peter-B29397 wrote:
> > > Hi all,
> > > 
> > > I am sorry I can't find linux-hid and linux-input ML, so I try to get a
> > 
> > help here.
> > 
> > > I have a device which follows usb-hid protocol (can be communicated by
> > 
> > WindowsXP),
> > 
> > > but it can't communicate with my linux box. The error is at below code
> > 
> > of function hid_parser_global:
> > > case HID_GLOBAL_ITEM_TAG_REPORT_SIZE:
> > >   parser->global.report_size = item_udata(item);
> > >   if (parser->global.report_size > 32) {
> > >   
> > >   dbg_hid("invalid report_size %d\n",
> > >   
> > >   parser->global.report_size);
> > >   
> > >   return -1;
> > >   
> > >   }
> > >   return 0;
> > > 
> > > So my question is that does larger than 32 bits report size is
> > 
> > supported at linux HID stack?
> > 
> > > Or it is supported but I need to have a hid driver for my device?
> > 
> > Is this a "real" hid device or does it just use the "we can write a
> > driver in userspace on XP if we fake out a HID device" functionality?
> > You can find out if you are forced to load a driver for the device on
> > Windows or not.
> 
> It is one of freescale SoC's ROM

Which one? The MX28 p-o-s? Skimming through it's sources, I already used two 
puking bags, more to be filled tho.

> , it can communicate with host using HID
> protocol. After adding raw hid device support and one hid driver for fsl
> SoC's rom, it can work using hidraw interface.
> 
> Thank you!
> 
> > What type of device is this?

MX28 bootrom does this. I barely have words for the flubs I unveiled so far in 
it.

Greg, do you please have any solution for this problem? Basically, the probe of 
hid fails on the fact that the HID_GLOBAL_ITEM_TAG_REPORT_SIZE is 128, while 
right now it's capped at 96.

Shall I submit a patch that increases it to 128 maybe? Please advise. Thanks!

[...]

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Does larger than 32 bits report size is supported at HID?

[Marek Vasut]

Dear Chen Peter-B29397,

> > Which one? The MX28 p-o-s? Skimming through it's sources, I already used
> > two
> > puking bags, more to be filled tho.
> 
> mx23,mx28, maybe mx6x is the same.

I use the MX28, so I made these two patches (follow). I'll roll them into an 
official thingie and submit properly, but I'd like to know Greg's opinion first.

First patch is the obvious increment, but I also need the second one which adds 
quirk for the MX28 bootrom. If you issue such command to it, the bootrom 
crashes, therefore the quirk.

With these in place, I can get some output from the bootrom finally using 
libusb.

Greg, can you please take an experienced look on these (and avoid tearing me 
limb to limb because I didn't submit them properly ;-) ) and tell me your 
opinion?

Thanks!

-->8--

--- a/drivers/hid/hid-core.c2012-08-04 23:53:38.900126160 +0200
+++ b/drivers/hid/hid-core.c2012-08-04 23:54:03.148127188 +0200
@@ -374,7 +374,7 @@
 
case HID_GLOBAL_ITEM_TAG_REPORT_SIZE:
parser->global.report_size = item_udata(item);
-   if (parser->global.report_size > 96) {
+   if (parser->global.report_size > 128) {
hid_err(parser->device, "invalid report_size %d\n",
parser->global.report_size);
return -1;

--8<--

--- a/drivers/hid/usbhid/hid-quirks.c   2012-08-05 02:58:15.880274023 +0200
+++ b/drivers/hid/usbhid/hid-quirks.c   2012-08-05 02:58:10.228273782 +0200
@@ -99,6 +99,7 @@
{ USB_VENDOR_ID_SIGMA_MICRO, USB_DEVICE_ID_SIGMA_MICRO_KEYBOARD, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_MOUSEPEN_I608X, 
HID_QUIRK_MULTI_INPUT },
{ USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X, 
HID_QUIRK_MULTI_INPUT },
+   { USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, 
HID_QUIRK_NOGET 
},
{ 0, 0 }
 };
 
--- a/drivers/hid/hid-ids.h 2012-08-05 02:59:00.340275908 +0200
+++ b/drivers/hid/hid-ids.h 2012-08-05 02:58:56.008275723 +0200
@@ -282,6 +282,9 @@
 #define USB_VENDOR_ID_EZKEY0x0518
 #define USB_DEVICE_ID_BTC_8193 0x0002
 
+#define USB_VENDOR_ID_FREESCALE0x15A2
+#define USB_DEVICE_ID_FREESCALE_MX28   0x004F
+
 #define USB_VENDOR_ID_FRUCTEL  0x25B6
 #define USB_DEVICE_ID_GAMETEL_MT_MODE  0x0002

-->8--

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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] input: usb: hid: Bump maximum global item tag report size to 128 bytes

[Marek Vasut]

The Freescale i.MX28 BootROM USB recovery mode implements the USB HID
protocol, yet the global item tag report size is 128. Linux checks if
this is 96 as of now, see [1]. This causes Linux to refuse to communicate
with this device, making it impossible to use the recovery mode.

This is not a standard HID device per se, but rather a software emulation
implemented within the BootROM code and realized through USB OTG-capable
port switched to device mode present on the device.

Previous attempt to discuss this issue dates back to 2011, see [2]. There
has been not much response. Also noteworthy is the [3], where there seems
to be a pointing device that has issue similar to this one.

The tool making use of the USB recovery mode is available at [4].

[1] http://comments.gmane.org/gmane.linux.kernel.input/22328
[2] http://www.spinics.net/lists/linux-usb/msg43463.html
[3] https://bbs.archlinux.org/viewtopic.php?pid=1141340
[4] http://git.bfuser.eu/?p=marex/mxsldr.git;a=summary

Signed-off-by: Marek Vasut 
Cc: Chen Peter 
Cc: Greg KH 
Cc: Jiri Kosina 
---
 drivers/hid/hid-core.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 60ea284..2b9dd76 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -374,7 +374,7 @@ static int hid_parser_global(struct hid_parser *parser, 
struct hid_item *item)
 
case HID_GLOBAL_ITEM_TAG_REPORT_SIZE:
parser->global.report_size = item_udata(item);
-   if (parser->global.report_size > 96) {
+   if (parser->global.report_size > 128) {
hid_err(parser->device, "invalid report_size %d\n",
parser->global.report_size);
return -1;
-- 
1.7.10.4

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

[PATCH 2/2] input: usb: hid: Add quirk for Freescale i.MX28 ROM recovery

[Marek Vasut]

The USB recovery mode present in i.MX28 ROM emulates USB HID.
It needs this quirk to behave properly.

Signed-off-by: Marek Vasut 
Cc: Chen Peter 
Cc: Greg KH 
Cc: Jiri Kosina 
---
 drivers/hid/hid-ids.h   |3 +++
 drivers/hid/usbhid/hid-quirks.c |1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 1dcb76f..ab8ce9f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -296,6 +296,9 @@
 #define USB_VENDOR_ID_EZKEY0x0518
 #define USB_DEVICE_ID_BTC_8193 0x0002
 
+#define USB_VENDOR_ID_FREESCALE0x15A2
+#define USB_DEVICE_ID_FREESCALE_MX28   0x004F
+
 #define USB_VENDOR_ID_FRUCTEL  0x25B6
 #define USB_DEVICE_ID_GAMETEL_MT_MODE  0x0002
 
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 903eef3..6a09570 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -101,6 +101,7 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_SIGMA_MICRO, USB_DEVICE_ID_SIGMA_MICRO_KEYBOARD, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_MOUSEPEN_I608X, 
HID_QUIRK_MULTI_INPUT },
{ USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X, 
HID_QUIRK_MULTI_INPUT },
+   { USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, 
HID_QUIRK_NOGET },
{ 0, 0 }
 };
 
-- 
1.7.10.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] input: usb: hid: Add quirk for Freescale i.MX28 ROM recovery

[Marek Vasut]

[...]

> diff --git a/drivers/hid/usbhid/hid-quirks.c
> b/drivers/hid/usbhid/hid-quirks.c index 903eef3..6a09570 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -101,6 +101,7 @@ static const struct hid_blacklist {
>   { USB_VENDOR_ID_SIGMA_MICRO, USB_DEVICE_ID_SIGMA_MICRO_KEYBOARD,
> HID_QUIRK_NO_INIT_REPORTS }, { USB_VENDOR_ID_KYE,
> USB_DEVICE_ID_KYE_MOUSEPEN_I608X, HID_QUIRK_MULTI_INPUT }, {
> USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X, HID_QUIRK_MULTI_INPUT
> }, +  { USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28,
> HID_QUIRK_NOGET }, { 0, 0 }

OT: btw. the comment mentioned the list should be sorted, yet it's not so for a 
while. Maybe a sorting patch would be favored?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: mxs-phy on i.MX233 not enumerating

[Marek Vasut]

Dear Chen Peter-B29397,

> > On 8/13/12, Felipe Balbi  wrote:
> > > Will there be another version for this or is this final ? Won't this
> > > cause any regressions to other boards ?
> > 
> > The proposed patch is not meant to be a final one.
> > 
> > We still need to figure out the proper way to handle HOSTDISCONDETECT
> > for all i.mx SoCs, including mx23, and would like to get some feedback
> > from the FSL guys in Cc.
> 
> According to IC guys, the logic of handling HOSTDISCONDETECT is the same
> between i.mx28 and i.mx23.

I recall we had issues with the discon detector on MX28, but on mx28 the chip 
was completely deaf (no messages in kernel log).

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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] input: usb: hid: Bump maximum global item tag report size to 128 bytes

[Marek Vasut]

Dear Jiri Kosina,

> On Sun, 5 Aug 2012, Marek Vasut wrote:
> > The Freescale i.MX28 BootROM USB recovery mode implements the USB HID
> > protocol, yet the global item tag report size is 128. Linux checks if
> > this is 96 as of now, see [1]. This causes Linux to refuse to communicate
> > with this device, making it impossible to use the recovery mode.
> > 
> > This is not a standard HID device per se, but rather a software emulation
> > implemented within the BootROM code and realized through USB OTG-capable
> > port switched to device mode present on the device.
> > 
> > Previous attempt to discuss this issue dates back to 2011, see [2]. There
> > has been not much response. Also noteworthy is the [3], where there seems
> > to be a pointing device that has issue similar to this one.
> > 
> > The tool making use of the USB recovery mode is available at [4].
> > 
> > [1] http://comments.gmane.org/gmane.linux.kernel.input/22328
> > [2] http://www.spinics.net/lists/linux-usb/msg43463.html
> > [3] https://bbs.archlinux.org/viewtopic.php?pid=1141340
> > [4] http://git.bfuser.eu/?p=marex/mxsldr.git;a=summary
> > 
> > Signed-off-by: Marek Vasut 
> > Cc: Chen Peter 
> > Cc: Greg KH 
> > Cc: Jiri Kosina 
> 
> I have not admit I don't remember where the original limit came from --
> the spec doesn't mandate it as far as I can tell.
> 
> I am queueing this patch now for next merge window. Thanks.
[...]

Thanks!

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: otg: mxs-phy: Fix mx23 operation

[Marek Vasut]

Dear Fabio Estevam,

> From: Mike Thompson 
> 
> Currently mx23 fails to enumerate a USB device:
> 
> [ 1.30] hub 1-0:1.0: unable to enumerate USB device on port 1
> [ 1.52] hub 1-0:1.0: unable to enumerate USB device on port 1
> [ 1.74] hub 1-0:1.0: unable to enumerate USB device on port 1
> [ 1.96] hub 1-0:1.0: unable to enumerate USB device on port 1
> [ 2.18] hub 1-0:1.0: unable to enumerate USB device on port 1
> 
> Use a kernel workqueue to asynchronously delay the setting of
> ENHOSTDISCONDETECT bit until after higher level hub connect/reset
> processing is complete.  Prematurely setting the bit prevents the
> connection
> processing from completing and not setting it prevents disconnect from
> being detected. No delay is needed for clearing of ENHOSTDISCONDETECT.
> 
> Successfully tested on mx23-olinuxino (micro, mini and maxi variants) and
> mx28evk.
> 
> Signed-off-by: Mike Thompson 
> Signed-off-by: Fabio Estevam 
> ---
>  drivers/usb/otg/mxs-phy.c |   34 +++---
>  1 file changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
> index c1a67cb..8188380 100644
> --- a/drivers/usb/otg/mxs-phy.c
> +++ b/drivers/usb/otg/mxs-phy.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #define DRIVER_NAME "mxs_phy"
> 
> @@ -34,9 +35,12 @@
>  #define BM_USBPHY_CTRL_ENUTMILEVEL2  BIT(14)
>  #define BM_USBPHY_CTRL_ENHOSTDISCONDETECTBIT(1)
> 
> +#define MXY_PHY_ENHOSTDISCONDETECT_DELAY 250
> +

Why 250  ? :)

>  struct mxs_phy {
>   struct usb_phy phy;
>   struct clk *clk;
> + struct delayed_work enhostdiscondetect_work;
>  };
> 
>  #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
> @@ -62,6 +66,7 @@ static int mxs_phy_init(struct usb_phy *phy)
> 
>   clk_prepare_enable(mxs_phy->clk);
>   mxs_phy_hw_init(mxs_phy);
> + INIT_DELAYED_WORK(&mxs_phy->enhostdiscondetect_work, NULL);
> 
>   return 0;
>  }
> @@ -76,13 +81,34 @@ static void mxs_phy_shutdown(struct usb_phy *phy)
>   clk_disable_unprepare(mxs_phy->clk);
>  }
> 
> +static void mxs_phy_enhostdiscondetect_delay(struct work_struct *ws)
> +{
> + struct mxs_phy *mxs_phy = container_of(ws, struct mxs_phy,
> + enhostdiscondetect_work.work);
> +
> + /* Enable HOSTDISCONDETECT after delay. */
> + dev_dbg(mxs_phy->phy.dev, "Setting ENHOSTDISCONDETECT\n");
> + writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
> + mxs_phy->phy.io_priv + HW_USBPHY_CTRL_SET);
> +}
> +
>  static int mxs_phy_on_connect(struct usb_phy *phy, int port)
>  {
> + struct mxs_phy *mxs_phy = to_mxs_phy(phy);
> +
>   dev_dbg(phy->dev, "Connect on port %d\n", port);
> 
> - mxs_phy_hw_init(to_mxs_phy(phy));
> - writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
> - phy->io_priv + HW_USBPHY_CTRL_SET);
> + mxs_phy_hw_init(mxs_phy);
> +
> + /*
> +  * Delay enabling ENHOSTDISCONDETECT so that connection and
> +  * reset processing can be completed for the root hub.
> +  */
> + dev_dbg(phy->dev, "Delaying setting ENHOSTDISCONDETECT\n");
> + PREPARE_DELAYED_WORK(&mxs_phy->enhostdiscondetect_work,
> + mxs_phy_enhostdiscondetect_delay);
> + schedule_delayed_work(&mxs_phy->enhostdiscondetect_work,
> + msecs_to_jiffies(MXY_PHY_ENHOSTDISCONDETECT_DELAY));

Isn't this mx23 specific?

>   return 0;
>  }
> @@ -91,6 +117,8 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy,
> int port) {
>   dev_dbg(phy->dev, "Disconnect on port %d\n", port);
> 
> + /* No need to delay before clearing ENHOSTDISCONDETECT. */
> + dev_dbg(phy->dev, "Clearing ENHOSTDISCONDETECT\n");
>   writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
>   phy->io_priv + HW_USBPHY_CTRL_CLR);

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: otg: mxs-phy: Fix mx23 operation

[Marek Vasut]

Dear Fabio Estevam,

> Hi Marek,
> 
> On Thu, Aug 30, 2012 at 12:20 PM, Marek Vasut  wrote:
> >> +#define MXY_PHY_ENHOSTDISCONDETECT_DELAY 250
> >> +
> > 
> > Why 250  ? :)
> 
> It is 250 ms. We found this one to be a safe value to enable
> ENHOSTDISCONDETECT in our tests.

Documentation / comment would be handy ... but why 250mS and not 500mS for 
example? Cant it get racy here?

> >> + /*
> >> +  * Delay enabling ENHOSTDISCONDETECT so that connection and
> >> +  * reset processing can be completed for the root hub.
> >> +  */
> >> + dev_dbg(phy->dev, "Delaying setting ENHOSTDISCONDETECT\n");
> >> + PREPARE_DELAYED_WORK(&mxs_phy->enhostdiscondetect_work,
> >> + mxs_phy_enhostdiscondetect_delay);
> >> + schedule_delayed_work(&mxs_phy->enhostdiscondetect_work,
> >> +
> >> msecs_to_jiffies(MXY_PHY_ENHOSTDISCONDETECT_DELAY));
> > 
> > Isn't this mx23 specific?
> 
> Checked with Peter Chen and also the design team confirmed that the
> mxs phy block is the same on mx23/mx28/mx6q.
> 
> Regards,
> 
> Fabio Estevam

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Hi,

> Hi,
> 
> On Wed, May 29, 2013 at 08:07:55AM +, Chen Peter-B29397 wrote:
> > > Indeed, I've been using the patchset "Add tested id switch and vbus
> > > connect detect support for Chipidea" from Peter for quite some time on
> > > top of 3.9 and it works like a charm for the gadget mode on an MX28
> > > platform.
> > > 
> > > BTW, Peter, I've seen that these patches are still not merged in 3.10,
> > > is there a reason for that? do you plan on sending a version rebased on
> > > top of 3.10 some time in the future? I tried to do the rebasing myself,
> > > but the chipidea driver seems to have changed quite heavily, which
> > > makes the process quite difficult when you don't know what you're
> > > doing :)
> 
> we already have Peter's patches on v3.10-rc3 [1].
> 
> > I can spend few bandwidth on upstream work recently, I may have more
> > bandwidth after June 15th.
> > 
> > Currently, we still have no conclusion for chipidea core driver's coming
> > work, like Device tree support, how to identify if the controller is OTG
> > supported.
> 
> Yes, the next important step is getting the of propertys "dr_mode" and
> "phy_type" properly used in the chipidea core.
> 
> [1] http://git.pengutronix.de/?p=mgr/linux.git;a=summary ->
> v3.10/topic/usb-peterchen

Is anyone planning to work on this stuff and start pushing it mainline or is 
this effort stalled completely? What is it that's missing before these can be 
applied?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Dear Michael Grzeschik,

> Hi Marek,
> 
> On Fri, Jun 21, 2013 at 01:26:10PM +0200, Marek Vasut wrote:
> > Hi,
> > 
> > > Hi,
> > > 
> > > On Wed, May 29, 2013 at 08:07:55AM +, Chen Peter-B29397 wrote:
> > > > > Indeed, I've been using the patchset "Add tested id switch and vbus
> > > > > connect detect support for Chipidea" from Peter for quite some time
> > > > > on top of 3.9 and it works like a charm for the gadget mode on an
> > > > > MX28 platform.
> > > > > 
> > > > > BTW, Peter, I've seen that these patches are still not merged in
> > > > > 3.10, is there a reason for that? do you plan on sending a version
> > > > > rebased on top of 3.10 some time in the future? I tried to do the
> > > > > rebasing myself, but the chipidea driver seems to have changed
> > > > > quite heavily, which makes the process quite difficult when you
> > > > > don't know what you're doing :)
> > > 
> > > we already have Peter's patches on v3.10-rc3 [1].
> > > 
> > > > I can spend few bandwidth on upstream work recently, I may have more
> > > > bandwidth after June 15th.
> > > > 
> > > > Currently, we still have no conclusion for chipidea core driver's
> > > > coming work, like Device tree support, how to identify if the
> > > > controller is OTG supported.
> > > 
> > > Yes, the next important step is getting the of propertys "dr_mode" and
> > > "phy_type" properly used in the chipidea core.
> > > 
> > > [1] http://git.pengutronix.de/?p=mgr/linux.git;a=summary ->
> > > v3.10/topic/usb-peterchen
> > 
> > Is anyone planning to work on this stuff and start pushing it mainline or
> > is this effort stalled completely? What is it that's missing before
> > these can be applied?
> 
> AFAIK the latest commit to that work is:
> 
> http://permalink.gmane.org/gmane.linux.usb.general/88121

Cool, so it seems Peter is back at it. Thanks

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Hi,

> Dear Michael Grzeschik,
> 
> > Hi Marek,
> > 
> > On Fri, Jun 21, 2013 at 01:26:10PM +0200, Marek Vasut wrote:
> > > Hi,
> > > 
> > > > Hi,
> > > > 
> > > > On Wed, May 29, 2013 at 08:07:55AM +, Chen Peter-B29397 wrote:
> > > > > > Indeed, I've been using the patchset "Add tested id switch and
> > > > > > vbus connect detect support for Chipidea" from Peter for quite
> > > > > > some time on top of 3.9 and it works like a charm for the gadget
> > > > > > mode on an MX28 platform.
> > > > > > 
> > > > > > BTW, Peter, I've seen that these patches are still not merged in
> > > > > > 3.10, is there a reason for that? do you plan on sending a
> > > > > > version rebased on top of 3.10 some time in the future? I tried
> > > > > > to do the rebasing myself, but the chipidea driver seems to have
> > > > > > changed quite heavily, which makes the process quite difficult
> > > > > > when you don't know what you're doing :)
> > > > 
> > > > we already have Peter's patches on v3.10-rc3 [1].
> > > > 
> > > > > I can spend few bandwidth on upstream work recently, I may have
> > > > > more bandwidth after June 15th.
> > > > > 
> > > > > Currently, we still have no conclusion for chipidea core driver's
> > > > > coming work, like Device tree support, how to identify if the
> > > > > controller is OTG supported.
> > > > 
> > > > Yes, the next important step is getting the of propertys "dr_mode"
> > > > and "phy_type" properly used in the chipidea core.
> > > > 
> > > > [1] http://git.pengutronix.de/?p=mgr/linux.git;a=summary ->
> > > > v3.10/topic/usb-peterchen
> > > 
> > > Is anyone planning to work on this stuff and start pushing it mainline
> > > or is this effort stalled completely? What is it that's missing before
> > > these can be applied?
> > 
> > AFAIK the latest commit to that work is:
> > 
> > http://permalink.gmane.org/gmane.linux.usb.general/88121
> 
> Cool, so it seems Peter is back at it. Thanks

Peter, I dunno if you are already aware of it, but the USB peripheral mode 
hangs 
on MX233. It's easy to replicate for example if you try to run CDC ethernet 
over 
the USB peripheral mode, then telnet into the board and run "dmesg" . This will 
trigger a "larger" data transfer which will make the UDC driver hang. This 
doesn't happen on MX28 so it must be some MX233-specific thing.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Hi Peter,

> > Peter, I dunno if you are already aware of it, but the USB peripheral
> > mode hangs
> > on MX233. It's easy to replicate for example if you try to run CDC
> > ethernet over
> > the USB peripheral mode, then telnet into the board and run "dmesg" .
> > This will
> > trigger a "larger" data transfer which will make the UDC driver hang.
> > This
> > doesn't happen on MX28 so it must be some MX233-specific thing.
> 
> Marek, Have you tried below?
> 
> http://marc.info/?l=linux-usb&m=136537318510847&w=2
> 
> It may not the USB itself problem, the mx23's usb is almost
> the same with mx28's.

This also happens on a different MX23-based board for me. This other board I 
tried has mDDR DRAM and the DRAM is operating correctly. The 96MHz thing people 
complained about in the Olimex forum is resolved now. 

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Hello Peter,

> > Hi Peter,
> > 
> > > > Peter, I dunno if you are already aware of it, but the USB peripheral
> > > > mode hangs
> > > > on MX233. It's easy to replicate for example if you try to run CDC
> > > > ethernet over
> > > > the USB peripheral mode, then telnet into the board and run "dmesg" .
> > > > This will
> > > > trigger a "larger" data transfer which will make the UDC driver hang.
> > > > This
> > > > doesn't happen on MX28 so it must be some MX233-specific thing.
> > > 
> > > Marek, Have you tried below?
> > > 
> > > http://marc.info/?l=linux-usb&m=136537318510847&w=2
> > > 
> > > It may not the USB itself problem, the mx23's usb is almost
> > > the same with mx28's.
> > 
> > This also happens on a different MX23-based board for me. This other
> > board I
> > tried has mDDR DRAM and the DRAM is operating correctly. The 96MHz thing
> > people
> > complained about in the Olimex forum is resolved now.
> 
> Add shawn.
> 
> Marek, have you tried mx23 evk?

No, I don't have this one available right now. I tried both MX23 Olinuxino maxi 
(I just mutilated the board so that I cut traces to the USB devices on the 
board 
and routed out USB gadget connector) and a custom MX23 board.

Note that both have working USB peripheral mode in U-Boot too, but the driver 
in 
U-Boot is very minimalistic.

> Shawn, marek reported the udc function at mx23 works abnormal, but it works
> good at mx28. Have you tried mx23 udc recently?

Fabio, can you possibly test on MX23EVK please?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Dear Fabio Estevam,

> On Tue, Jun 25, 2013 at 11:23 AM, Fabio Estevam  wrote:
> > On Mon, Jun 24, 2013 at 10:51 AM, Marek Vasut  wrote:
> >> Fabio, can you possibly test on MX23EVK please?
> > 
> > I never used USB gadget with chipidea driver.
> > 
> > Could you please explain what are the changes I need to do in the dts
> > file (I want to try on mx28evk first) and defconfig in order to be
> > able to test gadget? I am running today's linux-next.
> 
> I tried the following:

Merge [1] into your tree, enable USB peripheral mode for CI13xxx, enable 
ethernet gadget, adjust your DT file like this for the usb0 node:

usb0: usb@8008 {
dr_mode = "peripheral";
status = "okay";
};

And you should have gadget working.

[1] 
http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/v3.10/topic/usb-
peterchen

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Hi Peter,

> > No, I don't have this one available right now. I tried both MX23
> > Olinuxino maxi
> > (I just mutilated the board so that I cut traces to the USB devices on
> > the board
> > and routed out USB gadget connector) and a custom MX23 board.
> > 
> > Note that both have working USB peripheral mode in U-Boot too, but the
> > driver in
> > U-Boot is very minimalistic.
> 
> You mean, the larger data transfer with USB peripheral mode is ok at
> u-boot? but it can't work at Linux Kernel?

Yes, but the U-Boot driver is completely different beast.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Hi Peter,

> > No, I don't have this one available right now. I tried both MX23
> > Olinuxino maxi
> > (I just mutilated the board so that I cut traces to the USB devices on
> > the board
> > and routed out USB gadget connector) and a custom MX23 board.
> > 
> > Note that both have working USB peripheral mode in U-Boot too, but the
> > driver in
> > U-Boot is very minimalistic.
> 
> You mean, the larger data transfer with USB peripheral mode is ok at
> u-boot? but it can't work at Linux Kernel?

I stumbled across one more problem with the patches:

- I use OTG on MX28
- I configured the USB ID pin
- I set the dr_mode to "otg"
(so far so good)
- I use the port in HOST mode by plugging in a USB pendrive over the 
OTG-capable 
reduction ; this works OK
- I disconnect the reduction and connect it to the computer ; here I get 
"timeout waiting for 0800 in 11".

This means the BSV bit in OTGSC register wasn't unset.

Interestingly enough, transition the other way around (first use in gadget mode 
then in host mode) works fine, but if I do host mode -> gadget mode, I get the 
timeout. Do you have any hint for me?

btw. what is the plan about cleaning up and upstreaming all these patches we 
have here? Is anyone working on it? I'd hate to stomp on anyones' efforts,but 
I'd also like to see this mess sorted out.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Hi Peter,

> > - I use the port in HOST mode by plugging in a USB pendrive over the OTG-
> > capable
> > reduction ; this works OK
> > - I disconnect the reduction and connect it to the computer ; here I get
> > "timeout waiting for 0800 in 11".
> > 
> > This means the BSV bit in OTGSC register wasn't unset.
> > 
> > Interestingly enough, transition the other way around (first use in
> > gadget mode
> > then in host mode) works fine, but if I do host mode -> gadget mode, I
> > get the
> > timeout. Do you have any hint for me?
> 
> It means your OTG VBUS does not lower than BSV (B SESSION VALID, 0.8v)
> after plugging out Micro-B-TO-A cable.

Where can I measure this?

> Two possible reasons:
> 
> 1. You have not gpio control for vbus toggle when role switches.

This works well.

> 2. Your hardware has some problems that the vbus can't lower than 0.8v.

How can I check that?

> > btw. what is the plan about cleaning up and upstreaming all these patches
> > we
> > have here? Is anyone working on it? I'd hate to stomp on anyones'
> > efforts,but
> > I'd also like to see this mess sorted out.
> 
> We had something un-decided before, now, things almost are cleared.
> But I am a little busy recently, I hope I can begin to work on it
> from next week.

Ok then, I won't interfere.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Dear Chen Peter-B29397,

> > Where can I measure this?
> > 
> > > Two possible reasons:
> > > 
> > > 1. You have not gpio control for vbus toggle when role switches.
> > 
> > This works well.
> > 
> > > 2. Your hardware has some problems that the vbus can't lower than 0.8v.
> > 
> > How can I check that?
> 
> Just measure the voltage of your otg vbus pin.

Ok, I see it's 2.25 volts after I disconnect the microB-A thing . I should see 
0.8V constantly with nothing connected on the port?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Hi Peter,

> > Where can I measure this?
> > 
> > > Two possible reasons:
> > > 
> > > 1. You have not gpio control for vbus toggle when role switches.
> > 
> > This works well.
> > 
> > > 2. Your hardware has some problems that the vbus can't lower than 0.8v.
> > 
> > How can I check that?
> 
> Just measure the voltage of your otg vbus pin.

It's 2.5 volt for me. What am I supposed to observe happening there?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Hi Peter,

> > -Original Message-
> > From: Marek Vasut [mailto:ma...@denx.de]
> > Sent: Sunday, July 07, 2013 9:10 AM
> > To: Chen Peter-B29397
> > Cc: shawn@linaro.org; Michael Grzeschik; maxime.ripard@free-
> > electrons.com; Hector Palacios; linux-usb@vger.kernel.org;
> > s.ha...@pengutronix.de; alexander.shish...@linux.intel.com; Estevam
> > Fabio-R49496; Marc Kleine-Budde; m.grzesc...@pengutronix.de; linux-arm-
> > ker...@lists.infradead.org
> > Subject: Re: Chipidea usb otg support for IMX/MXS (device functionality)
> > 
> > Hi Peter,
> > 
> > > > Where can I measure this?
> > > > 
> > > > > Two possible reasons:
> > > > > 
> > > > > 1. You have not gpio control for vbus toggle when role switches.
> > > > 
> > > > This works well.
> > > > 
> > > > > 2. Your hardware has some problems that the vbus can't lower than
> > 
> > 0.8v.
> > 
> > > > How can I check that?
> > > 
> > > Just measure the voltage of your otg vbus pin.
> > 
> > It's 2.5 volt for me. What am I supposed to observe happening there?
> 
> http://marc.info/?l=linux-usb&m=137242465920952&w=2

Dang, must have missed that one, sorry.

The VBUS is slowly discharging from around 5V to 2.5V , what does that tell me?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Hi Peter,
[...]

> 
> It needs to check your hardware, it is probably there is a high capacitance
> at vbus pin. Besides, the vbus should be lower than 0.8v if it is
> disconnected from pc.

I just wonder why I can no longer use device mode only after I plug in the 
adaptor for host device. Before that, I can plug in the device mode cable from 
PC all I want and this works. Then I plug in the host adaptor, that works as 
well , but if I plug in the device cable, this works no longer.

I rather suspect it might be something going on in the driver that's not right. 
I will check the hardware too, thanks.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Chipidea usb otg support for IMX/MXS (device functionality)

[Marek Vasut]

Hi Peter,

> > Hi Peter,
> > 
> > > > Peter, I dunno if you are already aware of it, but the USB peripheral
> > > > mode hangs
> > > > on MX233. It's easy to replicate for example if you try to run CDC
> > > > ethernet over
> > > > the USB peripheral mode, then telnet into the board and run "dmesg" .
> > > > This will
> > > > trigger a "larger" data transfer which will make the UDC driver hang.
> > > > This
> > > > doesn't happen on MX28 so it must be some MX233-specific thing.
> > > 
> > > Marek, Have you tried below?
> > > 
> > > http://marc.info/?l=linux-usb&m=136537318510847&w=2
> > > 
> > > It may not the USB itself problem, the mx23's usb is almost
> > > the same with mx28's.
> > 
> > This also happens on a different MX23-based board for me. This other
> > board I
> > tried has mDDR DRAM and the DRAM is operating correctly. The 96MHz thing
> > people
> > complained about in the Olimex forum is resolved now.
> 
> Add shawn.
> 
> Marek, have you tried mx23 evk?

Ok, I just tried the MX23EVK. Running "dmesg" via telnet connection resulted in 
a hang of the USB driver (likely), but since I have serial connection to the 
board, I was able to verify the board itself didn't crash. I can still operate 
the board via serial console, it's only the USB in peripheral mode ethernet 
that 
is hung.

> Shawn, marek reported the udc function at mx23 works abnormal, but it works
> good at mx28. Have you tried mx23 udc recently?
> 
> Best regards,
> Peter

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 v12 06/13] usb: chipidea: add otg_cap attribute for otg capable

[Marek Vasut]

Dear Peter Chen,

> Since we need otgsc to know vbus's status at some chipidea
> controllers even it is peripheral-only mode. Besides, some
> SoCs (eg, AR9331 SoC) don't have otgsc register even
> the DCCPARAMS_DC and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS.
> We inroduce otg_cap attribute to indicate if the controller
> is otg capable, defaultly, we follow the rule that if DCCPARAMS_DC
> and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS are otg capable, but if there
> is exception, the platform can override it by device tree or platform data.
> 
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/chipidea/core.c  |   35 ---
>  include/linux/usb/chipidea.h |   13 +
>  2 files changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 93961ff..e8ceb04 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -405,6 +405,18 @@ static inline void ci_role_destroy(struct ci_hdrc *ci)
>   ci_hdrc_host_destroy(ci);
>  }
> 
> +static void ci_get_otg_capable(struct ci_hdrc *ci)
> +{
> + if (ci->platdata->otg_cap != OTG_CAP_ATTR_IS_NOT_EXISTED)

IS_NONEXISTENT , no?

[..]

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Hi Peter,

> This patchset adds tested otg id switch function and
> vbus connect and disconnect detection for chipidea driver.
> And fix kinds of bugs found at chipidea drivers after enabling
> id and vbus detection.
> 
> This patch is fully tested at imx6 sabresd platform.
> My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> 
> Changes for v12:
> - Rebased greg's usb-next tree (3.10.0-rc7+)
> - Split more small patches for single function and fix.

I tested the patchset. Here are the results:

- VBUS switching

I'm no longer getting any ID interrupts at all when I apply the patch below. 
The 
board stays in HOST mode all the time. If I configure it as peripheral, it 
works 
as peripheral. Note with [1], I was able to switch from Peripheral->Host , not 
the other way around.

--- a/arch/arm/boot/dts/imx28-m28evk.dts
+++ b/arch/arm/boot/dts/imx28-m28evk.dts
@@ -240,6 +240,8 @@
 
ahb@8008 {
usb0: usb@8008 {
+   dr_mode = "otg";
+   phy_mode = "utmi";
vbus-supply = <®_usb0_vbus>;
pinctrl-names = "default";
pinctrl-0 = <&usbphy0_pins_a>;

---

- MX23 UDC issue

I found a workaround. Now running 'dmesg' via telnet through USB CDC link no 
longer hangs the USB driver, but works as expected. I applied this small patch 
that enables the streaming mode. Works on MX23EVK. It's surprising this issue 
doesn't manifest on MX28, maybe MX28 contains a new revision of the controller. 
I remember there was some discussion about the streaming mode on MXS some time 
ago.

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
b/drivers/usb/chipidea/ci_hdrc_imx.c
index d06355e..bba8e25 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -92,8 +92,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
.name   = "ci_hdrc_imx",
.capoffset  = DEF_CAPOFFSET,
.flags  = CI_HDRC_REQUIRE_TRANSCEIVER |
- CI_HDRC_PULLUP_ON_VBUS |
- CI_HDRC_DISABLE_STREAMING,
+ CI_HDRC_PULLUP_ON_VBUS,
};
struct resource *res;
int ret;

[1] 
http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/v3.10/topic/usb-
peterchen

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 v12 06/13] usb: chipidea: add otg_cap attribute for otg capable

[Marek Vasut]

Dear Peter Chen,

> On Thu, Jul 11, 2013 at 05:36:10PM +0200, Marek Vasut wrote:
> > Dear Peter Chen,
> > 
> > > Since we need otgsc to know vbus's status at some chipidea
> > > controllers even it is peripheral-only mode. Besides, some
> > > SoCs (eg, AR9331 SoC) don't have otgsc register even
> > > the DCCPARAMS_DC and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS.
> > > We inroduce otg_cap attribute to indicate if the controller
> > > is otg capable, defaultly, we follow the rule that if DCCPARAMS_DC
> > > and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS are otg capable, but if
> > > there is exception, the platform can override it by device tree or
> > > platform data.
> > > 
> > > Signed-off-by: Peter Chen 
> > > ---
> > > 
> > >  drivers/usb/chipidea/core.c  |   35
> > >  --- include/linux/usb/chipidea.h |  
> > >  13 +
> > >  2 files changed, 41 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > > index 93961ff..e8ceb04 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -405,6 +405,18 @@ static inline void ci_role_destroy(struct ci_hdrc
> > > *ci)
> > > 
> > >   ci_hdrc_host_destroy(ci);
> > >  
> > >  }
> > > 
> > > +static void ci_get_otg_capable(struct ci_hdrc *ci)
> > > +{
> > > + if (ci->platdata->otg_cap != OTG_CAP_ATTR_IS_NOT_EXISTED)
> > 
> > IS_NONEXISTENT , no?
> 
> You mean the English for this string? ok, I can change.

Yes, IS_NOT_EXISTED doesn't make much sense. Sorry, don't mean to nitpick.

> It means if the "otg_cap" is not existed at platform data,
> then, the "otg_cap" can be determined by DCCPARAMS

Then maybe just call it OTG_CAP_ATTR_NOT_SET or something .

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Hi Peter,

> On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > This patchset adds tested otg id switch function and
> > > vbus connect and disconnect detection for chipidea driver.
> > > And fix kinds of bugs found at chipidea drivers after enabling
> > > id and vbus detection.
> > > 
> > > This patch is fully tested at imx6 sabresd platform.
> > > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > > 
> > > Changes for v12:
> > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > - Split more small patches for single function and fix.
> > 
> > I tested the patchset. Here are the results:
> > 
> > - VBUS switching
> > 
> > I'm no longer getting any ID interrupts at all when I apply the patch
> > below. The board stays in HOST mode all the time. If I configure it as
> > peripheral, it works as peripheral. Note with [1], I was able to switch
> > from Peripheral->Host , not the other way around.
> 
> Thanks for your testing. But first, can you have me check
> if your ID wakeup is enabled?

ID wakeup? How do I check?

> I can have a test at mx28evk.
> Is it current upstream kernel can boot mx28evk run?

Yes, I use next-20130711 and it works fine.

> I have a RevC board, I would like to know if
> any patches needed.

Apply just this patchset and you should be all set.

> > --- a/arch/arm/boot/dts/imx28-m28evk.dts
> > +++ b/arch/arm/boot/dts/imx28-m28evk.dts
> > @@ -240,6 +240,8 @@
> > 
> > ahb@8008 {
> > 
> > usb0: usb@8008 {
> > 
> > +   dr_mode = "otg";
> > +   phy_mode = "utmi";
> > 
> > vbus-supply = <®_usb0_vbus>;
> > pinctrl-names = "default";
> > pinctrl-0 = <&usbphy0_pins_a>;
> > 
> > ---
> > 
> > - MX23 UDC issue
> > 
> > I found a workaround. Now running 'dmesg' via telnet through USB CDC link
> > no longer hangs the USB driver, but works as expected. I applied this
> > small patch that enables the streaming mode. Works on MX23EVK. It's
> > surprising this issue doesn't manifest on MX28, maybe MX28 contains a
> > new revision of the controller. I remember there was some discussion
> > about the streaming mode on MXS some time ago.
> 
> It seems not reasonable

I talked to Fabio and he said he met similar issue on MX6, where disabling the 
streaming mode fixed the problem. Could it possibly be so that the 
non-streaming 
mode is broken on mx23?

btw. I also tested this on STMP3780-based device, works the same way as MX23 
does, streaming mode has to be enabled.

> the same question for mx23evk,
> it can run well with current kernel? I have no board on hand,
> let me see if I can find one.

Yes, MX23EVK rev. B2 works just fine with next-20130711 . You need to add the 
DT 
USB sections for MX23EVK, but they can be copied from imx28.dtsi + 
imx28-evk.dts 
.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Hi Peter,

> On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > > Hi Peter,
> > > > 
> > > > > This patchset adds tested otg id switch function and
> > > > > vbus connect and disconnect detection for chipidea driver.
> > > > > And fix kinds of bugs found at chipidea drivers after enabling
> > > > > id and vbus detection.
> > > > > 
> > > > > This patch is fully tested at imx6 sabresd platform.
> > > > > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > > > > 
> > > > > Changes for v12:
> > > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > > - Split more small patches for single function and fix.
> > > > 
> > > > I tested the patchset. Here are the results:
> > > > 
> > > > - VBUS switching
> > > > 
> > > > I'm no longer getting any ID interrupts at all when I apply the patch
> > > > below. The board stays in HOST mode all the time. If I configure it
> > > > as peripheral, it works as peripheral. Note with [1], I was able to
> > > > switch from Peripheral->Host , not the other way around.
> > > 
> > > Thanks for your testing. But first, can you have me check
> > > if your ID wakeup is enabled?
> > 
> > ID wakeup? How do I check?
> 
> See otgsc at controller register, the ID wakeup enable is bit 24.

Yes, ID interrupt (IDIE) is set.

I noticed this backtrace in the kernel bootlog, but this only happens if the 
dr_mode="otg" , it comes from the host-mode irq handler :

[2.757563] irq 238: nobody cared (try booting with the "irqpoll" option)
[2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted 3.10.0-
next-20130711-00013-g011c4b3-dirty #703
[2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from [<80011644>] 
(show_stack+0x10/0x14)
[2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>] 
(__report_bad_irq.isra.6+0x20/0xe0)
[2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from 
[<80065c98>] (note_interrupt+0x16c/0x230)
[2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from [<80064000>] 
(handle_irq_event_percpu+0x10c/0x1a4)
[2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4) from 
[<800640e8>] (handle_irq_event+0x50/0x78)
[2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from [<8006652c>] 
(handle_level_irq+0x88/0x10c)
[2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from [<800638d0>] 
(generic_handle_irq+0x28/0x3c)
[2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from [<8000f84c>] 
(handle_IRQ+0x30/0x84)
[2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>] 
(__irq_svc+0x40/0x6c)
[2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>] 
(__do_softirq+0x90/0x1d8)
[2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from [<80022edc>] 
(irq_exit+0x98/0xd4)
[2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from [<8000f850>] 
(handle_IRQ+0x34/0x84)
[2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from [<80012160>] 
(__irq_svc+0x40/0x6c)
[2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from [<8001d724>] 
(vprintk_emit+0x1bc/0x524)
[2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from [<804da5a4>] 
(printk+0x30/0x40)
[2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>] 
(mousedev_init+0x4c/0x60)
[2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from [<806178fc>] 
(do_one_initcall+0x94/0x14c)
[2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from [<80617b20>] 
(kernel_init_freeable+0x16c/0x22c)
[2.932230] [<80617b20>] (kernel_init_freeable+0x16c/0x22c) from 
[<804d8cbc>] 
(kernel_init+0x8/0x150)
[2.941486] [<804d8cbc>] (kernel_init+0x8/0x150) from [<8000ea70>] 
(ret_from_fork+0x14/0x24)
[2.949932] handlers:
[2.952227] [<8033fc58>] ci_irq
[2.955388] Disabling IRQ #238

btw. do you have any kind of other CI13xxx documentation than what's in the CPU 
datasheets ?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Hi Peter,

> On Fri, Jul 12, 2013 at 03:18:31PM +0200, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> > > > Hi Peter,
> > > > 
> > > > > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > > > > Hi Peter,
> > > > > > 
> > > > > > > This patchset adds tested otg id switch function and
> > > > > > > vbus connect and disconnect detection for chipidea driver.
> > > > > > > And fix kinds of bugs found at chipidea drivers after enabling
> > > > > > > id and vbus detection.
> > > > > > > 
> > > > > > > This patch is fully tested at imx6 sabresd platform.
> > > > > > > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > > > > > > 
> > > > > > > Changes for v12:
> > > > > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > > > > - Split more small patches for single function and fix.
> > > > > > 
> > > > > > I tested the patchset. Here are the results:
> > > > > > 
> > > > > > - VBUS switching
> > > > > > 
> > > > > > I'm no longer getting any ID interrupts at all when I apply the
> > > > > > patch below. The board stays in HOST mode all the time. If I
> > > > > > configure it as peripheral, it works as peripheral. Note with
> > > > > > [1], I was able to switch from Peripheral->Host , not the other
> > > > > > way around.
> > > > > 
> > > > > Thanks for your testing. But first, can you have me check
> > > > > if your ID wakeup is enabled?
> > > > 
> > > > ID wakeup? How do I check?
> > > 
> > > See otgsc at controller register, the ID wakeup enable is bit 24.
> > 
> > Yes, ID interrupt (IDIE) is set.
> > 
> > I noticed this backtrace in the kernel bootlog, but this only happens if
> > the dr_mode="otg" , it comes from the host-mode irq handler :
> > 
> > [2.757563] irq 238: nobody cared (try booting with the "irqpoll"
> > option) [2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted 3.10.0-
> > next-20130711-00013-g011c4b3-dirty #703
> > [2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from [<80011644>]
> > (show_stack+0x10/0x14)
> > [2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>]
> > (__report_bad_irq.isra.6+0x20/0xe0)
> > [2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from
> > [<80065c98>] (note_interrupt+0x16c/0x230)
> > [2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from
> > [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > [2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4) from
> > [<800640e8>] (handle_irq_event+0x50/0x78)
> > [2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from
> > [<8006652c>] (handle_level_irq+0x88/0x10c)
> > [2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from
> > [<800638d0>] (generic_handle_irq+0x28/0x3c)
> > [2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from
> > [<8000f84c>] (handle_IRQ+0x30/0x84)
> > [2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>]
> > (__irq_svc+0x40/0x6c)
> > [2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>]
> > (__do_softirq+0x90/0x1d8)
> > [2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from [<80022edc>]
> > (irq_exit+0x98/0xd4)
> > [2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from [<8000f850>]
> > (handle_IRQ+0x34/0x84)
> > [2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from [<80012160>]
> > (__irq_svc+0x40/0x6c)
> > [2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from [<8001d724>]
> > (vprintk_emit+0x1bc/0x524)
> > [2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from [<804da5a4>]
> > (printk+0x30/0x40)
> > [2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>]
> > (mousedev_init+0x4c/0x60)
> > [2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from [<806178fc>]
> > (do_one_initcall+0x94/0x14c)
> > [2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from
> > [<80617b20>] (kernel_init_

Re: [PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Dear Peter Chen,

> On Mon, Jul 22, 2013 at 03:15:28AM +0200, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > On Fri, Jul 12, 2013 at 03:18:31PM +0200, Marek Vasut wrote:
> > > > Hi Peter,
> > > > 
> > > > > On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> > > > > > Hi Peter,
> > > > > > 
> > > > > > > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > > > > > > Hi Peter,
> > > > > > > > 
> > > > > > > > > This patchset adds tested otg id switch function and
> > > > > > > > > vbus connect and disconnect detection for chipidea driver.
> > > > > > > > > And fix kinds of bugs found at chipidea drivers after
> > > > > > > > > enabling id and vbus detection.
> > > > > > > > > 
> > > > > > > > > This patch is fully tested at imx6 sabresd platform.
> > > > > > > > > My chipidea repo:
> > > > > > > > > https://github.com/hzpeterchen/linux-usb.git
> > > > > > > > > 
> > > > > > > > > Changes for v12:
> > > > > > > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > > > > > > - Split more small patches for single function and fix.
> > > > > > > > 
> > > > > > > > I tested the patchset. Here are the results:
> > > > > > > > 
> > > > > > > > - VBUS switching
> > > > > > > > 
> > > > > > > > I'm no longer getting any ID interrupts at all when I apply
> > > > > > > > the patch below. The board stays in HOST mode all the time.
> > > > > > > > If I configure it as peripheral, it works as peripheral.
> > > > > > > > Note with [1], I was able to switch from Peripheral->Host ,
> > > > > > > > not the other way around.
> > > > > > > 
> > > > > > > Thanks for your testing. But first, can you have me check
> > > > > > > if your ID wakeup is enabled?
> > > > > > 
> > > > > > ID wakeup? How do I check?
> > > > > 
> > > > > See otgsc at controller register, the ID wakeup enable is bit 24.
> > > > 
> > > > Yes, ID interrupt (IDIE) is set.
> > > > 
> > > > I noticed this backtrace in the kernel bootlog, but this only happens
> > > > if the dr_mode="otg" , it comes from the host-mode irq handler :
> > > > 
> > > > [2.757563] irq 238: nobody cared (try booting with the "irqpoll"
> > > > option) [2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted
> > > > 3.10.0- next-20130711-00013-g011c4b3-dirty #703
> > > > [2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from
> > > > [<80011644>] (show_stack+0x10/0x14)
> > > > [2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>]
> > > > (__report_bad_irq.isra.6+0x20/0xe0)
> > > > [2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from
> > > > [<80065c98>] (note_interrupt+0x16c/0x230)
> > > > [2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from
> > > > [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > > > [2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > > > from [<800640e8>] (handle_irq_event+0x50/0x78)
> > > > [2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from
> > > > [<8006652c>] (handle_level_irq+0x88/0x10c)
> > > > [2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from
> > > > [<800638d0>] (generic_handle_irq+0x28/0x3c)
> > > > [2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from
> > > > [<8000f84c>] (handle_IRQ+0x30/0x84)
> > > > [2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>]
> > > > (__irq_svc+0x40/0x6c)
> > > > [2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>]
> > > > (__do_softirq+0x90/0x1d8)
> > > > [2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from
> > > > [<80022edc>] (irq_exit+0x98/0xd4)
> > > > [2.873025]

Re: [PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Hi Peter,

> On Mon, Jul 22, 2013 at 03:40:32AM +0200, Marek Vasut wrote:
> > Dear Peter Chen,
> > 
> > > On Mon, Jul 22, 2013 at 03:15:28AM +0200, Marek Vasut wrote:
> > > > Hi Peter,
> > > > 
> > > > > On Fri, Jul 12, 2013 at 03:18:31PM +0200, Marek Vasut wrote:
> > > > > > Hi Peter,
> > > > > > 
> > > > > > > On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> > > > > > > > Hi Peter,
> > > > > > > > 
> > > > > > > > > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > > > > > > > > Hi Peter,
> > > > > > > > > > 
> > > > > > > > > > > This patchset adds tested otg id switch function and
> > > > > > > > > > > vbus connect and disconnect detection for chipidea
> > > > > > > > > > > driver. And fix kinds of bugs found at chipidea
> > > > > > > > > > > drivers after enabling id and vbus detection.
> > > > > > > > > > > 
> > > > > > > > > > > This patch is fully tested at imx6 sabresd platform.
> > > > > > > > > > > My chipidea repo:
> > > > > > > > > > > https://github.com/hzpeterchen/linux-usb.git
> > > > > > > > > > > 
> > > > > > > > > > > Changes for v12:
> > > > > > > > > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > > > > > > > > - Split more small patches for single function and fix.
> > > > > > > > > > 
> > > > > > > > > > I tested the patchset. Here are the results:
> > > > > > > > > > 
> > > > > > > > > > - VBUS switching
> > > > > > > > > > 
> > > > > > > > > > I'm no longer getting any ID interrupts at all when I
> > > > > > > > > > apply the patch below. The board stays in HOST mode all
> > > > > > > > > > the time. If I configure it as peripheral, it works as
> > > > > > > > > > peripheral. Note with [1], I was able to switch from
> > > > > > > > > > Peripheral->Host , not the other way around.
> > > > > > > > > 
> > > > > > > > > Thanks for your testing. But first, can you have me check
> > > > > > > > > if your ID wakeup is enabled?
> > > > > > > > 
> > > > > > > > ID wakeup? How do I check?
> > > > > > > 
> > > > > > > See otgsc at controller register, the ID wakeup enable is bit
> > > > > > > 24.
> > > > > > 
> > > > > > Yes, ID interrupt (IDIE) is set.
> > > > > > 
> > > > > > I noticed this backtrace in the kernel bootlog, but this only
> > > > > > happens if the dr_mode="otg" , it comes from the host-mode irq
> > > > > > handler :
> > > > > > 
> > > > > > [2.757563] irq 238: nobody cared (try booting with the
> > > > > > "irqpoll" option) [2.764398] CPU: 0 PID: 1 Comm: swapper Not
> > > > > > tainted 3.10.0- next-20130711-00013-g011c4b3-dirty #703
> > > > > > [2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from
> > > > > > [<80011644>] (show_stack+0x10/0x14)
> > > > > > [2.782027] [<80011644>] (show_stack+0x10/0x14) from
> > > > > > [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0)
> > > > > > [2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0)
> > > > > > from [<80065c98>] (note_interrupt+0x16c/0x230)
> > > > > > [2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from
> > > > > > [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > > > > > [2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > > > > > from [<800640e8>] (handle_irq_event+0x50/0x78)
> > > > > > [2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from
> > > > > > [<8006652c>] (handle_level_irq+0x88/0x1

Re: [PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Dear Peter Chen,

> This patchset adds tested otg id switch function and
> vbus connect and disconnect detection for chipidea driver.
> And fix kinds of bugs found at chipidea drivers after enabling
> id and vbus detection.
> 
> This patch is fully tested at imx6 sabresd platform.
> My chipidea repo: https://github.com/hzpeterchen/linux-usb.git

Patchset:

Tested-by: Marek Vasut 

on two STMP3780-based boards (not yet mainline) and two MX28-based boards.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Dear Peter Chen,

> On Thu, Jul 25, 2013 at 07:55:23AM +0200, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > I have not tried the manual switching, but first, you need to close
> > > your vbus supply.
> > 
> > I think we can close this issue, I will now be also getting MX28EVK.
> > Thanks for all your help!
> 
> Great. What's the problem? Any changes for this patchset?

No changes needed, I had VDD5V constantly tied to 5V so I didn't get the VBUS 
interrupt.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: USB gadget on mx6

[Marek Vasut]

Dear Fabio Estevam,

> Hi,
> 
> I am running today's linux-next and I am trying to enable usb gadget on
> mx6:
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
> b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
> index a55113e..ef757d2 100644
> --- a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
> @@ -115,6 +115,14 @@
> status = "okay";
>  };
> 
> +&usbotg {
> +   pinctrl-names = "default";
> +   pinctrl-0 = <&pinctrl_usbotg_1>;
> +   disable-over-current;
> +   dr_mode = "peripheral";
> +   status = "okay";
> +};
> +
>  &usdhc1 {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_usdhc1_2>;
> 
> 
> However, I am not able to get USB ether to work.
> 
> Any suggestions?

Are you not missing usbmisc or vbus regulator ?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: USB gadget on mx6

[Marek Vasut]

Dear Fabio Estevam,

> On Tue, Aug 20, 2013 at 10:32 PM, Peter Chen  wrote:
> > On Wed, Aug 21, 2013 at 12:26:33AM -0300, Fabio Estevam wrote:
> > 
> > Have you enabled CONFIG_USB_CHIPIDEA_UDC?
> > It is not enabled by default.
> 
> Yes, I did.
> 
> I managed to test g_ether on mx28, but not on mx6.
> 
> Will try tomorrow on mx6q-sabresd.
> 

Does OTG work for you on MX28 ?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: USB gadget on mx6

[Marek Vasut]

Dear Peter Chen,

[...]

> > Setting up networking on loopback device:
> > rm: can't remove '/etc/resolv.conf': Permission denied
> > Setting up networking on eth0:
> > You need to manually set your nameserver in /etc/resolv.conf
> > chown: /home/user/.rhosts: Operation not permitted
> > chown: /home/user: Operation not permitted
> > chown: /home/user: Operation not permitted
> > starting pid 696, tty '': '/etc/rc.d/rc_gpu.S'
> > can't run '/etc/rc.d/rc_gpu.S': No such file or directory
> > starting pid 697, tty '': '-/bin/sh'
> > 
> > 
> > BusyBox v1.20.2 () built-in shell (ash)
> > Enter 'help' for a list of built-in commands.
> > 
> > -/bin/sh: can't access tty; job control turned off
> > root@freescale /$
> > root@freescale /$
> > 
> > As you can see there is no g_ether being detected.
> 
> Please compile g_ether as loadable module, not build-in.

Would that make a difference? If so (and I can guess it might), it's a bug 
anyway.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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] usb: phy-mxs: Add auto clock and power setting

[Marek Vasut]

Dear Peter Chen,

> With the auto setting, the PHY's clock and power can be
> recovered correctly from low power mode, it is ganranteed by IC logic.
> 
> Besides, we enable the IC fixes for this PHY at mx6 platform.
> 
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/phy/phy-mxs-usb.c |   30 ++
>  1 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index f80e2e6..868c445 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright 2012 Freescale Semiconductor, Inc.
> + * Copyright 2012-2013 Freescale Semiconductor, Inc.
>   * Copyright (C) 2012 Marek Vasut 
>   * on behalf of DENX Software Engineering GmbH
>   *
> @@ -29,8 +29,17 @@
>  #define HW_USBPHY_CTRL_SET   0x34
>  #define HW_USBPHY_CTRL_CLR   0x38
> 
> +#define HW_USBPHY_IP 0x90
> +#define HW_USBPHY_IP_SET 0x94
> +#define HW_USBPHY_IP_CLR 0x98
> +
>  #define BM_USBPHY_CTRL_SFTRSTBIT(31)
>  #define BM_USBPHY_CTRL_CLKGATE   BIT(30)
> +#define BM_USBPHY_CTRL_ENAUTOSET_USBCLKS BIT(26)
> +#define BM_USBPHY_CTRL_ENAUTOCLR_USBCLKGATE  BIT(25)
> +#define BM_USBPHY_CTRL_ENAUTOCLR_PHY_PWD BIT(20)
> +#define BM_USBPHY_CTRL_ENAUTOCLR_CLKGATE BIT(19)
> +#define BM_USBPHY_CTRL_ENAUTO_PWRON_PLL  BIT(18)
>  #define BM_USBPHY_CTRL_ENUTMILEVEL3  BIT(15)
>  #define BM_USBPHY_CTRL_ENUTMILEVEL2  BIT(14)
>  #define BM_USBPHY_CTRL_ENHOSTDISCONDETECTBIT(1)
> @@ -100,11 +109,24 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
>   /* Power up the PHY */
>   writel(0, base + HW_USBPHY_PWD);
> 
> - /* enable FS/LS device */
> - writel(BM_USBPHY_CTRL_ENUTMILEVEL2 |
> -BM_USBPHY_CTRL_ENUTMILEVEL3,
> + /*
> +  * USB PHY Ctrl Setting
> +  * - Auto clock/power on
> +  * - Enable full/low speed support
> +  */
> + writel(BM_USBPHY_CTRL_ENAUTOSET_USBCLKS |
> + BM_USBPHY_CTRL_ENAUTOCLR_USBCLKGATE |
> + BM_USBPHY_CTRL_ENAUTOCLR_PHY_PWD |
> + BM_USBPHY_CTRL_ENAUTOCLR_CLKGATE |
> + BM_USBPHY_CTRL_ENAUTO_PWRON_PLL |
> + BM_USBPHY_CTRL_ENUTMILEVEL2 |
> + BM_USBPHY_CTRL_ENUTMILEVEL3,
>  base + HW_USBPHY_CTRL_SET);
> 
> + /* Enable IC solution */
> + if (is_mx6q_phy(mxs_phy) || is_mx6sl_phy(mxs_phy))
> + writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);

So why not just add fsl,imx6-phy instead of adding three phy types ?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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] usb: phy-mxs: Add auto clock and power setting

[Marek Vasut]

Dear Peter Chen,

> On Mon, Sep 09, 2013 at 04:41:15PM +0200, Marek Vasut wrote:
> > Dear Peter Chen,
> > 
> > > With the auto setting, the PHY's clock and power can be
> > > recovered correctly from low power mode, it is ganranteed by IC logic.
> > > 
> > > Besides, we enable the IC fixes for this PHY at mx6 platform.
> > > 
> > > Signed-off-by: Peter Chen 
> > > ---
> > > 
> > >  drivers/usb/phy/phy-mxs-usb.c |   30 ++
> > >  1 files changed, 26 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/usb/phy/phy-mxs-usb.c
> > > b/drivers/usb/phy/phy-mxs-usb.c index f80e2e6..868c445 100644
> > > --- a/drivers/usb/phy/phy-mxs-usb.c
> > > +++ b/drivers/usb/phy/phy-mxs-usb.c
> > > @@ -1,5 +1,5 @@
> > > 
> > >  /*
> > > 
> > > - * Copyright 2012 Freescale Semiconductor, Inc.
> > > + * Copyright 2012-2013 Freescale Semiconductor, Inc.
> > > 
> > >   * Copyright (C) 2012 Marek Vasut 
> > >   * on behalf of DENX Software Engineering GmbH
> > >   *
> > > 
> > > @@ -29,8 +29,17 @@
> > > 
> > >  #define HW_USBPHY_CTRL_SET   0x34
> > >  #define HW_USBPHY_CTRL_CLR   0x38
> > > 
> > > +#define HW_USBPHY_IP 0x90
> > > +#define HW_USBPHY_IP_SET 0x94
> > > +#define HW_USBPHY_IP_CLR 0x98
> > > +
> > > 
> > >  #define BM_USBPHY_CTRL_SFTRSTBIT(31)
> > >  #define BM_USBPHY_CTRL_CLKGATE   BIT(30)
> > > 
> > > +#define BM_USBPHY_CTRL_ENAUTOSET_USBCLKS BIT(26)
> > > +#define BM_USBPHY_CTRL_ENAUTOCLR_USBCLKGATE  BIT(25)
> > > +#define BM_USBPHY_CTRL_ENAUTOCLR_PHY_PWD BIT(20)
> > > +#define BM_USBPHY_CTRL_ENAUTOCLR_CLKGATE BIT(19)
> > > +#define BM_USBPHY_CTRL_ENAUTO_PWRON_PLL  BIT(18)
> > > 
> > >  #define BM_USBPHY_CTRL_ENUTMILEVEL3  BIT(15)
> > >  #define BM_USBPHY_CTRL_ENUTMILEVEL2  BIT(14)
> > >  #define BM_USBPHY_CTRL_ENHOSTDISCONDETECTBIT(1)
> > > 
> > > @@ -100,11 +109,24 @@ static int mxs_phy_hw_init(struct mxs_phy
> > > *mxs_phy)
> > > 
> > >   /* Power up the PHY */
> > >   writel(0, base + HW_USBPHY_PWD);
> > > 
> > > - /* enable FS/LS device */
> > > - writel(BM_USBPHY_CTRL_ENUTMILEVEL2 |
> > > -BM_USBPHY_CTRL_ENUTMILEVEL3,
> > > + /*
> > > +  * USB PHY Ctrl Setting
> > > +  * - Auto clock/power on
> > > +  * - Enable full/low speed support
> > > +  */
> > > + writel(BM_USBPHY_CTRL_ENAUTOSET_USBCLKS |
> > > + BM_USBPHY_CTRL_ENAUTOCLR_USBCLKGATE |
> > > + BM_USBPHY_CTRL_ENAUTOCLR_PHY_PWD |
> > > + BM_USBPHY_CTRL_ENAUTOCLR_CLKGATE |
> > > + BM_USBPHY_CTRL_ENAUTO_PWRON_PLL |
> > > + BM_USBPHY_CTRL_ENUTMILEVEL2 |
> > > + BM_USBPHY_CTRL_ENUTMILEVEL3,
> > > 
> > >  base + HW_USBPHY_CTRL_SET);
> > > 
> > > + /* Enable IC solution */
> > > + if (is_mx6q_phy(mxs_phy) || is_mx6sl_phy(mxs_phy))
> > > + writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
> > 
> > So why not just add fsl,imx6-phy instead of adding three phy types ?
> 
> Please see my commit log, the mx6sl-phy has some improvements compared
> to mx6q-phy.

But they're not yet implemented as so far, this stuff is compatible with mx6q , 
no ?

Ok, this situation is something about the DT I would like to know. Shall we use 
mx6q-phy ID for both so far (as the differences between 6sl and 6q are still 
not 
implemented) or go for 6sl-phy and 6q-phy right away?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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] usb: phy-mxs: Add auto clock and power setting

[Marek Vasut]

Dear Peter Chen,

> On Tue, Sep 10, 2013 at 10:18:04AM +0200, Marek Vasut wrote:
> > Dear Peter Chen,
> > 
> > > On Mon, Sep 09, 2013 at 04:41:15PM +0200, Marek Vasut wrote:
> > > > Dear Peter Chen,
> > > 
> > > Please see my commit log, the mx6sl-phy has some improvements compared
> > > to mx6q-phy.
> > 
> > But they're not yet implemented as so far, this stuff is compatible with
> > mx6q , no ?
> > 
> > Ok, this situation is something about the DT I would like to know. Shall
> > we use mx6q-phy ID for both so far (as the differences between 6sl and
> > 6q are still not implemented) or go for 6sl-phy and 6q-phy right away?
> 
> Although it has not implemented, the 6sl DT has already name
> "imx6sl-usbphy", I don't think we should change node name at DT
> file, and change it back when we add different things for mx6q and mx6sl.

OK

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Testing USB connectors on iMX28EVK

[Marek Vasut]

Dear Peter Chen,

> On Thu, Sep 12, 2013 at 12:55:54PM +0200, gianluca wrote:
> > Hello,
> > I just compiled the kernel from git repo of peter chen
> > (git://github.com/hzpeterchen/linux-usb)
> 
> Please use current linux-next tree, it has already supported
> mx28 evk for vbus and id detection.
> I am afraid I have not updated my tree with mx28 updates.

Just curious, how do you boot the linux kernel on your EVK? Do you use imx 
bootlets or uboot (which one, mainline?) or even some other setup ?

[...]

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: 'ci_hdrc.0: failed to start' when using gadgetfs

[Marek Vasut]

Dear Fabio Estevam,

> Hi,
> 
> I am using mx53 (which uses the chipidea driver) and it works fine with
> g_ether.
> 
> Now, I need to test gadgetfs and this is what I get:
> 
> $ modprobe gadgetfs
> gadgetfs: USB Gadget filesystem, version 24 Aug 2004
> 
> $ mkdir  /dev/gadget
> $ mount -t gadgetfs none /dev/gadget
> nop ci_hdrc.0: failed to start (null): -120
> 
> This error message comes from:
> 
> ret = driver->bind(udc->gadget, driver);
> 
> inside udc_bind_to_driver() from udc_core.c
> 
> Any idea as to why driver->bind fails?
> 
> I will start debugging it, but would appreciate any suggestions.

gadgetfs_probe() returns -EISNAM unconditionally. That's rather strange.

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

Re: [PATCH 1/1] usb: gadget: mark init as late_initcall

[Marek Vasut]

Dear Peter Chen,

> Since we introduce -EPROBE_DEFER for udc driver, it will be
> probed at late_initcall if it is defered. When the gadget
> is built in, it will return "couldn't find an available UDC"
> at such case. That's the problem we met at below link:
> 
> http://marc.info/?l=linux-usb&m=137706435611447&w=2
> 
> We have no driver's probe at gadget driver, so we can't return
> -EPROBE_DEFER. And it is also not suitable to defer udc_bind_to_driver
> if the udc is not found temporarily, since it is hard to decide the
> return value for usb_gadget_probe_driver.
> 
> Due to above reasons, mark gadget's init as late_initcall may be a
> moderate solution.
> 
> Signed-off-by: Peter Chen 

Seems this tries to paper over an issue with module dependencies , no?

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

Re: [PATCH 1/1] usb: gadget: mark init as late_initcall

[Marek Vasut]

Dear Chen Peter-B29397,

> > > Since we introduce -EPROBE_DEFER for udc driver, it will be
> > > probed at late_initcall if it is defered. When the gadget
> > > is built in, it will return "couldn't find an available UDC"
> > > at such case. That's the problem we met at below link:
> > > 
> > > http://marc.info/?l=linux-usb&m=137706435611447&w=2
> > > 
> > > We have no driver's probe at gadget driver, so we can't return
> > > -EPROBE_DEFER. And it is also not suitable to defer udc_bind_to_driver
> > > if the udc is not found temporarily, since it is hard to decide the
> > > return value for usb_gadget_probe_driver.
> > > 
> > > Due to above reasons, mark gadget's init as late_initcall may be a
> > > moderate solution.
> > > 
> > > Signed-off-by: Peter Chen 
> > 
> > Seems this tries to paper over an issue with module dependencies , no?
> 
> In fact, there are module dependencies between udc and composite gadget.
> The udc must be created before composite gadget. If the creation of udc
> (or its controller driver) is deferred, the composite gadget driver has no
> way to know it, unless we change gadget framework a lot, eg add probe API
> for composite driver, create a thread to check udc creation if there is no
> udc, etc.

I do see the problem. I'm just worried this change is only pushing the 
inevitable problem away for a bit. I think a bit more discussion would be a 
good 
idea now and this patch is a good starter for that.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 02/12] usb: phy-mxs: Enable IC fixes for mx6 SoC serial

[Marek Vasut]

Hi,

> After adding IC fixes bits, some PHY bugs are fixed by
> IC logic.

Can you please elaborate what those bits do exactly ? They seem like a magic 
stuff to me thus far, which is not exactly helpful . I can't find them in the 
datasheet either.

> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/phy/phy-mxs-usb.c |   10 ++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index 87ba429..831b13e 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -29,6 +29,10 @@
>  #define HW_USBPHY_CTRL_SET   0x34
>  #define HW_USBPHY_CTRL_CLR   0x38
> 
> +#define HW_USBPHY_IP 0x90
> +#define HW_USBPHY_IP_SET 0x94
> +#define HW_USBPHY_IP_CLR 0x98
> +
>  #define BM_USBPHY_CTRL_SFTRSTBIT(31)
>  #define BM_USBPHY_CTRL_CLKGATE   BIT(30)
>  #define BM_USBPHY_CTRL_ENAUTOSET_USBCLKS BIT(26)
> @@ -40,6 +44,8 @@
>  #define BM_USBPHY_CTRL_ENUTMILEVEL2  BIT(14)
>  #define BM_USBPHY_CTRL_ENHOSTDISCONDETECTBIT(1)
> 
> +#define BM_USBPHY_IP_FIX   (BIT(17) | BIT(18))
> +
>  #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
> 
>  enum imx_phy_type {
> @@ -118,6 +124,10 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
>   BM_USBPHY_CTRL_ENUTMILEVEL3,
>  base + HW_USBPHY_CTRL_SET);
> 
> + /* Enable IC solution */
> + if (is_mx6q_phy(mxs_phy) || is_mx6sl_phy(mxs_phy))
> + writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
> +
>   return 0;
>  }

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 05/12] usb: phy-mxs: Add implementation of nofity_suspend and notify_resume

[Marek Vasut]

Dear Peter Chen,

> Add notify_suspend and notify_resume according to different SoCs.
> 
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/phy/phy-mxs-usb.c |   73
> + 1 files changed, 73
> insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index e0b0de0..8661dae 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -197,6 +197,78 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy,
>   return 0;
>  }
> 
> +static int mxs_phy_on_suspend_workaround(struct usb_phy *phy,
> + enum usb_device_speed speed)
> +{
> + dev_dbg(phy->dev, "%s speed device has suspended\n",
> + (speed == USB_SPEED_HIGH) ? "high" : "non-high");

HS : FS/LS you mean?

> +
> + /* delay 4ms to wait bus entering idle */
> + usleep_range(4000, 5000);
> +
> + /*
> +  * Workaround for wakeup signal between portsc.suspendM
> +  * and PHY enters low power mode.
> +  */
> + writel_relaxed(0x, phy->io_priv + HW_USBPHY_PWD);
> + writel_relaxed(0, phy->io_priv + HW_USBPHY_PWD);
> +
> + if (speed == USB_SPEED_HIGH)
> + writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
> + phy->io_priv + HW_USBPHY_CTRL_CLR);
> +
> + return 0;
> +}

[...]

> +/*
> + * For mxs PHY, there are two PHY issues related to suspend/resume.
> + * For mx23 and mx28, both of two issues are existed.
> + * For mx6q and mx6dl, only one issue is existed.
> + * For mx6 sololite, none issue is existed.
> + */
> +static void mxs_phy_workaround(struct mxs_phy *mxs_phy)
> +{
> + if (is_mx23_phy(mxs_phy)) {

This is_mx23_phy() returns 1 for mx28 too? It's not too clear, not even from 
the 
comment above, a short comment here would help for sure.

> + mxs_phy->phy.notify_suspend = mxs_phy_on_suspend_workaround;
> + mxs_phy->phy.notify_resume = mxs_phy_on_resume;
> + } else if (is_mx6q_phy(mxs_phy)) {
> + mxs_phy->phy.notify_suspend = mxs_phy_on_suspend;
> + mxs_phy->phy.notify_resume = mxs_phy_on_resume;
> + }
> +}
> +
>  static int mxs_phy_probe(struct platform_device *pdev)
>  {
>   struct resource *res;
> @@ -257,6 +329,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
> 
>   mxs_phy->clk = clk;
>   mxs_phy->devtype = pdev->id_entry->driver_data;
> + mxs_phy_workaround(mxs_phy);
> 
>   platform_set_drvdata(pdev, &mxs_phy->phy);
--
To unsubscribe from this list: send the line "unsubscribe 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 07/12] usb: phy-mxs: Add implementation of set_wakeup

[Marek Vasut]

Dear Peter Chen,

> When we need the PHY can be waken up by external signals,
> we can call this API. Besides, we call mxs_phy_disconnect_line
> at this API to close the connection between USB PHY and
> controller, after that, the line state from controller is SE0.
> Once the PHY is out of power, without calling mxs_phy_disconnect_line,
> there are unknown wakeups due to dp/dm floating at device mode.
> 
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/phy/phy-mxs-usb.c |   82
> - 1 files changed, 81
> insertions(+), 1 deletions(-)

[...]

> +static int mxs_phy_set_wakeup(struct usb_phy *x, bool enabled)
> +{
> + struct mxs_phy *mxs_phy = to_mxs_phy(x);
> + u32 value = BM_USBPHY_CTRL_ENVBUSCHG_WKUP |
> + BM_USBPHY_CTRL_ENDPDMCHG_WKUP |
> + BM_USBPHY_CTRL_ENIDCHG_WKUP;

Does this stuff pass checkpatch at all? I mean, this alignment seems a bit 
strange.

> + if (enabled) {
> + mxs_phy_disconnect_line(mxs_phy, true);
> + writel_relaxed(value, x->io_priv + HW_USBPHY_CTRL_SET);
> + } else {
> + writel_relaxed(value, x->io_priv + HW_USBPHY_CTRL_CLR);
> + mxs_phy_disconnect_line(mxs_phy, false);
> + }
> +
> + return 0;
> +}
> +
>  static int mxs_phy_on_connect(struct usb_phy *phy,
>   enum usb_device_speed speed)
>  {
> @@ -315,6 +390,10 @@ static int mxs_phy_probe(struct platform_device *pdev)
>   }
>   }
> 
> + if (of_find_property(np, "disconnect_line_without_vbus", NULL) &&
> + mxs_phy->regmap_anatop)

You might want to introduce a variable here to make the condition shorter:

var = of_find
if (var && mxs_phy->...)

> + mxs_phy->disconnect_line_without_vbus_is_needed = true;
> +
>   mxs_phy->phy.io_priv= base;
>   mxs_phy->phy.dev= &pdev->dev;
>   mxs_phy->phy.label  = DRIVER_NAME;
> @@ -324,6 +403,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
>   mxs_phy->phy.notify_connect = mxs_phy_on_connect;
>   mxs_phy->phy.notify_disconnect  = mxs_phy_on_disconnect;
>   mxs_phy->phy.type   = USB_PHY_TYPE_USB2;
> + mxs_phy->phy.set_wakeup = mxs_phy_set_wakeup;
> 
>   ATOMIC_INIT_NOTIFIER_HEAD(&mxs_phy->phy.notifier);
--
To unsubscribe from this list: send the line "unsubscribe 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 11/12] usb: phy-mxs: update binding for adding disconnect line property

[Marek Vasut]

Dear Peter Chen,

> This property is used to disconnect line between USB PHY and
> USB controller.
> 
> Signed-off-by: Peter Chen 
> ---
>  Documentation/devicetree/bindings/usb/mxs-phy.txt |4 
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt
> b/Documentation/devicetree/bindings/usb/mxs-phy.txt index 1a9bd85..099b0bb
> 100644
> --- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
> +++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> @@ -5,6 +5,9 @@ Required properties:
>  - reg: Should contain registers location and length
>  - interrupts: Should contain phy interrupt
>  - fsl,anatop: phandle for anatop register
> +- disconnect_line_without_vbus: needs to disconnect
> +connection between USB PHY and controller, it can avoid
> +unexpected wakeup interrupt when the PHY is out of power

Uh oh, this might needs some rewording. I didn't understand the reason for this 
prop before I checked the 12/12 patch.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 10/11] usb: chipidea: imx: add binding for supporting runtime pm

[Marek Vasut]

Dear Peter Chen,

> On Sat, Oct 12, 2013 at 10:40:37AM -0400, Alan Stern wrote:
> > On Sat, 12 Oct 2013, Peter Chen wrote:
> > > Add property for supporting runtime power management
> > > 
> > > Signed-off-by: Peter Chen 
> > > ---
> > > 
> > >  .../devicetree/bindings/usb/ci13xxx-imx.txt|2 ++
> > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
> > > b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt index
> > > b4b5b79..f666598 100644
> > > --- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
> > > +++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
> > > 
> > > @@ -18,6 +18,7 @@ Optional properties:
> > >  - vbus-supply: regulator for vbus
> > >  - disable-over-current: disable over current detect
> > >  - external-vbus-divider: enables off-chip resistor divider for Vbus
> > > 
> > > +- supports_runtime_pm: enable runtime pm support
> > > 
> > >  Examples:
> > >  usb@02184000 { /* USB OTG */
> > > 
> > > @@ -28,4 +29,5 @@ usb@02184000 { /* USB OTG */
> > > 
> > >   fsl,usbmisc = <&usbmisc 0>;
> > >   disable-over-current;
> > >   external-vbus-divider;
> > > 
> > > + supports_runtime_pm;
> > > 
> > >  };
> > 
> > This does not sound like a property of the hardware.  What's the
> > _hardware_ difference between parts that support runtime PM and parts
> > that don't?
> 
> Thanks.
> 
> From my point, all hardware using chipidea core should support runtime pm.
> But some of platforms need special glue layer operations to support
> it, it will break other platforms if enable chipidea core runtime pm.
> Since device tree describes hardware property, maybe I should move
> it to glue layer, or do you have any suggestions?

You should certainly move it out of DT. This is linux-specific property, it has 
nothing to do with HW. The best course of action would be to fix those 
platforms 
that are broken by runtime PM.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 02/12] usb: phy-mxs: Enable IC fixes for mx6 SoC serial

[Marek Vasut]

Dear Peter Chen,

> On Sat, Oct 12, 2013 at 11:38:16AM +0200, Marek Vasut wrote:
> > Hi,
> > 
> > > After adding IC fixes bits, some PHY bugs are fixed by
> > > IC logic.
> > 
> > Can you please elaborate what those bits do exactly ? They seem like a
> > magic stuff to me thus far, which is not exactly helpful . I can't find
> > them in the datasheet either.
> 
> Yes, these bits are added at late TO verion for i.mx 6, and these TO
> versions will be for mass production, unfortunately, the related doc
> update may be forgotten.
> 
> These two bits are related to two PHY bugs, two PHY bugs are still existed
> at mx28 and mx23, one bug is fixed at mx6dq and mx6dl, and both of two
> bugs are fixed at later mx6 (like mx6sololite and later SoCs), but the IC
> fixes are not enabled by default, it needs software opens it.

Sure, I get it. But what exactly does that bit do? Can you add a proper (and 
likely beefy) comment into the code to supplement the missing parts in the 
datasheet?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 05/12] usb: phy-mxs: Add implementation of nofity_suspend and notify_resume

[Marek Vasut]

Dear Peter Chen,

> On Sat, Oct 12, 2013 at 11:42:06AM +0200, Marek Vasut wrote:
> > Dear Peter Chen,
> > 
> > > Add notify_suspend and notify_resume according to different SoCs.
> > > 
> > > Signed-off-by: Peter Chen 
> > > ---
> > > 
> > >  drivers/usb/phy/phy-mxs-usb.c |   73
> > > 
> > > + 1 files changed, 73
> > > insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/usb/phy/phy-mxs-usb.c
> > > b/drivers/usb/phy/phy-mxs-usb.c index e0b0de0..8661dae 100644
> > > --- a/drivers/usb/phy/phy-mxs-usb.c
> > > +++ b/drivers/usb/phy/phy-mxs-usb.c
> > > @@ -197,6 +197,78 @@ static int mxs_phy_on_disconnect(struct usb_phy
> > > *phy,
> > > 
> > >   return 0;
> > >  
> > >  }
> > > 
> > > +static int mxs_phy_on_suspend_workaround(struct usb_phy *phy,
> > > + enum usb_device_speed speed)
> > > +{
> > > + dev_dbg(phy->dev, "%s speed device has suspended\n",
> > > + (speed == USB_SPEED_HIGH) ? "high" : "non-high");
> > 
> > HS : FS/LS you mean?
> 
> Yes, it is what I mean.
> OK, I will change to HS and FS/LS.
> 
> > > +/*
> > > + * For mxs PHY, there are two PHY issues related to suspend/resume.
> > > + * For mx23 and mx28, both of two issues are existed.
> > > + * For mx6q and mx6dl, only one issue is existed.
> > > + * For mx6 sololite, none issue is existed.
> > > + */
> > > +static void mxs_phy_workaround(struct mxs_phy *mxs_phy)
> > > +{
> > > + if (is_mx23_phy(mxs_phy)) {
> > 
> > This is_mx23_phy() returns 1 for mx28 too? It's not too clear, not even
> > from the comment above, a short comment here would help for sure.
> 
> mx23 and mx28 PHY are the same PHY, so the fixes are the same.
> Anything I need to add?

I believe it's OK. But identifying MX28 PHY via is_mx23_phy() might be a little 
confusing. I'm just trying to keep this driver flushed out of my brain here and 
do an unbiased review ;-)

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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/5] usb: chipidea: imx: set CI_HDRC_IMX28_WRITE_FIX for imx28

[Marek Vasut]

Dear Peter Chen,

> Due to imx28 needs ARM swp instruction for writing, we set
> CI_HDRC_IMX28_WRITE_FIX for imx28.
> 
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c |   32 ++--
>  1 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> b/drivers/usb/chipidea/ci_hdrc_imx.c index 023d3cb..68f7f5e 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -23,6 +23,26 @@
>  #include "ci.h"
>  #include "ci_hdrc_imx.h"
> 
> +#define CI_HDRC_IMX_IMX28_WRITE_FIX BIT(0)
> +
> +struct ci_hdrc_imx_platform_flag {
> + unsigned int flags;
> +};
> +
> +static const struct ci_hdrc_imx_platform_flag imx27_usb_data = {
> +};
> +
> +static const struct ci_hdrc_imx_platform_flag imx28_usb_data = {
> + .flags = CI_HDRC_IMX_IMX28_WRITE_FIX,
> +};
> +
> +static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
> + { .compatible = "fsl,imx28-usb", .data = &imx28_usb_data},
> + { .compatible = "fsl,imx27-usb", .data = &imx27_usb_data},

Just a nit-pick, but the order here is wrong ;-)
[...]
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 4/5] usb: doc: chipidea: imx: add compatiable string for imx28 SoC

[Marek Vasut]

Dear Peter Chen,

> Due to imx28 usb has special write requirement compared to other imx SoCs.
> 
> Signed-off-by: Peter Chen 
> ---
> Changes for v3:
> - 4/5, 5/5 are added
> 
>  .../devicetree/bindings/usb/ci13xxx-imx.txt|3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
> b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt index
> b4b5b79..a502d41 100644
> --- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
> +++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
> @@ -1,7 +1,8 @@
>  * Freescale i.MX ci13xxx usb controllers
> 
>  Required properties:
> -- compatible: Should be "fsl,imx27-usb"
> +- compatible: "fsl,imx28-usb" for imx28 SoC, "fsl,imx27-usb" for
> +non-imx28 SoC.
>  - reg: Should contain registers location and length
>  - interrupts: Should contain controller interrupt

Does the ERRATA apply to MX23 as well btw?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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/5] usb: chipidea: imx: set CI_HDRC_IMX28_WRITE_FIX for imx28

[Marek Vasut]

Hi Shawn,

> On Sun, Oct 27, 2013 at 05:25:36PM +0100, Marek Vasut wrote:
> > > +static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
> > > + { .compatible = "fsl,imx28-usb", .data = &imx28_usb_data},
> > > + { .compatible = "fsl,imx27-usb", .data = &imx27_usb_data},
> > 
> > Just a nit-pick, but the order here is wrong ;-)
> 
> Oh, no.  Before of_match_device() gets improved to find the best match,
> we have to sort the table from the most specific entry to the most
> generic one.

Oh, thanks for explaining!

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 2/5] usb: chipidea: add freescale imx28 special write register method

[Marek Vasut]

Dear Hector Palacios,

> Dear Peter,
> 
> On 10/25/2013 08:02 AM, Peter Chen wrote:
> > According to Freescale imx28 Errata, "ENGR119653 USB: ARM to USB
> > register error issue", All USB register write operations must
> > use the ARM SWP instruction. So, we implement special hw_write
> > and hw_test_and_clear for imx28.
> > 
> > Discussion for it at below:
> > http://marc.info/?l=linux-usb&m=137996395529294&w=2
> > 
> > Signed-off-by: Peter Chen
> >  ---
> > Changes for v2:
> > - Rebase to latest usb-next tree
> > 
> >   drivers/usb/chipidea/ci.h|   23 +++
> >   drivers/usb/chipidea/core.c  |2 ++
> >   drivers/usb/chipidea/host.c  |1 +
> >   include/linux/usb/chipidea.h |1 +
> >   4 files changed, 27 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > index 1c94fc5..4eb61d0 100644
> > --- a/drivers/usb/chipidea/ci.h
> > +++ b/drivers/usb/chipidea/ci.h
> > @@ -173,6 +173,8 @@ struct ci_hdrc {
> > 
> > struct dentry   *debugfs;
> > boolid_event;
> > boolb_sess_valid_event;
> > 
> > +   /* imx28 needs swp instruction for writing */
> > +   boolimx28_write_fix;
> > 
> >   };
> >   
> >   static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> > 
> > @@ -253,6 +255,13 @@ static inline u32 hw_read(struct ci_hdrc *ci, enum
> > ci_hw_regs reg, u32 mask)
> > 
> > return ioread32(ci->hw_bank.regmap[reg]) & mask;
> >   
> >   }
> > 
> > +#ifdef CONFIG_SOC_IMX28
> > +static inline void imx28_ci_writel(u32 val32, volatile u32 *addr)
> > +{
> > +   __asm__ ("swp %0, %0, [%1]" : : "r"(val32), "r"(addr));
> > +}
> > +#endif
> > +
> > 
> >   /**
> >   
> >* hw_write: writes to a hw register
> >* @reg:  register index
> > 
> > @@ -266,7 +275,14 @@ static inline void hw_write(struct ci_hdrc *ci, enum
> > ci_hw_regs reg,
> > 
> > data = (ioread32(ci->hw_bank.regmap[reg]) & ~mask)
> > 
> > | (data & mask);
> > 
> > +#ifdef CONFIG_SOC_IMX28
> > +   if (ci->imx28_write_fix)
> > +   imx28_ci_writel(data, ci->hw_bank.regmap[reg]);
> > +   else
> > +   iowrite32(data, ci->hw_bank.regmap[reg]);
> > +#else
> > 
> > iowrite32(data, ci->hw_bank.regmap[reg]);
> > 
> > +#endif
> > 
> >   }
> >   
> >   /**
> > 
> > @@ -281,7 +297,14 @@ static inline u32 hw_test_and_clear(struct ci_hdrc
> > *ci, enum ci_hw_regs reg,
> > 
> >   {
> >   
> > u32 val = ioread32(ci->hw_bank.regmap[reg]) & mask;
> > 
> > +#ifdef CONFIG_SOC_IMX28
> > +   if (ci->imx28_write_fix)
> > +   imx28_ci_writel(val, ci->hw_bank.regmap[reg]);
> > +   else
> > +   iowrite32(val, ci->hw_bank.regmap[reg]);
> > +#else
> > 
> > iowrite32(val, ci->hw_bank.regmap[reg]);
> > 
> > +#endif
> > 
> > return val;
> >   
> >   }
> 
> Can't we remove the #ifdefs CONFIG_SOC_IMX28 all around?
> The check is done on the new flag ci->imx28_write_fix which exists for all
> platforms, isn't it?.

The SWP instruction is specific to ARM, so you'd need to stub-out the 
imx28_ci_writel() with ifdef then.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: chipidea: i.MX: Probe usbmisc early

[Marek Vasut]

Probe the usbmisc driver earlier, otherwise the ci_hdrc_imx.c will
get -EPROBE_DEFER from usbmisc when both are compiled into the kernel
and thus USB gadget mode won't work.

Signed-off-by: Marek Vasut 
Cc: Alexander Shishkin 
Cc: Peter Chen 
---
 drivers/usb/chipidea/usbmisc_imx.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

NOTE: I'm not sure if this is a correct approach. The fs_initcall() is
  what we use in the PCI express driver to circumvent similar ordering
  issue, but maybe it should be in some arch_initcall() or something?

diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c
index 8a1094b..7764101 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -224,7 +224,6 @@ static int usbmisc_imx_remove(struct platform_device *pdev)
 }
 
 static struct platform_driver usbmisc_imx_driver = {
-   .probe = usbmisc_imx_probe,
.remove = usbmisc_imx_remove,
.driver = {
.name = "usbmisc_imx",
@@ -233,7 +232,17 @@ static struct platform_driver usbmisc_imx_driver = {
 },
 };
 
-module_platform_driver(usbmisc_imx_driver);
+static int __init usbmisc_imx_init(void)
+{
+   return platform_driver_probe(&usbmisc_imx_driver, usbmisc_imx_probe);
+}
+fs_initcall(usbmisc_imx_init);
+
+static void __exit usbmisc_imx_exit(void)
+{
+   platform_driver_unregister(&usbmisc_imx_driver);
+}
+module_exit(usbmisc_imx_exit);
 
 MODULE_ALIAS("platform:usbmisc-imx");
 MODULE_LICENSE("GPL v2");
-- 
1.8.4.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] USB: chipidea: i.MX: Probe usbmisc early

[Marek Vasut]

Dear Peter Chen,

> > Probe the usbmisc driver earlier, otherwise the ci_hdrc_imx.c will
> > get -EPROBE_DEFER from usbmisc when both are compiled into the kernel
> > and thus USB gadget mode won't work.
> 
> Hi Marek, you know the root cause of it, you suggested using
> a better way to fix it.
> 
> http://marc.info/?l=linux-usb&m=138060107809076&w=2

Argh, I see. So what's the solution here really ?

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

Re: [PATCH] USB: chipidea: i.MX: Probe usbmisc early

[Marek Vasut]

Hi Michael,

> Hi Marek,
> 
> On Sun, Nov 17, 2013 at 11:59:10PM +0100, Marek Vasut wrote:
> > Dear Peter Chen,
> > 
> > > > Probe the usbmisc driver earlier, otherwise the ci_hdrc_imx.c will
> > > > get -EPROBE_DEFER from usbmisc when both are compiled into the kernel
> > > > and thus USB gadget mode won't work.
> > > 
> > > Hi Marek, you know the root cause of it, you suggested using
> > > a better way to fix it.
> > > 
> > > http://marc.info/?l=linux-usb&m=138060107809076&w=2
> > 
> > Argh, I see. So what's the solution here really ?
> 
> I think one solution here is to load the composite
> driver as a module, after you are sure the corresponding
> udc is already registered.
> 
> Alternativly, we also have the configfs interface now. Look into:
> 
> Documentation/usb/gadget_configfs.txt
> 
> It should be possible to attach an udc to the gadget-driver
> afterwards with:
> 
> echo $UDC_NAME > $CONFIGFS_HOME/usb_gadget/$GADGET_NAME/UDC

Sure, but all these need manual intervention or are otherwise limited. One 
would 
expect this probing would work automagically (and it usually does). Thus my 
question.

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

Re: [PATCH 1/1] usb: chipidea: imx: avoid unnecessary probe defer every time

[Marek Vasut]

Hello Peter,

> The ci_hdrc_imx's probe needs usbmisc_imx to be loadded beforehand,
> so it is better we load usbmisc_imx first.
> 
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/chipidea/Makefile |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index a99d980..7345d21 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -17,5 +17,5 @@ ifneq ($(CONFIG_PCI),)
>  endif
> 
>  ifneq ($(CONFIG_OF),)
> - obj-$(CONFIG_USB_CHIPIDEA)  += ci_hdrc_imx.o usbmisc_imx.o
> + obj-$(CONFIG_USB_CHIPIDEA)  += usbmisc_imx.o ci_hdrc_imx.o
>  endif

How is this supposed to work please? Can you explain me the trick here please?

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

Re: [PATCH 1/1] usb: chipidea: imx: avoid unnecessary probe defer every time

[Marek Vasut]

Hi Peter,

> > >  endif
> > >  
> > >  ifneq ($(CONFIG_OF),)
> > > 
> > > - obj-$(CONFIG_USB_CHIPIDEA)  += ci_hdrc_imx.o usbmisc_imx.o
> > > + obj-$(CONFIG_USB_CHIPIDEA)  += usbmisc_imx.o ci_hdrc_imx.o
> > > 
> > >  endif
> > 
> > How is this supposed to work please? Can you explain me the trick here
> > please?
> 
> The probe will be executed according to compile order which is decided by
> Makefile, you can try if it can fix your problem you reported several days
> before.

Why does the compile order here matter please? What if I use 'make -j N', which 
will cause the files to be compiled in parallel?

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

Re: [PATCH 1/1] usb: chipidea: imx: avoid unnecessary probe defer every time

[Marek Vasut]

Hello Greg,

> On Fri, Nov 22, 2013 at 04:34:14PM +0100, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > > >  endif
> > > > >  
> > > > >  ifneq ($(CONFIG_OF),)
> > > > > 
> > > > > - obj-$(CONFIG_USB_CHIPIDEA)  += ci_hdrc_imx.o usbmisc_imx.o
> > > > > + obj-$(CONFIG_USB_CHIPIDEA)  += usbmisc_imx.o ci_hdrc_imx.o
> > > > > 
> > > > >  endif
> > > > 
> > > > How is this supposed to work please? Can you explain me the trick
> > > > here please?
> > > 
> > > The probe will be executed according to compile order which is decided
> > > by Makefile, you can try if it can fix your problem you reported
> > > several days before.
> > 
> > Why does the compile order here matter please? What if I use 'make -j N',
> > which will cause the files to be compiled in parallel?
> 
> Yes, but not linked "in parallel".  Link order matters, not building
> object order.

Ah! So this is the trick here. Thank you for explaining!

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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/4 V2] ARM: mx28: Add USB PHY overcurrent pinmux

[Marek Vasut]

Hello Michael,

> Hi Marek, Peter,
> 
> On Sat, Aug 25, 2012 at 01:51:38AM +0200, Marek Vasut wrote:
> > Add the pinmux settings for USB PHY overcurrent pins.
> > 
> > Signed-off-by: Marek Vasut 
> > Cc: Chris Ball 
> > Cc: Fabio Estevam 
> > Cc: Mark Brown 
> > Cc: Shawn Guo 
> > ---
> > 
> >  arch/arm/boot/dts/imx28.dtsi |   30 ++
> >  1 file changed, 30 insertions(+)
> > 
> > V2: This is pulled from the M28EVK and SPS1 board files, put it into
> > common
> > 
> > file.
> > 
> > diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> > index b996c2d..3cba62d 100644
> > --- a/arch/arm/boot/dts/imx28.dtsi
> > +++ b/arch/arm/boot/dts/imx28.dtsi
> > @@ -520,6 +520,36 @@
> > 
> > fsl,voltage = <1>;
> > fsl,pull-up = <1>;
> > 
> > };
> > 
> > +
> > +   usbphy0_pins_a: usbphy0@0 {
> > +   reg = <0>;
> > +   fsl,pinmux-ids = <
> > +   0x2152 /* 
MX28_PAD_SSP2_SS2__USB0_OVERCURRENT */
> > +   >;
> > +   fsl,drive-strength = <2>;
> > +   fsl,voltage = <1>;
> > +   fsl,pull-up = <0>;
> > +   };
> > +
> > +   usbphy0_pins_b: usbphy0@1 {
> > +   reg = <0>;
> > +   fsl,pinmux-ids = <
> > +   0x3061 /* 
MX28_PAD_AUART1_CTS__USB0_OVERCURRENT */
> > +   >;
> > +   fsl,drive-strength = <2>;
> > +   fsl,voltage = <1>;
> > +   fsl,pull-up = <0>;
> > +   };
> > +
> > +   usbphy1_pins_a: usbphy1@0 {
> > +   reg = <0>;
> > +   fsl,pinmux-ids = <
> > +   0x2142 /* 
MX28_PAD_SSP2_SS1__USB1_OVERCURRENT */
> > +   >;
> > +   fsl,drive-strength = <2>;
> > +   fsl,voltage = <1>;
> > +   fsl,pull-up = <0>;
> > +   };
> > 
> > };
> > 
> > digctl@8001c000 {
> 
> @Marek: Did you test the overcurrent functionality with the MX28 and this
> pinmux?
> 
> I currently can not trigger any overcurrent events and also don't
> see changes in the PORTSC register after pulling the OC pin to 3V3.

Sorry for the late reply, I see Peter already replied. Do you see the changes 
if 
you configure the pin as a GPIO at least ?

Best regards,
--
To unsubscribe from this list: send the line "unsubscribe 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 RESEND] USB: chipidea: convert to use devm_request_irq

[Marek Vasut]

Dear Richard Zhao,

You know, commit message would be nice to have.

> Signed-off-by: Richard Zhao 

Otherwise

Reviewed-by: Marek Vasut 

> ---
>  drivers/usb/chipidea/core.c |5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 307651b..0942b9b 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -486,8 +486,8 @@ static int __devinit ci_hdrc_probe(struct
> platform_device *pdev) }
> 
>   platform_set_drvdata(pdev, ci);
> - ret = request_irq(ci->irq, ci_irq, IRQF_SHARED, ci->platdata->name,
> -   ci);
> + ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED,
> + ci->platdata->name, ci);
>   if (ret)
>   goto stop;
> 
> @@ -518,7 +518,6 @@ static int __devexit ci_hdrc_remove(struct
> platform_device *pdev) flush_workqueue(ci->wq);
>   destroy_workqueue(ci->wq);
>   device_remove_file(ci->dev, &dev_attr_role);
> - free_irq(ci->irq, ci);
>   ci_role_stop(ci);
> 
>   return 0;

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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/4] usb: chipidea: add otg id switch and vbus connect/disconnect detect

[Marek Vasut]

Dear Peter Chen,

[...]

> +static void ci_wait_vbus_stable(struct ci13xxx *ci, bool low)
> +{
> + unsigned long timeout;
> + u32 otgsc = hw_read(ci, OP_OTGSC, ~0);
> +
> + timeout = jiffies + CI_WAIT_VBUS_STABLE_TIMEOUT;
> +
> + if (low) {
> + while (otgsc & OTGSC_BSV) {
> + if (time_after(jiffies, timeout)) {
> + dev_err(ci->dev, "wait vbus lower than\
> + B_SESS_VALID timeout!\n");
> + return;
> + }
> + msleep(20);
> + otgsc = hw_read(ci, OP_OTGSC, ~0);
> + }
> + } else {
> + while (!(otgsc & OTGSC_AVV)) {
> + if (time_after(jiffies, timeout)) {
> + dev_err(ci->dev, "wait vbus higher than\
> + A_VBUS_VALID timeout!\n");
> + return;
> + }
> + msleep(20);
> + otgsc = hw_read(ci, OP_OTGSC, ~0);
> + }

Just a nitpick really, but what about parametrizing this code so we won't have 
two copies of the same loop?

Say:

uint32_t bit = low ? OTGSC_BSV : OTGSC_AWW;
const char *name = low ? "B_SESS_VALID" : "A_VBUS_VALID";

while (!(otgsc & bit)) {
if (time_after(jiffies, timeout)) {
dev_err(ci->dev, "wait vbus higher than\
%s timeout!\n", name);
return;
}
msleep(20);
otgsc = hw_read(ci, OP_OTGSC, ~0);
}

And shall we not be more careful about endless loops?

[...]
--
To unsubscribe from this list: send the line "unsubscribe 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: chipidea: imx: add internal vbus regulator control

[Marek Vasut]

Dear Peter Chen,

> - For host, the vbus should always be on.
> - For otg, the vbus is off defaultly, the vbus needs to be
> turned on/off when usb role switches.
> 
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/chipidea/ci.h  |2 +
>  drivers/usb/chipidea/ci13xxx_imx.c |   67
> ++-- 2 files changed, 51 insertions(+), 18
> deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index d32f932..cd42b59 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -170,6 +170,8 @@ struct ci13xxx {
>  #define B_SESS_VALID 1
>   unsigned long events;
>   struct usb_otg  otg;
> + /* used to control internal vbus regulator */
> + struct regulator *reg_vbus;
>  };
> 
>  static inline struct ci_role_driver *ci_role(struct ci13xxx *ci)
> diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
> b/drivers/usb/chipidea/ci13xxx_imx.c index 0f5ca4b..935de97 100644
> --- a/drivers/usb/chipidea/ci13xxx_imx.c
> +++ b/drivers/usb/chipidea/ci13xxx_imx.c
> @@ -89,14 +89,34 @@ static struct ci13xxx_platform_data
> ci13xxx_imx_platdata __devinitdata  = { .name = "ci13xxx_imx",
>   .flags  = CI13XXX_REQUIRE_TRANSCEIVER |
> CI13XXX_PULLUP_ON_VBUS |
> -   CI13XXX_DISABLE_STREAMING,
> +   CI13XXX_DISABLE_STREAMING |
> +   CI13XXX_REGS_SHARED,

Why is this REGS_SHARED change needed?

>   .capoffset  = DEF_CAPOFFSET,
>  };
> 
> +static int ci13xxx_otg_set_vbus(struct usb_otg *otg, bool enabled)
> +{
> +
> + struct ci13xxx  *ci = container_of(otg, struct ci13xxx, otg);
> + struct regulator *reg_vbus = ci->reg_vbus;
> +
> + WARN_ON(!reg_vbus);
> +
> + if (reg_vbus) {

if (!reg_vbus) {
WARN and return;

}


if (enabled)
...

You'll cut down on the indent and will make it more readable I believe.

[...]
--
To unsubscribe from this list: send the line "unsubscribe 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/5] usb: phy: mxs-usb: Remove redundant platform_set_drvdata()

[Marek Vasut]

Dear Sachin Kamat,

fixing CC here

> Commit 0998d06310 (device-core: Ensure drvdata = NULL when no
> driver is bound) removes the need to set driver data field to
> NULL.
> 
> Signed-off-by: Sachin Kamat 
> Cc: Marek Vasut 
> ---
>  drivers/usb/phy/phy-mxs-usb.c |2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index 9d4381e..3b64277 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -180,8 +180,6 @@ static int mxs_phy_remove(struct platform_device *pdev)
> 
>   usb_remove_phy(&mxs_phy->phy);
> 
> - platform_set_drvdata(pdev, NULL);
> -
>   return 0;
>  }

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 v9 REBASE 0/9] add imx usb driver based on Greg next tree

[Marek Vasut]

Dear Greg KH,

> On Sat, Jul 07, 2012 at 10:56:39PM +0800, Richard Zhao wrote:
> > The work is based on  ci13xxx rework done by Alexander Shishkin.
> > 
> > To let Greg pick up, I also added patches Alex queued that I depends on.
> 
> I've applied everything but patch number 6, which I've responded to
> with some questions about it.

That #6 emerged after discussion with Alan. It's unlikely on most hardware, but 
there are a few pieces where that case will happen.

> thanks,
> 
> greg k-h

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 v9 REBASE 6/9] USB: notify phy when root hub port connect change

[Marek Vasut]

Dear Richard Zhao,

> On Sat, Jul 07, 2012 at 10:56:45PM +0800, Richard Zhao wrote:
> > Phy may need to change settings when port connect change.
> > 
> > Signed-off-by: Richard Zhao 
> > Tested-by: Subodh Nijsure 
> > ---
> > 
> >  drivers/usb/core/hub.c |8 
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 4cc8dc9..2ba9d84 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -20,6 +20,7 @@
> > 
> >  #include 
> >  #include 
> >  #include 
> > 
> > +#include 
> > 
> >  #include 
> >  #include 
> >  #include 
> > 
> > @@ -4037,6 +4038,13 @@ static void hub_port_connect_change(struct usb_hub
> > *hub, int port1,
> > 
> > }
> > 
> > }
> > 
> > +   if (unlikely(hcd->phy && !hdev->parent)) {
> > +   if (portstatus & USB_PORT_STAT_CONNECTION)
> > +   usb_phy_notify_connect(hcd->phy, port1);
> > +   else
> > +   usb_phy_notify_disconnect(hcd->phy, port1);
> 
> There's another issue. When hcd is removed, notify disconnect is not
> called. Is it ok, if I remove the above two line and add below patch:
> 
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1924,6 +1924,11 @@ void usb_disconnect(struct usb_device **pdev)
>*/
>   device_del(&udev->dev);
> 
> + if (udev->parent && !udev->parent->parent) {
> + struct usb_hcd *hcd = bus_to_hcd(udev->bus);
> + usb_phy_notify_disconnect(hcd->phy, udev->portnum);
> + }

Shouldn't that go before device_del() ?

> +
>   /* Free the device number and delete the parent's children[]
>* (or root_hub) pointer.
>*/
> 
> 
> Thanks
> Richard

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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 v9 REBASE 6/9] USB: notify phy when root hub port connect change

[Marek Vasut]

Dear Richard Zhao,

[...]

> > > --- a/drivers/usb/core/hub.c
> > > +++ b/drivers/usb/core/hub.c
> > > @@ -1924,6 +1924,11 @@ void usb_disconnect(struct usb_device **pdev)
> > > 
> > >*/
> > >   
> > >   device_del(&udev->dev);
> > > 
> > > + if (udev->parent && !udev->parent->parent) {
> > > + struct usb_hcd *hcd = bus_to_hcd(udev->bus);
> > > + usb_phy_notify_disconnect(hcd->phy, udev->portnum);
> > > + }
> > 
> > Shouldn't that go before device_del() ?
> 
> Any difference?

I was worried some corruption of other members in udev structure might happen, 
but I'm not so sure anymore after taking deer look.

> Thanks
> Richard

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Toggling green led on Olinuxino Maxi, also toggles USB

[Marek Vasut]

On Sunday, April 12, 2015 at 12:06:10 PM, Stefan Wahren wrote:
> Hi,
> 
> toggling the green LED (GPIO 65) on Olinuxino Maxi unexpectedly also
> toggles the USB Host support.
> 
> Here is the console output:
> 
> # Switching the led off (USB drive connected)
> echo 255 > /sys/class/leds/green/brightness
> [  318.65] ci_hdrc ci_hdrc.0: timeout waiting for 0800 in 11
> [  318.65] ci_hdrc ci_hdrc.0: EHCI Host Controller
> [  318.67] ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus
> number 1 [  318.71] ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00
> [  318.75] hub 1-0:1.0: USB hub found
> [  318.78] hub 1-0:1.0: 1 port detected
> [  319.14] usb 1-1: new high-speed USB device number 2 using ci_hdrc
> [  319.31] hub 1-1:1.0: USB hub found
> [  319.34] hub 1-1:1.0: 3 ports detected
> [  319.64] usb 1-1.1: new high-speed USB device number 3 using ci_hdrc
> [  319.88] usb 1-1.2: new high-speed USB device number 4 using ci_hdrc
> [  320.03] usb-storage 1-1.2:1.0: USB Mass Storage device detected
> [  320.04] scsi host0: usb-storage 1-1.2:1.0
> [  321.09] scsi 0:0:0:0: Direct-Access LG   USB Drive   
> 1100 PQ: 0 ANSI: 0 CCS
> 
> # Switching the led on (USB drive connected)
> echo "0" > /sys/class/leds/green/brightness
> [ 1068.89] ci_hdrc ci_hdrc.0: remove, state 1
> [ 1068.89] usb usb1: USB disconnect, device number 1
> [ 1068.92] usb 1-1: USB disconnect, device number 2
> [ 1068.92] usb 1-1.1: USB disconnect, device number 3
> [ 1069.07] usb 1-1.2: USB disconnect, device number 4
> [ 1069.45] ci_hdrc ci_hdrc.0: USB bus 1 deregistered
> [ 1074.46] ci_hdrc ci_hdrc.0: timeout waiting for 0800 in 11
> 
> Kernel: 4.0.0-rc4-next-20150320
> Bootloader: U-Boot 2014-10
> 
> Harald discovered this problem before me on Olinuxino Mini [1].
> 
> I think the problem has something to with USB OTG, because GPIO 65 is on
> the same pin for USB_OTG_ID.
> My idea was to set "dr_mode" in olinuxino dts explicit to "host" and it
> works, but i'm not sure that is the right fix.
> 
> Shouldn't chipidea driver complain about missing pinctrl or something else?

Is the MX23_PAD_SSP1_DETECT pin muxed as a GPIO ? ie. you should have such an
entry in the DTS pinmux setup -- MX23_PAD_SSP1_DETECT__GPIO_2_1 .

If it is, then it'd probably mean that the pin state is leaking into the USB
core even if it's muxed as GPIO, in which case this would be a silicon problem.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: Toggling green led on Olinuxino Maxi, also toggles USB

[Marek Vasut]

On Monday, April 13, 2015 at 07:59:29 AM, har...@ccbib.org wrote:
> On Mon, 13 Apr 2015 01:18:07 +0200, Marek Vasut  wrote:
> > On Sunday, April 12, 2015 at 12:06:10 PM, Stefan Wahren wrote:
> >> Hi,
> >> 
> >> toggling the green LED (GPIO 65) on Olinuxino Maxi unexpectedly also
> >> toggles the USB Host support.
> >> 
> >> Here is the console output:
> >> 
> >> # Switching the led off (USB drive connected)
> >> echo 255 > /sys/class/leds/green/brightness
> >> [  318.65] ci_hdrc ci_hdrc.0: timeout waiting for 0800 in 11
> >> [  318.65] ci_hdrc ci_hdrc.0: EHCI Host Controller
> >> [  318.67] ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus
> >> number 1 [  318.71] ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00
> >> [  318.75] hub 1-0:1.0: USB hub found
> >> [  318.78] hub 1-0:1.0: 1 port detected
> >> [  319.14] usb 1-1: new high-speed USB device number 2 using
> 
> ci_hdrc
> 
> >> [  319.31] hub 1-1:1.0: USB hub found
> >> [  319.34] hub 1-1:1.0: 3 ports detected
> >> [  319.64] usb 1-1.1: new high-speed USB device number 3 using
> >> ci_hdrc
> >> [  319.88] usb 1-1.2: new high-speed USB device number 4 using
> >> ci_hdrc
> >> [  320.03] usb-storage 1-1.2:1.0: USB Mass Storage device detected
> >> [  320.04] scsi host0: usb-storage 1-1.2:1.0
> >> [  321.09] scsi 0:0:0:0: Direct-Access LG   USB Drive
> >> 
> >> 1100 PQ: 0 ANSI: 0 CCS
> >> 
> >> # Switching the led on (USB drive connected)
> >> echo "0" > /sys/class/leds/green/brightness
> >> [ 1068.89] ci_hdrc ci_hdrc.0: remove, state 1
> >> [ 1068.89] usb usb1: USB disconnect, device number 1
> >> [ 1068.92] usb 1-1: USB disconnect, device number 2
> >> [ 1068.92] usb 1-1.1: USB disconnect, device number 3
> >> [ 1069.07] usb 1-1.2: USB disconnect, device number 4
> >> [ 1069.45] ci_hdrc ci_hdrc.0: USB bus 1 deregistered
> >> [ 1074.46] ci_hdrc ci_hdrc.0: timeout waiting for 0800 in 11
> >> 
> >> Kernel: 4.0.0-rc4-next-20150320
> >> Bootloader: U-Boot 2014-10
> >> 
> >> Harald discovered this problem before me on Olinuxino Mini [1].
> >> 
> >> I think the problem has something to with USB OTG, because GPIO 65 is
> 
> on
> 
> >> the same pin for USB_OTG_ID.
> >> My idea was to set "dr_mode" in olinuxino dts explicit to "host" and it
> >> works, but i'm not sure that is the right fix.
> >> 
> >> Shouldn't chipidea driver complain about missing pinctrl or something
> >> else?
> > 
> > Is the MX23_PAD_SSP1_DETECT pin muxed as a GPIO ? ie. you should have
> 
> such
> 
> > an
> > entry in the DTS pinmux setup -- MX23_PAD_SSP1_DETECT__GPIO_2_1 .
> 
> Yes:
> led_pin_gpio2_1: led_gpio2_1@0 {
>   reg = <0>;
>   fsl,pinmux-ids = <
>   MX23_PAD_SSP1_DETECT__GPIO_2_1
> 
>   >;
> 
>   fsl,drive-strength = ;
>   fsl,voltage = ;
>   fsl,pull-up = ;
> };
> 
> > If it is, then it'd probably mean that the pin state is leaking into the
> > USB
> > core even if it's muxed as GPIO, in which case this would be a silicon
> > problem.
> 
> Well, silicon problem or not: It is actually documented in the iMX23
> 
> Reference Manual 37.2.2:
> | Readback registers are never affected by the operation of the
> | HW_PINCTRL_MUXSELx registers and always sense the actual value
> | on the data pin.
> | 
> | For example, if a pin is programmed to be a GPIO output and then
> | driven high, any specialized hardware interfaces that are actively
> | monitoring that pin will read the high logic value. Conversely, if
> | the pin mux is programmed to give a specialized hardware interface
> | such as the EMI block control of a particular pin, the current state
> | of that pin can be read through its GPIO read register at any time,
> | even while active EMI cycles are in progress.
> 
> So it seems like the driver has to take care not to read pins it isn't
> actually in charge of.

Yikes. But good find, 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: Official bugreport 4.1 kernel (audio gadget and ChipIdea)

[Marek Vasut]

On Tuesday, June 30, 2015 at 04:23:01 AM, Peter Chen wrote:
> On Fri, Jun 26, 2015 at 07:15:18PM +0200, Sébastien Pruvost wrote:
> > Hello,
> > 
> > I'm sending this mail to report a bug concerning the latest kernel 4.1.
> > 
> > Here is the problem (and the test I've done):
> > I have firstly used the 3.10.53 kernel for my two
> > sabrelites in
> > 
> > order to use the audio gadget driver with the Dual Role ChipIdea
> > Controller (in order to switch roles between my two IMX6 sabreLite).
> > After loading g_audio in my two sabreLite and plugging the cable (microA
> > – microB), there is an error “ci_hdrc.0 request length too big for
> > isochronous snd_uac2.0 1116 Error”.
> > And even after running aplay command, I still got this error and there is
> > no sound getting out of the jack port.
> > I've switched roles between the two boards by following this: https://
> > www.kernel.org/doc/Documentation/usb/chipidea.txt.
> > This works fine with the serial driver, I can see a new serial interface
> > (host side) and after switching role a new serial interfaces at device
> > side. Same thing for ethernet gadget: this works fine too. But not with
> > the audio gadget. In fact, there is a new audio interface at host side
> > but I can not interact with it (even alsamixer doesn’t see any controls
> > on this new sound card). I’ve tested that audio gadget works fine if I
> > don’t use ChipIdea HighSpeed Dual Role Controller.
> > 
> > Secondly I have tested this audio gadget with the latest
> > Kernel
> > 
> > 4.1 for my two IMX6 sabrelites (imx_v6_v7_defconfig). Now these previous
> > errors are gone but there are still no sound getting out of the jack
> > port (even if there are a new sound card in host side)
> 
> It is may not a role switch problem, please check if the g_audio can
> work well with an ubuntu PC (make sure your codec works well).

ci_hdrc.0 request length too big for isochronous

Doesn't this just mean it cannot transfer such a long buffer via ISO pipe ?
I guess the UAC should send smaller buffers to work with the CI HDRC?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: disable erroneous overcurrent condition

[Marek Vasut]

On 10/16/2017 03:57 PM, Dinh Nguyen wrote:
> For the case where an external VBUS is used, we should enable the external
> VBUS comparator in the driver. This would prevent an unnecessary
> overcurrent error which would then disable the host port.
> 
> This patch uses the standard 'disable-over-current' binding to allow of the
> option of disabling the over-current condition.
> 
> Signed-off-by: Dinh Nguyen 

Reviewed-by: Marek Vasut 

Similar patch was in U-Boot for two years now and it does the trick indeed.

> ---
>  drivers/usb/dwc2/core.h   | 4 
>  drivers/usb/dwc2/hcd.c| 5 +
>  drivers/usb/dwc2/params.c | 3 +++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 8367d4f9..730d7eb 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -395,6 +395,9 @@ enum dwc2_ep0_state {
>   *   (default when phy_type is UTMI+ or ULPI)
>   *   1 - 6 MHz
>   *   (default when phy_type is Full Speed)
> + * @oc_disable:  Flag to disable overcurrent condition.
> + *   0 - Allow overcurrent condition to get detected
> + *   1 - Disable overcurrent condtion to get detected
>   * @ts_dline:   Enable Term Select Dline pulsing
>   *   0 - No (default)
>   *   1 - Yes
> @@ -492,6 +495,7 @@ struct dwc2_core_params {
>   bool dma_desc_fs_enable;
>   bool host_support_fs_ls_low_power;
>   bool host_ls_low_power_phy_clk;
> + bool oc_disable;
>  
>   u8 host_channels;
>   u16 host_rx_fifo_size;
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index c263114..5e20336 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -213,6 +213,11 @@ static int dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, 
> bool select_phy)
>   usbcfg &= ~(GUSBCFG_PHYIF16 | GUSBCFG_DDRSEL);
>   if (hsotg->params.phy_ulpi_ddr)
>   usbcfg |= GUSBCFG_DDRSEL;
> +
> + /* Set external VBUS indicator as needed. */
> + if (hsotg->params.oc_disable)
> + usbcfg |= (GUSBCFG_ULPI_INT_VBUS_IND |
> +GUSBCFG_INDICATORPASSTHROUGH);
>   break;
>   case DWC2_PHY_TYPE_PARAM_UTMI:
>   /* UTMI+ interface */
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index a3ffe97..39e02cd 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -335,6 +335,9 @@ static void dwc2_get_device_properties(struct dwc2_hsotg 
> *hsotg)
>  num);
>   }
>   }
> +
> +     if (of_find_property(hsotg->dev->of_node, "disable-over-current", NULL))
> + p->oc_disable = true;
>  }
>  
>  static void dwc2_check_param_otg_cap(struct dwc2_hsotg *hsotg)
> 


-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: dwc2: usb: Unable to clear channel error

[Marek Vasut]

On 10/18/2017 04:05 PM, Dinh Nguyen wrote:
> Hi,
> 
> I'm trying to bringup the DWC2 USB IP version 330A on a new Stratix10
> SoC and have encountered this error in both Linux and U-Boot:
> 
> U-Boot(version v2017.09)
> 
> # usb start
> starting USB...
> USB0:   Core Release: 3.30a
> dwc_otg_core_host_init: Timeout!
> dwc_otg_core_host_init: Timeout!
> 
> Linux(kernel v4.13)
> 
> [1.299891] dwc2 ffb0.usb: DWC OTG Controller
> [1.304628] dwc2 ffb0.usb: new USB bus registered, assigned bus
> number 1
> [1.311698] dwc2 ffb0.usb: irq 13, io mem 0xffb0
> [1.318309] dwc2 ffb0.usb: Unable to clear enable on channel 0
> [1.325749] dwc2 ffb0.usb: Unable to clear enable on channel 1
> [1.333187] dwc2 ffb0.usb: Unable to clear enable on channel 2
> [1.340626] dwc2 ffb0.usb: Unable to clear enable on channel 3
> [1.348064] dwc2 ffb0.usb: Unable to clear enable on channel 4
> [1.355503] dwc2 ffb0.usb: Unable to clear enable on channel 5
> [1.362941] dwc2 ffb0.usb: Unable to clear enable on channel 6
> [1.370379] dwc2 ffb0.usb: Unable to clear enable on channel 7
> [1.377818] dwc2 ffb0.usb: Unable to clear enable on channel 8
> [1.385256] dwc2 ffb0.usb: Unable to clear enable on channel 9
> [1.392694] dwc2 ffb0.usb: Unable to clear enable on channel 10
> [1.400218] dwc2 ffb0.usb: Unable to clear enable on channel 11
> [1.407743] dwc2 ffb0.usb: Unable to clear enable on channel 12
> [1.415269] dwc2 ffb0.usb: Unable to clear enable on channel 13
> [1.422794] dwc2 ffb0.usb: Unable to clear enable on channel 14
> 
> Just wondering if anyone might have an idea on what could be causing
> this error?

Maybe some clock are not enabled ?

-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: High CPU load produced by USB (DW2)

[Marek Vasut]

On 02/16/2018 06:59 AM, Minas Harutyunyan wrote:
> On 2/15/2018 5:20 PM, Mirza Krak wrote:
>> On 14 February 2018 at 13:07, Minas Harutyunyan
>>  wrote:
>>> On 2/14/2018 12:57 PM, Mirza Krak wrote:
>>>> On 8 February 2018 at 14:53, Minas Harutyunyan
>>>>  wrote:
>>
>> < snip >
>>
>>>
>>> I reviewed your interrupt count log again. About 140,000 interrupts in 2
>>> seconds, obviously it's not SOF only interrupts. More probably, its NAK
>>> respond interrupts to SSPLIT/CSPLIT transactions. For this case I can
>>> recommend you to apply patch from Douglas Anderson: "[PATCH v2] usb:
>>> dwc2: host: Don't retry NAKed transactions right away" which already
>>> merged to 4.16-rc1.
>>>
>>> In your setups you see different behavior on different HUBs. Your HUBs
>>> have different "TT think time": 8 and 32. In USB2.0 spec "TT think time"
>>> described as follow "TT requires at most 8/32 FS bit times of inter
>>> transaction gap on a full-/low-speed downstream bus". So, your "worst"
>>> HUB with "TT think time"=8 sending more frequently SSPLIT/CSPLIT
>>> transactions which replied by NAK. As result you see about 4 time more
>>> interrupts comparing to "good" HUB. Could you please check interrupts
>>> count for "good" HUB and check "4 time" hypothesis.
>>
>> Did some further testing. The "good" HUB is actually as bad as the
>> "bad" HUB, and it was my setup that caused the different behavior. Did
>> not use the same devices etc.
>>
>> Once I made sure that the configuration and setup was the same on both
>> board I could see that the behaved similarly. And that is the
>> following interrupt load:
>>
>> - BT USB (FS) = ~80k interrupts / second
>> - Keyboard (FS) = ~80k interrupts / second
>> - WiFI USB (HS) = ~8k interrupts / second
>>
>> After applying the suggested patch [1], it is steady around 8k
>> interrupts / second no matter what device I connect (HS/FS,
>> HUB/NO-HUB). Which is acceptable and usable.
> 
> Great!

So 8k IRQs per second is what I should expect from this HW with HS
device attached, that's normal and cannot be reduced ?

-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: High CPU load produced by USB (DW2)

[Marek Vasut]

On 02/19/2018 09:19 AM, Minas Harutyunyan wrote:
> On 2/17/2018 12:07 AM, Marek Vasut wrote:
>> On 02/16/2018 06:59 AM, Minas Harutyunyan wrote:
>>> On 2/15/2018 5:20 PM, Mirza Krak wrote:
>>>> On 14 February 2018 at 13:07, Minas Harutyunyan
>>>>  wrote:
>>>>> On 2/14/2018 12:57 PM, Mirza Krak wrote:
>>>>>> On 8 February 2018 at 14:53, Minas Harutyunyan
>>>>>>  wrote:
>>>>
>>>> < snip >
>>>>
>>>>>
>>>>> I reviewed your interrupt count log again. About 140,000 interrupts in 2
>>>>> seconds, obviously it's not SOF only interrupts. More probably, its NAK
>>>>> respond interrupts to SSPLIT/CSPLIT transactions. For this case I can
>>>>> recommend you to apply patch from Douglas Anderson: "[PATCH v2] usb:
>>>>> dwc2: host: Don't retry NAKed transactions right away" which already
>>>>> merged to 4.16-rc1.
>>>>>
>>>>> In your setups you see different behavior on different HUBs. Your HUBs
>>>>> have different "TT think time": 8 and 32. In USB2.0 spec "TT think time"
>>>>> described as follow "TT requires at most 8/32 FS bit times of inter
>>>>> transaction gap on a full-/low-speed downstream bus". So, your "worst"
>>>>> HUB with "TT think time"=8 sending more frequently SSPLIT/CSPLIT
>>>>> transactions which replied by NAK. As result you see about 4 time more
>>>>> interrupts comparing to "good" HUB. Could you please check interrupts
>>>>> count for "good" HUB and check "4 time" hypothesis.
>>>>
>>>> Did some further testing. The "good" HUB is actually as bad as the
>>>> "bad" HUB, and it was my setup that caused the different behavior. Did
>>>> not use the same devices etc.
>>>>
>>>> Once I made sure that the configuration and setup was the same on both
>>>> board I could see that the behaved similarly. And that is the
>>>> following interrupt load:
>>>>
>>>> - BT USB (FS) = ~80k interrupts / second
>>>> - Keyboard (FS) = ~80k interrupts / second
>>>> - WiFI USB (HS) = ~8k interrupts / second
>>>>
>>>> After applying the suggested patch [1], it is steady around 8k
>>>> interrupts / second no matter what device I connect (HS/FS,
>>>> HUB/NO-HUB). Which is acceptable and usable.
>>>
>>> Great!
>>
>> So 8k IRQs per second is what I should expect from this HW with HS
>> device attached, that's normal and cannot be reduced ?
>>
> If core acting as Host in Buffer DMA mode then 8k IRQs per second (SOF 
> interrupts) is expected if connected device(s) has periodic endpoint.

Is there a way to reduce that or is that the absolute minimum in HS mode?

-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: High CPU load produced by USB (DW2)

[Marek Vasut]

On 02/19/2018 11:11 AM, Minas Harutyunyan wrote:
> On 2/19/2018 12:51 PM, Marek Vasut wrote:
>> On 02/19/2018 09:19 AM, Minas Harutyunyan wrote:
>>> On 2/17/2018 12:07 AM, Marek Vasut wrote:
>>>> On 02/16/2018 06:59 AM, Minas Harutyunyan wrote:
>>>>> On 2/15/2018 5:20 PM, Mirza Krak wrote:
>>>>>> On 14 February 2018 at 13:07, Minas Harutyunyan
>>>>>>  wrote:
>>>>>>> On 2/14/2018 12:57 PM, Mirza Krak wrote:
>>>>>>>> On 8 February 2018 at 14:53, Minas Harutyunyan
>>>>>>>>  wrote:
>>>>>>
>>>>>> < snip >
>>>>>>
>>>>>>>
>>>>>>> I reviewed your interrupt count log again. About 140,000 interrupts in 2
>>>>>>> seconds, obviously it's not SOF only interrupts. More probably, its NAK
>>>>>>> respond interrupts to SSPLIT/CSPLIT transactions. For this case I can
>>>>>>> recommend you to apply patch from Douglas Anderson: "[PATCH v2] usb:
>>>>>>> dwc2: host: Don't retry NAKed transactions right away" which already
>>>>>>> merged to 4.16-rc1.
>>>>>>>
>>>>>>> In your setups you see different behavior on different HUBs. Your HUBs
>>>>>>> have different "TT think time": 8 and 32. In USB2.0 spec "TT think time"
>>>>>>> described as follow "TT requires at most 8/32 FS bit times of inter
>>>>>>> transaction gap on a full-/low-speed downstream bus". So, your "worst"
>>>>>>> HUB with "TT think time"=8 sending more frequently SSPLIT/CSPLIT
>>>>>>> transactions which replied by NAK. As result you see about 4 time more
>>>>>>> interrupts comparing to "good" HUB. Could you please check interrupts
>>>>>>> count for "good" HUB and check "4 time" hypothesis.
>>>>>>
>>>>>> Did some further testing. The "good" HUB is actually as bad as the
>>>>>> "bad" HUB, and it was my setup that caused the different behavior. Did
>>>>>> not use the same devices etc.
>>>>>>
>>>>>> Once I made sure that the configuration and setup was the same on both
>>>>>> board I could see that the behaved similarly. And that is the
>>>>>> following interrupt load:
>>>>>>
>>>>>> - BT USB (FS) = ~80k interrupts / second
>>>>>> - Keyboard (FS) = ~80k interrupts / second
>>>>>> - WiFI USB (HS) = ~8k interrupts / second
>>>>>>
>>>>>> After applying the suggested patch [1], it is steady around 8k
>>>>>> interrupts / second no matter what device I connect (HS/FS,
>>>>>> HUB/NO-HUB). Which is acceptable and usable.
>>>>>
>>>>> Great!
>>>>
>>>> So 8k IRQs per second is what I should expect from this HW with HS
>>>> device attached, that's normal and cannot be reduced ?
>>>>
>>> If core acting as Host in Buffer DMA mode then 8k IRQs per second (SOF
>>> interrupts) is expected if connected device(s) has periodic endpoint.
>>
>> Is there a way to reduce that or is that the absolute minimum in HS mode?
>>
> We already discussed, in this email thread earlier, why SOF interrupts 
> required and unmasked.
> Only in case when connected device with CTRL+BLK EP's only (like flash 
> drive) and directly connected to cores root HUB, SOF's will be masked.

That's the setup I have on Altera SoCFPGA, yet the SOFs are still present.

-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: High CPU load produced by USB (DW2)

[Marek Vasut]

On 02/20/2018 06:51 AM, Minas Harutyunyan wrote:
[...]
>>>> Is there a way to reduce that or is that the absolute minimum in HS mode?
>>>>
>>> We already discussed, in this email thread earlier, why SOF interrupts
>>> required and unmasked.
>>> Only in case when connected device with CTRL+BLK EP's only (like flash
>>> drive) and directly connected to cores root HUB, SOF's will be masked.
>>
>> That's the setup I have on Altera SoCFPGA, yet the SOFs are still present.
>>
> Could you please send verbose lsusb on connected to dwc2 device

See attached

> and driver debug log.

What exactly do you mean by this one ?

-- 
Best regards,
Marek Vasut

Bus 001 Device 003: ID 0781:5581 SanDisk Corp. Ultra
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   2.10
  bDeviceClass0 (Defined at Interface level)
  bDeviceSubClass 0 
  bDeviceProtocol 0 
  bMaxPacketSize064
  idVendor   0x0781 SanDisk Corp.
  idProduct  0x5581 Ultra
  bcdDevice1.00
  iManufacturer   1 SanDisk
  iProduct2 Ultra
  iSerial 3 4C530001110620109113
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength   32
bNumInterfaces  1
bConfigurationValue 1
iConfiguration  0 
bmAttributes 0x80
  (Bus Powered)
MaxPower  224mA
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   2
  bInterfaceClass 8 Mass Storage
  bInterfaceSubClass  6 SCSI
  bInterfaceProtocol 80 Bulk-Only
  iInterface  0 
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81  EP 1 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02  EP 2 OUT
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
Binary Object Store Descriptor:
  bLength 5
  bDescriptorType15
  wTotalLength   22
  bNumDeviceCaps  2
  USB 2.0 Extension Device Capability:
bLength 7
bDescriptorType16
bDevCapabilityType  2
bmAttributes   0x0002
  Link Power Management (LPM) Supported
  SuperSpeed USB Device Capability:
bLength10
bDescriptorType16
bDevCapabilityType  3
bmAttributes 0x00
wSpeedsSupported   0x000e
  Device can operate at Full Speed (12Mbps)
  Device can operate at High Speed (480Mbps)
  Device can operate at SuperSpeed (5Gbps)
bFunctionalitySupport   1
  Lowest fully-functional device speed is Full Speed (12Mbps)
bU1DevExitLat  10 micro seconds
bU2DevExitLat 256 micro seconds
Device Status: 0x
  (Bus Powered)

Bus 001 Device 002: ID 0424:2514 Standard Microsystems Corp. USB 2.0 Hub
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass9 Hub
  bDeviceSubClass 0 Unused
  bDeviceProtocol 2 TT per port
  bMaxPacketSize064
  idVendor   0x0424 Standard Microsystems Corp.
  idProduct  0x2514 USB 2.0 Hub
  bcdDeviceb.b3
  iManufacturer   0 
  iProduct0 
  iSerial 0 
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength   41
bNumInterfaces  1
bConfigurationValue 1
iConfiguration  0 
bmAttributes 0xe0
  Self Powered
  Remote Wakeup
MaxPower2mA
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   1
  bInterfaceClass 9 Hub
  bInterfaceSubClass  0 Unused
  bInterfaceProtocol  1 Single TT
  iInterface  0 
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81  EP 1 IN
bmAttributes3
  Transfer Type  

Re: High CPU load produced by USB (DW2)

[Marek Vasut]

On 03/06/2018 09:09 AM, Minas Harutyunyan wrote:

Hi,

On 3/6/2018 10:45 AM, Minas Harutyunyan wrote:

Hi,

On 3/5/2018 11:14 PM, Marek Vasut wrote:

On 02/20/2018 06:51 AM, Minas Harutyunyan wrote:
[...]

Is there a way to reduce that or is that the absolute minimum in HS mode?


We already discussed, in this email thread earlier, why SOF interrupts
required and unmasked.
Only in case when connected device with CTRL+BLK EP's only (like flash
drive) and directly connected to cores root HUB, SOF's will be masked.


That's the setup I have on Altera SoCFPGA, yet the SOFs are still present.


Could you please send verbose lsusb on connected to dwc2 device


See attached


and driver debug log.


What exactly do you mean by this one ?

Enable debugging messages and verbose debugging messages for dwc2 from
make menuconfig. Provide dmesg starting from dwc2 load till HS device
connected to dwc2 port and enumerated.

Thanks,
Minas







Driver debug log not required.
Based on lsusb output: to dwc2 port connected "Standard Microsystems
Corp. USB 2.0 Hub" to which connected your HS device "SanDisk Corp.
Ultra". Because of connected HUB, which have periodic endpoint
(Interrupt IN EP1) like all HUB's, dwc2 forced in Buffer DMA mode unmask
SOF interrupts.


I don't understand the last part, can you rephrase ?

What does that mean regarding the SOF question above ?
--
To unsubscribe from this list: send the line "unsubscribe 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: serial: ftdi_sio: add device ID for Microsemi/Arrow SF2PLUS Dev Kit

[Marek Vasut]

This development kit has an FT4232 on it with a custom USB VID/PID.
The FT4232 provides four UARTs, but only two are used. The UART 0
is used by the FlashPro5 programmer and UART 2 is connected to the
SmartFusion2 CortexM3 SoC UART port.

Note that the USB VID is registered to Actel according to Linux USB
VID database, but that was acquired by Microsemi.

Signed-off-by: Marek Vasut 
Cc: stable 
Cc: usb 
Cc: Johan Hovold 
---
V2: - Move the ID in ftdi_sio_ids.h to keep it sorted by VIDs
- Use the JTAG quirk to avoid binding channel 0, this is needed by
  the Libero software, otherwise it cannot use the programmer.
---
 drivers/usb/serial/ftdi_sio.c |  2 ++
 drivers/usb/serial/ftdi_sio_ids.h | 10 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 80c6d3deb80d..a42e7f2af420 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -873,6 +873,8 @@ static const struct usb_device_id id_table_combined[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(MICROCHIP_VID, MICROCHIP_USB_BOARD_PID,
USB_CLASS_VENDOR_SPEC,
USB_SUBCLASS_VENDOR_SPEC, 0x00) },
+   { USB_DEVICE(ACTEL_VID, MICROSEMI_ARROW_SF2PLUS_BOARD_PID),
+   .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
{ USB_DEVICE(JETI_VID, JETI_SPC1201_PID) },
{ USB_DEVICE(MARVELL_VID, MARVELL_SHEEVAPLUG_PID),
.driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
diff --git a/drivers/usb/serial/ftdi_sio_ids.h 
b/drivers/usb/serial/ftdi_sio_ids.h
index 168ccb03ce08..7b6edb8e9ee2 100644
--- a/drivers/usb/serial/ftdi_sio_ids.h
+++ b/drivers/usb/serial/ftdi_sio_ids.h
@@ -877,6 +877,16 @@
 #defineFIC_VID 0x1457
 #defineFIC_NEO1973_DEBUG_PID   0x5118
 
+/*
+ * Microsemi/Arrow SF2PLUS Dev Kit
+ *
+ * This board has an FT4232 on it which provides four UART ports.
+ * UART 0 is used by the FlashPro5 programmer, UART 2 is connected
+ * to the UART of an CortexM3 SoC-FPGA on the board.
+ */
+#define ACTEL_VID  0x1514
+#define MICROSEMI_ARROW_SF2PLUS_BOARD_PID  0x2008
+
 /* Olimex */
 #define OLIMEX_VID 0x15BA
 #define OLIMEX_ARM_USB_OCD_PID 0x0003
-- 
2.11.0

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

Re: [PATCH] USB: serial: ftdi_sio: add device ID for Microsemi/Arrow SF2PLUS Dev Kit

[Marek Vasut]

On 02/13/2017 10:29 AM, Johan Hovold wrote:
> [+CC: linux-usb]
> 
> Always make sure to CC linux-usb for USB patches.
> 
> On Fri, Feb 10, 2017 at 05:16:12PM +0100, Marek Vasut wrote:
>> This development kit has an FT4232 on it with a custom USB VID/PID.
>> The FT4232 provides four UARTs, but only two are used. The UART 0
>> is used by the FlashPro5 programmer and UART 2 is connected to the
>> SmartFusion2 CortexM3 SoC UART port.
> 
> Don't you want to use the "jtag" quirk for this one then to prevent the
> driver from binding to interface 0? Or do you still need this interface
> to be accessible as a tty device?

I just got the programmer working and yes, I need the JTAG quirk, thanks
for pointing that out. V2 is out.

NOTE that the MS Libero software experience on Linux is really abysmal,
neither the flashpro5 or flashpro express are usable, one has to use the
inobvious "configure programmer" in the libero gui after the synthesis
was done to make the programmer usable. And that's not the only
inobvious thing when it comes to Microsemi ... :(

>> Note that the USB VID is registered to Actel according to Linux USB
>> VID database, but that was acquired by Microsemi.
>>
>> Signed-off-by: Marek Vasut 
>> Cc: stable 
>> Cc: Johan Hovold 
>> ---
>>  drivers/usb/serial/ftdi_sio.c |  1 +
>>  drivers/usb/serial/ftdi_sio_ids.h | 10 ++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
>> index c98cf10be5af..14f0fb34f655 100644
>> --- a/drivers/usb/serial/ftdi_sio.c
>> +++ b/drivers/usb/serial/ftdi_sio.c
>> @@ -873,6 +873,7 @@ static const struct usb_device_id id_table_combined[] = {
>>  { USB_DEVICE_AND_INTERFACE_INFO(MICROCHIP_VID, MICROCHIP_USB_BOARD_PID,
>>  USB_CLASS_VENDOR_SPEC,
>>  USB_SUBCLASS_VENDOR_SPEC, 0x00) },
>> +{ USB_DEVICE(ACTEL_VID, MICROSEMI_ARROW_SF2PLUS_BOARD_PID) },
>>  { USB_DEVICE(JETI_VID, JETI_SPC1201_PID) },
>>  { USB_DEVICE(MARVELL_VID, MARVELL_SHEEVAPLUG_PID),
>>  .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
>> diff --git a/drivers/usb/serial/ftdi_sio_ids.h 
>> b/drivers/usb/serial/ftdi_sio_ids.h
>> index 168ccb03ce08..a9d538d18344 100644
>> --- a/drivers/usb/serial/ftdi_sio_ids.h
>> +++ b/drivers/usb/serial/ftdi_sio_ids.h
>> @@ -627,6 +627,16 @@
>>  #define MICROCHIP_USB_BOARD_PID 0x000A /* CDC RS-232 Emulation Demo */
>>  
>>  /*
>> + * Microsemi/Arrow SF2PLUS Dev Kit
>> + *
>> + * This board has an FT4232 on it which provides four UART ports.
>> + * UART 0 is used by the FlashPro5 programmer, UART 2 is connected
>> + * to the UART of an CortexM3 SoC-FPGA on the board.
>> + */
>> +#define ACTEL_VID   0x1514
>> +#define MICROSEMI_ARROW_SF2PLUS_BOARD_PID   0x2008
>> +
> 
> Please place this before the Olimex section to try to maintain some
> order based on VID.

Done

-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: serial: ftdi_sio: add device ID for Microsemi/Arrow SF2PLUS Dev Kit

[Marek Vasut]

This development kit has an FT4232 on it with a custom USB VID/PID.
The FT4232 provides four UARTs, but only two are used. The UART 0
is used by the FlashPro5 programmer and UART 2 is connected to the
SmartFusion2 CortexM3 SoC UART port.

Note that the USB VID is registered to Actel according to Linux USB
VID database, but that was acquired by Microsemi.

Signed-off-by: Marek Vasut 
Cc: stable 
Cc: usb 
Cc: Johan Hovold 
---
V2: - Move the ID in ftdi_sio_ids.h to keep it sorted by VIDs
- Use the JTAG quirk to avoid binding channel 0, this is needed by
  the Libero software, otherwise it cannot use the programmer.
V3: - Remove the verbose comment
- Use USB_DEVICE_INTERFACE_NUMBER() instead of the JTAG quirk
---
 drivers/usb/serial/ftdi_sio.c | 1 +
 drivers/usb/serial/ftdi_sio_ids.h | 4 
 2 files changed, 5 insertions(+)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 80c6d3deb80d..91a4d7346549 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -873,6 +873,7 @@ static const struct usb_device_id id_table_combined[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(MICROCHIP_VID, MICROCHIP_USB_BOARD_PID,
USB_CLASS_VENDOR_SPEC,
USB_SUBCLASS_VENDOR_SPEC, 0x00) },
+   { USB_DEVICE_INTERFACE_NUMBER(ACTEL_VID, 
MICROSEMI_ARROW_SF2PLUS_BOARD_PID, 2) },
{ USB_DEVICE(JETI_VID, JETI_SPC1201_PID) },
{ USB_DEVICE(MARVELL_VID, MARVELL_SHEEVAPLUG_PID),
.driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
diff --git a/drivers/usb/serial/ftdi_sio_ids.h 
b/drivers/usb/serial/ftdi_sio_ids.h
index 168ccb03ce08..58cf17d362c5 100644
--- a/drivers/usb/serial/ftdi_sio_ids.h
+++ b/drivers/usb/serial/ftdi_sio_ids.h
@@ -877,6 +877,10 @@
 #defineFIC_VID 0x1457
 #defineFIC_NEO1973_DEBUG_PID   0x5118
 
+/* Microsemi/Arrow SF2PLUS Dev Kit */
+#define ACTEL_VID  0x1514
+#define MICROSEMI_ARROW_SF2PLUS_BOARD_PID  0x2008
+
 /* Olimex */
 #define OLIMEX_VID 0x15BA
 #define OLIMEX_ARM_USB_OCD_PID 0x0003
-- 
2.11.0

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

Re: [PATCH V2] USB: serial: ftdi_sio: add device ID for Microsemi/Arrow SF2PLUS Dev Kit

[Marek Vasut]

On 04/18/2017 11:56 AM, Johan Hovold wrote:
> On Mon, Apr 17, 2017 at 01:56:54PM +0200, Marek Vasut wrote:
>> This development kit has an FT4232 on it with a custom USB VID/PID.
>> The FT4232 provides four UARTs, but only two are used. The UART 0
>> is used by the FlashPro5 programmer and UART 2 is connected to the
>> SmartFusion2 CortexM3 SoC UART port.
> 
> Thanks for verifying that you do not want to bind to interface 0.
> 
> Judging from the above description, it seems we can do even even better
> and only bind to interface 2 using
> 
>   USB_DEVICE_INTERFACE_NUMBER()
> 
> since you only have one uart connected that you want to expose, right?

Yes

>> Note that the USB VID is registered to Actel according to Linux USB
>> VID database, but that was acquired by Microsemi.
>>
>> Signed-off-by: Marek Vasut 
>> Cc: stable 
>> Cc: usb 
>> Cc: Johan Hovold 
>> ---
>> V2: - Move the ID in ftdi_sio_ids.h to keep it sorted by VIDs
>> - Use the JTAG quirk to avoid binding channel 0, this is needed by
>>   the Libero software, otherwise it cannot use the programmer.
>> ---
>>  drivers/usb/serial/ftdi_sio.c |  2 ++
>>  drivers/usb/serial/ftdi_sio_ids.h | 10 ++
>>  2 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
>> index 80c6d3deb80d..a42e7f2af420 100644
>> --- a/drivers/usb/serial/ftdi_sio.c
>> +++ b/drivers/usb/serial/ftdi_sio.c
>> @@ -873,6 +873,8 @@ static const struct usb_device_id id_table_combined[] = {
>>  { USB_DEVICE_AND_INTERFACE_INFO(MICROCHIP_VID, MICROCHIP_USB_BOARD_PID,
>>  USB_CLASS_VENDOR_SPEC,
>>  USB_SUBCLASS_VENDOR_SPEC, 0x00) },
>> +{ USB_DEVICE(ACTEL_VID, MICROSEMI_ARROW_SF2PLUS_BOARD_PID),
>> +.driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
>>  { USB_DEVICE(JETI_VID, JETI_SPC1201_PID) },
>>  { USB_DEVICE(MARVELL_VID, MARVELL_SHEEVAPLUG_PID),
>>  .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
>> diff --git a/drivers/usb/serial/ftdi_sio_ids.h 
>> b/drivers/usb/serial/ftdi_sio_ids.h
>> index 168ccb03ce08..7b6edb8e9ee2 100644
>> --- a/drivers/usb/serial/ftdi_sio_ids.h
>> +++ b/drivers/usb/serial/ftdi_sio_ids.h
>> @@ -877,6 +877,16 @@
>>  #define FIC_VID 0x1457
>>  #define FIC_NEO1973_DEBUG_PID   0x5118
>>  
>> +/*
>> + * Microsemi/Arrow SF2PLUS Dev Kit
>> + *
>> + * This board has an FT4232 on it which provides four UART ports.
>> + * UART 0 is used by the FlashPro5 programmer, UART 2 is connected
>> + * to the UART of an CortexM3 SoC-FPGA on the board.
> 
> You can remove this comment (having it in the git history should be
> sufficient), and just make this a "Microsemi (formerly Actel)" header.

OK

>> + */
>> +#define ACTEL_VID   0x1514
>> +#define MICROSEMI_ARROW_SF2PLUS_BOARD_PID   0x2008
>> +
>>  /* Olimex */
>>  #define OLIMEX_VID  0x15BA
>>  #define OLIMEX_ARM_USB_OCD_PID  0x0003
> 
> Thanks,
> Johan
> 


-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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: serial: ftdi_sio: add device ID for Microsemi/Arrow SF2PLUS Dev Kit

[Marek Vasut]

On 04/19/2017 10:00 AM, Johan Hovold wrote:
> On Tue, Apr 18, 2017 at 08:07:56PM +0200, Marek Vasut wrote:
>> This development kit has an FT4232 on it with a custom USB VID/PID.
>> The FT4232 provides four UARTs, but only two are used. The UART 0
>> is used by the FlashPro5 programmer and UART 2 is connected to the
>> SmartFusion2 CortexM3 SoC UART port.
>>
>> Note that the USB VID is registered to Actel according to Linux USB
>> VID database, but that was acquired by Microsemi.
>>
>> Signed-off-by: Marek Vasut 
>> Cc: stable 
>> Cc: usb 
>> Cc: Johan Hovold 
>> ---
>> V2: - Move the ID in ftdi_sio_ids.h to keep it sorted by VIDs
>> - Use the JTAG quirk to avoid binding channel 0, this is needed by
>>   the Libero software, otherwise it cannot use the programmer.
>> V3: - Remove the verbose comment
>> - Use USB_DEVICE_INTERFACE_NUMBER() instead of the JTAG quirk
> 
> Applied for -next, thanks.

Super, thanks.

-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe 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] smsc95xx: Add support for automated PHY address detection

[Marek Vasut]

The SMSC95xx chip can use either the internal PHY or an external one.
Currently, the driver hard-codes support for the internal PHY only.

This patch reads out the HW_CFG register to determine whether external
PHY is attached or not. If an external PHY is not attached, the driver
falls back to internal PHY with fixed PHY address 0x1.

If an external PHY is attached, the driver scans the entire address
range of the MDIO bus to determine whether there is a valid external
PHY attached. The scanning happens from the end of the address range,
since some PHYs also respond to address 0, which is considered a MDIO
broadcast address by them. We want to obtain their real MDIO address
instead.

Signed-off-by: Marek Vasut 
Cc: David S. Miller 
Cc: Nisar Sayed 
Cc: Woojung Huh 
---
 drivers/net/usb/smsc95xx.c | 42 +-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 06b4d290784d..014bb71ce8a8 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -983,6 +983,40 @@ static int smsc95xx_start_rx_path(struct usbnet *dev, int 
in_pm)
return __smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr, in_pm);
 }
 
+static int smsc95xx_phy_address(struct usbnet *dev)
+{
+   u32 read_buf;
+   int ret, id1, id2, phyad;
+
+   ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
+   if (ret < 0)
+   return ret;
+
+   /* Check if using external PHY, if not, use internal PHY address */
+   if (!(read_buf & HW_CFG_PSEL_))
+   return SMSC95XX_INTERNAL_PHY_ID;
+
+   /*
+* Detect external PHY address. Here we probe the MDIO bus from
+* the highest address, since some PHYs respond also on address
+* zero, which they consider MDIO broadcast address. We really
+* want to get their proper address instead though, so we scan
+* address zero last.
+*/
+   for (phyad = 0x1f; phyad >= 0; phyad--) {
+   id1 = smsc95xx_mdio_read(dev->net, phyad, MII_PHYSID1);
+   id2 = smsc95xx_mdio_read(dev->net, phyad, MII_PHYSID2);
+   /* Check for valid response from the PHY */
+   if (id1 > 0 && id2 > 0 && id1 != 0x7fff && id2 != 0x)
+   return phyad;
+   }
+
+   /* No PHY found. */
+   netdev_warn(dev->net, "cannot detect any PHY");
+
+   return -ENODEV;
+}
+
 static int smsc95xx_phy_initialize(struct usbnet *dev)
 {
int bmcr, ret, timeout = 0;
@@ -993,7 +1027,13 @@ static int smsc95xx_phy_initialize(struct usbnet *dev)
dev->mii.mdio_write = smsc95xx_mdio_write;
dev->mii.phy_id_mask = 0x1f;
dev->mii.reg_num_mask = 0x1f;
-   dev->mii.phy_id = SMSC95XX_INTERNAL_PHY_ID;
+
+   /* Determine PHY address */
+   ret = smsc95xx_phy_address(dev);
+   if (ret)
+   return ret;
+
+   dev->mii.phy_id = ret;
 
/* reset phy and wait for reset to complete */
smsc95xx_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, BMCR_RESET);
-- 
2.18.0

[PATCH] smsc95xx: Add quirk for TJA1100 BroadRReach PHY

[Marek Vasut]

The company atmes.de manufactures a SMSC95xx device with default USB
ID 0424:9e00 , but with external NXP TJA1100 PHY at address 0x4. This
PHY is not 802.3 c22 compliant, but rather c96 compliant. The register
set is slightly different and does not provide link state information
in c22-compliant manner and any duplex information.

This patch adds a quirk for such a setup. The PHY is detected by its
PHY ID register values and if present, link detection is not performed
and duplex is always forced to full.

Signed-off-by: Marek Vasut 
Cc: David S. Miller 
Cc: Nisar Sayed 
Cc: Woojung Huh 
---
 drivers/net/usb/smsc95xx.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 014bb71ce8a8..454a3994133d 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -74,6 +74,7 @@ struct smsc95xx_priv {
u8 suspend_flags;
u8 mdix_ctrl;
bool link_ok;
+   bool has_tja1100_phy;
struct delayed_work carrier_check;
struct usbnet *dev;
 };
@@ -578,7 +579,8 @@ static int smsc95xx_link_reset(struct usbnet *dev)
if (ret < 0)
return ret;
 
-   mii_check_media(mii, 1, 1);
+   if (!pdata->has_tja1100_phy)
+   mii_check_media(mii, 1, 1);
mii_ethtool_gset(&dev->mii, &ecmd);
lcladv = smsc95xx_mdio_read(dev->net, mii->phy_id, MII_ADVERTISE);
rmtadv = smsc95xx_mdio_read(dev->net, mii->phy_id, MII_LPA);
@@ -588,7 +590,7 @@ static int smsc95xx_link_reset(struct usbnet *dev)
  ethtool_cmd_speed(&ecmd), ecmd.duplex, lcladv, rmtadv);
 
spin_lock_irqsave(&pdata->mac_cr_lock, flags);
-   if (ecmd.duplex != DUPLEX_FULL) {
+   if (!pdata->has_tja1100_phy && ecmd.duplex != DUPLEX_FULL) {
pdata->mac_cr &= ~MAC_CR_FDPX_;
pdata->mac_cr |= MAC_CR_RCVOWN_;
} else {
@@ -985,6 +987,7 @@ static int smsc95xx_start_rx_path(struct usbnet *dev, int 
in_pm)
 
 static int smsc95xx_phy_address(struct usbnet *dev)
 {
+   struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
u32 read_buf;
int ret, id1, id2, phyad;
 
@@ -1006,9 +1009,19 @@ static int smsc95xx_phy_address(struct usbnet *dev)
for (phyad = 0x1f; phyad >= 0; phyad--) {
id1 = smsc95xx_mdio_read(dev->net, phyad, MII_PHYSID1);
id2 = smsc95xx_mdio_read(dev->net, phyad, MII_PHYSID2);
+
/* Check for valid response from the PHY */
-   if (id1 > 0 && id2 > 0 && id1 != 0x7fff && id2 != 0x)
+   if (id1 > 0 && id2 > 0 && id1 != 0x7fff && id2 != 0x) {
+   /*
+* Check for special mutation of the SMSC95xx USB
+* device with NXP TJA1100 BroadRReach PHY. If this
+* is present, enable quirk.
+*/
+   if (id1 == 0x0180 && id2 == 0xdc48)
+   pdata->has_tja1100_phy = true;
+
return phyad;
+   }
}
 
/* No PHY found. */
-- 
2.18.0

Re: [PATCH] smsc95xx: Add support for automated PHY address detection

[Marek Vasut]

On 9/11/18 12:12 PM, Marek Vasut wrote:
> The SMSC95xx chip can use either the internal PHY or an external one.
> Currently, the driver hard-codes support for the internal PHY only.
> 
> This patch reads out the HW_CFG register to determine whether external
> PHY is attached or not. If an external PHY is not attached, the driver
> falls back to internal PHY with fixed PHY address 0x1.
> 
> If an external PHY is attached, the driver scans the entire address
> range of the MDIO bus to determine whether there is a valid external
> PHY attached. The scanning happens from the end of the address range,
> since some PHYs also respond to address 0, which is considered a MDIO
> broadcast address by them. We want to obtain their real MDIO address
> instead.
> 
> Signed-off-by: Marek Vasut 
> Cc: David S. Miller 
> Cc: Nisar Sayed 
> Cc: Woojung Huh 

Expanding the CC, since I received no feedback.
Still applies cleanly on next/master.

> ---
>  drivers/net/usb/smsc95xx.c | 42 +-
>  1 file changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index 06b4d290784d..014bb71ce8a8 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -983,6 +983,40 @@ static int smsc95xx_start_rx_path(struct usbnet *dev, 
> int in_pm)
>   return __smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr, in_pm);
>  }
>  
> +static int smsc95xx_phy_address(struct usbnet *dev)
> +{
> + u32 read_buf;
> + int ret, id1, id2, phyad;
> +
> + ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
> + if (ret < 0)
> + return ret;
> +
> + /* Check if using external PHY, if not, use internal PHY address */
> + if (!(read_buf & HW_CFG_PSEL_))
> + return SMSC95XX_INTERNAL_PHY_ID;
> +
> + /*
> +  * Detect external PHY address. Here we probe the MDIO bus from
> +  * the highest address, since some PHYs respond also on address
> +  * zero, which they consider MDIO broadcast address. We really
> +  * want to get their proper address instead though, so we scan
> +  * address zero last.
> +  */
> + for (phyad = 0x1f; phyad >= 0; phyad--) {
> + id1 = smsc95xx_mdio_read(dev->net, phyad, MII_PHYSID1);
> + id2 = smsc95xx_mdio_read(dev->net, phyad, MII_PHYSID2);
> + /* Check for valid response from the PHY */
> + if (id1 > 0 && id2 > 0 && id1 != 0x7fff && id2 != 0x)
> + return phyad;
> + }
> +
> + /* No PHY found. */
> + netdev_warn(dev->net, "cannot detect any PHY");
> +
> + return -ENODEV;
> +}
> +
>  static int smsc95xx_phy_initialize(struct usbnet *dev)
>  {
>   int bmcr, ret, timeout = 0;
> @@ -993,7 +1027,13 @@ static int smsc95xx_phy_initialize(struct usbnet *dev)
>   dev->mii.mdio_write = smsc95xx_mdio_write;
>   dev->mii.phy_id_mask = 0x1f;
>   dev->mii.reg_num_mask = 0x1f;
> - dev->mii.phy_id = SMSC95XX_INTERNAL_PHY_ID;
> +
> + /* Determine PHY address */
> + ret = smsc95xx_phy_address(dev);
> + if (ret)
> + return ret;
> +
> + dev->mii.phy_id = ret;
>  
>   /* reset phy and wait for reset to complete */
>   smsc95xx_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, BMCR_RESET);
> 

Re: [PATCH] smsc95xx: Add support for automated PHY address detection

[Marek Vasut]

On 12/23/18 11:23 AM, Andrew Lunn wrote:
>>> +static int smsc95xx_phy_address(struct usbnet *dev)
>>> +{
>>> +   u32 read_buf;
>>> +   int ret, id1, id2, phyad;
>>> +
>>> +   ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
>>> +   if (ret < 0)
>>> +   return ret;
>>> +
>>> +   /* Check if using external PHY, if not, use internal PHY address */
>>> +   if (!(read_buf & HW_CFG_PSEL_))
>>> +   return SMSC95XX_INTERNAL_PHY_ID;
>>> +
>>> +   /*
>>> +* Detect external PHY address. Here we probe the MDIO bus from
>>> +* the highest address, since some PHYs respond also on address
>>> +* zero, which they consider MDIO broadcast address. We really
>>> +* want to get their proper address instead though, so we scan
>>> +* address zero last.
>>> +*/
>>> +   for (phyad = 0x1f; phyad >= 0; phyad--) {
>>> +   id1 = smsc95xx_mdio_read(dev->net, phyad, MII_PHYSID1);
>>> +   id2 = smsc95xx_mdio_read(dev->net, phyad, MII_PHYSID2);
>>> +   /* Check for valid response from the PHY */
>>> +   if (id1 > 0 && id2 > 0 && id1 != 0x7fff && id2 != 0x)
>>> +   return phyad;
>>> +   }
> 
> This would be so much easier if the driver used the core mdio/phy
> code. Just set mdio->phy_mask to ~BIT(0) and then use
> phy_find_first().

That's in the pipeline, along with PM cleanups, but low prio.

> Anyway, net is closed at the moment, so please repost in three weeks
> time.

OK

-- 
Best regards,
Marek Vasut

Re: [PATCH] smsc95xx: Add support for automated PHY address detection

[Marek Vasut]

On 12/23/18 11:56 AM, Andrew Lunn wrote:
> On Sun, Dec 23, 2018 at 11:43:05AM +0100, Marek Vasut wrote:
>> On 12/23/18 11:23 AM, Andrew Lunn wrote:
>>>>> +static int smsc95xx_phy_address(struct usbnet *dev)
>>>>> +{
>>>>> + u32 read_buf;
>>>>> + int ret, id1, id2, phyad;
>>>>> +
>>>>> + ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + /* Check if using external PHY, if not, use internal PHY address */
>>>>> + if (!(read_buf & HW_CFG_PSEL_))
>>>>> + return SMSC95XX_INTERNAL_PHY_ID;
>>>>> +
>>>>> + /*
>>>>> +  * Detect external PHY address. Here we probe the MDIO bus from
>>>>> +  * the highest address, since some PHYs respond also on address
>>>>> +  * zero, which they consider MDIO broadcast address. We really
>>>>> +  * want to get their proper address instead though, so we scan
>>>>> +  * address zero last.
>>>>> +  */
>>>>> + for (phyad = 0x1f; phyad >= 0; phyad--) {
>>>>> + id1 = smsc95xx_mdio_read(dev->net, phyad, MII_PHYSID1);
>>>>> + id2 = smsc95xx_mdio_read(dev->net, phyad, MII_PHYSID2);
>>>>> + /* Check for valid response from the PHY */
>>>>> + if (id1 > 0 && id2 > 0 && id1 != 0x7fff && id2 != 0x)
>>>>> + return phyad;
>>>>> + }
>>>
>>> This would be so much easier if the driver used the core mdio/phy
>>> code. Just set mdio->phy_mask to ~BIT(0) and then use
>>> phy_find_first().
>>
>> That's in the pipeline, along with PM cleanups, but low prio.
> 
> Great. Does using the broadcast address actually cause a problem?  If
> not, i would say lets drop this part of the patch until you do the
> cleanup.

Yeah, at least the TJA11xx doesn't respond on 0x0, so I wouldn't depend
on the broadcast address. In my case, the PHY just sits at 0x4.

-- 
Best regards,
Marek Vasut

[PATCH 02/19] usbnet: smsc95xx: Stop propagation of in_pm

[Marek Vasut]

The information whether the SMSC95xx is in_pm or not can be derived from
the usbdev->suspend_count. First thing called in smsc95xx_suspend() is
usbnet_suspend(), which increments the usbdev->suspend_count and since
then the driver only calls _nopm() functions and functions with in_pm
set to 1. The smsc95xx_resume() does the inverse, it calls functions
with _nopm() suffix and with in_pm set to 1 until usbnet_resume(), which
sets the usbdev->suspend_count to 0.

Thus, this patch replaces all the in_pm variables used throughout the
driver with simple call to smsc95xx_in_pm(), which checks whether the
usbdev->suspend_count is zero or not.

Signed-off-by: Marek Vasut 
Cc: David S. Miller 
Cc: Nisar Sayed 
Cc: Woojung Huh 
Cc: Andrew Lunn 
Cc: Florian Fainelli 
Cc: linux-usb@vger.kernel.org
To: net...@vger.kernel.org
---
 drivers/net/usb/smsc95xx.c | 63 --
 1 file changed, 33 insertions(+), 30 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 772429c17ae1..c74183b51d5b 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -82,8 +82,13 @@ static bool turbo_mode = true;
 module_param(turbo_mode, bool, 0644);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
 
+static int smsc95xx_in_pm(struct usbnet *dev)
+{
+   return dev->suspend_count != 0;
+}
+
 static int __must_check __smsc95xx_read_reg(struct usbnet *dev, u32 index,
-   u32 *data, int in_pm)
+   u32 *data)
 {
u32 buf;
int ret;
@@ -91,7 +96,7 @@ static int __must_check __smsc95xx_read_reg(struct usbnet 
*dev, u32 index,
 
BUG_ON(!dev);
 
-   if (!in_pm)
+   if (!smsc95xx_in_pm(dev))
fn = usbnet_read_cmd;
else
fn = usbnet_read_cmd_nopm;
@@ -112,7 +117,7 @@ static int __must_check __smsc95xx_read_reg(struct usbnet 
*dev, u32 index,
 }
 
 static int __must_check __smsc95xx_write_reg(struct usbnet *dev, u32 index,
-u32 data, int in_pm)
+u32 data)
 {
u32 buf;
int ret;
@@ -120,7 +125,7 @@ static int __must_check __smsc95xx_write_reg(struct usbnet 
*dev, u32 index,
 
BUG_ON(!dev);
 
-   if (!in_pm)
+   if (!smsc95xx_in_pm(dev))
fn = usbnet_write_cmd;
else
fn = usbnet_write_cmd_nopm;
@@ -141,38 +146,37 @@ static int __must_check __smsc95xx_write_reg(struct 
usbnet *dev, u32 index,
 static int __must_check smsc95xx_read_reg_nopm(struct usbnet *dev, u32 index,
   u32 *data)
 {
-   return __smsc95xx_read_reg(dev, index, data, 1);
+   return __smsc95xx_read_reg(dev, index, data);
 }
 
 static int __must_check smsc95xx_write_reg_nopm(struct usbnet *dev, u32 index,
u32 data)
 {
-   return __smsc95xx_write_reg(dev, index, data, 1);
+   return __smsc95xx_write_reg(dev, index, data);
 }
 
 static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
  u32 *data)
 {
-   return __smsc95xx_read_reg(dev, index, data, 0);
+   return __smsc95xx_read_reg(dev, index, data);
 }
 
 static int __must_check smsc95xx_write_reg(struct usbnet *dev, u32 index,
   u32 data)
 {
-   return __smsc95xx_write_reg(dev, index, data, 0);
+   return __smsc95xx_write_reg(dev, index, data);
 }
 
 /* Loop until the read is completed with timeout
  * called with phy_mutex held */
-static int __must_check __smsc95xx_phy_wait_not_busy(struct usbnet *dev,
-int in_pm)
+static int __must_check __smsc95xx_phy_wait_not_busy(struct usbnet *dev)
 {
unsigned long start_time = jiffies;
u32 val;
int ret;
 
do {
-   ret = __smsc95xx_read_reg(dev, MII_ADDR, &val, in_pm);
+   ret = __smsc95xx_read_reg(dev, MII_ADDR, &val);
if (ret < 0) {
netdev_warn(dev->net, "Error reading MII_ACCESS\n");
return ret;
@@ -185,8 +189,7 @@ static int __must_check __smsc95xx_phy_wait_not_busy(struct 
usbnet *dev,
return -EIO;
 }
 
-static int __smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx,
-   int in_pm)
+static int __smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx)
 {
struct usbnet *dev = netdev_priv(netdev);
u32 val, addr;
@@ -195,7 +198,7 @@ static int __smsc95xx_mdio_read(struct net_device *netdev, 
int phy_id, int idx,
mutex_lock(&dev->phy_mutex);
 
/* confirm MII not busy */
-   ret = __smsc95xx_phy_wait_not_busy(dev, in_pm);
+   ret