[PATCH 1/2] USB: MUSB: PHY: scrap singleton am335x_get_usb_phy()

[PATCH 1/2] USB: MUSB: PHY: scrap singleton am335x_get_usb_phy()

[Ahmad Fatoum]

am335x_get_usb_phy() retrieves the last probed USB phy. On the BeagleBone
with both PHYs enabled, this means that dependent on probe order, both
MUSB instances could end up with the same PHY.

Remove the global variable and have the MUSB driver parse the "phys"
property instead.

The cleaner way to achieve this would be to migrate phy-am335x.c
and phy-am335x-control.c to the generic phy framework and have MUSB use
of_phy_get, alas, even Linux hasn't done this so far and we need
a short patch for master anyway, thus just do it the easy way.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Prerequisite for fix in next commit. Please apply both to master.
---
 drivers/usb/musb/musb_dsps.c  | 16 +++++++++++-----
 drivers/usb/musb/phy-am335x.c | 11 ++---------
 drivers/usb/musb/phy-am335x.h |  6 ------
 3 files changed, 13 insertions(+), 20 deletions(-)
 delete mode 100644 drivers/usb/musb/phy-am335x.h

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 3b76b6cc610d..f30672914830 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -39,7 +39,6 @@
 #include <linux/barebox-wrapper.h>
 
 #include "musb_core.h"
-#include "phy-am335x.h"
 
 static __maybe_unused struct of_device_id musb_dsps_dt_ids[];
 
@@ -217,10 +216,6 @@ static int dsps_musb_init(struct musb *musb)
 	const struct dsps_musb_wrapper *wrp = glue->wrp;
 	u32 rev, val, mode;
 
-	musb->xceiv = am335x_get_usb_phy();
-	if (IS_ERR(musb->xceiv))
-		return PTR_ERR(musb->xceiv);
-
 	/* Returns zero if e.g. not clocked */
 	rev = dsps_readl(musb->ctrl_base, wrp->revision);
 	if (!rev)
@@ -324,6 +319,8 @@ static int dsps_probe(struct device_d *dev)
 	struct musb_hdrc_config	*config;
 	struct device_node *dn = dev->device_node;
 	const struct dsps_musb_wrapper *wrp;
+	struct device_node *phy_node;
+	struct device_d *phy_dev;
 	struct dsps_glue *glue;
 	int ret;
 
@@ -337,6 +334,14 @@ static int dsps_probe(struct device_d *dev)
 		return -ENODEV;
 	}
 
