All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
@ 2015-08-04 21:25 Hans de Goede
  2015-08-04 21:35 ` Felipe Balbi
  2015-08-06  8:22 ` [linux-sunxi] " Olliver Schinagl
  0 siblings, 2 replies; 28+ messages in thread
From: Hans de Goede @ 2015-08-04 21:25 UTC (permalink / raw)
  To: linux-arm-kernel

For some unclear reason sometimes we get VBus errors in host-only mode,
even though we do not have any vbus-detection then. Ignore these.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/musb/sunxi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index f9f6304..34ce5df 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -194,6 +194,10 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
 		musb_writeb(musb->mregs, MUSB_FADDR, 0);
 	}
 
+	/*  Ignore Vbus errors when in host only mode */
+	if (musb->port_mode == MUSB_PORT_MODE_HOST)
+		musb->int_usb &= ~MUSB_INTR_VBUSERROR;
+
 	musb->int_tx = readw(musb->mregs + SUNXI_MUSB_INTRTX);
 	if (musb->int_tx)
 		writew(musb->int_tx, musb->mregs + SUNXI_MUSB_INTRTX);
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-08-04 21:25 [PATCH] musb: sunxi: Ignore VBus errors in host-only mode Hans de Goede
@ 2015-08-04 21:35 ` Felipe Balbi
  2015-08-04 22:05   ` Hans de Goede
  2015-08-06  8:22 ` [linux-sunxi] " Olliver Schinagl
  1 sibling, 1 reply; 28+ messages in thread
From: Felipe Balbi @ 2015-08-04 21:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Aug 04, 2015 at 11:25:53PM +0200, Hans de Goede wrote:
> For some unclear reason sometimes we get VBus errors in host-only mode,
> even though we do not have any vbus-detection then. Ignore these.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/musb/sunxi.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> index f9f6304..34ce5df 100644
> --- a/drivers/usb/musb/sunxi.c
> +++ b/drivers/usb/musb/sunxi.c
> @@ -194,6 +194,10 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
>  		musb_writeb(musb->mregs, MUSB_FADDR, 0);
>  	}
>  
> +	/*  Ignore Vbus errors when in host only mode */
> +	if (musb->port_mode == MUSB_PORT_MODE_HOST)
> +		musb->int_usb &= ~MUSB_INTR_VBUSERROR;

check with a scope if VBUS is really dropping. Host does VBUS detection
indeed, at a minimum, for overcurrent protection. You might have
something causing VBUS to drop and that something needs to be found,
rather than masked.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150804/4dbbb53a/attachment.sig>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-08-04 21:35 ` Felipe Balbi
@ 2015-08-04 22:05   ` Hans de Goede
  2015-08-04 22:57     ` Felipe Balbi
  0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2015-08-04 22:05 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 08/04/2015 11:35 PM, Felipe Balbi wrote:
> On Tue, Aug 04, 2015 at 11:25:53PM +0200, Hans de Goede wrote:
>> For some unclear reason sometimes we get VBus errors in host-only mode,
>> even though we do not have any vbus-detection then. Ignore these.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/usb/musb/sunxi.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
>> index f9f6304..34ce5df 100644
>> --- a/drivers/usb/musb/sunxi.c
>> +++ b/drivers/usb/musb/sunxi.c
>> @@ -194,6 +194,10 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
>>   		musb_writeb(musb->mregs, MUSB_FADDR, 0);
>>   	}
>>
>> +	/*  Ignore Vbus errors when in host only mode */
>> +	if (musb->port_mode == MUSB_PORT_MODE_HOST)
>> +		musb->int_usb &= ~MUSB_INTR_VBUSERROR;
>
> check with a scope if VBUS is really dropping. Host does VBUS detection
> indeed, at a minimum, for overcurrent protection. You might have
> something causing VBUS to drop and that something needs to be found,
> rather than masked.

The boards in question do not have any vbus detection, the usb-phy on
allwinner boards do not have a vbus sense pin, instead a gpio is used
in designs which use otg. Designs which use host-only mode typically
do not have any form of vbus detection at all. In this case we always
report vbus as being valid to the musb core, so I've no idea why
the musb core is still generating vbus errors.

Regards,

Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-08-04 22:05   ` Hans de Goede
@ 2015-08-04 22:57     ` Felipe Balbi
  2015-08-05 13:30       ` Hans de Goede
  0 siblings, 1 reply; 28+ messages in thread
From: Felipe Balbi @ 2015-08-04 22:57 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Wed, Aug 05, 2015 at 12:05:02AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 08/04/2015 11:35 PM, Felipe Balbi wrote:
> >On Tue, Aug 04, 2015 at 11:25:53PM +0200, Hans de Goede wrote:
> >>For some unclear reason sometimes we get VBus errors in host-only mode,
> >>even though we do not have any vbus-detection then. Ignore these.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>---
> >>  drivers/usb/musb/sunxi.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >>diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> >>index f9f6304..34ce5df 100644
> >>--- a/drivers/usb/musb/sunxi.c
> >>+++ b/drivers/usb/musb/sunxi.c
> >>@@ -194,6 +194,10 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
> >>  		musb_writeb(musb->mregs, MUSB_FADDR, 0);
> >>  	}
> >>
> >>+	/*  Ignore Vbus errors when in host only mode */
> >>+	if (musb->port_mode == MUSB_PORT_MODE_HOST)
> >>+		musb->int_usb &= ~MUSB_INTR_VBUSERROR;
> >
> >check with a scope if VBUS is really dropping. Host does VBUS detection
> >indeed, at a minimum, for overcurrent protection. You might have
> >something causing VBUS to drop and that something needs to be found,
> >rather than masked.
> 
> The boards in question do not have any vbus detection, the usb-phy on
> allwinner boards do not have a vbus sense pin, instead a gpio is used
> in designs which use otg. Designs which use host-only mode typically
> do not have any form of vbus detection at all. In this case we always
> report vbus as being valid to the musb core, so I've no idea why
> the musb core is still generating vbus errors.

PHY must tell MUSB about VBUS level using either ULPI or UTMI+. I'd
check to see if the PHY is at fault, at least.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150804/573864e2/attachment.sig>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-08-04 22:57     ` Felipe Balbi
@ 2015-08-05 13:30       ` Hans de Goede
  0 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2015-08-05 13:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 05-08-15 00:57, Felipe Balbi wrote:
> Hi,
>
> On Wed, Aug 05, 2015 at 12:05:02AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 08/04/2015 11:35 PM, Felipe Balbi wrote:
>>> On Tue, Aug 04, 2015 at 11:25:53PM +0200, Hans de Goede wrote:
>>>> For some unclear reason sometimes we get VBus errors in host-only mode,
>>>> even though we do not have any vbus-detection then. Ignore these.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>   drivers/usb/musb/sunxi.c | 4 ++++
>>>>   1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
>>>> index f9f6304..34ce5df 100644
>>>> --- a/drivers/usb/musb/sunxi.c
>>>> +++ b/drivers/usb/musb/sunxi.c
>>>> @@ -194,6 +194,10 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
>>>>   		musb_writeb(musb->mregs, MUSB_FADDR, 0);
>>>>   	}
>>>>
>>>> +	/*  Ignore Vbus errors when in host only mode */
>>>> +	if (musb->port_mode == MUSB_PORT_MODE_HOST)
>>>> +		musb->int_usb &= ~MUSB_INTR_VBUSERROR;
>>>
>>> check with a scope if VBUS is really dropping. Host does VBUS detection
>>> indeed, at a minimum, for overcurrent protection. You might have
>>> something causing VBUS to drop and that something needs to be found,
>>> rather than masked.
>>
>> The boards in question do not have any vbus detection, the usb-phy on
>> allwinner boards do not have a vbus sense pin, instead a gpio is used
>> in designs which use otg. Designs which use host-only mode typically
>> do not have any form of vbus detection at all. In this case we always
>> report vbus as being valid to the musb core, so I've no idea why
>> the musb core is still generating vbus errors.
>
> PHY must tell MUSB about VBUS level using either ULPI or UTMI+. I'd
> check to see if the PHY is at fault, at least.

Right, and the phy code has:

                 if (data->id_det_gpio) {
                         /* OTG mode, force ISCR and cable state updates */
                         data->id_det = -1;
                         data->vbus_det = -1;
                         queue_delayed_work(system_wq, &data->detect, 0);
                 } else {
                         /* Host only mode */
                         sun4i_usb_phy0_set_id_detect(_phy, 0);
                         sun4i_usb_phy0_set_vbus_detect(_phy, 1);
                 }

Where we enter the host only path (no id-pin in host only mode) and
then sun4i_usb_phy0_set_vbus_detect updates the ISCR register of
the phy to make it report vbus valid. I've done an mmio-dump of
the iscr register while running Linux, and this code does the right
thing.

I agree with you that this is weird, but atm I see no other way
to fix this then the submitted patch.

Regards,

Hans


>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-08-04 21:25 [PATCH] musb: sunxi: Ignore VBus errors in host-only mode Hans de Goede
  2015-08-04 21:35 ` Felipe Balbi
