Discussion:
[PATCH] musb: sunxi: Ignore VBus errors in host-only mode
Hans de Goede
2015-08-04 21:25:53 UTC
Permalink
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 <***@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
Felipe Balbi
2015-08-04 21:35:53 UTC
Permalink
Post by 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.
---
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
Hans de Goede
2015-08-04 22:05:02 UTC
Permalink
Hi,
Post by Felipe Balbi
Post by 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.
---
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
Felipe Balbi
2015-08-04 22:57:22 UTC
Permalink
Hi,
Post by Hans de Goede
Hi,
Post by Felipe Balbi
Post by 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.
---
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
Hans de Goede
2015-08-05 13:30:36 UTC
Permalink
Hi,
Post by Felipe Balbi
Hi,
Post by Hans de Goede
Hi,
Post by Felipe Balbi
Post by 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.
---
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
Olliver Schinagl
2015-08-06 08:22:59 UTC
Permalink
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
Post by 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.
---
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);
Hans de Goede
2015-08-06 14:35:08 UTC
Permalink
Hi,
Post by 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]
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.
Post by Olliver Schinagl
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
Olliver Schinagl
2015-08-07 08:45:53 UTC
Permalink
Hey Hans,
Post by Hans de Goede
Hi,
Post by 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]
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
Post by Hans de Goede
Post by Olliver Schinagl
and b) I got an error about an known dr_mode before and this was the
quick and easy way.
Post by Olliver Schinagl
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.
Post by Hans de Goede
Regards,
Hans
Olliver Schinagl
2015-09-04 06:43:24 UTC
Permalink
Hey Hans,
<snip>
Post by Hans de Goede
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
Hans de Goede
2015-09-10 18:23:23 UTC
Permalink
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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
Maxime Ripard
2015-09-10 18:30:17 UTC
Permalink
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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
Hans de Goede
2015-09-10 18:38:38 UTC
Permalink
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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
Bin Liu
2015-09-14 14:44:41 UTC
Permalink
Hi,
Post by Hans de Goede
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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.
Post by Hans de Goede
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hans de Goede
2015-09-14 14:59:22 UTC
Permalink
Hi,
Post by Bin Liu
Hi,
Post by Hans de Goede
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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
Bin Liu
2015-09-14 16:58:42 UTC
Permalink
Hi,
Post by Hans de Goede
Hi,
Post by Bin Liu
Hi,
Post by Hans de Goede
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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.
Post by Hans de Goede
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.
Post by Hans de Goede
Regards,
Hans
Hans de Goede
2015-09-14 17:08:56 UTC
Permalink
Hi,
Post by Bin Liu
Hi,
Post by Hans de Goede
Hi,
Post by Bin Liu
Hi,
Post by Hans de Goede
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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.
Post by Bin Liu
Felipe, could you please give your comments on this issue? I have to
do the similar change for musb_dsps.
Post by Hans de Goede
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
Bin Liu
2015-09-14 17:14:00 UTC
Permalink
Hi,
Post by Hans de Goede
Hi,
Post by Bin Liu
Hi,
Post by Hans de Goede
Hi,
Post by Bin Liu
Hi,
Post by Hans de Goede
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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.
Post by Bin Liu
Felipe, could you please give your comments on this issue? I have to
do the similar change for musb_dsps.
Post by Hans de Goede
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.
Post by Hans de Goede
Regards,
Hans
Hans de Goede
2015-09-14 17:25:02 UTC
Permalink
Hi,
Post by Bin Liu
Hi,
Post by Hans de Goede
Hi,
Post by Bin Liu
Hi,
Post by Hans de Goede
Hi,
Post by Bin Liu
Hi,
Post by Hans de Goede
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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.
Post by Bin Liu
Felipe, could you please give your comments on this issue? I have to
do the similar change for musb_dsps.
Post by Hans de Goede
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
Bin Liu
2015-09-14 17:53:30 UTC
Permalink
Hi,
Post by Hans de Goede
Hi,
Post by Bin Liu
Hi,
Post by Hans de Goede
Hi,
Post by Bin Liu
Hi,
Post by Hans de Goede
Hi,
Post by Bin Liu
Hi,
Post by Hans de Goede
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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.
Post by Bin Liu
Felipe, could you please give your comments on this issue? I have to
do the similar change for musb_dsps.
Post by Hans de Goede
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.
Post by Hans de Goede
Regards,
Hans
Hans de Goede
2015-09-14 17:56:55 UTC
Permalink
Hi,

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

<snip>
Post by Bin Liu
Post by Hans de Goede
Post by Bin Liu
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.
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
Bin Liu
2015-09-14 19:06:41 UTC
Permalink
Hi,
Hi,
<snip>
Post by Bin Liu
Post by Hans de Goede
Post by Bin Liu
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.
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
Maxime Ripard
2015-09-14 20:25:33 UTC
Permalink
Post by Hans de Goede
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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
Hans de Goede
2015-09-15 02:54:00 UTC
Permalink
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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
Bin Liu
2015-09-15 04:20:22 UTC
Permalink
Hi,
Post by Hans de Goede
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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.
Post by Hans de Goede
Felipe, Kishon any comments on this ?
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Suchanek
2016-05-04 10:25:33 UTC
Permalink
On 14 September 2015 at 22:25, Maxime Ripard
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Maxime Ripard
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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
Maxime Ripard
2017-05-12 09:16:41 UTC
Permalink
Hi,
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
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
- 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
Olliver Schinagl
2015-09-26 12:50:18 UTC
Permalink
Hey Hans,
Post by Hans de Goede
Hi,
Post by Olliver Schinagl
Hey Hans,
<snip>
Post by Hans de Goede
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.
Post by Hans de Goede
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 :)
Post by Hans de Goede
Regards,
Hans
Hans de Goede
2015-09-28 07:04:28 UTC
Permalink
Hi,
Post by Olliver Schinagl
&usbphy {
/* Unset otg detect pins as we force dr_mode */
/delete-property/ usb0_id_det-gpio;
/delete-property/ usb0_vbus_det-gpio;
};
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 <***@vger.kernel.org>,
that way people who may actually know the answer will see the mail :)

Regards,

Hans
Loading...