+	phy_node = of_parse_phandle(dn, "phys", 0);
+	if (!phy_node)
+		return -ENODEV;
+
+	phy_dev = of_find_device_by_node(phy_node);
+	if (!phy_dev || !phy_dev->priv)
+		return -EPROBE_DEFER;
+
 	/* allocate glue */
 	glue = kzalloc(sizeof(*glue), GFP_KERNEL);
 	if (!glue) {
@@ -360,6 +365,7 @@ static int dsps_probe(struct device_d *dev)
 	glue->musb.ctrl_base = IOMEM(iores->start);
 
 	glue->musb.controller = dev;
+	glue->musb.xceiv = phy_dev->priv;
 
 	config = &glue->config;
 
diff --git a/drivers/usb/musb/phy-am335x.c b/drivers/usb/musb/phy-am335x.c
index df31255d891c..6991f4402d3f 100644
--- a/drivers/usb/musb/phy-am335x.c
+++ b/drivers/usb/musb/phy-am335x.c
@@ -5,7 +5,6 @@
 #include <linux/err.h>
 #include "am35x-phy-control.h"
 #include "musb_core.h"
-#include "phy-am335x.h"
 
 struct am335x_usbphy {
 	void __iomem *base;
@@ -14,13 +13,6 @@ struct am335x_usbphy {
 	struct usb_phy phy;
 };
 
-static struct am335x_usbphy *am_usbphy;
-
-struct usb_phy *am335x_get_usb_phy(void)
-{
-	return &am_usbphy->phy;
-}
-
 static int am335x_init(struct usb_phy *phy)
 {
 	struct am335x_usbphy *am_usbphy = container_of(phy, struct am335x_usbphy, phy);
@@ -31,6 +23,7 @@ static int am335x_init(struct usb_phy *phy)
 
 static int am335x_phy_probe(struct device_d *dev)
 {
+	struct am335x_usbphy *am_usbphy;
 	struct resource *iores;
 	int ret;
 
@@ -54,7 +47,7 @@ static int am335x_phy_probe(struct device_d *dev)
 	}
 
 	am_usbphy->phy.init = am335x_init;
-	dev->priv = am_usbphy;
+	dev->priv = &am_usbphy->phy;
 
 	dev_info(dev, "am_usbphy %p enabled\n", &am_usbphy->phy);
 
diff --git a/drivers/usb/musb/phy-am335x.h b/drivers/usb/musb/phy-am335x.h
deleted file mode 100644
index 27da2e3b1057..000000000000
--- a/drivers/usb/musb/phy-am335x.h
+++ /dev/null
@@ -1,6 +0,0 @@
-#ifndef _PHY_AM335x_H_
-#define _PHY_AM335x_H_
-
-struct usb_phy *am335x_get_usb_phy(void);
-
-#endif
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] USB: MUSB: defer driver probes where necessary

[Ahmad Fatoum]

Kernel commit 0782e8572c ("ARM: dts: Probe am335x musb with ti-sysc")
which we pulled in during the v2020.02.0 dts/ sync moved the USB nodes
to be under a ti-sysc bus instead of ti,am33xx-usb.

This new probe order broke am335x USB under barebox, because the MUSB
drivers couldn't cope with the now different device probe order.

Pepper some -EPROBE_DEFER around to make USB work again.

Fixes: 574eed3f6f ("dts: update to v5.5-rc1")
Reported-by: Yegor Yefremov <yegorslists@googlemail.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Fix for v2020.02.0. Please apply to master.
---
 drivers/usb/musb/musb_core.c          |  5 ++--
 drivers/usb/musb/musb_dsps.c          | 39 +++++++++++++++++++--------
 drivers/usb/musb/phy-am335x-control.c | 25 ++++++++++++-----
 drivers/usb/musb/phy-am335x.c         | 12 ++++++---
 4 files changed, 58 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 4c11e6580c0f..b84da5516c4a 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1136,8 +1136,9 @@ fail2:
 	musb_platform_exit(musb);
 
 fail1:
-	dev_err(musb->controller,
-		"musb_init_controller failed with status %d\n", status);
+	if (status != -EPROBE_DEFER)
+		dev_err(musb->controller,
+			"musb_init_controller failed with status %d\n", status);
 
 	musb_free(musb);
 
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index f30672914830..d54a663e9d8b 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -314,7 +314,7 @@ static int dsps_set_mode(void *ctx, enum usb_dr_mode mode)
 
 static int dsps_probe(struct device_d *dev)
 {
-	struct resource *iores;
+	struct resource *iores[2];
 	struct musb_hdrc_platform_data *pdata;
 	struct musb_hdrc_config	*config;
 	struct device_node *dn = dev->device_node;
@@ -354,15 +354,19 @@ static int dsps_probe(struct device_d *dev)
 
 	pdata = &glue->pdata;
 
-	iores = dev_request_mem_resource(dev, 0);
-	if (IS_ERR(iores))
-		return PTR_ERR(iores);
-	glue->musb.mregs = IOMEM(iores->start);
+	iores[0] = dev_request_mem_resource(dev, 0);
+	if (IS_ERR(iores[0])) {
+		ret = PTR_ERR(iores[0]);
+		goto free_glue;
+	}
+	glue->musb.mregs = IOMEM(iores[0]->start);
 
-	iores = dev_request_mem_resource(dev, 1);
-	if (IS_ERR(iores))
-		return PTR_ERR(iores);
-	glue->musb.ctrl_base = IOMEM(iores->start);
+	iores[1] = dev_request_mem_resource(dev, 1);
+	if (IS_ERR(iores[1])) {
+		ret = PTR_ERR(iores[1]);
+		goto release_iores0;
+	}
+	glue->musb.ctrl_base = IOMEM(iores[1]->start);
 
 	glue->musb.controller = dev;
 	glue->musb.xceiv = phy_dev->priv;
@@ -383,11 +387,24 @@ static int dsps_probe(struct device_d *dev)
 	if (pdata->mode == MUSB_PORT_MODE_DUAL_ROLE) {
 		ret = usb_register_otg_device(dev, dsps_set_mode, glue);
 		if (ret)
-			return ret;
+			goto release_iores1;
 		return 0;
 	}
 
-	return musb_init_controller(&glue->musb, pdata);
+	ret = musb_init_controller(&glue->musb, pdata);
+	if (ret)
+		goto release_iores1;
+
+	return 0;
+
+release_iores1:
+	release_region(iores[1]);
+release_iores0:
+	release_region(iores[0]);
+free_glue:
+	free(glue);
+
+	return ret;
 }
 
 static const struct dsps_musb_wrapper am33xx_driver_data = {
diff --git a/drivers/usb/musb/phy-am335x-control.c b/drivers/usb/musb/phy-am335x-control.c
index c84525ec7eb4..41a3689ed3f9 100644
--- a/drivers/usb/musb/phy-am335x-control.c
+++ b/drivers/usb/musb/phy-am335x-control.c
@@ -109,15 +109,15 @@ struct phy_control *am335x_get_phy_control(struct device_d *dev)
 
 	node = of_parse_phandle(dev->device_node, "ti,ctrl_mod", 0);
 	if (!node)
-		return NULL;
+		return ERR_PTR(-ENOENT);
 
 	dev = of_find_device_by_node(node);
 	if (!dev)
-		return NULL;
+		return ERR_PTR(-EPROBE_DEFER);
 
 	ctrl_usb = dev->priv;
 	if (!ctrl_usb)
-		return NULL;
+		return ERR_PTR(-EPROBE_DEFER);
 
 	return &ctrl_usb->phy_ctrl;
 }
@@ -141,13 +141,17 @@ static int am335x_control_usb_probe(struct device_d *dev)
 	ctrl_usb->dev = dev;
 
 	iores = dev_request_mem_resource(dev, 0);
-	if (IS_ERR(iores))
-		return PTR_ERR(iores);
+	if (IS_ERR(iores)) {
+		ret = PTR_ERR(iores);
+		goto free_ctrl;
+	}
 	ctrl_usb->phy_reg = IOMEM(iores->start);
 
 	iores = dev_request_mem_resource(dev, 1);
-	if (IS_ERR(iores))
-		return PTR_ERR(iores);
+	if (IS_ERR(iores)) {
+		ret = PTR_ERR(iores);
+		goto release_resource;
+	}
 	ctrl_usb->wkup = IOMEM(iores->start);
 
 	spin_lock_init(&ctrl_usb->lock);
@@ -155,6 +159,13 @@ static int am335x_control_usb_probe(struct device_d *dev)
 
 	dev->priv = ctrl_usb;
 	return 0;
+
+release_resource:
+	release_region(iores);
+free_ctrl:
+	free(ctrl_usb);
+
+	return 0;
 };
 
 static struct driver_d am335x_control_driver = {
diff --git a/drivers/usb/musb/phy-am335x.c b/drivers/usb/musb/phy-am335x.c
index 6991f4402d3f..7dda8caf2a3c 100644
--- a/drivers/usb/musb/phy-am335x.c
+++ b/drivers/usb/musb/phy-am335x.c
@@ -37,13 +37,16 @@ static int am335x_phy_probe(struct device_d *dev)
 	am_usbphy->base = IOMEM(iores->start);
 
 	am_usbphy->phy_ctrl = am335x_get_phy_control(dev);
-	if (!am_usbphy->phy_ctrl)
-		return -ENODEV;
+	if (IS_ERR(am_usbphy->phy_ctrl)) {
+		ret = PTR_ERR(am_usbphy->phy_ctrl);
+		goto err_release;
+	}
 
 	am_usbphy->id = of_alias_get_id(dev->device_node, "phy");
 	if (am_usbphy->id < 0) {
 		dev_err(dev, "Missing PHY id: %d\n", am_usbphy->id);
-		return am_usbphy->id;
+		ret = am_usbphy->id;
+		goto err_release;
 	}
 
 	am_usbphy->phy.init = am335x_init;
@@ -53,8 +56,11 @@ static int am335x_phy_probe(struct device_d *dev)
 
 	return 0;
 
+err_release:
+	release_region(iores);
 err_free:
 	free(am_usbphy);
+	am_usbphy = NULL;
 
 	return ret;
 };
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] USB: MUSB: defer driver probes where necessary