@ 2015-08-06  8:22 ` Olliver Schinagl
  2015-08-06 14:35   ` Hans de Goede
  1 sibling, 1 reply; 28+ messages in thread
From: Olliver Schinagl @ 2015-08-06  8:22 UTC (permalink / raw)
  To: linux-arm-kernel

Hey Hans,

I've tried getting your musb stuff working on a cubietruck, but i don't 
seem to see this patch on your linux-sunxi/sunxi-wip branch on github? 
Is your github branch fully functional at the moment?

What I have done so far, is build the kernel using sunxi_defconfig and 
enabled USB_MUSB_SUNXI with its dependancies (musb isn't enabled there 
by default): USB_SUPPORT [=y] && USB_MUSB_HDRC [=y] && ARCH_SUNXI [=y] 
&& NOP_USB_XCEIV [=y] && PHY_SUN4I_USB [=y] && EXTCON [=y] && 
GENERIC_PHY [=y]  Selects: SUNXI_SRAM [=y]

I changed the dts from dr_mode='otg' to dr_mode='host', a) we only need 
host mode anyway (the id pin isn't properly connected) and b) I got an 
error about an known dr_mode before and this was the quick and easy way.

Dmesg produces the following related to musb.

[    1.691062] usb_phy_generic.0.auto supply vcc not found, using dummy 
regulator
[    1.691445] musb-hdrc: ConfigData=0xde (UTMI-8, dyn FIFOs, bulk 
combine, bulk split, HB-ISO Rx, HB-ISO Tx, SoftConn)
[    1.691453] musb-hdrc: MHDRC RTL version 0.0
[    1.691467] musb-hdrc: 11/11 max ep, 5184/8192 memory
[    1.691543] musb-hdrc musb-hdrc.1.auto: MUSB HDRC host driver
[    1.691553] musb-hdrc musb-hdrc.1.auto: new USB bus registered, 
assigned bus number 5
[    1.692470] hub 5-0:1.0: USB hub found
[    1.692529] hub 5-0:1.0: 1 port detected
[    1.699956] sunxi-rtc 1c20d00.rtc: setting system clock to 2015-08-06 
07:59:08 UTC (1438847948)
[    1.704733] usb0-vbus: disabling
[    1.765695] ldo4: disabling
[    1.808351] ldo3: disabling
[    1.848769] vcc5v0: disabling
[    1.848774] vcc3v0: disabling

The usb_phy_generic missing shouldn't be too bad? But the usb0-vbus 
being disabled obviously might be related to the musb port not working? 
What causes this though? I went through all the musb patch series mails 
but don't recall seing anything special being needed.

If there are fixes missing in the sunxi-next stuff that could explain 
this, could you be so kind and push your latest work so I can try it? 
Thanks Hans!

Olliver

On 04-08-15 23:25, Hans de Goede wrote:
> For some unclear reason sometimes we get VBus errors in host-only mode,
> even though we do not have any vbus-detection then. Ignore these.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   drivers/usb/musb/sunxi.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> index f9f6304..34ce5df 100644
> --- a/drivers/usb/musb/sunxi.c
> +++ b/drivers/usb/musb/sunxi.c
> @@ -194,6 +194,10 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
>   		musb_writeb(musb->mregs, MUSB_FADDR, 0);
>   	}
>   
> +	/*  Ignore Vbus errors when in host only mode */
> +	if (musb->port_mode == MUSB_PORT_MODE_HOST)
> +		musb->int_usb &= ~MUSB_INTR_VBUSERROR;
> +
>   	musb->int_tx = readw(musb->mregs + SUNXI_MUSB_INTRTX);
>   	if (musb->int_tx)
>   		writew(musb->int_tx, musb->mregs + SUNXI_MUSB_INTRTX);

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-08-06  8:22 ` [linux-sunxi] " Olliver Schinagl
@ 2015-08-06 14:35   ` Hans de Goede
  2015-08-07  8:45     ` Olliver Schinagl
  0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2015-08-06 14:35 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 06-08-15 10:22, Olliver Schinagl wrote:
> Hey Hans,
>
> I've tried getting your musb stuff working on a cubietruck, but i don't seem to see this patch on your linux-sunxi/sunxi-wip branch on github? Is your github branch fully functional at the moment?
>
> What I have done so far, is build the kernel using sunxi_defconfig and enabled USB_MUSB_SUNXI with its dependancies (musb isn't enabled there by default): USB_SUPPORT [=y] && USB_MUSB_HDRC [=y] && ARCH_SUNXI [=y] && NOP_USB_XCEIV [=y] && PHY_SUN4I_USB [=y] && EXTCON [=y] && GENERIC_PHY [=y]  Selects: SUNXI_SRAM [=y]
>
> I changed the dts from dr_mode='otg' to dr_mode='host', a) we only need host mode anyway (the id pin isn't properly connected)

If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det
gpio settings from the usb_phy node in the dts, as the sun4i phy code detects
host vs otg mode by checking for the presence of these.

 > and b) I got an error about an known dr_mode before and this was the quick and easy way.
>
> Dmesg produces the following related to musb.
>
> [    1.691062] usb_phy_generic.0.auto supply vcc not found, using dummy regulator
> [    1.691445] musb-hdrc: ConfigData=0xde (UTMI-8, dyn FIFOs, bulk combine, bulk split, HB-ISO Rx, HB-ISO Tx, SoftConn)
> [    1.691453] musb-hdrc: MHDRC RTL version 0.0
> [    1.691467] musb-hdrc: 11/11 max ep, 5184/8192 memory
> [    1.691543] musb-hdrc musb-hdrc.1.auto: MUSB HDRC host driver
> [    1.691553] musb-hdrc musb-hdrc.1.auto: new USB bus registered, assigned bus number 5
> [    1.692470] hub 5-0:1.0: USB hub found
> [    1.692529] hub 5-0:1.0: 1 port detected
> [    1.699956] sunxi-rtc 1c20d00.rtc: setting system clock to 2015-08-06 07:59:08 UTC (1438847948)
> [    1.704733] usb0-vbus: disabling
> [    1.765695] ldo4: disabling
> [    1.808351] ldo3: disabling
> [    1.848769] vcc5v0: disabling
> [    1.848774] vcc3v0: disabling
>
> The usb_phy_generic missing shouldn't be too bad? But the usb0-vbus being disabled obviously might be related to the musb port not working?

Correct.

For starters I would try the dts changes I suggested above.

Regards,

Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-08-06 14:35   ` Hans de Goede
@ 2015-08-07  8:45     ` Olliver Schinagl
  2015-09-04  6:43       ` Olliver Schinagl
  0 siblings, 1 reply; 28+ messages in thread
From: Olliver Schinagl @ 2015-08-07  8:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hey Hans,

On 06-08-15 16:35, Hans de Goede wrote:
> Hi,
>
> On 06-08-15 10:22, Olliver Schinagl wrote:
>> Hey Hans,
>>
>> I've tried getting your musb stuff working on a cubietruck, but i 
>> don't seem to see this patch on your linux-sunxi/sunxi-wip branch on 
>> github? Is your github branch fully functional at the moment?
>>
>> What I have done so far, is build the kernel using sunxi_defconfig 
>> and enabled USB_MUSB_SUNXI with its dependancies (musb isn't enabled 
>> there by default): USB_SUPPORT [=y] && USB_MUSB_HDRC [=y] && 
>> ARCH_SUNXI [=y] && NOP_USB_XCEIV [=y] && PHY_SUN4I_USB [=y] && EXTCON 
>> [=y] && GENERIC_PHY [=y]  Selects: SUNXI_SRAM [=y]
>>
>> I changed the dts from dr_mode='otg' to dr_mode='host', a) we only 
>> need host mode anyway (the id pin isn't properly connected)
>
> If you change the dr_mode to host then you _must_ also remove any 
> id_det and vbus_det
> gpio settings from the usb_phy node in the dts, as the sun4i phy code 
> detects
> host vs otg mode by checking for the presence of these.
Yes, this fixes it and makes it work. Thanks.

Also I apologize for not noticing this patch in your queue before :) 
Looks like everything is all ready in your WIP branch.

Now, I see that Chen-Yu found the same problems here:
http://permalink.gmane.org/gmane.comp.hardware.netbook.arm.sunxi/17843

So far, with your mentioned changes to remove the id and vbus gpio's you 
can have my

Tested-by: Olliver Schinagl <o.schinagl@ultimaker.com>

