| Message ID | 1458081473-8223-3-git-send-email-david@lechnology.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello. On 03/16/2016 01:37 AM, David Lechner wrote: > The usb ohci driver has been change to not include mach/*, so we need > to pass the cfgchip2 address to the driver so that it can turn the usb > phy on and off. > > Signed-off-by: David Lechner <david@lechnology.com> > --- > arch/arm/mach-davinci/usb.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c > index b0a6b52..9607b0c 100644 > --- a/arch/arm/mach-davinci/usb.c > +++ b/arch/arm/mach-davinci/usb.c > @@ -148,6 +148,11 @@ static struct resource da8xx_usb11_resources[] = { > .flags = IORESOURCE_MEM, > }, > [1] = { > + .start = DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP2_REG, > + .end = DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP2_REG + 4 - 1, > + .flags = IORESOURCE_MEM, > + }, No, this register is shared b/w MUSB and OHCI. The proper thing to do is to write the PHY driver and let it control this shared register. [...] MBR, Sergei
On 03/15/2016 05:45 PM, Sergei Shtylyov wrote: > > No, this register is shared b/w MUSB and OHCI. The proper thing to > do is to write the PHY driver and let it control this shared register. > OK. I've started working on this. I am looking at using struct usb_phy, however, enum usb_phy_type only has USB_PHY_TYPE_UNDEFINED, USB_PHY_TYPE_USB2, and USB_PHY_TYPE_USB3. Would it be acceptable to use USB_PHY_TYPE_UNDEFINED for the ohci since it is USB 1.1? Or perhaps I should use the more generic struct phy for that one?
On 03/15/2016 10:46 PM, David Lechner wrote: > On 03/15/2016 05:45 PM, Sergei Shtylyov wrote: >> >> No, this register is shared b/w MUSB and OHCI. The proper thing to >> do is to write the PHY driver and let it control this shared register. >> > > OK. I've started working on this. I am looking at using struct usb_phy, > however, enum usb_phy_type only has USB_PHY_TYPE_UNDEFINED, > USB_PHY_TYPE_USB2, and USB_PHY_TYPE_USB3. Would it be acceptable to use > USB_PHY_TYPE_UNDEFINED for the ohci since it is USB 1.1? Or perhaps I > should use the more generic struct phy for that one? > Also, I am not finding any existing data structure to pass the musb set_mode function to the phy in either usb_phy or usb_otg. Setting the mode (host/peripheral/otg) is done in the same PHY register, so it seems like it should be implemented in the new phy driver as well. I guess I could use a generic phy instead and use phy_set_drvdata() to share data between the phy driver and the musb driver. Does this sound like a reasonable thing to do?
Hello. On 3/16/2016 6:46 AM, David Lechner wrote: >> No, this register is shared b/w MUSB and OHCI. The proper thing to >> do is to write the PHY driver and let it control this shared register. > OK. I've started working on this. I am looking at using struct usb_phy, > however, enum usb_phy_type only has USB_PHY_TYPE_UNDEFINED, USB_PHY_TYPE_USB2, > and USB_PHY_TYPE_USB3. Would it be acceptable to use USB_PHY_TYPE_UNDEFINED > for the ohci since it is USB 1.1? Or perhaps I should use the more generic > struct phy for that one? No new USB PHY drivers accepted, look at drivers/phy/ instead please. MBR, Sergei
On 03/16/2016 07:57 AM, David Lechner wrote: >>> No, this register is shared b/w MUSB and OHCI. The proper thing to >>> do is to write the PHY driver and let it control this shared register. >>> >> OK. I've started working on this. I am looking at using struct usb_phy, >> however, enum usb_phy_type only has USB_PHY_TYPE_UNDEFINED, >> USB_PHY_TYPE_USB2, and USB_PHY_TYPE_USB3. Would it be acceptable to use >> USB_PHY_TYPE_UNDEFINED for the ohci since it is USB 1.1? Or perhaps I >> should use the more generic struct phy for that one? >> > Also, I am not finding any existing data structure to pass the musb set_mode > function to the phy in either usb_phy or usb_otg. Setting the mode > (host/peripheral/otg) is done in the same PHY register, so it seems like it > should be implemented in the new phy driver as well. Perhaps we'd have to sacrifice that functionality... > I guess I could use a generic phy instead and use phy_set_drvdata() to share > data between the phy driver and the musb driver. Does this sound like a > reasonable thing to do? Not sure what you mean, could you elaborate? MBR, Sergei
On 03/16/2016 12:38 PM, Sergei Shtylyov wrote: > On 03/16/2016 07:57 AM, David Lechner wrote: > >> Also, I am not finding any existing data structure to pass the musb >> set_mode >> function to the phy in either usb_phy or usb_otg. Setting the mode >> (host/peripheral/otg) is done in the same PHY register, so it seems >> like it >> should be implemented in the new phy driver as well. > > Perhaps we'd have to sacrifice that functionality... The device I am working on (LEGO MINDSTORMS EV3) has the port wired as peripheral only, so I don't think leaving this out is an option. Leaving it in OTG mode doesn't work because the required electrical connections are just not there. >> I guess I could use a generic phy instead and use phy_set_drvdata() to >> share >> data between the phy driver and the musb driver. Does this sound like a >> reasonable thing to do? > > Not sure what you mean, could you elaborate? I found another driver that essentially does what I was trying to explain here. See the sun4i_usb_phy_set_squelch_detect function in drivers/phy/phy-sun4i-usb.c:394[1] as an example. It is called at drivers/usb/musb/sunxi.c:160[2] and :167. I would move the da8xx_musb_set_mode function from drivers/usb/musb/da8xx.c to the new drivers/phy/phy-da8xx-usb.c and call it in a similar manner to the sunix example I gave. --- [1]: drivers/phy/phy-sun4i-usb.c void sun4i_usb_phy_set_squelch_detect(struct phy *_phy, bool enabled) { struct sun4i_usb_phy *phy = phy_get_drvdata(_phy); sun4i_usb_phy_write(phy, PHY_SQUELCH_DETECT, enabled ? 0 : 2, 2); } EXPORT_SYMBOL_GPL(sun4i_usb_phy_set_squelch_detect); [2]: drivers/usb/musb/sunxi.c static void sunxi_musb_pre_root_reset_end(struct musb *musb) { struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent); sun4i_usb_phy_set_squelch_detect(glue->phy, false); }
On 03/16/2016 09:14 PM, David Lechner wrote: >>> Also, I am not finding any existing data structure to pass the musb >>> set_mode >>> function to the phy in either usb_phy or usb_otg. Setting the mode >>> (host/peripheral/otg) is done in the same PHY register, so it seems >>> like it >>> should be implemented in the new phy driver as well. >> >> Perhaps we'd have to sacrifice that functionality... > > The device I am working on (LEGO MINDSTORMS EV3) has the port wired as > peripheral only, so I don't think leaving this out is an option. Leaving it in > OTG mode doesn't work because the required electrical connections are just not > there. The set_mode() method doesn't have anything to do with the predefined roles. What CFGCHIP2 setting do is to override the ID input (and also the VBUS level comparator). This is not required for the normal functioning of either host or peripheral AFAIR. [...] MBR, Sergei
On 03/16/2016 09:22 PM, Sergei Shtylyov wrote: >>>> Also, I am not finding any existing data structure to pass the musb >>>> set_mode >>>> function to the phy in either usb_phy or usb_otg. Setting the mode >>>> (host/peripheral/otg) is done in the same PHY register, so it seems >>>> like it >>>> should be implemented in the new phy driver as well. >>> >>> Perhaps we'd have to sacrifice that functionality... >> >> The device I am working on (LEGO MINDSTORMS EV3) has the port wired as >> peripheral only, so I don't think leaving this out is an option. Leaving it in >> OTG mode doesn't work because the required electrical connections are just not >> there. > > The set_mode() method doesn't have anything to do with the predefined > roles. What CFGCHIP2 setting do is to override the ID input (and also the VBUS > level comparator). This is not required for the normal functioning of either > host or peripheral AFAIR. Or at least it wasn't when I last looked. Now it does... :-/ > [...] MBR, Sergei
On 03/16/2016 01:22 PM, Sergei Shtylyov wrote: > > The set_mode() method doesn't have anything to do with the > predefined roles. What CFGCHIP2 setting do is to override the ID input > (and also the VBUS level comparator). This is not required for the > normal functioning of either host or peripheral AFAIR. > OK, so it sounds like I should just remove set_mode from the musb driver completely and make this configurable in the phy driver via platform data or device tree.
diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c index b0a6b52..9607b0c 100644 --- a/arch/arm/mach-davinci/usb.c +++ b/arch/arm/mach-davinci/usb.c @@ -148,6 +148,11 @@ static struct resource da8xx_usb11_resources[] = { .flags = IORESOURCE_MEM, }, [1] = { + .start = DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP2_REG, + .end = DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP2_REG + 4 - 1, + .flags = IORESOURCE_MEM, + }, + [2] = { .start = IRQ_DA8XX_IRQN, .end = IRQ_DA8XX_IRQN, .flags = IORESOURCE_IRQ,
The usb ohci driver has been change to not include mach/*, so we need to pass the cfgchip2 address to the driver so that it can turn the usb phy on and off. Signed-off-by: David Lechner <david@lechnology.com> --- arch/arm/mach-davinci/usb.c | 5 +++++ 1 file changed, 5 insertions(+)