| Message ID | 528F7E8F.5050807@newflow.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/22/2013 04:55 PM, Mark Jackson wrote: > The IDDIG input pin is normally used to determine the USB mode > (i.e. HOST or DEVICE). > > On some systems (e.g. AM335x) leaving this pin floating allows > the USB mode to be set via software. So you have a board where musb is used only as host or only as device and the ID pin not on ground or 3.3V? What are the side effects? I remember correctly Bin wanted to avoid settings this if it could be avoided. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hallo, On Fri, Nov 22, 2013 at 03:55:59PM +0000, Mark Jackson wrote: > The IDDIG input pin is normally used to determine the USB mode > (i.e. HOST or DEVICE). > > On some systems (e.g. AM335x) leaving this pin floating allows > the USB mode to be set via software. > > This patch adds support for this via the device tree. > > Signed-off-by: Mark Jackson <mpfj@newflow.co.uk> > --- > .../devicetree/bindings/usb/am33xx-usb.txt | 2 ++ > drivers/usb/musb/musb_dsps.c | 14 ++++++++++++++ > include/linux/usb/musb.h | 1 + > 3 files changed, 17 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/am33xx-usb.txt b/Documentation/devicetree/bindings/usb/am33xx-usb.txt > index 20c2ff2..560b7ff 100644 > --- a/Documentation/devicetree/bindings/usb/am33xx-usb.txt > +++ b/Documentation/devicetree/bindings/usb/am33xx-usb.txt > @@ -47,6 +47,8 @@ USB > - dmas: specifies the dma channels > - dma-names: specifies the names of the channels. Use "rxN" for receive > and "txN" for transmit endpoints. N specifies the endpoint number. > +- ti,force-host: specifies that the IDDIG input be ignored and the device be > + put into host mode regardless. You should always CC devicetree-discuss if adding new bindings. Why another binding anyway? We have the common binding dr_mode already. Please use this and of_usb_get_dr_mode from drivers/usb/usb-common.c instead. Thanks, Michael
On 22/11/13 16:38, Michael Grzeschik wrote: > Hallo, > > On Fri, Nov 22, 2013 at 03:55:59PM +0000, Mark Jackson wrote: >> The IDDIG input pin is normally used to determine the USB mode >> (i.e. HOST or DEVICE). >> >> On some systems (e.g. AM335x) leaving this pin floating allows >> the USB mode to be set via software. >> >> This patch adds support for this via the device tree. >> >> Signed-off-by: Mark Jackson <mpfj@newflow.co.uk> >> --- >> .../devicetree/bindings/usb/am33xx-usb.txt | 2 ++ >> drivers/usb/musb/musb_dsps.c | 14 ++++++++++++++ >> include/linux/usb/musb.h | 1 + >> 3 files changed, 17 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/usb/am33xx-usb.txt b/Documentation/devicetree/bindings/usb/am33xx-usb.txt >> index 20c2ff2..560b7ff 100644 >> --- a/Documentation/devicetree/bindings/usb/am33xx-usb.txt >> +++ b/Documentation/devicetree/bindings/usb/am33xx-usb.txt >> @@ -47,6 +47,8 @@ USB >> - dmas: specifies the dma channels >> - dma-names: specifies the names of the channels. Use "rxN" for receive >> and "txN" for transmit endpoints. N specifies the endpoint number. >> +- ti,force-host: specifies that the IDDIG input be ignored and the device be >> + put into host mode regardless. > > You should always CC devicetree-discuss if adding new bindings. Why > another binding anyway? We have the common binding dr_mode already. > Please use this and of_usb_get_dr_mode from drivers/usb/usb-common.c > instead. Sure ... that's a nicer solution. I'll do that for v2 Mark J. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 22/11/13 16:33, Sebastian Andrzej Siewior wrote: > On 11/22/2013 04:55 PM, Mark Jackson wrote: >> The IDDIG input pin is normally used to determine the USB mode >> (i.e. HOST or DEVICE). >> >> On some systems (e.g. AM335x) leaving this pin floating allows >> the USB mode to be set via software. > > So you have a board where musb is used only as host or only as device > and the ID pin not on ground or 3.3V? > What are the side effects? I remember correctly Bin wanted to avoid > settings this if it could be avoided. Yes ... we have a host only USB port and an unconnected ID pin. AFAIK it defaults to device mode so I can't see any devices that get plugged into the USB port. If I tweak the s/w to "force" host mode on, then everything appears to work okay. I guess it's more of a hardware oversight that we left the pin floating but in the real world, I guess someone may want this feature to they can change the usb port type ? Either way, I need to fix the current h/w (which can be done via s/w) hence the patch. Mark J. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/22/2013 05:49 PM, Mark Jackson wrote: >> and the ID pin not on ground or 3.3V? >> What are the side effects? I remember correctly Bin wanted to avoid >> settings this if it could be avoided. > > Yes ... we have a host only USB port and an unconnected ID pin. Is it too late to connect that pin? > AFAIK it defaults to device mode so I can't see any devices that get > plugged into the USB port. > > If I tweak the s/w to "force" host mode on, then everything appears to > work okay. > > I guess it's more of a hardware oversight that we left the pin floating > but in the real world, I guess someone may want this feature to they > can change the usb port type ? This is something I would prefer to avoid if possible. We have the dr_mode attribute in DT. Based on that one we act as a device or host. Now if you decide (via dr_mode) either for host or device that means we have to set that bit. Where I feel a little uncomfortable is when someone having OTG runs in hostmode and attaches a host. > > Either way, I need to fix the current h/w (which can be done via s/w) > hence the patch. I've seen many projects where this pin has been forgotten and it could not be changed in SW and they patched the HW. Usually this is noticed in the early phase and a wire is just soldered and the redesign has it fixed. So I don't think that this is a big issue. Do you insists on having this change merged upstream? > > Mark J. > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 22/11/13 17:01, Sebastian Andrzej Siewior wrote: > On 11/22/2013 05:49 PM, Mark Jackson wrote: >>> and the ID pin not on ground or 3.3V? >>> What are the side effects? I remember correctly Bin wanted to avoid >>> settings this if it could be avoided. >> >> Yes ... we have a host only USB port and an unconnected ID pin. > > Is it too late to connect that pin? Unfortunately, yes. The USB portion of the CPU board has not been needed until now, and although I did know about the unconnected pin, I also knew it could be fixed in s/w, so I wasn't too concerned. >> AFAIK it defaults to device mode so I can't see any devices that get >> plugged into the USB port. >> >> If I tweak the s/w to "force" host mode on, then everything appears to >> work okay. >> >> I guess it's more of a hardware oversight that we left the pin floating >> but in the real world, I guess someone may want this feature to they >> can change the usb port type ? > > This is something I would prefer to avoid if possible. We have the > dr_mode attribute in DT. Based on that one we act as a device or host. > Now if you decide (via dr_mode) either for host or device that means we > have to set that bit. Where I feel a little uncomfortable is when > someone having OTG runs in hostmode and attaches a host. > >> >> Either way, I need to fix the current h/w (which can be done via s/w) >> hence the patch. > I've seen many projects where this pin has been forgotten and it could > not be changed in SW and they patched the HW. Usually this is noticed > in the early phase and a wire is just soldered and the redesign has it > fixed. So I don't think that this is a big issue. > Do you insists on having this change merged upstream? No ... I didn't think it would be such an issue, so I'm happy to keep this as a local patch if that's any better. Mark J. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/22/2013 06:07 PM, Mark Jackson wrote: >> Do you insists on having this change merged upstream? > > No ... I didn't think it would be such an issue, so I'm happy to keep this > as a local patch if that's any better. Felipe, Bin: Second opinion on that? > > Mark J. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/usb/am33xx-usb.txt b/Documentation/devicetree/bindings/usb/am33xx-usb.txt index 20c2ff2..560b7ff 100644 --- a/Documentation/devicetree/bindings/usb/am33xx-usb.txt +++ b/Documentation/devicetree/bindings/usb/am33xx-usb.txt @@ -47,6 +47,8 @@ USB - dmas: specifies the dma channels - dma-names: specifies the names of the channels. Use "rxN" for receive and "txN" for transmit endpoints. N specifies the endpoint number. +- ti,force-host: specifies that the IDDIG input be ignored and the device be + put into host mode regardless. The controller should have an "usb" alias numbered properly in the alias node. diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 1901f6f..6439809 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -105,6 +105,7 @@ struct dsps_musb_wrapper { unsigned otg_disable:5; /* bit positions for mode */ + unsigned iddig_mux:5; unsigned iddig:5; /* miscellaneous stuff */ u8 poll_seconds; @@ -387,6 +388,15 @@ static int dsps_musb_init(struct musb *musb) musb->isr = dsps_interrupt; + /* Force host mode, rather than relying on IDDIG input */ + if (musb->config->force_host) { + val = dsps_readl(reg_base, wrp->mode); + /* clear IDDIG bit, set IDDIG_MUX bit */ + val &= ~(1 << wrp->iddig); + val |= (1 << wrp->iddig_mux); + dsps_writel(musb->ctrl_base, wrp->mode, val); + } + /* reset the otgdisable bit, needed for host mode to work */ val = dsps_readl(reg_base, wrp->phy_utmi); val &= ~(1 << wrp->otg_disable); @@ -512,6 +522,9 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, pdata.power = get_int_prop(dn, "mentor,power") / 2; config->multipoint = of_property_read_bool(dn, "mentor,multipoint"); + if (of_property_read_bool(dn, "ti,force-host")) + config->force_host = true; + ret = platform_device_add_data(musb, &pdata, sizeof(pdata)); if (ret) { dev_err(dev, "failed to add platform_data\n"); @@ -607,6 +620,7 @@ static const struct dsps_musb_wrapper am33xx_driver_data = { .mode = 0xe8, .reset = 0, .otg_disable = 21, + .iddig_mux = 7, .iddig = 8, .usb_shift = 0, .usb_mask = 0x1ff, diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h index eb50525..a0a81c5 100644 --- a/include/linux/usb/musb.h +++ b/include/linux/usb/musb.h @@ -75,6 +75,7 @@ struct musb_hdrc_config { unsigned high_iso_rx:1; /* Rx ep required for HD iso */ unsigned dma:1 __deprecated; /* supports DMA */ unsigned vendor_req:1 __deprecated; /* vendor registers required */ + unsigned force_host:1; /* force host mode */ u8 num_eps; /* number of endpoints _with_ ep0 */ u8 dma_channels __deprecated; /* number of dma channels */
The IDDIG input pin is normally used to determine the USB mode (i.e. HOST or DEVICE). On some systems (e.g. AM335x) leaving this pin floating allows the USB mode to be set via software. This patch adds support for this via the device tree. Signed-off-by: Mark Jackson <mpfj@newflow.co.uk> --- .../devicetree/bindings/usb/am33xx-usb.txt | 2 ++ drivers/usb/musb/musb_dsps.c | 14 ++++++++++++++ include/linux/usb/musb.h | 1 + 3 files changed, 17 insertions(+)