>
> > and b) I got an error about an known dr_mode before and this was the 
> quick and easy way.
>>
>> Dmesg produces the following related to musb.
>>
>> [    1.691062] usb_phy_generic.0.auto supply vcc not found, using 
>> dummy regulator
>> [    1.691445] musb-hdrc: ConfigData=0xde (UTMI-8, dyn FIFOs, bulk 
>> combine, bulk split, HB-ISO Rx, HB-ISO Tx, SoftConn)
>> [    1.691453] musb-hdrc: MHDRC RTL version 0.0
>> [    1.691467] musb-hdrc: 11/11 max ep, 5184/8192 memory
>> [    1.691543] musb-hdrc musb-hdrc.1.auto: MUSB HDRC host driver
>> [    1.691553] musb-hdrc musb-hdrc.1.auto: new USB bus registered, 
>> assigned bus number 5
>> [    1.692470] hub 5-0:1.0: USB hub found
>> [    1.692529] hub 5-0:1.0: 1 port detected
>> [    1.699956] sunxi-rtc 1c20d00.rtc: setting system clock to 
>> 2015-08-06 07:59:08 UTC (1438847948)
>> [    1.704733] usb0-vbus: disabling
>> [    1.765695] ldo4: disabling
>> [    1.808351] ldo3: disabling
>> [    1.848769] vcc5v0: disabling
>> [    1.848774] vcc3v0: disabling
>>
>> The usb_phy_generic missing shouldn't be too bad? But the usb0-vbus 
>> being disabled obviously might be related to the musb port not working?
>
> Correct.
>
> For starters I would try the dts changes I suggested above.
Yep that fixed it.
>
> Regards,
>
> Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-08-07  8:45     ` Olliver Schinagl
@ 2015-09-04  6:43       ` Olliver Schinagl
  2015-09-10 18:23         ` Hans de Goede
  0 siblings, 1 reply; 28+ messages in thread
From: Olliver Schinagl @ 2015-09-04  6:43 UTC (permalink / raw)
  To: linux-arm-kernel

Hey Hans,

On 07-08-15 10:45, Olliver Schinagl wrote:
> <snip>
>> If you change the dr_mode to host then you _must_ also remove any 
>> id_det and vbus_det
>> gpio settings from the usb_phy node in the dts, as the sun4i phy code 
>> detects
>> host vs otg mode by checking for the presence of these.
> Yes, this fixes it and makes it work. Thanks.
>
I've been going back to this and am wondering if this is something I can 
look into to fix properly? E.g. if the dts sets dr_mode = host, can we 
simply ignore the pins and treat them as unset?

Reason why I'm asking, the board we are using, the Lime2 has all the 
stuff required for regular otg mode. The cables however that we connect 
to the miniusb however do not force host only mode via the id pin, which 
would have been the preferred way. We use a 'shield' for our lime with 
various connections and thus have a dts for this shield, which includes 
the lime2 dts. In the lime2 dts you want the otg pins enabled and set in 
the generic usbphy section.

In my overlay, I ideally don't want to unset the pins, but do want to 
set dr_mode (as i can't force it from userspace can I?). The logical 
thing in my mind, would be to then just ignore those pins even when set, 
right?

Olliver

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-04  6:43       ` Olliver Schinagl
@ 2015-09-10 18:23         ` Hans de Goede
  2015-09-10 18:30           ` Maxime Ripard
  2015-09-26 12:50           ` Olliver Schinagl
  0 siblings, 2 replies; 28+ messages in thread
From: Hans de Goede @ 2015-09-10 18:23 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 04-09-15 08:43, Olliver Schinagl wrote:
> Hey Hans,
>
> On 07-08-15 10:45, Olliver Schinagl wrote:
>> <snip>
>>> If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det
>>> gpio settings from the usb_phy node in the dts, as the sun4i phy code detects
>>> host vs otg mode by checking for the presence of these.
>> Yes, this fixes it and makes it work. Thanks.
>>
> I've been going back to this and am wondering if this is something I can look into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore the pins and treat them as unset?

AFAIK you cannot unset something in dts. The only solution I
can comeup with is to add a dr_mode argument to the phy like
we already have for the otg controller itself.

This is something which we likely need to do anyways to add
support for peripheral only mode, which we seem to need for
some "hdmi sticks".

Patches doing this are welcome from my pov.

Regards,

Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-10 18:23         ` Hans de Goede
@ 2015-09-10 18:30           ` Maxime Ripard
  2015-09-10 18:38             ` Hans de Goede
  2015-09-26 12:50           ` Olliver Schinagl
  1 sibling, 1 reply; 28+ messages in thread
From: Maxime Ripard @ 2015-09-10 18:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 04-09-15 08:43, Olliver Schinagl wrote:
> >Hey Hans,
> >
> >On 07-08-15 10:45, Olliver Schinagl wrote:
> >><snip>
> >>>If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det
> >>>gpio settings from the usb_phy node in the dts, as the sun4i phy code detects
> >>>host vs otg mode by checking for the presence of these.
> >>Yes, this fixes it and makes it work. Thanks.
> >>
> >I've been going back to this and am wondering if this is something I can look into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore the pins and treat them as unset?
> 
> AFAIK you cannot unset something in dts. The only solution I
> can comeup with is to add a dr_mode argument to the phy like
> we already have for the otg controller itself.
> 
> This is something which we likely need to do anyways to add
> support for peripheral only mode, which we seem to need for
> some "hdmi sticks".

I haven't really followed the rest of the discussion, so sorry if you
already talked about that, but why can't you just set the dr_mode to
peripheral in such a case?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150910/9489360c/attachment.sig>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-10 18:30           ` Maxime Ripard
@ 2015-09-10 18:38             ` Hans de Goede
  2015-09-14 14:44               ` Bin Liu
  2015-09-14 20:25               ` Maxime Ripard
  0 siblings, 2 replies; 28+ messages in thread
From: Hans de Goede @ 2015-09-10 18:38 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 10-09-15 20:30, Maxime Ripard wrote:
> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 04-09-15 08:43, Olliver Schinagl wrote:
>>> Hey Hans,
>>>
>>> On 07-08-15 10:45, Olliver Schinagl wrote:
>>>> <snip>
>>>>> If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det
>>>>> gpio settings from the usb_phy node in the dts, as the sun4i phy code detects
>>>>> host vs otg mode by checking for the presence of these.
>>>> Yes, this fixes it and makes it work. Thanks.
>>>>
>>> I've been going back to this and am wondering if this is something I can look into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore the pins and treat them as unset?
>>
>> AFAIK you cannot unset something in dts. The only solution I
>> can comeup with is to add a dr_mode argument to the phy like
>> we already have for the otg controller itself.
>>
>> This is something which we likely need to do anyways to add
>> support for peripheral only mode, which we seem to need for
>> some "hdmi sticks".
>
> I haven't really followed the rest of the discussion, so sorry if you
> already talked about that, but why can't you just set the dr_mode to
> peripheral in such a case?

This is about the usbphy code not the musb-controller code, which are
2 different dts nodes, atm only the musb-controller node has a
dr_mode property, and the phy code decides between host-only
and otg mode based on whether an id pin is assigned or not.

My proposal is to get rid of the id-pin hack to determine the mode
and add a dr_mode property to the usbphy dts node.

Regards,

Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-10 18:38             ` Hans de Goede
@ 2015-09-14 14:44               ` Bin Liu
  2015-09-14 14:59                 ` Hans de Goede
  2015-09-14 20:25               ` Maxime Ripard
  1 sibling, 1 reply; 28+ messages in thread
From: Bin Liu @ 2015-09-14 14:44 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 10-09-15 20:30, Maxime Ripard wrote:
>>
>> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
>>>
>>> Hi,
>>>
>>> On 04-09-15 08:43, Olliver Schinagl wrote:
>>>>
>>>> Hey Hans,
>>>>
>>>> On 07-08-15 10:45, Olliver Schinagl wrote:
>>>>>
>>>>> <snip>
>>>>>>
>>>>>> If you change the dr_mode to host then you _must_ also remove any
>>>>>> id_det and vbus_det
>>>>>> gpio settings from the usb_phy node in the dts, as the sun4i phy code
>>>>>> detects
>>>>>> host vs otg mode by checking for the presence of these.
>>>>>
>>>>> Yes, this fixes it and makes it work. Thanks.
>>>>>
>>>> I've been going back to this and am wondering if this is something I can
>>>> look into to fix properly? E.g. if the dts sets dr_mode = host, can we
>>>> simply ignore the pins and treat them as unset?
>>>
>>>
>>> AFAIK you cannot unset something in dts. The only solution I
>>> can comeup with is to add a dr_mode argument to the phy like
>>> we already have for the otg controller itself.
>>>
>>> This is something which we likely need to do anyways to add
>>> support for peripheral only mode, which we seem to need for
>>> some "hdmi sticks".
>>
>>
>> I haven't really followed the rest of the discussion, so sorry if you
>> already talked about that, but why can't you just set the dr_mode to
>> peripheral in such a case?
>
>
> This is about the usbphy code not the musb-controller code, which are
> 2 different dts nodes, atm only the musb-controller node has a
> dr_mode property, and the phy code decides between host-only
> and otg mode based on whether an id pin is assigned or not.
>
> My proposal is to get rid of the id-pin hack to determine the mode
> and add a dr_mode property to the usbphy dts node.

I would try to avoid adding dr_mode in the usbphy node if possible,
since it is already specified in the controller node.

Since the phy node is referenced in the controller node, is it
possible that the phy driver looks up the controller of properties to
find its dr_mode setting?

I am currently doing the same thing for musb_dsps.
Experienmenting...since I don't much about dts handling yet.

Regards,
-Bin.

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-14 14:44               ` Bin Liu
@ 2015-09-14 14:59                 ` Hans de Goede
  2015-09-14 16:58                   ` Bin Liu
  0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2015-09-14 14:59 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 14-09-15 16:44, Bin Liu wrote:
