| Message ID | 1371482014-5244-2-git-send-email-bigeasy@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jun 17, 2013 at 05:13:33PM +0200, Sebastian Andrzej Siewior wrote: > If we specify right now more than once instance then we attempt to add > the platform device twice. The nop driver does not mind the second add > because it checks for it and returns without a word. At removal time a > segfault is likely because the first intance clean ups the phy and > second, well, goes boom. > This patchs adds two dummy device node to the am33xx for the dummy phy > which we have now. During probe time, we grab the phy based on the > device node if we have one. If not, we use the same hack we used so far. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > arch/arm/boot/dts/am33xx.dtsi | 8 ++++++++ > drivers/usb/musb/musb_dsps.c | 18 +++++++++++++----- > include/linux/usb/musb.h | 1 + > 3 files changed, 22 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi > index 8e1248f..30d0d45 100644 > --- a/arch/arm/boot/dts/am33xx.dtsi > +++ b/arch/arm/boot/dts/am33xx.dtsi > @@ -341,6 +341,14 @@ > port1-mode = <3>; > power = <250>; > ti,hwmods = "usb_otg_hs"; > + phys = <&nopphy0 &nopphy1>; > + }; > + > + nopphy0: usbphy@0 { > + compatible = "usb-nop-xceiv"; > + }; > + nopphy1: usbphy@1 { > + compatible = "usb-nop-xceiv"; > }; > > mac: ethernet@4a100000 { > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c > index e1b661d..d9ff390 100644 > --- a/drivers/usb/musb/musb_dsps.c > +++ b/drivers/usb/musb/musb_dsps.c > @@ -415,9 +415,14 @@ static int dsps_musb_init(struct musb *musb) > /* mentor core register starts at offset of 0x400 from musb base */ > musb->mregs += wrp->musb_core_offset; > > - /* NOP driver needs change if supporting dual instance */ > - usb_nop_xceiv_register(); > - musb->xceiv = usb_get_phy(USB_PHY_TYPE_USB2); > + if (!glue->dev->of_node) { > + /* This hack works only for a single instance. */ > + usb_nop_xceiv_register(); > + musb->xceiv = usb_get_phy(USB_PHY_TYPE_USB2); I think you can drop this altogether, am335x is DT-only anyway :-) > + } else { > + musb->xceiv = devm_usb_get_phy_by_phandle(glue->dev, "phys", > + musb->config->instance); > + } after doing all this, perhaps we should re-factor phy_get into musb_core.c, so that we can remove this sort of support from all glue layers.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On 06/18/2013 10:27 AM, Felipe Balbi wrote: >> diff --git a/drivers/usb/musb/musb_dsps.c >> b/drivers/usb/musb/musb_dsps.c index e1b661d..d9ff390 100644 --- >> a/drivers/usb/musb/musb_dsps.c +++ >> b/drivers/usb/musb/musb_dsps.c @@ -415,9 +415,14 @@ static int >> dsps_musb_init(struct musb *musb) /* mentor core register starts >> at offset of 0x400 from musb base */ musb->mregs += >> wrp->musb_core_offset; >> >> - /* NOP driver needs change if supporting dual instance */ - >> usb_nop_xceiv_register(); - musb->xceiv = >> usb_get_phy(USB_PHY_TYPE_USB2); + if (!glue->dev->of_node) { + >> /* This hack works only for a single instance. */ + >> usb_nop_xceiv_register(); + musb->xceiv = >> usb_get_phy(USB_PHY_TYPE_USB2); > > I think you can drop this altogether, am335x is DT-only anyway :-) Yes, but this is also used by: $ git grep "musb-ti81xx" arch/arm/mach-omap2/usb-musb.c: name = "musb-ti81xx"; drivers/usb/musb/musb_dsps.c: .name = "musb-ti81xx", Is that one also am33xx? > >> + } else { + musb->xceiv = >> devm_usb_get_phy_by_phandle(glue->dev, "phys", + >> musb->config->instance); + } > > after doing all this, perhaps we should re-factor phy_get into > musb_core.c, so that we can remove this sort of support from all > glue layers. So keep this is as is and add later a phy_get into musb? I would have check if everyone has a phy instance but I guess so. Sebastian -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iQIcBAEBCAAGBQJRwBucAAoJEHuW6BYqjPXRhK4P/3SjhljlSmWO0XN1pjXCu+L+ m6yxlz+epFfJSnXKX8FWmVQOahtQrbclv+KhcMo/KWeHj/epp30u924avkexwOc7 RRPCYEldhJdySNEkv14MpAan5WfOXNm8NuD7wCnrADahQxw6vQPoU3wnDIRKuW5i b4kR+V2cSJatlg5tNeXiYR+LLygtSaBprBwcdUSeJ/BsivZBYx4TnwQ/LpMD24br vORIbivMf+SKkJAmxBqHg956L38vL6C3iLYR0GHnk7a4x9cQ4Kk5jJVz6HbphYkJ zzIrYojFXQ5SE7F4+jyEidUx2hQnNhNT6TAFGXG9fCpdNoi+3zhxBKUW1b7aIAHP fbQJbNM4wRNBmxKvQPTLzlsnLTFbjfBb+vd1KBst8niIjLfhaYiV9oLiwAUilrvw ugndxyzGgfKWbBDwcS6pVNwfP3eE54ZunOrRJnS/EF1GRrC1yaPZb0xuTxuMiqN2 97R9H1SPyUv9qJidB/EoJKFMyoq6fWlNTMTMGK+S80ctdQp+bOWdNcpJGaPx7XGc 8z+AKoQZapC/TunWqrJrNz6mcTh2ibyG+hfTD7FJULq9NYM7oAtAn9MOser4gwTB YxsJnEIj29TNJQx4g7/29rr+vNKNX1DPNTgW7r3v4JFKPClx4wZpXLe9qOG48LKa ciA5Sp9sWio6AwtyhEyl =xL1i -----END PGP SIGNATURE----- -- 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
Hi, On Tue, Jun 18, 2013 at 10:34:41AM +0200, Sebastian Andrzej Siewior wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > On 06/18/2013 10:27 AM, Felipe Balbi wrote: > >> diff --git a/drivers/usb/musb/musb_dsps.c > >> b/drivers/usb/musb/musb_dsps.c index e1b661d..d9ff390 100644 --- > >> a/drivers/usb/musb/musb_dsps.c +++ > >> b/drivers/usb/musb/musb_dsps.c @@ -415,9 +415,14 @@ static int > >> dsps_musb_init(struct musb *musb) /* mentor core register starts > >> at offset of 0x400 from musb base */ musb->mregs += > >> wrp->musb_core_offset; > >> > >> - /* NOP driver needs change if supporting dual instance */ - > >> usb_nop_xceiv_register(); - musb->xceiv = > >> usb_get_phy(USB_PHY_TYPE_USB2); + if (!glue->dev->of_node) { + > >> /* This hack works only for a single instance. */ + > >> usb_nop_xceiv_register(); + musb->xceiv = > >> usb_get_phy(USB_PHY_TYPE_USB2); > > > > I think you can drop this altogether, am335x is DT-only anyway :-) > > Yes, but this is also used by: > > $ git grep "musb-ti81xx" > arch/arm/mach-omap2/usb-musb.c: name = "musb-ti81xx"; > drivers/usb/musb/musb_dsps.c: .name = "musb-ti81xx", > > Is that one also am33xx? my bad, it's not :-) > >> + } else { + musb->xceiv = > >> devm_usb_get_phy_by_phandle(glue->dev, "phys", + > >> musb->config->instance); + } > > > > after doing all this, perhaps we should re-factor phy_get into > > musb_core.c, so that we can remove this sort of support from all > > glue layers. > > So keep this is as is and add later a phy_get into musb? I would have > check if everyone has a phy instance but I guess so. sure, let's keep it as is :-)
On 06/18/2013 10:27 AM, Felipe Balbi wrote: >> --- a/arch/arm/boot/dts/am33xx.dtsi +++ >> b/arch/arm/boot/dts/am33xx.dtsi @@ -341,6 +341,14 @@ port1-mode = >> <3>; power = <250>; ti,hwmods = "usb_otg_hs"; + phys = >> <&nopphy0 &nopphy1>; + }; + + nopphy0: usbphy@0 { + >> compatible = "usb-nop-xceiv"; + }; + nopphy1: usbphy@1 { + >> compatible = "usb-nop-xceiv"; }; >> >> mac: ethernet@4a100000 { Any opinion on those phy nodes? Is it likely that we need a real phy driver here and also expose a little more information like the memory register or reset / vcc supply? 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/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index 8e1248f..30d0d45 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -341,6 +341,14 @@ port1-mode = <3>; power = <250>; ti,hwmods = "usb_otg_hs"; + phys = <&nopphy0 &nopphy1>; + }; + + nopphy0: usbphy@0 { + compatible = "usb-nop-xceiv"; + }; + nopphy1: usbphy@1 { + compatible = "usb-nop-xceiv"; }; mac: ethernet@4a100000 { diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index e1b661d..d9ff390 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -415,9 +415,14 @@ static int dsps_musb_init(struct musb *musb) /* mentor core register starts at offset of 0x400 from musb base */ musb->mregs += wrp->musb_core_offset; - /* NOP driver needs change if supporting dual instance */ - usb_nop_xceiv_register(); - musb->xceiv = usb_get_phy(USB_PHY_TYPE_USB2); + if (!glue->dev->of_node) { + /* This hack works only for a single instance. */ + usb_nop_xceiv_register(); + musb->xceiv = usb_get_phy(USB_PHY_TYPE_USB2); + } else { + musb->xceiv = devm_usb_get_phy_by_phandle(glue->dev, "phys", + musb->config->instance); + } if (IS_ERR_OR_NULL(musb->xceiv)) return -EPROBE_DEFER; @@ -449,7 +454,8 @@ static int dsps_musb_init(struct musb *musb) return 0; err0: usb_put_phy(musb->xceiv); - usb_nop_xceiv_unregister(); + if (!glue->dev->of_node) + usb_nop_xceiv_unregister(); return status; } @@ -466,7 +472,8 @@ static int dsps_musb_exit(struct musb *musb) /* NOP driver needs change if supporting dual instance */ usb_put_phy(musb->xceiv); - usb_nop_xceiv_unregister(); + if (!glue->dev->of_node) + usb_nop_xceiv_unregister(); return 0; } @@ -570,6 +577,7 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, u8 id) of_property_read_u32(np, res_name, (u32 *)&pdata->mode); of_property_read_u32(np, "power", (u32 *)&pdata->power); config->multipoint = of_property_read_bool(np, "multipoint"); + config->instance = id; pdata->config = config; } diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h index 053c268..e027705 100644 --- a/include/linux/usb/musb.h +++ b/include/linux/usb/musb.h @@ -83,6 +83,7 @@ struct musb_hdrc_config { u8 vendor_stat __deprecated; /* vendor status reg witdh */ u8 dma_req_chan __deprecated; /* bitmask for required dma channels */ u8 ram_bits; /* ram address size */ + u8 instance; struct musb_hdrc_eps_bits *eps_bits __deprecated; #ifdef CONFIG_BLACKFIN
If we specify right now more than once instance then we attempt to add the platform device twice. The nop driver does not mind the second add because it checks for it and returns without a word. At removal time a segfault is likely because the first intance clean ups the phy and second, well, goes boom. This patchs adds two dummy device node to the am33xx for the dummy phy which we have now. During probe time, we grab the phy based on the device node if we have one. If not, we use the same hack we used so far. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- arch/arm/boot/dts/am33xx.dtsi | 8 ++++++++ drivers/usb/musb/musb_dsps.c | 18 +++++++++++++----- include/linux/usb/musb.h | 1 + 3 files changed, 22 insertions(+), 5 deletions(-)