[Yegor Yefremov]

On Tue, Feb 25, 2020 at 5:57 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Kernel commit 0782e8572c ("ARM: dts: Probe am335x musb with ti-sysc")
> which we pulled in during the v2020.02.0 dts/ sync moved the USB nodes
> to be under a ti-sysc bus instead of ti,am33xx-usb.
>
> This new probe order broke am335x USB under barebox, because the MUSB
> drivers couldn't cope with the now different device probe order.
>
> Pepper some -EPROBE_DEFER around to make USB work again.
>
> Fixes: 574eed3f6f ("dts: update to v5.5-rc1")
> Reported-by: Yegor Yefremov <yegorslists@googlemail.com>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Tested against the next branch.

Tested-by: Yegor Yefremov <yegorslists@googlemail.com>

Yegor

> ---
> Fix for v2020.02.0. Please apply to master.
> ---
>  drivers/usb/musb/musb_core.c          |  5 ++--
>  drivers/usb/musb/musb_dsps.c          | 39 +++++++++++++++++++--------
>  drivers/usb/musb/phy-am335x-control.c | 25 ++++++++++++-----
>  drivers/usb/musb/phy-am335x.c         | 12 ++++++---
>  4 files changed, 58 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 4c11e6580c0f..b84da5516c4a 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -1136,8 +1136,9 @@ fail2:
>         musb_platform_exit(musb);
>
>  fail1:
> -       dev_err(musb->controller,
> -               "musb_init_controller failed with status %d\n", status);
> +       if (status != -EPROBE_DEFER)
> +               dev_err(musb->controller,
> +                       "musb_init_controller failed with status %d\n", status);
>
>         musb_free(musb);
>
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index f30672914830..d54a663e9d8b 100644
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -314,7 +314,7 @@ static int dsps_set_mode(void *ctx, enum usb_dr_mode mode)
>
>  static int dsps_probe(struct device_d *dev)
>  {
> -       struct resource *iores;
> +       struct resource *iores[2];
>         struct musb_hdrc_platform_data *pdata;
>         struct musb_hdrc_config *config;
>         struct device_node *dn = dev->device_node;
> @@ -354,15 +354,19 @@ static int dsps_probe(struct device_d *dev)
>
>         pdata = &glue->pdata;
>
> -       iores = dev_request_mem_resource(dev, 0);
> -       if (IS_ERR(iores))
> -               return PTR_ERR(iores);
> -       glue->musb.mregs = IOMEM(iores->start);
> +       iores[0] = dev_request_mem_resource(dev, 0);
> +       if (IS_ERR(iores[0])) {
> +               ret = PTR_ERR(iores[0]);
> +               goto free_glue;
> +       }
> +       glue->musb.mregs = IOMEM(iores[0]->start);
>
> -       iores = dev_request_mem_resource(dev, 1);
> -       if (IS_ERR(iores))
> -               return PTR_ERR(iores);
> -       glue->musb.ctrl_base = IOMEM(iores->start);
> +       iores[1] = dev_request_mem_resource(dev, 1);
> +       if (IS_ERR(iores[1])) {
> +               ret = PTR_ERR(iores[1]);
> +               goto release_iores0;
> +       }
> +       glue->musb.ctrl_base = IOMEM(iores[1]->start);
>
>         glue->musb.controller = dev;
>         glue->musb.xceiv = phy_dev->priv;
> @@ -383,11 +387,24 @@ static int dsps_probe(struct device_d *dev)
>         if (pdata->mode == MUSB_PORT_MODE_DUAL_ROLE) {
>                 ret = usb_register_otg_device(dev, dsps_set_mode, glue);
>                 if (ret)
> -                       return ret;
> +                       goto release_iores1;
>                 return 0;
>         }
>
> -       return musb_init_controller(&glue->musb, pdata);
> +       ret = musb_init_controller(&glue->musb, pdata);
> +       if (ret)
> +               goto release_iores1;
> +
> +       return 0;
> +
> +release_iores1:
> +       release_region(iores[1]);
> +release_iores0:
> +       release_region(iores[0]);
> +free_glue:
> +       free(glue);
> +
> +       return ret;
>  }
>
>  static const struct dsps_musb_wrapper am33xx_driver_data = {
> diff --git a/drivers/usb/musb/phy-am335x-control.c b/drivers/usb/musb/phy-am335x-control.c
> index c84525ec7eb4..41a3689ed3f9 100644
> --- a/drivers/usb/musb/phy-am335x-control.c
> +++ b/drivers/usb/musb/phy-am335x-control.c
> @@ -109,15 +109,15 @@ struct phy_control *am335x_get_phy_control(struct device_d *dev)
>
>         node = of_parse_phandle(dev->device_node, "ti,ctrl_mod", 0);
>         if (!node)
> -               return NULL;
> +               return ERR_PTR(-ENOENT);
>
>         dev = of_find_device_by_node(node);
>         if (!dev)
> -               return NULL;
> +               return ERR_PTR(-EPROBE_DEFER);
>
>         ctrl_usb = dev->priv;
>         if (!ctrl_usb)
> -               return NULL;
> +               return ERR_PTR(-EPROBE_DEFER);
>
>         return &ctrl_usb->phy_ctrl;
>  }
> @@ -141,13 +141,17 @@ static int am335x_control_usb_probe(struct device_d *dev)
>         ctrl_usb->dev = dev;
>
>         iores = dev_request_mem_resource(dev, 0);
> -       if (IS_ERR(iores))
> -               return PTR_ERR(iores);
> +       if (IS_ERR(iores)) {
> +               ret = PTR_ERR(iores);
> +               goto free_ctrl;
> +       }
>         ctrl_usb->phy_reg = IOMEM(iores->start);
>
>         iores = dev_request_mem_resource(dev, 1);
> -       if (IS_ERR(iores))
> -               return PTR_ERR(iores);
> +       if (IS_ERR(iores)) {
> +               ret = PTR_ERR(iores);
> +               goto release_resource;
> +       }
>         ctrl_usb->wkup = IOMEM(iores->start);
>
>         spin_lock_init(&ctrl_usb->lock);
> @@ -155,6 +159,13 @@ static int am335x_control_usb_probe(struct device_d *dev)
>
>         dev->priv = ctrl_usb;
>         return 0;
> +
> +release_resource:
> +       release_region(iores);
> +free_ctrl:
> +       free(ctrl_usb);
> +
> +       return 0;
>  };
>
>  static struct driver_d am335x_control_driver = {
> diff --git a/drivers/usb/musb/phy-am335x.c b/drivers/usb/musb/phy-am335x.c
> index 6991f4402d3f..7dda8caf2a3c 100644
> --- a/drivers/usb/musb/phy-am335x.c
> +++ b/drivers/usb/musb/phy-am335x.c
> @@ -37,13 +37,16 @@ static int am335x_phy_probe(struct device_d *dev)
>         am_usbphy->base = IOMEM(iores->start);
>
>         am_usbphy->phy_ctrl = am335x_get_phy_control(dev);
> -       if (!am_usbphy->phy_ctrl)
> -               return -ENODEV;
> +       if (IS_ERR(am_usbphy->phy_ctrl)) {
> +               ret = PTR_ERR(am_usbphy->phy_ctrl);
> +               goto err_release;
> +       }
>
>         am_usbphy->id = of_alias_get_id(dev->device_node, "phy");
>         if (am_usbphy->id < 0) {
>                 dev_err(dev, "Missing PHY id: %d\n", am_usbphy->id);
> -               return am_usbphy->id;
> +               ret = am_usbphy->id;
> +               goto err_release;
>         }
>
>         am_usbphy->phy.init = am335x_init;
> @@ -53,8 +56,11 @@ static int am335x_phy_probe(struct device_d *dev)
>
>         return 0;
>
> +err_release:
> +       release_region(iores);
>  err_free:
>         free(am_usbphy);
> +       am_usbphy = NULL;
>
>         return ret;
>  };
> --
> 2.25.0
>

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH] fixup! USB: MUSB: defer driver probes where necessary