> Hi,
>
> On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 10-09-15 20:30, Maxime Ripard wrote:
>>>
>>> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 04-09-15 08:43, Olliver Schinagl wrote:
>>>>>
>>>>> Hey Hans,
>>>>>
>>>>> On 07-08-15 10:45, Olliver Schinagl wrote:
>>>>>>
>>>>>> <snip>
>>>>>>>
>>>>>>> If you change the dr_mode to host then you _must_ also remove any
>>>>>>> id_det and vbus_det
>>>>>>> gpio settings from the usb_phy node in the dts, as the sun4i phy code
>>>>>>> detects
>>>>>>> host vs otg mode by checking for the presence of these.
>>>>>>
>>>>>> Yes, this fixes it and makes it work. Thanks.
>>>>>>
>>>>> I've been going back to this and am wondering if this is something I can
>>>>> look into to fix properly? E.g. if the dts sets dr_mode = host, can we
>>>>> simply ignore the pins and treat them as unset?
>>>>
>>>>
>>>> AFAIK you cannot unset something in dts. The only solution I
>>>> can comeup with is to add a dr_mode argument to the phy like
>>>> we already have for the otg controller itself.
>>>>
>>>> This is something which we likely need to do anyways to add
>>>> support for peripheral only mode, which we seem to need for
>>>> some "hdmi sticks".
>>>
>>>
>>> I haven't really followed the rest of the discussion, so sorry if you
>>> already talked about that, but why can't you just set the dr_mode to
>>> peripheral in such a case?
>>
>>
>> This is about the usbphy code not the musb-controller code, which are
>> 2 different dts nodes, atm only the musb-controller node has a
>> dr_mode property, and the phy code decides between host-only
>> and otg mode based on whether an id pin is assigned or not.
>>
>> My proposal is to get rid of the id-pin hack to determine the mode
>> and add a dr_mode property to the usbphy dts node.
>
> I would try to avoid adding dr_mode in the usbphy node if possible,
> since it is already specified in the controller node.
>
> Since the phy node is referenced in the controller node, is it
> possible that the phy driver looks up the controller of properties to
> find its dr_mode setting?

That is possible, but it very much goes against how devicetree normally
works, where every node is more or less a standalone unit which
contains enough info for the associated driver to do its work.

Currently there is no link from the phy node to the controller node
(only the other way around) and adding such a link requires more code
then simple having a duplicate dr_mode.

Regards,

Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-14 14:59                 ` Hans de Goede
@ 2015-09-14 16:58                   ` Bin Liu
  2015-09-14 17:08                     ` Hans de Goede
  0 siblings, 1 reply; 28+ messages in thread
From: Bin Liu @ 2015-09-14 16:58 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Mon, Sep 14, 2015 at 9:59 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 14-09-15 16:44, Bin Liu wrote:
>>
>> Hi,
>>
>> On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> Hi,
>>>
>>> On 10-09-15 20:30, Maxime Ripard wrote:
>>>>
>>>>
>>>> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 04-09-15 08:43, Olliver Schinagl wrote:
>>>>>>
>>>>>>
>>>>>> Hey Hans,
>>>>>>
>>>>>> On 07-08-15 10:45, Olliver Schinagl wrote:
>>>>>>>
>>>>>>>
>>>>>>> <snip>
>>>>>>>>
>>>>>>>>
>>>>>>>> If you change the dr_mode to host then you _must_ also remove any
>>>>>>>> id_det and vbus_det
>>>>>>>> gpio settings from the usb_phy node in the dts, as the sun4i phy
>>>>>>>> code
>>>>>>>> detects
>>>>>>>> host vs otg mode by checking for the presence of these.
>>>>>>>
>>>>>>>
>>>>>>> Yes, this fixes it and makes it work. Thanks.
>>>>>>>
>>>>>> I've been going back to this and am wondering if this is something I
>>>>>> can
>>>>>> look into to fix properly? E.g. if the dts sets dr_mode = host, can we
>>>>>> simply ignore the pins and treat them as unset?
>>>>>
>>>>>
>>>>>
>>>>> AFAIK you cannot unset something in dts. The only solution I
>>>>> can comeup with is to add a dr_mode argument to the phy like
>>>>> we already have for the otg controller itself.
>>>>>
>>>>> This is something which we likely need to do anyways to add
>>>>> support for peripheral only mode, which we seem to need for
>>>>> some "hdmi sticks".
>>>>
>>>>
>>>>
>>>> I haven't really followed the rest of the discussion, so sorry if you
>>>> already talked about that, but why can't you just set the dr_mode to
>>>> peripheral in such a case?
>>>
>>>
>>>
>>> This is about the usbphy code not the musb-controller code, which are
>>> 2 different dts nodes, atm only the musb-controller node has a
>>> dr_mode property, and the phy code decides between host-only
>>> and otg mode based on whether an id pin is assigned or not.
>>>
>>> My proposal is to get rid of the id-pin hack to determine the mode
>>> and add a dr_mode property to the usbphy dts node.
>>
>>
>> I would try to avoid adding dr_mode in the usbphy node if possible,
>> since it is already specified in the controller node.
>>
>> Since the phy node is referenced in the controller node, is it
>> possible that the phy driver looks up the controller of properties to
>> find its dr_mode setting?
>
>
> That is possible, but it very much goes against how devicetree normally
> works, where every node is more or less a standalone unit which
> contains enough info for the associated driver to do its work.

I guess you are right, but duplicating dr_mode would cause more
trouble for end user.

Felipe, could you please give your comments on this issue? I have to
do the similar change for musb_dsps.

>
> Currently there is no link from the phy node to the controller node
> (only the other way around) and adding such a link requires more code
> then simple having a duplicate dr_mode.

I guess I did not make my previous comment clearly, we don't need to
add the link from the phy node to the controller node. We don't need
to change the current dts, just in the phy driver to look up dr_mode
of the controller node, if possible.

Regards,
-Bin.

>
> Regards,
>
> Hans
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-14 16:58                   ` Bin Liu
@ 2015-09-14 17:08                     ` Hans de Goede
  2015-09-14 17:14                       ` Bin Liu
  0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2015-09-14 17:08 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 14-09-15 18:58, Bin Liu wrote:
> Hi,
>
> On Mon, Sep 14, 2015 at 9:59 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 14-09-15 16:44, Bin Liu wrote:
>>>
>>> Hi,
>>>
>>> On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede <hdegoede@redhat.com>
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 10-09-15 20:30, Maxime Ripard wrote:
>>>>>
>>>>>
>>>>> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 04-09-15 08:43, Olliver Schinagl wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hey Hans,
>>>>>>>
>>>>>>> On 07-08-15 10:45, Olliver Schinagl wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> <snip>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> If you change the dr_mode to host then you _must_ also remove any
>>>>>>>>> id_det and vbus_det
>>>>>>>>> gpio settings from the usb_phy node in the dts, as the sun4i phy
>>>>>>>>> code
>>>>>>>>> detects
>>>>>>>>> host vs otg mode by checking for the presence of these.
>>>>>>>>
>>>>>>>>
>>>>>>>> Yes, this fixes it and makes it work. Thanks.
>>>>>>>>
>>>>>>> I've been going back to this and am wondering if this is something I
>>>>>>> can
>>>>>>> look into to fix properly? E.g. if the dts sets dr_mode = host, can we
>>>>>>> simply ignore the pins and treat them as unset?
>>>>>>
>>>>>>
>>>>>>
>>>>>> AFAIK you cannot unset something in dts. The only solution I
>>>>>> can comeup with is to add a dr_mode argument to the phy like
>>>>>> we already have for the otg controller itself.
>>>>>>
>>>>>> This is something which we likely need to do anyways to add
>>>>>> support for peripheral only mode, which we seem to need for
>>>>>> some "hdmi sticks".
>>>>>
>>>>>
>>>>>
>>>>> I haven't really followed the rest of the discussion, so sorry if you
>>>>> already talked about that, but why can't you just set the dr_mode to
>>>>> peripheral in such a case?
>>>>
>>>>
>>>>
>>>> This is about the usbphy code not the musb-controller code, which are
>>>> 2 different dts nodes, atm only the musb-controller node has a
>>>> dr_mode property, and the phy code decides between host-only
>>>> and otg mode based on whether an id pin is assigned or not.
>>>>
>>>> My proposal is to get rid of the id-pin hack to determine the mode
>>>> and add a dr_mode property to the usbphy dts node.
>>>
>>>
>>> I would try to avoid adding dr_mode in the usbphy node if possible,
>>> since it is already specified in the controller node.
>>>
>>> Since the phy node is referenced in the controller node, is it
>>> possible that the phy driver looks up the controller of properties to
>>> find its dr_mode setting?
>>
>>
>> That is possible, but it very much goes against how devicetree normally
>> works, where every node is more or less a standalone unit which
>> contains enough info for the associated driver to do its work.
>
> I guess you are right, but duplicating dr_mode would cause more
> trouble for end user.

