musb: sunxi: Ignore VBus errors in host-only mode

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 <[email protected]>

---
 drivers/usb/musb/sunxi.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/usb/musb/sunxi.c +0 -0

index f9f6304..34ce5df 100644

Quoted 16 lines · Hans de Goede

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 <[email protected]>

---
  drivers/usb/musb/sunxi.c | 4 ++++
  1 file changed, 4 insertions(+)

drivers/usb/musb/sunxi.c +0 -0

index f9f6304..34ce5df 100644
@@ -194,6 +194,10 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
  		musb_writeb(musb->mregs, MUSB_FADDR, 0);
  	}
 

drivers/usb/musb/sunxi.c static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci) · line 199

Quoted 3 lines

drivers/usb/musb/sunxi.c +3 -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.

Hi,

Quoted 17 lines · Felipe Balbi

Quoted 16 lines · Hans de Goede

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 <[email protected]>

---
 drivers/usb/musb/sunxi.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/usb/musb/sunxi.c +0 -0

index f9f6304..34ce5df 100644
@@ -194,6 +194,10 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
 		musb_writeb(musb->mregs, MUSB_FADDR, 0);
 	}
 

Quoted 8 lines

drivers/usb/musb/sunxi.c static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci) · line 199

Quoted 3 lines

drivers/usb/musb/sunxi.c +3 -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.

Hi,

Quoted 29 lines · Hans de Goede

Hi,

Quoted 17 lines · Felipe Balbi

Quoted 16 lines · Hans de Goede

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 <[email protected]>

---
  drivers/usb/musb/sunxi.c | 4 ++++
  1 file changed, 4 insertions(+)

drivers/usb/musb/sunxi.c +0 -0

index f9f6304..34ce5df 100644
@@ -194,6 +194,10 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
  		musb_writeb(musb->mregs, MUSB_FADDR, 0);
  	}
 

Quoted 8 lines

drivers/usb/musb/sunxi.c static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci) · line 199

Quoted 3 lines

drivers/usb/musb/sunxi.c +3 -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.

Quoted 6 lines

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.

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 <[email protected]>

---
  drivers/usb/musb/sunxi.c | 4 ++++
  1 file changed, 4 insertions(+)

drivers/usb/musb/sunxi.c +4 -0

index f9f6304..34ce5df 100644
@@ -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);

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?

Hi,

Quoted 12 lines · Olliver Schinagl

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]

Quoted 2 lines

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.

Quoted 2 lines

and b) I got an error about an known dr_mode before and this was the

quick and easy way.

Quoted 22 lines

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

Quoted 2 lines

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

<snip>

Quoted 5 lines

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.

Hey Hans,

Quoted 6 lines · Olliver Schinagl

<snip>

Quoted 3 lines

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?

Hi,

Quoted 10 lines · Olliver Schinagl

Hey Hans,

Quoted 6 lines · Olliver Schinagl

<snip>

Quoted 3 lines

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".

Quoted 18 lines · Hans de Goede

Hi,

Quoted 10 lines · Olliver Schinagl

Hey Hans,

Quoted 6 lines · Olliver Schinagl

<snip>

Quoted 3 lines

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.

Quoted 3 lines

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?

Hi,

Quoted 30 lines · Maxime Ripard

Quoted 28 lines · Hans de Goede

Hi,

Quoted 13 lines · Olliver Schinagl

Hey Hans,

Quoted 9 lines · Olliver Schinagl

<snip>

Quoted 6 lines

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 8 lines

Quoted 3 lines

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
--
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

Hi,

Quoted 43 lines · Hans de Goede

Hi,

Quoted 30 lines · Maxime Ripard

Quoted 28 lines · Hans de Goede

Hi,

Quoted 13 lines · Olliver Schinagl

Hey Hans,

Quoted 9 lines · Olliver Schinagl

<snip>

Quoted 6 lines

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 8 lines

Quoted 3 lines

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?

Quoted 7 lines

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?

Hi,

Quoted 72 lines · Bin Liu

Hi,

Quoted 55 lines · Hans de Goede

Hi,

Quoted 45 lines · Maxime Ripard

Quoted 32 lines · Hans de Goede

Hi,

Quoted 18 lines · Olliver Schinagl

Hey Hans,

Quoted 13 lines · Olliver Schinagl

<snip>

Quoted 8 lines

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.

Quoted 6 lines

Quoted 2 lines

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?

Quoted 7 lines

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".

Quoted 3 lines

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?

Quoted 7 lines

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.

Quoted 3 lines

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

Hi,

Quoted 81 lines · Hans de Goede

Hi,

Quoted 75 lines · Bin Liu

Hi,

Quoted 55 lines · Hans de Goede

Hi,

Quoted 45 lines · Maxime Ripard

Quoted 32 lines · Hans de Goede

Hi,

Quoted 18 lines · Olliver Schinagl

Hey Hans,

Quoted 13 lines · Olliver Schinagl

<snip>

Quoted 8 lines

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.

Quoted 6 lines

Quoted 2 lines

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?