[Ahmad Fatoum]

The NULL assignment is a left over from when it was a global variable.
am_usbphy is local now and is just going out of scope, so need to NULL
it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/musb/phy-am335x.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/musb/phy-am335x.c b/drivers/usb/musb/phy-am335x.c
index 7dda8caf2a3c..f2e870d7ee61 100644
--- a/drivers/usb/musb/phy-am335x.c
+++ b/drivers/usb/musb/phy-am335x.c
@@ -60,7 +60,6 @@ err_release:
 	release_region(iores);
 err_free:
 	free(am_usbphy);
-	am_usbphy = NULL;
 
 	return ret;
 };
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] fixup! USB: MUSB: defer driver probes where necessary

[Yegor Yefremov]

Hi Ahmad,

On Wed, Feb 26, 2020 at 11:48 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> The NULL assignment is a left over from when it was a global variable.
> am_usbphy is local now and is just going out of scope, so need to NULL
> it.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/usb/musb/phy-am335x.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/usb/musb/phy-am335x.c b/drivers/usb/musb/phy-am335x.c
> index 7dda8caf2a3c..f2e870d7ee61 100644
> --- a/drivers/usb/musb/phy-am335x.c
> +++ b/drivers/usb/musb/phy-am335x.c
> @@ -60,7 +60,6 @@ err_release:
>         release_region(iores);
>  err_free:
>         free(am_usbphy);
> -       am_usbphy = NULL;
>
>         return ret;
>  };