The user already needs to set up regulators, vbus-det and id-det
for otg mode in the usbphy node. Although there are 2 nodes in dts /
2 separate hardware blocks involved in reality the 2 are closely
related and the user already must take care to have the settings match.

Besides that writing dts files is not something which end users do,
so a normal user will never see this.

> Felipe, could you please give your comments on this issue? I have to
> do the similar change for musb_dsps.
>
>>
>> Currently there is no link from the phy node to the controller node
>> (only the other way around) and adding such a link requires more code
>> then simple having a duplicate dr_mode.
>
> I guess I did not make my previous comment clearly, we don't need to
> add the link from the phy node to the controller node. We don't need
> to change the current dts, just in the phy driver to look up dr_mode
> of the controller node, if possible.

And how does the phy code now where the controller node lives without
having a link to it in its node? Hardcode the full path to the
controller node ? That is both ugly and error prone.

Regards,

Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-14 17:08                     ` Hans de Goede
@ 2015-09-14 17:14                       ` Bin Liu
  2015-09-14 17:25                         ` Hans de Goede
  0 siblings, 1 reply; 28+ messages in thread
From: Bin Liu @ 2015-09-14 17:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Mon, Sep 14, 2015 at 12:08 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 14-09-15 18:58, Bin Liu wrote:
>>
>> Hi,
>>
>> On Mon, Sep 14, 2015 at 9:59 AM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> Hi,
>>>
>>>
>>> On 14-09-15 16:44, Bin Liu wrote:
>>>>
>>>>
>>>> Hi,
>>>>
>>>> On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede <hdegoede@redhat.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 10-09-15 20:30, Maxime Ripard wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 04-09-15 08:43, Olliver Schinagl wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Hey Hans,
>>>>>>>>
>>>>>>>> On 07-08-15 10:45, Olliver Schinagl wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> <snip>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> If you change the dr_mode to host then you _must_ also remove any
>>>>>>>>>> id_det and vbus_det
>>>>>>>>>> gpio settings from the usb_phy node in the dts, as the sun4i phy
>>>>>>>>>> code
>>>>>>>>>> detects
>>>>>>>>>> host vs otg mode by checking for the presence of these.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Yes, this fixes it and makes it work. Thanks.
>>>>>>>>>
>>>>>>>> I've been going back to this and am wondering if this is something I
>>>>>>>> can
>>>>>>>> look into to fix properly? E.g. if the dts sets dr_mode = host, can
>>>>>>>> we
>>>>>>>> simply ignore the pins and treat them as unset?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> AFAIK you cannot unset something in dts. The only solution I
>>>>>>> can comeup with is to add a dr_mode argument to the phy like
>>>>>>> we already have for the otg controller itself.
>>>>>>>
>>>>>>> This is something which we likely need to do anyways to add
>>>>>>> support for peripheral only mode, which we seem to need for
>>>>>>> some "hdmi sticks".
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> I haven't really followed the rest of the discussion, so sorry if you
>>>>>> already talked about that, but why can't you just set the dr_mode to
>>>>>> peripheral in such a case?
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> This is about the usbphy code not the musb-controller code, which are
>>>>> 2 different dts nodes, atm only the musb-controller node has a
>>>>> dr_mode property, and the phy code decides between host-only
>>>>> and otg mode based on whether an id pin is assigned or not.
>>>>>
>>>>> My proposal is to get rid of the id-pin hack to determine the mode
>>>>> and add a dr_mode property to the usbphy dts node.
>>>>
>>>>
>>>>
>>>> I would try to avoid adding dr_mode in the usbphy node if possible,
>>>> since it is already specified in the controller node.
>>>>
>>>> Since the phy node is referenced in the controller node, is it
>>>> possible that the phy driver looks up the controller of properties to
>>>> find its dr_mode setting?
>>>
>>>
>>>
>>> That is possible, but it very much goes against how devicetree normally
>>> works, where every node is more or less a standalone unit which
>>> contains enough info for the associated driver to do its work.
>>
>>
>> I guess you are right, but duplicating dr_mode would cause more
>> trouble for end user.
>
>
> The user already needs to set up regulators, vbus-det and id-det
> for otg mode in the usbphy node. Although there are 2 nodes in dts /
> 2 separate hardware blocks involved in reality the 2 are closely
> related and the user already must take care to have the settings match.
>
> Besides that writing dts files is not something which end users do,
> so a normal user will never see this.
>
>> Felipe, could you please give your comments on this issue? I have to
>> do the similar change for musb_dsps.
>>
>>>
>>> Currently there is no link from the phy node to the controller node
>>> (only the other way around) and adding such a link requires more code
>>> then simple having a duplicate dr_mode.
>>
>>
>> I guess I did not make my previous comment clearly, we don't need to
>> add the link from the phy node to the controller node. We don't need
>> to change the current dts, just in the phy driver to look up dr_mode
>> of the controller node, if possible.
>
>
> And how does the phy code now where the controller node lives without
> having a link to it in its node? Hardcode the full path to the
> controller node ? That is both ugly and error prone.

This is my first time looking at dts handling in drivers, so I might
be completely wrong, but I am thinking that since the controller node
links to the phy node, so the controller node is the parent of the phy
node, so if there is an of api can look it up?

Regards,
-Bin.

>
> Regards,
>
> Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-14 17:14                       ` Bin Liu
@ 2015-09-14 17:25                         ` Hans de Goede
  2015-09-14 17:53                           ` Bin Liu
  0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2015-09-14 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 14-09-15 19:14, Bin Liu wrote:
> Hi,
>
> On Mon, Sep 14, 2015 at 12:08 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 14-09-15 18:58, Bin Liu wrote:
>>>
>>> Hi,
>>>
>>> On Mon, Sep 14, 2015 at 9:59 AM, Hans de Goede <hdegoede@redhat.com>
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>>
>>>> On 14-09-15 16:44, Bin Liu wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede <hdegoede@redhat.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 10-09-15 20:30, Maxime Ripard wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 04-09-15 08:43, Olliver Schinagl wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hey Hans,
>>>>>>>>>
>>>>>>>>> On 07-08-15 10:45, Olliver Schinagl wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> <snip>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> If you change the dr_mode to host then you _must_ also remove any
>>>>>>>>>>> id_det and vbus_det
>>>>>>>>>>> gpio settings from the usb_phy node in the dts, as the sun4i phy
>>>>>>>>>>> code
>>>>>>>>>>> detects
>>>>>>>>>>> host vs otg mode by checking for the presence of these.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Yes, this fixes it and makes it work. Thanks.
>>>>>>>>>>
>>>>>>>>> I've been going back to this and am wondering if this is something I
>>>>>>>>> can
>>>>>>>>> look into to fix properly? E.g. if the dts sets dr_mode = host, can
>>>>>>>>> we
>>>>>>>>> simply ignore the pins and treat them as unset?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> AFAIK you cannot unset something in dts. The only solution I
>>>>>>>> can comeup with is to add a dr_mode argument to the phy like
>>>>>>>> we already have for the otg controller itself.
>>>>>>>>
>>>>>>>> This is something which we likely need to do anyways to add
>>>>>>>> support for peripheral only mode, which we seem to need for
>>>>>>>> some "hdmi sticks".
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I haven't really followed the rest of the discussion, so sorry if you
>>>>>>> already talked about that, but why can't you just set the dr_mode to
>>>>>>> peripheral in such a case?
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> This is about the usbphy code not the musb-controller code, which are
>>>>>> 2 different dts nodes, atm only the musb-controller node has a
>>>>>> dr_mode property, and the phy code decides between host-only
>>>>>> and otg mode based on whether an id pin is assigned or not.
>>>>>>
>>>>>> My proposal is to get rid of the id-pin hack to determine the mode
>>>>>> and add a dr_mode property to the usbphy dts node.
>>>>>
>>>>>
>>>>>
>>>>> I would try to avoid adding dr_mode in the usbphy node if possible,
>>>>> since it is already specified in the controller node.
>>>>>
>>>>> Since the phy node is referenced in the controller node, is it
>>>>> possible that the phy driver looks up the controller of properties to
>>>>> find its dr_mode setting?
>>>>
>>>>
>>>>
>>>> That is possible, but it very much goes against how devicetree normally
>>>> works, where every node is more or less a standalone unit which
>>>> contains enough info for the associated driver to do its work.
>>>
>>>
>>> I guess you are right, but duplicating dr_mode would cause more
>>> trouble for end user.
>>
>>
>> The user already needs to set up regulators, vbus-det and id-det
>> for otg mode in the usbphy node. Although there are 2 nodes in dts /
>> 2 separate hardware blocks involved in reality the 2 are closely
>> related and the user already must take care to have the settings match.
>>
>> Besides that writing dts files is not something which end users do,
>> so a normal user will never see this.
>>
>>> Felipe, could you please give your comments on this issue? I have to
>>> do the similar change for musb_dsps.
>>>
>>>>
>>>> Currently there is no link from the phy node to the controller node
>>>> (only the other way around) and adding such a link requires more code
>>>> then simple having a duplicate dr_mode.
>>>
>>>
>>> I guess I did not make my previous comment clearly, we don't need to
>>> add the link from the phy node to the controller node. We don't need
>>> to change the current dts, just in the phy driver to look up dr_mode
>>> of the controller node, if possible.
>>
>>
>> And how does the phy code now where the controller node lives without
>> having a link to it in its node? Hardcode the full path to the
>> controller node ? That is both ugly and error prone.
>
> This is my first time looking at dts handling in drivers, so I might
> be completely wrong, but I am thinking that since the controller node
> links to the phy node, so the controller node is the parent of the phy
> node, so if there is an of api can look it up?

