| Message ID | 20190509201142.10543-6-chris.brandt@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | usb: Add host and device support for RZ/A2 | expand |
On 09.05.2019 23:11, Chris Brandt wrote: > When not using OTG, the PHY will need to know if it should function as > host or peripheral by checking dr_mode in the PHY node (not the parent > controller node). > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > --- > v2: > * added braces to else statement > * check if dr_mode is "host" > --- > drivers/phy/renesas/phy-rcar-gen3-usb2.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > index 1ebb08f8323f..5e5e5e938f80 100644 > --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c > +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > @@ -391,6 +391,7 @@ static int rcar_gen3_phy_usb2_init(struct phy *p) > struct rcar_gen3_phy *rphy = phy_get_drvdata(p); > struct rcar_gen3_chan *channel = rphy->ch; > void __iomem *usb2_base = channel->base; > + enum usb_dr_mode mode; > u32 val; > > if (channel->uses_usb_x1) > @@ -408,6 +409,13 @@ static int rcar_gen3_phy_usb2_init(struct phy *p) > if (rcar_gen3_needs_init_otg(channel)) > rcar_gen3_init_otg(channel); > rphy->otg_initialized = true; > + } else { > + /* Not OTG, so dr_mode should be set in PHY node */ > + mode = usb_get_dr_mode(channel->dev); > + if (mode == USB_DR_MODE_HOST) > + writel(0x00000000, usb2_base + USB2_COMMCTRL); > + else if (mode == USB_DR_MODE_PERIPHERAL) > + writel(0x80000000, usb2_base + USB2_COMMCTRL); Maybe a *switch* instead? MBR, Sergei
On Fri, May 10, 2019, Sergei Shtylyov wrote: > > + } else { > > + /* Not OTG, so dr_mode should be set in PHY node */ > > + mode = usb_get_dr_mode(channel->dev); > > + if (mode == USB_DR_MODE_HOST) > > + writel(0x00000000, usb2_base + USB2_COMMCTRL); > > + else if (mode == USB_DR_MODE_PERIPHERAL) > > + writel(0x80000000, usb2_base + USB2_COMMCTRL); > > Maybe a *switch* instead? I like that idea because I can get rid of the dr_mode variable. However... I just tried it, but if I only have a case for HOST and PERIPHERAL, I get this gcc warning: warning: enumeration value ‘USB_DR_MODE_UNKNOWN’ not handled in switch [-Wswitch] warning: enumeration value ‘USB_DR_MODE_OTG’ not handled in switch [-Wswitch] So, my code would have to be: } else { /* Not OTG, so dr_mode should be set in PHY node */ switch (usb_get_dr_mode(channel->dev)) { case USB_DR_MODE_HOST: writel(0x00000000, usb2_base + USB2_COMMCTRL); break; case USB_DR_MODE_PERIPHERAL: writel(0x80000000, usb2_base + USB2_COMMCTRL); break; case USB_DR_MODE_UNKNOWN: case USB_DR_MODE_OTG: break; } } I guess that is still OK. Chris
On 10.05.2019 16:55, Chris Brandt wrote: >>> + } else { >>> + /* Not OTG, so dr_mode should be set in PHY node */ >>> + mode = usb_get_dr_mode(channel->dev); >>> + if (mode == USB_DR_MODE_HOST) >>> + writel(0x00000000, usb2_base + USB2_COMMCTRL); >>> + else if (mode == USB_DR_MODE_PERIPHERAL) >>> + writel(0x80000000, usb2_base + USB2_COMMCTRL); >> >> Maybe a *switch* instead? > > I like that idea because I can get rid of the dr_mode variable. Yes. :-) > However... > I just tried it, but if I only have a case for HOST and PERIPHERAL, I > get this gcc warning: > > warning: enumeration value ‘USB_DR_MODE_UNKNOWN’ not handled in switch [-Wswitch] > warning: enumeration value ‘USB_DR_MODE_OTG’ not handled in switch [-Wswitch] > > > So, my code would have to be: > > } else { > /* Not OTG, so dr_mode should be set in PHY node */ > switch (usb_get_dr_mode(channel->dev)) { > case USB_DR_MODE_HOST: > writel(0x00000000, usb2_base + USB2_COMMCTRL); > break; > case USB_DR_MODE_PERIPHERAL: > writel(0x80000000, usb2_base + USB2_COMMCTRL); > break; > case USB_DR_MODE_UNKNOWN: > case USB_DR_MODE_OTG: Maybe default: instead? > break; > } > } > > I guess that is still OK. Yes. :-) > Chris MBR, Sergei
On Sat, May 11, 2019, Sergei Shtylyov wrote: > > case USB_DR_MODE_UNKNOWN: > > case USB_DR_MODE_OTG: > > Maybe default: instead? Yes, using default instead works. Thank you! Chris
diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c index 1ebb08f8323f..5e5e5e938f80 100644 --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c @@ -391,6 +391,7 @@ static int rcar_gen3_phy_usb2_init(struct phy *p) struct rcar_gen3_phy *rphy = phy_get_drvdata(p); struct rcar_gen3_chan *channel = rphy->ch; void __iomem *usb2_base = channel->base; + enum usb_dr_mode mode; u32 val; if (channel->uses_usb_x1) @@ -408,6 +409,13 @@ static int rcar_gen3_phy_usb2_init(struct phy *p) if (rcar_gen3_needs_init_otg(channel)) rcar_gen3_init_otg(channel); rphy->otg_initialized = true; + } else { + /* Not OTG, so dr_mode should be set in PHY node */ + mode = usb_get_dr_mode(channel->dev); + if (mode == USB_DR_MODE_HOST) + writel(0x00000000, usb2_base + USB2_COMMCTRL); + else if (mode == USB_DR_MODE_PERIPHERAL) + writel(0x80000000, usb2_base + USB2_COMMCTRL); } rphy->initialized = true;
When not using OTG, the PHY will need to know if it should function as host or peripheral by checking dr_mode in the PHY node (not the parent controller node). Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- v2: * added braces to else statement * check if dr_mode is "host" --- drivers/phy/renesas/phy-rcar-gen3-usb2.c | 8 ++++++++ 1 file changed, 8 insertions(+)