Do you want me to test this fixup as well or are you going to post v2?

Yegor

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] fixup! USB: MUSB: defer driver probes where necessary

[Ahmad Fatoum]

Hello Yegor,

On 2/28/20 11:27 AM, Yegor Yefremov wrote:
> Hi Ahmad,
> 
> On Wed, Feb 26, 2020 at 11:48 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> The NULL assignment is a left over from when it was a global variable.
>> am_usbphy is local now and is just going out of scope, so need to NULL
>> it.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/usb/musb/phy-am335x.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/usb/musb/phy-am335x.c b/drivers/usb/musb/phy-am335x.c
>> index 7dda8caf2a3c..f2e870d7ee61 100644
>> --- a/drivers/usb/musb/phy-am335x.c
>> +++ b/drivers/usb/musb/phy-am335x.c
>> @@ -60,7 +60,6 @@ err_release:
>>         release_region(iores);
>>  err_free:
>>         free(am_usbphy);
>> -       am_usbphy = NULL;
>>
>>         return ret;
>>  };
> 
> Do you want me to test this fixup as well or are you going to post v2?

You don't need to. It's a line that the compiler optimizes out anyway.

Cheers
Ahmad

> 
> Yegor> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] USB: MUSB: PHY: scrap singleton am335x_get_usb_phy()