If the phy is a child of the controller, then yes this would work,
but in the case of sunxi the phy is a built-in mmio mapped peripheral
just like the controller, so they sit at the same level and have no
parent child relation.

Regards,

Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-14 17:25                         ` Hans de Goede
@ 2015-09-14 17:53                           ` Bin Liu
  2015-09-14 17:56                             ` Hans de Goede
  0 siblings, 1 reply; 28+ messages in thread
From: Bin Liu @ 2015-09-14 17:53 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Mon, Sep 14, 2015 at 12:25 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 14-09-15 19:14, Bin Liu wrote:
>>
>> Hi,
>>
>> On Mon, Sep 14, 2015 at 12:08 PM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> Hi,
>>>
>>>
>>> On 14-09-15 18:58, Bin Liu wrote:
>>>>
>>>>
>>>> Hi,
>>>>
>>>> On Mon, Sep 14, 2015 at 9:59 AM, Hans de Goede <hdegoede@redhat.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>>
>>>>> On 14-09-15 16:44, Bin Liu wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede <hdegoede@redhat.com>
>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 10-09-15 20:30, Maxime Ripard wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On 04-09-15 08:43, Olliver Schinagl wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hey Hans,
>>>>>>>>>>
>>>>>>>>>> On 07-08-15 10:45, Olliver Schinagl wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> <snip>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> If you change the dr_mode to host then you _must_ also remove
>>>>>>>>>>>> any
>>>>>>>>>>>> id_det and vbus_det
>>>>>>>>>>>> gpio settings from the usb_phy node in the dts, as the sun4i phy
>>>>>>>>>>>> code
>>>>>>>>>>>> detects
>>>>>>>>>>>> host vs otg mode by checking for the presence of these.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Yes, this fixes it and makes it work. Thanks.
>>>>>>>>>>>
>>>>>>>>>> I've been going back to this and am wondering if this is something
>>>>>>>>>> I
>>>>>>>>>> can
>>>>>>>>>> look into to fix properly? E.g. if the dts sets dr_mode = host,
>>>>>>>>>> can
>>>>>>>>>> we
>>>>>>>>>> simply ignore the pins and treat them as unset?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> AFAIK you cannot unset something in dts. The only solution I
>>>>>>>>> can comeup with is to add a dr_mode argument to the phy like
>>>>>>>>> we already have for the otg controller itself.
>>>>>>>>>
>>>>>>>>> This is something which we likely need to do anyways to add
>>>>>>>>> support for peripheral only mode, which we seem to need for
>>>>>>>>> some "hdmi sticks".
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> I haven't really followed the rest of the discussion, so sorry if
>>>>>>>> you
>>>>>>>> already talked about that, but why can't you just set the dr_mode to
>>>>>>>> peripheral in such a case?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> This is about the usbphy code not the musb-controller code, which are
>>>>>>> 2 different dts nodes, atm only the musb-controller node has a
>>>>>>> dr_mode property, and the phy code decides between host-only
>>>>>>> and otg mode based on whether an id pin is assigned or not.
>>>>>>>
>>>>>>> My proposal is to get rid of the id-pin hack to determine the mode
>>>>>>> and add a dr_mode property to the usbphy dts node.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> I would try to avoid adding dr_mode in the usbphy node if possible,
>>>>>> since it is already specified in the controller node.
>>>>>>
>>>>>> Since the phy node is referenced in the controller node, is it
>>>>>> possible that the phy driver looks up the controller of properties to
>>>>>> find its dr_mode setting?
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> That is possible, but it very much goes against how devicetree normally
>>>>> works, where every node is more or less a standalone unit which
>>>>> contains enough info for the associated driver to do its work.
>>>>
>>>>
>>>>
>>>> I guess you are right, but duplicating dr_mode would cause more
>>>> trouble for end user.
>>>
>>>
>>>
>>> The user already needs to set up regulators, vbus-det and id-det
>>> for otg mode in the usbphy node. Although there are 2 nodes in dts /
>>> 2 separate hardware blocks involved in reality the 2 are closely
>>> related and the user already must take care to have the settings match.
>>>
>>> Besides that writing dts files is not something which end users do,
>>> so a normal user will never see this.
>>>
>>>> Felipe, could you please give your comments on this issue? I have to
>>>> do the similar change for musb_dsps.
>>>>
>>>>>
>>>>> Currently there is no link from the phy node to the controller node
>>>>> (only the other way around) and adding such a link requires more code
>>>>> then simple having a duplicate dr_mode.
>>>>
>>>>
>>>>
>>>> I guess I did not make my previous comment clearly, we don't need to
>>>> add the link from the phy node to the controller node. We don't need
>>>> to change the current dts, just in the phy driver to look up dr_mode
>>>> of the controller node, if possible.
>>>
>>>
>>>
>>> And how does the phy code now where the controller node lives without
>>> having a link to it in its node? Hardcode the full path to the
>>> controller node ? That is both ugly and error prone.
>>
>>
>> This is my first time looking at dts handling in drivers, so I might
>> be completely wrong, but I am thinking that since the controller node
>> links to the phy node, so the controller node is the parent of the phy
>> node, so if there is an of api can look it up?
>
>
> If the phy is a child of the controller, then yes this would work,
> but in the case of sunxi the phy is a built-in mmio mapped peripheral
> just like the controller, so they sit at the same level and have no
> parent child relation.

musb_dsps dts is the same.

sun8i-a33.dtsi:

    soc {
        usb_otg: {
            phys = <&usbphy 0>;
        }
        usbphy: {
        }
    }

As in the example above, usb_otg node refers to usbphy node, so I am
wondering if there is an of api to look up the usb_otg properties in
the usbphy driver.

Regards,
-Bin.

>
> Regards,
>
> Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-14 17:53                           ` Bin Liu
@ 2015-09-14 17:56                             ` Hans de Goede
  2015-09-14 19:06                               ` Bin Liu
  0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2015-09-14 17:56 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 14-09-15 19:53, Bin Liu wrote:

<snip>

>>> This is my first time looking at dts handling in drivers, so I might
>>> be completely wrong, but I am thinking that since the controller node
>>> links to the phy node, so the controller node is the parent of the phy
>>> node, so if there is an of api can look it up?
>>
>>
>> If the phy is a child of the controller, then yes this would work,
>> but in the case of sunxi the phy is a built-in mmio mapped peripheral
>> just like the controller, so they sit at the same level and have no
>> parent child relation.
>
> musb_dsps dts is the same.
>
> sun8i-a33.dtsi:
>
>      soc {
>          usb_otg: {
>              phys = <&usbphy 0>;
>          }
>          usbphy: {
>          }
>      }
>
> As in the example above, usb_otg node refers to usbphy node, so I am
> wondering if there is an of api to look up the usb_otg properties in
> the usbphy driver.

That would boil down to hardcoding the node name / path compatible
which is not acceptable from a devicetree pov.

Really having to duplicate the dr_mode is not that bad / such big
a deal.

Regards,

Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-14 17:56                             ` Hans de Goede
@ 2015-09-14 19:06                               ` Bin Liu
  0 siblings, 0 replies; 28+ messages in thread
From: Bin Liu @ 2015-09-14 19:06 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Mon, Sep 14, 2015 at 12:56 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 14-09-15 19:53, Bin Liu wrote:
>
> <snip>
>
>>>> This is my first time looking at dts handling in drivers, so I might
>>>> be completely wrong, but I am thinking that since the controller node
>>>> links to the phy node, so the controller node is the parent of the phy
>>>> node, so if there is an of api can look it up?
>>>
>>>
>>>
>>> If the phy is a child of the controller, then yes this would work,
>>> but in the case of sunxi the phy is a built-in mmio mapped peripheral
>>> just like the controller, so they sit at the same level and have no
>>> parent child relation.
>>
>>
>> musb_dsps dts is the same.
>>
>> sun8i-a33.dtsi:
>>
>>      soc {
>>          usb_otg: {
>>              phys = <&usbphy 0>;
>>          }
>>          usbphy: {
>>          }
>>      }
>>
>> As in the example above, usb_otg node refers to usbphy node, so I am
>> wondering if there is an of api to look up the usb_otg properties in
>> the usbphy driver.
>
>
> That would boil down to hardcoding the node name / path compatible
> which is not acceptable from a devicetree pov.

Okay, I got this sorted out - querying dr_mode in the phy driver:

struct device_node *p;
struct device_node *t = NULL;
const char *dr_mode_str;
enum usb_dr_mode dr_mode;

do {
    t = of_find_node_with_property(t, "phys");
    if (!t)
        goto end;

    p = of_parse_phandle(t, "phys", 0);
    if (p == pdev->dev.of_node)
        break;
while (true);

of_property_read_string(t, "dr_mode", &dr_mode_str);
dr_mode = of_usb_get_dr_mode(dr_mode_str);

end:

Regards,
-Bin.

>
> Really having to duplicate the dr_mode is not that bad / such big
> a deal.
>
> Regards,
>
> Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-10 18:38             ` Hans de Goede
  2015-09-14 14:44               ` Bin Liu
@ 2015-09-14 20:25               ` Maxime Ripard
  2015-09-15  2:54                 ` Hans de Goede
  2016-05-04 10:25                 ` Michal Suchanek
  1 sibling, 2 replies; 28+ messages in thread
From: Maxime Ripard @ 2015-09-14 20:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 10, 2015 at 08:38:38PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 10-09-15 20:30, Maxime Ripard wrote:
> >On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
> >>Hi,
> >>
> >>On 04-09-15 08:43, Olliver Schinagl wrote:
> >>>Hey Hans,
> >>>
> >>>On 07-08-15 10:45, Olliver Schinagl wrote:
> >>>><snip>
> >>>>>If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det
> >>>>>gpio settings from the usb_phy node in the dts, as the sun4i phy code detects
> >>>>>host vs otg mode by checking for the presence of these.
> >>>>Yes, this fixes it and makes it work. Thanks.
> >>>>
> >>>I've been going back to this and am wondering if this is something I can look into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore the pins and treat them as unset?
> >>
> >>AFAIK you cannot unset something in dts. The only solution I
> >>can comeup with is to add a dr_mode argument to the phy like
> >>we already have for the otg controller itself.
> >>
> >>This is something which we likely need to do anyways to add
> >>support for peripheral only mode, which we seem to need for
> >>some "hdmi sticks".
> >
> >I haven't really followed the rest of the discussion, so sorry if you
> >already talked about that, but why can't you just set the dr_mode to
> >peripheral in such a case?
> 
> This is about the usbphy code not the musb-controller code, which are
> 2 different dts nodes, atm only the musb-controller node has a
> dr_mode property, and the phy code decides between host-only
> and otg mode based on whether an id pin is assigned or not.
> 
> My proposal is to get rid of the id-pin hack to determine the mode
> and add a dr_mode property to the usbphy dts node.

I agree that we should get rid of that hack, especially since a lack
of an ID pin might also be used on a peripheral-only device.

However, we already have that information in the musb node, and
duplicating the info seems error prone. We already have a custom
function, maybe that's a case for another one, and that would allow to
handle "hard" cases more easily (like CONFIG_USB_MUSB_HOST selected,
with the otg node set to otg).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150914/58e2107d/attachment.sig>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-14 20:25               ` Maxime Ripard
@ 2015-09-15  2:54                 ` Hans de Goede
  2015-09-15  4:20                   ` Bin Liu
  2016-05-04 10:25                 ` Michal Suchanek
  1 sibling, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2015-09-15  2:54 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 09/14/2015 10:25 PM, Maxime Ripard wrote:
> On Thu, Sep 10, 2015 at 08:38:38PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 10-09-15 20:30, Maxime Ripard wrote:
>>> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 04-09-15 08:43, Olliver Schinagl wrote:
>>>>> Hey Hans,
>>>>>
>>>>> On 07-08-15 10:45, Olliver Schinagl wrote:
>>>>>> <snip>
>>>>>>> If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det
>>>>>>> gpio settings from the usb_phy node in the dts, as the sun4i phy code detects
>>>>>>> host vs otg mode by checking for the presence of these.
>>>>>> Yes, this fixes it and makes it work. Thanks.
>>>>>>
>>>>> I've been going back to this and am wondering if this is something I can look into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore the pins and treat them as unset?
>>>>
>>>> AFAIK you cannot unset something in dts. The only solution I
>>>> can comeup with is to add a dr_mode argument to the phy like
>>>> we already have for the otg controller itself.
>>>>
>>>> This is something which we likely need to do anyways to add
>>>> support for peripheral only mode, which we seem to need for
>>>> some "hdmi sticks".
>>>
>>> I haven't really followed the rest of the discussion, so sorry if you
>>> already talked about that, but why can't you just set the dr_mode to
>>> peripheral in such a case?
>>
>> This is about the usbphy code not the musb-controller code, which are
>> 2 different dts nodes, atm only the musb-controller node has a
>> dr_mode property, and the phy code decides between host-only
>> and otg mode based on whether an id pin is assigned or not.
>>
>> My proposal is to get rid of the id-pin hack to determine the mode
>> and add a dr_mode property to the usbphy dts node.
>
> I agree that we should get rid of that hack, especially since a lack
> of an ID pin might also be used on a peripheral-only device.
>
> However, we already have that information in the musb node, and
> duplicating the info seems error prone. We already have a custom
> function, maybe that's a case for another one

AFAIK the musb / phy maintainers really want to avoid adding more
custom functions ...

Felipe, Kishon any comments on this ?

Regards,

Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-15  2:54                 ` Hans de Goede
@ 2015-09-15  4:20                   ` Bin Liu
  0 siblings, 0 replies; 28+ messages in thread
From: Bin Liu @ 2015-09-15  4:20 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Mon, Sep 14, 2015 at 9:54 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 09/14/2015 10:25 PM, Maxime Ripard wrote:
>>
>> On Thu, Sep 10, 2015 at 08:38:38PM +0200, Hans de Goede wrote:
>>>
>>> Hi,
>>>
>>> On 10-09-15 20:30, Maxime Ripard wrote:
>>>>
>>>> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 04-09-15 08:43, Olliver Schinagl wrote:
>>>>>>
>>>>>> Hey Hans,
>>>>>>
>>>>>> On 07-08-15 10:45, Olliver Schinagl wrote:
>>>>>>>
>>>>>>> <snip>
>>>>>>>>
>>>>>>>> If you change the dr_mode to host then you _must_ also remove any
>>>>>>>> id_det and vbus_det
>>>>>>>> gpio settings from the usb_phy node in the dts, as the sun4i phy
>>>>>>>> code detects
>>>>>>>> host vs otg mode by checking for the presence of these.
>>>>>>>
>>>>>>> Yes, this fixes it and makes it work. Thanks.
>>>>>>>
>>>>>> I've been going back to this and am wondering if this is something I
>>>>>> can look into to fix properly? E.g. if the dts sets dr_mode = host, can we
>>>>>> simply ignore the pins and treat them as unset?
>>>>>
>>>>>
>>>>> AFAIK you cannot unset something in dts. The only solution I
>>>>> can comeup with is to add a dr_mode argument to the phy like
>>>>> we already have for the otg controller itself.
>>>>>
>>>>> This is something which we likely need to do anyways to add
>>>>> support for peripheral only mode, which we seem to need for
>>>>> some "hdmi sticks".
>>>>
>>>>
>>>> I haven't really followed the rest of the discussion, so sorry if you
>>>> already talked about that, but why can't you just set the dr_mode to
>>>> peripheral in such a case?
>>>
>>>
>>> This is about the usbphy code not the musb-controller code, which are
>>> 2 different dts nodes, atm only the musb-controller node has a
>>> dr_mode property, and the phy code decides between host-only
>>> and otg mode based on whether an id pin is assigned or not.
>>>
>>> My proposal is to get rid of the id-pin hack to determine the mode
>>> and add a dr_mode property to the usbphy dts node.
>>
>>
>> I agree that we should get rid of that hack, especially since a lack
>> of an ID pin might also be used on a peripheral-only device.
>>
>> However, we already have that information in the musb node, and
>> duplicating the info seems error prone. We already have a custom
>> function, maybe that's a case for another one
>
>
> AFAIK the musb / phy maintainers really want to avoid adding more
> custom functions ...

I add a function called of_usb_get_dr_mode_by_phy() in usb/common.c,
so that other phy drivers can call it to get dr_mode of the
controller. I will send the patch tomorrow morning for review.

Regards,
-Bin.

>
> Felipe, Kishon any comments on this ?
>
>
> Regards,
>
> Hans
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-10 18:23         ` Hans de Goede
  2015-09-10 18:30           ` Maxime Ripard
@ 2015-09-26 12:50           ` Olliver Schinagl
       [not found]             ` <jwvmvw7zzg5.fsf-monnier+gmane.comp.hardware.netbook.arm.sunxi@gnu.org>
  1 sibling, 1 reply; 28+ messages in thread
From: Olliver Schinagl @ 2015-09-26 12:50 UTC (permalink / raw)
  To: linux-arm-kernel

Hey Hans,

On 10-09-15 20:23, Hans de Goede wrote:
> Hi,
>
> On 04-09-15 08:43, Olliver Schinagl wrote:
>> Hey Hans,
>>
>> On 07-08-15 10:45, Olliver Schinagl wrote:
>>> <snip>
>>>> If you change the dr_mode to host then you _must_ also remove any 
>>>> id_det and vbus_det
>>>> gpio settings from the usb_phy node in the dts, as the sun4i phy 
>>>> code detects
>>>> host vs otg mode by checking for the presence of these.
>>> Yes, this fixes it and makes it work. Thanks.
>>>
>> I've been going back to this and am wondering if this is something I 
>> can look into to fix properly? E.g. if the dts sets dr_mode = host, 
>> can we simply ignore the pins and treat them as unset?
>
> AFAIK you cannot unset something in dts. The only solution I
> can comeup with is to add a dr_mode argument to the phy like
> we already have for the otg controller itself.
Actually, it seems that you can :)


&usbphy {
     /* Unset otg detect pins as we force dr_mode */
     /delete-property/ usb0_id_det-gpio;
     /delete-property/ usb0_vbus_det-gpio;
};

is what i am using at the moment.
>
> This is something which we likely need to do anyways to add
> support for peripheral only mode, which we seem to need for
> some "hdmi sticks".
>
> Patches doing this are welcome from my pov.
While my plate is uite fullish too, i may look into it :)
>
> Regards,
>
> Hans
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] Re: [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
       [not found]             ` <jwvmvw7zzg5.fsf-monnier+gmane.comp.hardware.netbook.arm.sunxi@gnu.org>
@ 2015-09-28  7:04               ` Hans de Goede
  0 siblings, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2015-09-28  7:04 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 27-09-15 16:58, Stefan Monnier wrote:
>> &usbphy {
>>      /* Unset otg detect pins as we force dr_mode */
>>      /delete-property/ usb0_id_det-gpio;
>>      /delete-property/ usb0_vbus_det-gpio;
>> };
>
> Side question [ wearing my "contributor to Emacs's dts-mode" hat ]:
>
> How should this piece of syntax be parsed?  Is
>
>        /<something>/ blabla;
>
> a general syntax accepted in DTS files, with various possible
> <something> in there?  Could someone point me to some documentation
> explaining/describing this syntax?

It is probably best to start a new thread about this on <devicetree@vger.kernel.org>,
that way people who may actually know the answer will see the mail :)

Regards,

Hans

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
  2015-09-14 20:25               ` Maxime Ripard
  2015-09-15  2:54                 ` Hans de Goede
@ 2016-05-04 10:25                 ` Michal Suchanek
       [not found]                   ` <706aa130-3ace-4059-a7c9-44147cd56c28@googlegroups.com>
  1 sibling, 1 reply; 28+ messages in thread
From: Michal Suchanek @ 2016-05-04 10:25 UTC (permalink / raw)
  To: linux-arm-kernel

On 14 September 2015 at 22:25, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Thu, Sep 10, 2015 at 08:38:38PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 10-09-15 20:30, Maxime Ripard wrote:
>> >On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
>> >>Hi,
>> >>
>> >>On 04-09-15 08:43, Olliver Schinagl wrote:
>> >>>Hey Hans,
>> >>>
>> >>>On 07-08-15 10:45, Olliver Schinagl wrote:
>> >>>><snip>
>> >>>>>If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det
>> >>>>>gpio settings from the usb_phy node in the dts, as the sun4i phy code detects
>> >>>>>host vs otg mode by checking for the presence of these.
>> >>>>Yes, this fixes it and makes it work. Thanks.
>> >>>>
>> >>>I've been going back to this and am wondering if this is something I can look into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore the pins and treat them as unset?
>> >>
>> >>AFAIK you cannot unset something in dts. The only solution I
>> >>can comeup with is to add a dr_mode argument to the phy like
>> >>we already have for the otg controller itself.
>> >>
>> >>This is something which we likely need to do anyways to add
>> >>support for peripheral only mode, which we seem to need for
>> >>some "hdmi sticks".
>> >
>> >I haven't really followed the rest of the discussion, so sorry if you
>> >already talked about that, but why can't you just set the dr_mode to
>> >peripheral in such a case?
>>
>> This is about the usbphy code not the musb-controller code, which are
>> 2 different dts nodes, atm only the musb-controller node has a
>> dr_mode property, and the phy code decides between host-only
>> and otg mode based on whether an id pin is assigned or not.
>>
>> My proposal is to get rid of the id-pin hack to determine the mode
>> and add a dr_mode property to the usbphy dts node.
>
> I agree that we should get rid of that hack, especially since a lack
> of an ID pin might also be used on a peripheral-only device.
>
> However, we already have that information in the musb node, and
> duplicating the info seems error prone. We already have a custom
> function, maybe that's a case for another one, and that would allow to
> handle "hard" cases more easily (like CONFIG_USB_MUSB_HOST selected,
> with the otg node set to otg).
>

Hello,

was this solved somehow?

What problem is there with referencing the phy node?

Just like pinmux setting nodes and whatnot it can be named and
referenced by name.

Thanks

Michal

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
       [not found]                       ` <0106707e-21f3-4aec-92ca-8b951af8a34e@googlegroups.com>
@ 2017-05-12  9:16                         ` Maxime Ripard
  0 siblings, 0 replies; 28+ messages in thread
From: Maxime Ripard @ 2017-05-12  9:16 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Thu, May 11, 2017 at 02:45:11PM -0700, Bim Overbohm wrote:
> Hi All,
> 
> I had a similar problem running a mainline 4.11 kernel on an A33 tablet 
> (marked "Q8-2.4G 20140918". It came from a company called Trimeo and is 
> simply called 7". It contains an APX223 power controller, RTL8189ETV WiFi 
> controller, 2x Nanya NT5CB128M15FP-DY DRAM chips (2x256MB), a Hynix 
> H27UCG8T2BTR-BC NAND-flash chip and a Goodix GT818 touch controller. 
> Display resolution is 1024x600). The self-compiled u-boot dtb would 
> initialize the USB, but the kernel dtb would not work.

U-Boot doesn't use the DT for this USB setup.

> The kernel messages were something along the lines of "[ 9.366994]
> sun4i-usb-phy 1c19400.phy: could not find pctldev for node
> /soc at 01c00000/pinctrl at 01c20800/usb0_id_detect_pin at 0, deferring
> probe".

That's not likely to be your issue. The probe should be deferred, and
reattempted later.

> I found no way to activate the pinctrl devices, so I fixed this by
> decompiling the dts, removing all the USB id detect pin and vbus
> settings and recompiling the dts (diff at end of message). The
> device boots into LXDE now and I can use an USB keyboard and
> mouse. I still have no WiFi though...
> 
> Pastebin of unpatched dts file: https://pastebin.com/kL6WELuC
> 
> A couple of questions:
>
> - Is there a proper way to make the pinctrls and OTG dual mode work
>   on the device?! I suspect they are needed to activate WiFi too...

As I said, that's probably not your issue. Are you sure of your
micro-USB cable? Is it setting the ID pin to the ground as it's
supposed to?

> - Is this a bug I should report for sunxi-next?

sunxi-next is just a merge of various upstream branches. It doesn't
make sense to report bugs there. You've found the right place already.

> - Is there a driver for the GT818 in the mainline kernel or a way to
>   make that device work?

It doesn't look like there is.

> - Is there a driver for the RTL8189ETV in the mainline kernel or a way to 
> make that device work?

https://linux-sunxi.org/Wifi#RTL8189ES_.2F_RTL8189ETV

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170512/774c113d/attachment.sig>

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2017-05-12  9:16 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-04 21:25 [PATCH] musb: sunxi: Ignore VBus errors in host-only mode Hans de Goede
2015-08-04 21:35 ` Felipe Balbi
2015-08-04 22:05   ` Hans de Goede
2015-08-04 22:57     ` Felipe Balbi
2015-08-05 13:30       ` Hans de Goede
2015-08-06  8:22 ` [linux-sunxi] " Olliver Schinagl
2015-08-06 14:35   ` Hans de Goede
2015-08-07  8:45     ` Olliver Schinagl
2015-09-04  6:43       ` Olliver Schinagl
2015-09-10 18:23         ` Hans de Goede
2015-09-10 18:30           ` Maxime Ripard
2015-09-10 18:38             ` Hans de Goede
2015-09-14 14:44               ` Bin Liu
2015-09-14 14:59                 ` Hans de Goede
2015-09-14 16:58                   ` Bin Liu
2015-09-14 17:08                     ` Hans de Goede
2015-09-14 17:14                       ` Bin Liu
2015-09-14 17:25                         ` Hans de Goede
2015-09-14 17:53                           ` Bin Liu
2015-09-14 17:56                             ` Hans de Goede
2015-09-14 19:06                               ` Bin Liu
2015-09-14 20:25               ` Maxime Ripard
2015-09-15  2:54                 ` Hans de Goede
2015-09-15  4:20                   ` Bin Liu
2016-05-04 10:25                 ` Michal Suchanek
     [not found]                   ` <706aa130-3ace-4059-a7c9-44147cd56c28@googlegroups.com>
     [not found]                     ` <af013a0a-9257-4936-9e66-cb38f2e46369@googlegroups.com>
     [not found]                       ` <0106707e-21f3-4aec-92ca-8b951af8a34e@googlegroups.com>
2017-05-12  9:16                         ` Maxime Ripard
2015-09-26 12:50           ` Olliver Schinagl
     [not found]             ` <jwvmvw7zzg5.fsf-monnier+gmane.comp.hardware.netbook.arm.sunxi@gnu.org>
2015-09-28  7:04               ` [linux-sunxi] " Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.