Quoted 7 lines

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".

Quoted 3 lines

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?

Quoted 7 lines

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?

Quoted 3 lines

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.

Quoted 1 line

Quoted 3 lines

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.

Hi,

Quoted 101 lines · Bin Liu

Hi,

Quoted 96 lines · Hans de Goede

Hi,

Quoted 88 lines · Bin Liu

Hi,

Quoted 66 lines · Hans de Goede

Hi,

Quoted 54 lines · Maxime Ripard

Quoted 39 lines · Hans de Goede

Hi,

Quoted 22 lines · Olliver Schinagl

Hey Hans,

Quoted 16 lines · Olliver Schinagl

<snip>

Quoted 9 lines

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.

Quoted 7 lines

Quoted 2 lines

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?

Quoted 7 lines

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".

Quoted 3 lines

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?

Quoted 7 lines

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?

Quoted 7 lines

Quoted 3 lines

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.

Quoted 9 lines

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

Quoted 4 lines

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.

Quoted 4 lines

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

Hi,

Quoted 137 lines · Hans de Goede

Hi,

Quoted 101 lines · Bin Liu

Hi,

Quoted 96 lines · Hans de Goede

Hi,

Quoted 88 lines · Bin Liu

Hi,

Quoted 66 lines · Hans de Goede

Hi,

Quoted 54 lines · Maxime Ripard

Quoted 39 lines · Hans de Goede

Hi,

Quoted 22 lines · Olliver Schinagl

Hey Hans,

Quoted 16 lines · Olliver Schinagl

<snip>

Quoted 9 lines

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.

Quoted 7 lines

Quoted 2 lines

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?

Quoted 7 lines

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".

Quoted 3 lines

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?

Quoted 7 lines

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?

Quoted 7 lines

Quoted 3 lines

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.

Quoted 13 lines

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

Quoted 4 lines

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.

Quoted 3 lines

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?

Hi,

Quoted 171 lines · Bin Liu

Hi,

Quoted 161 lines · Hans de Goede

Hi,

Quoted 120 lines · Bin Liu

Hi,

Quoted 114 lines · Hans de Goede

Hi,

Quoted 104 lines · Bin Liu

Hi,

Quoted 80 lines · Hans de Goede

Hi,

Quoted 65 lines · Maxime Ripard

Quoted 48 lines · Hans de Goede

Hi,

Quoted 29 lines · Olliver Schinagl

Hey Hans,

Quoted 20 lines · Olliver Schinagl

<snip>

Quoted 11 lines

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.

Quoted 2 lines

Yes, this fixes it and makes it work. Thanks.

Quoted 7 lines

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?

Quoted 7 lines

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".

Quoted 4 lines

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?

Quoted 7 lines

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?

Quoted 8 lines

Quoted 3 lines

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.

Quoted 10 lines

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

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 4 lines

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

Quoted 10 lines

Quoted 4 lines

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.

Hi,

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

<snip>

Quoted 25 lines

Quoted 11 lines

Quoted 4 lines

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: {
         }
     }

Quoted 3 lines

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

Hi,

Quoted 19 lines · Maxime Ripard

Quoted 18 lines · Hans de Goede

Hi,

Quoted 10 lines · Olliver Schinagl

Hey Hans,

Quoted 6 lines · Olliver Schinagl

<snip>

Quoted 3 lines

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.

Quoted 7 lines

Quoted 3 lines

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.

Quoted 30 lines · Hans de Goede

Hi,

Quoted 19 lines · Maxime Ripard

Quoted 18 lines · Hans de Goede

Hi,

Quoted 10 lines · Olliver Schinagl

Hey Hans,

Quoted 6 lines · Olliver Schinagl

<snip>

Quoted 3 lines

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.

Quoted 7 lines

Quoted 3 lines

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?

Quoted 7 lines

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

Hi,

Quoted 58 lines · Maxime Ripard

Quoted 44 lines · Hans de Goede

Hi,

Quoted 30 lines · Maxime Ripard

Quoted 28 lines · Hans de Goede

Hi,

Quoted 13 lines · Olliver Schinagl

Hey Hans,

Quoted 9 lines · Olliver Schinagl

<snip>

Quoted 6 lines

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 8 lines

Quoted 3 lines

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?

Quoted 7 lines

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.

Quoted 3 lines

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
--
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

Quoted 30 lines · Hans de Goede

Hi,

Quoted 19 lines · Maxime Ripard

Quoted 18 lines · Hans de Goede

Hi,

Quoted 10 lines · Olliver Schinagl

Hey Hans,

Quoted 6 lines · Olliver Schinagl

<snip>

Quoted 3 lines

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.

Quoted 7 lines

Quoted 3 lines

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?

Quoted 7 lines

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).

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.

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".

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...

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

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

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

Hi,

Quoted 11 lines · Olliver Schinagl

Hey Hans,

Quoted 8 lines · Olliver Schinagl

<snip>

Quoted 5 lines

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.

Quoted 3 lines

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

Quoted 5 lines

&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?