[Yegor Yefremov]

On Tue, Feb 25, 2020 at 5:57 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> am335x_get_usb_phy() retrieves the last probed USB phy. On the BeagleBone
> with both PHYs enabled, this means that dependent on probe order, both
> MUSB instances could end up with the same PHY.
>
> Remove the global variable and have the MUSB driver parse the "phys"
> property instead.
>
> The cleaner way to achieve this would be to migrate phy-am335x.c
> and phy-am335x-control.c to the generic phy framework and have MUSB use
> of_phy_get, alas, even Linux hasn't done this so far and we need
> a short patch for master anyway, thus just do it the easy way.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Tested against the next branch.

Tested-by: Yegor Yefremov <yegorslists@googlemail.com>

Yegor

> ---
> Prerequisite for fix in next commit. Please apply both to master.
> ---
>  drivers/usb/musb/musb_dsps.c  | 16 +++++++++++-----
>  drivers/usb/musb/phy-am335x.c | 11 ++---------
>  drivers/usb/musb/phy-am335x.h |  6 ------
>  3 files changed, 13 insertions(+), 20 deletions(-)
>  delete mode 100644 drivers/usb/musb/phy-am335x.h
>
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index 3b76b6cc610d..f30672914830 100644
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -39,7 +39,6 @@
>  #include <linux/barebox-wrapper.h>
>
>  #include "musb_core.h"
> -#include "phy-am335x.h"
>
>  static __maybe_unused struct of_device_id musb_dsps_dt_ids[];
>
> @@ -217,10 +216,6 @@ static int dsps_musb_init(struct musb *musb)
>         const struct dsps_musb_wrapper *wrp = glue->wrp;
>         u32 rev, val, mode;
>
> -       musb->xceiv = am335x_get_usb_phy();
> -       if (IS_ERR(musb->xceiv))
> -               return PTR_ERR(musb->xceiv);
> -
>         /* Returns zero if e.g. not clocked */
>         rev = dsps_readl(musb->ctrl_base, wrp->revision);
>         if (!rev)
> @@ -324,6 +319,8 @@ static int dsps_probe(struct device_d *dev)
>         struct musb_hdrc_config *config;
>         struct device_node *dn = dev->device_node;
>         const struct dsps_musb_wrapper *wrp;
> +       struct device_node *phy_node;
> +       struct device_d *phy_dev;
>         struct dsps_glue *glue;
>         int ret;
>
> @@ -337,6 +334,14 @@ static int dsps_probe(struct device_d *dev)
>                 return -ENODEV;
>         }
>
> +       phy_node = of_parse_phandle(dn, "phys", 0);
> +       if (!phy_node)
> +               return -ENODEV;
> +
> +       phy_dev = of_find_device_by_node(phy_node);
> +       if (!phy_dev || !phy_dev->priv)
> +               return -EPROBE_DEFER;
> +
>         /* allocate glue */
>         glue = kzalloc(sizeof(*glue), GFP_KERNEL);
>         if (!glue) {
> @@ -360,6 +365,7 @@ static int dsps_probe(struct device_d *dev)
>         glue->musb.ctrl_base = IOMEM(iores->start);
>
>         glue->musb.controller = dev;
> +       glue->musb.xceiv = phy_dev->priv;
>
>         config = &glue->config;
>
> diff --git a/drivers/usb/musb/phy-am335x.c b/drivers/usb/musb/phy-am335x.c
> index df31255d891c..6991f4402d3f 100644
> --- a/drivers/usb/musb/phy-am335x.c
> +++ b/drivers/usb/musb/phy-am335x.c
> @@ -5,7 +5,6 @@
>  #include <linux/err.h>
>  #include "am35x-phy-control.h"
>  #include "musb_core.h"
> -#include "phy-am335x.h"
>
>  struct am335x_usbphy {
>         void __iomem *base;
> @@ -14,13 +13,6 @@ struct am335x_usbphy {
>         struct usb_phy phy;
>  };
>
> -static struct am335x_usbphy *am_usbphy;
> -
> -struct usb_phy *am335x_get_usb_phy(void)
> -{
> -       return &am_usbphy->phy;
> -}
> -
>  static int am335x_init(struct usb_phy *phy)
>  {
>         struct am335x_usbphy *am_usbphy = container_of(phy, struct am335x_usbphy, phy);
> @@ -31,6 +23,7 @@ static int am335x_init(struct usb_phy *phy)
>
>  static int am335x_phy_probe(struct device_d *dev)
>  {
> +       struct am335x_usbphy *am_usbphy;
>         struct resource *iores;
>         int ret;
>
> @@ -54,7 +47,7 @@ static int am335x_phy_probe(struct device_d *dev)
>         }
>
>         am_usbphy->phy.init = am335x_init;
> -       dev->priv = am_usbphy;
> +       dev->priv = &am_usbphy->phy;
>
>         dev_info(dev, "am_usbphy %p enabled\n", &am_usbphy->phy);
>
> diff --git a/drivers/usb/musb/phy-am335x.h b/drivers/usb/musb/phy-am335x.h
> deleted file mode 100644
> index 27da2e3b1057..000000000000
> --- a/drivers/usb/musb/phy-am335x.h
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -#ifndef _PHY_AM335x_H_
> -#define _PHY_AM335x_H_
> -
> -struct usb_phy *am335x_get_usb_phy(void);
> -
> -#endif
> --
> 2.25.0
>
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] USB: MUSB: PHY: scrap singleton am335x_get_usb_phy()

[Sascha Hauer]

On Tue, Feb 25, 2020 at 05:57:30PM +0100, Ahmad Fatoum wrote:
> am335x_get_usb_phy() retrieves the last probed USB phy. On the BeagleBone
> with both PHYs enabled, this means that dependent on probe order, both
> MUSB instances could end up with the same PHY.
> 
> Remove the global variable and have the MUSB driver parse the "phys"
> property instead.
> 
> The cleaner way to achieve this would be to migrate phy-am335x.c
> and phy-am335x-control.c to the generic phy framework and have MUSB use
> of_phy_get, alas, even Linux hasn't done this so far and we need
> a short patch for master anyway, thus just do it the easy way.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox