[U-Boot] [PATCH 0/8] musb: sunxi: Few fixes/improvements

[U-Boot] [PATCH 0/8] musb: sunxi: Few fixes/improvements

[Jagan Teki]

All these changes mostly related to musb/sunxi.c and 
msub core files with few fixes on sunxi front and
some code improvements on musb while handling shutdown code.

One patch mark it as 'RFC' about "including UCLASS_USB_DEV_GENERIC
into shutdown caller" so expecting some inputs on the same.

patch 1, fix improper musb host pointer

patch 2, musb-new/sunxi allocate phy in private

patch 3, use phy_passby even for PHY#0

patch 4, don't set usb_clk_cfg in PHY probe 

patch 5, update PHY#4 rst_mask for H3_H5

patch 6, shutdown UCLASS_USB_DEV_GENERIC

patch 7, musb-new/sunxi add proper exit

patch 8, call musb_platform_exit from musb_stop

Jagan Teki (8):
  usb: musb-new: Fix improper musb host pointer
  usb: musb-new: sunxi: Allocate struct phy in private
  phy: sun4i-usb: Call phy_passby even for PHY#0
  phy: sun4i-usb: Remove usb_clk_cfg set in probe
  phy: sun4i-usb: Update PHY#3 rst_mask only for H3_H5
  dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown
  usb: musb-new: sunxi: Add proper musb exit support
  usb: musb-new: core: Call musb_platform_exit from musb_stop

 board/compulab/cm_t3517/cm_t3517.c    |  4 +-
 drivers/phy/allwinner/phy-sun4i-usb.c |  7 +--
 drivers/usb/host/usb-uclass.c         | 43 +++++++++++++++
 drivers/usb/musb-new/musb_core.c      |  1 +
 drivers/usb/musb-new/musb_uboot.c     | 10 ++--
 drivers/usb/musb-new/pic32.c          |  6 ++-
 drivers/usb/musb-new/sunxi.c          | 76 +++++++++++++++------------
 include/linux/usb/musb.h              |  4 +-
 8 files changed, 101 insertions(+), 50 deletions(-)

-- 
2.17.1

[U-Boot] [PATCH 1/8] usb: musb-new: Fix improper musb host pointer

[Jagan Teki]

When MUSB is operating in peripheral mode, probe registering
musb core using musb_register which intern return int value
for validation. so there is no scope to preserve struct musb
pointer but the same can be used in .remove musb_stop.
So fix this by return musb_register with struct musb pointer.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 board/compulab/cm_t3517/cm_t3517.c |  4 ++--
 drivers/usb/musb-new/musb_uboot.c  | 10 +++++-----
 drivers/usb/musb-new/pic32.c       |  6 ++++--
 drivers/usb/musb-new/sunxi.c       |  8 +++++---
 include/linux/usb/musb.h           |  4 ++--
 5 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/board/compulab/cm_t3517/cm_t3517.c b/board/compulab/cm_t3517/cm_t3517.c
index 09cb27def0..668bb7631a 100644
--- a/board/compulab/cm_t3517/cm_t3517.c
+++ b/board/compulab/cm_t3517/cm_t3517.c
@@ -74,8 +74,8 @@ static void cm_t3517_musb_init(void)
 			CONF2_REFFREQ_13MHZ | CONF2_SESENDEN |
 			CONF2_VBDTCTEN | CONF2_DATPOL);
 
-	if (musb_register(&cm_t3517_musb_pdata, &cm_t3517_musb_board_data,
-			  (void *)AM35XX_IPSS_USBOTGSS_BASE))
+	if (!musb_register(&cm_t3517_musb_pdata, &cm_t3517_musb_board_data,
+			   (void *)AM35XX_IPSS_USBOTGSS_BASE))
 		printf("Failed initializing AM35x MUSB!\n");
 }
 #else
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
index 2b04fbd046..3cd89793c9 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -419,8 +419,8 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
 }
 #endif /* CONFIG_USB_MUSB_GADGET */
 
-int musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
-			void *ctl_regs)
+struct musb *musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
+			   void *ctl_regs)
 {
 	struct musb **musbp;
 
@@ -436,14 +436,14 @@ int musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
 		break;
 #endif
 	default:
-		return -EINVAL;
+		return NULL;
 	}
 
 	*musbp = musb_init_controller(plat, (struct device *)bdata, ctl_regs);
 	if (!*musbp) {
 		printf("Failed to init the controller\n");
-		return -EIO;
+		return NULL;
 	}
 
-	return 0;
+	return *musbp;
 }
diff --git a/drivers/usb/musb-new/pic32.c b/drivers/usb/musb-new/pic32.c
index f04719d7af..3a19900e21 100644
--- a/drivers/usb/musb-new/pic32.c
+++ b/drivers/usb/musb-new/pic32.c
@@ -251,9 +251,11 @@ static int musb_usb_probe(struct udevice *dev)
 	ret = musb_lowlevel_init(mdata);
 #else
 	pic32_musb_plat.mode = MUSB_PERIPHERAL;
-	ret = musb_register(&pic32_musb_plat, &pdata->dev, mregs);
+	mdata->host = musb_register(&pic32_musb_plat, &pdata->dev, mregs);
+	if (!mdata->host)
+		return -EIO;
 #endif
-	if (ret == 0)
+	if ((ret == 0) && mdata->host)
 		printf("PIC32 MUSB OTG\n");
 
 	return ret;
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index 08de9c69c7..b846afb094 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -444,9 +444,11 @@ static int musb_usb_probe(struct udevice *dev)
 		printf("Allwinner mUSB OTG (Host)\n");
 #else
 	pdata.mode = MUSB_PERIPHERAL;
-	ret = musb_register(&pdata, &glue->dev, base);
-	if (!ret)
-		printf("Allwinner mUSB OTG (Peripheral)\n");
+	host->host = musb_register(&pdata, &glue->dev, base);
+	if (!host->host)
+		return -EIO;
+
+	printf("Allwinner mUSB OTG (Peripheral)\n");
 #endif
 
 	return ret;
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 9104414cf0..a31ce67a81 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -150,7 +150,7 @@ extern int tusb6010_platform_retime(unsigned is_refclk);
 /*
  * U-Boot specfic stuff
  */
-int musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
-			void *ctl_regs);
+struct musb *musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
+			   void *ctl_regs);
 
 #endif /* __LINUX_USB_MUSB_H */
-- 
2.17.1

[U-Boot] [linux-sunxi] [PATCH 1/8] usb: musb-new: Fix improper musb host pointer

[Julian Calaby]

Hi Jagan,

On Tue, Jul 10, 2018 at 5:17 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> When MUSB is operating in peripheral mode, probe registering
> musb core using musb_register which intern return int value
> for validation. so there is no scope to preserve struct musb
> pointer but the same can be used in .remove musb_stop.
> So fix this by return musb_register with struct musb pointer.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  board/compulab/cm_t3517/cm_t3517.c |  4 ++--
>  drivers/usb/musb-new/musb_uboot.c  | 10 +++++-----
>  drivers/usb/musb-new/pic32.c       |  6 ++++--
>  drivers/usb/musb-new/sunxi.c       |  8 +++++---
>  include/linux/usb/musb.h           |  4 ++--
>  5 files changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
> index 2b04fbd046..3cd89793c9 100644
> --- a/drivers/usb/musb-new/musb_uboot.c
> +++ b/drivers/usb/musb-new/musb_uboot.c
> @@ -436,14 +436,14 @@ int musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
>                 break;
>  #endif
>         default:
> -               return -EINVAL;
> +               return NULL;

These error constants appear to have been chosen specifically, so
would it be nicer to preserve them and use ERR_PTR() and friends to
return them?

(I know nothing checks them today, but they might be useful in the future)

>         }
>
>         *musbp = musb_init_controller(plat, (struct device *)bdata, ctl_regs);
>         if (!*musbp) {
>                 printf("Failed to init the controller\n");
> -               return -EIO;
> +               return NULL;
>         }
>
> -       return 0;
> +       return *musbp;
>  }

Thanks,

-- 
Julian Calaby

Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

[U-Boot] [linux-sunxi] [PATCH 1/8] usb: musb-new: Fix improper musb host pointer

[Julian Calaby]

Hi,

Sorry, please ignore this, I didn't realise this patch was for u-boot.

On Tue, Jul 17, 2018 at 10:45 PM Julian Calaby <julian.calaby@gmail.com> wrote:
>
> Hi Jagan,
>
> On Tue, Jul 10, 2018 at 5:17 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > When MUSB is operating in peripheral mode, probe registering
> > musb core using musb_register which intern return int value
> > for validation. so there is no scope to preserve struct musb
> > pointer but the same can be used in .remove musb_stop.
> > So fix this by return musb_register with struct musb pointer.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  board/compulab/cm_t3517/cm_t3517.c |  4 ++--
> >  drivers/usb/musb-new/musb_uboot.c  | 10 +++++-----
> >  drivers/usb/musb-new/pic32.c       |  6 ++++--
> >  drivers/usb/musb-new/sunxi.c       |  8 +++++---
> >  include/linux/usb/musb.h           |  4 ++--
> >  5 files changed, 18 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
> > index 2b04fbd046..3cd89793c9 100644
> > --- a/drivers/usb/musb-new/musb_uboot.c
> > +++ b/drivers/usb/musb-new/musb_uboot.c
> > @@ -436,14 +436,14 @@ int musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
> >                 break;
> >  #endif
> >         default:
> > -               return -EINVAL;
> > +               return NULL;
>
> These error constants appear to have been chosen specifically, so
> would it be nicer to preserve them and use ERR_PTR() and friends to
> return them?
>
> (I know nothing checks them today, but they might be useful in the future)
>
> >         }
> >
> >         *musbp = musb_init_controller(plat, (struct device *)bdata, ctl_regs);
> >         if (!*musbp) {
> >                 printf("Failed to init the controller\n");
> > -               return -EIO;
> > +               return NULL;
> >         }
> >
> > -       return 0;
> > +       return *musbp;
> >  }

Thanks,

-- 
Julian Calaby

Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

[U-Boot] [PATCH 2/8] usb: musb-new: sunxi: Allocate struct phy in private

[Jagan Teki]

Allocate struct phy in private structure instead of allocating
locally and assign it to a pointer. This eventually fix miss
alignment phy which is used in another functions.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/usb/musb-new/sunxi.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index b846afb094..aa2880eeb9 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -87,7 +87,7 @@ struct sunxi_glue {
 	struct sunxi_ccm_reg *ccm;
 	struct sunxi_musb_config *cfg;
 	struct device dev;
-	struct phy *phy;
+	struct phy phy;
 };
 #define to_sunxi_glue(d)	container_of(d, struct sunxi_glue, dev)
 
@@ -235,19 +235,19 @@ static int sunxi_musb_enable(struct musb *musb)
 	musb_writeb(musb->mregs, USBC_REG_o_VEND0, 0);
 
 	if (is_host_enabled(musb)) {
-		ret = sun4i_usb_phy_vbus_detect(glue->phy);
+		ret = sun4i_usb_phy_vbus_detect(&glue->phy);
 		if (ret == 1) {
 			printf("A charger is plugged into the OTG: ");
 			return -ENODEV;
 		}
 
-		ret = sun4i_usb_phy_id_detect(glue->phy);
+		ret = sun4i_usb_phy_id_detect(&glue->phy);
 		if (ret == 1) {
 			printf("No host cable detected: ");
 			return -ENODEV;
 		}
 
-		ret = generic_phy_power_on(glue->phy);
+		ret = generic_phy_power_on(&glue->phy);
 		if (ret) {
 			pr_err("failed to power on USB PHY\n");
 			return ret;
@@ -271,7 +271,7 @@ static void sunxi_musb_disable(struct musb *musb)
 		return;
 
 	if (is_host_enabled(musb)) {
-		ret = generic_phy_power_off(glue->phy);
+		ret = generic_phy_power_off(&glue->phy);
 		if (ret) {
 			pr_err("failed to power off USB PHY\n");
 			return;
@@ -291,7 +291,7 @@ static int sunxi_musb_init(struct musb *musb)
 
 	pr_debug("%s():\n", __func__);
 
-	ret = generic_phy_init(glue->phy);
+	ret = generic_phy_init(&glue->phy);
 	if (ret) {
 		pr_err("failed to init USB PHY\n");
 		return ret;
@@ -330,14 +330,14 @@ static void sunxi_musb_pre_root_reset_end(struct musb *musb)
 {
 	struct sunxi_glue *glue = to_sunxi_glue(musb->controller);
 
-	sun4i_usb_phy_set_squelch_detect(glue->phy, false);
+	sun4i_usb_phy_set_squelch_detect(&glue->phy, false);
 }
 
 static void sunxi_musb_post_root_reset_end(struct musb *musb)
 {
 	struct sunxi_glue *glue = to_sunxi_glue(musb->controller);
 
-	sun4i_usb_phy_set_squelch_detect(glue->phy, true);
+	sun4i_usb_phy_set_squelch_detect(&glue->phy, true);
 }
 
 static const struct musb_platform_ops sunxi_musb_ops = {
@@ -405,7 +405,6 @@ static int musb_usb_probe(struct udevice *dev)
 	struct usb_bus_priv *priv = dev_get_uclass_priv(dev);
 	struct musb_hdrc_platform_data pdata;
 	void *base = dev_read_addr_ptr(dev);
-	struct phy phy;
 	int ret;
 
 	if (!base)
@@ -419,13 +418,12 @@ static int musb_usb_probe(struct udevice *dev)
 	if (IS_ERR(glue->ccm))
 		return PTR_ERR(glue->ccm);
 
-	ret = generic_phy_get_by_name(dev, "usb", &phy);
+	ret = generic_phy_get_by_name(dev, "usb", &glue->phy);
 	if (ret) {
 		pr_err("failed to get usb PHY\n");
 		return ret;
 	}
 
-	glue->phy = &phy;
 	priv->desc_before_addr = true;
 
 	memset(&pdata, 0, sizeof(pdata));
@@ -460,8 +458,8 @@ static int musb_usb_remove(struct udevice *dev)
 	struct musb_host_data *host = &glue->mdata;
 	int ret;
 
-	if (generic_phy_valid(glue->phy)) {
-		ret = generic_phy_exit(glue->phy);
+	if (generic_phy_valid(&glue->phy)) {
+		ret = generic_phy_exit(&glue->phy);
 		if (ret) {
 			pr_err("failed to exit %s USB PHY\n", dev->name);
 			return ret;
-- 
2.17.1

[U-Boot] [PATCH 3/8] phy: sun4i-usb: Call phy_passby even for PHY#0

[Jagan Teki]

phy_passby is not calling for phy id 0 in legacy code,
which has no information on why it restricted.
Since Linux phy driver doesn't restrict like this,
so call it directly with out any condition

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/phy/allwinner/phy-sun4i-usb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
index 2b3cf48025..01f585a283 100644
--- a/drivers/phy/allwinner/phy-sun4i-usb.c
+++ b/drivers/phy/allwinner/phy-sun4i-usb.c
@@ -300,8 +300,7 @@ static int sun4i_usb_phy_init(struct phy *phy)
 				    data->cfg->disc_thresh, PHY_DISCON_TH_LEN);
 	}
 
-	if (usb_phy->id != 0)
-		sun4i_usb_phy_passby(phy, true);
+	sun4i_usb_phy_passby(phy, true);
 
 	sun4i_usb_phy0_reroute(data, true);
 
-- 
2.17.1

[U-Boot] [PATCH 3/8] phy: sun4i-usb: Call phy_passby even for PHY#0

[Chen-Yu Tsai]

On Tue, Jul 10, 2018 at 3:17 AM, Jagan Teki <jagan@amarulasolutions.com> wrote:
> phy_passby is not calling for phy id 0 in legacy code,
> which has no information on why it restricted.
> Since Linux phy driver doesn't restrict like this,
> so call it directly with out any condition

See

https://github.com/wens/u-boot-sunxi/commit/b1d3cb2973e75097bbee1835efdc07afb8a7ef2f

for a better commit message. This is an older version. I tried
rebasing on the latest version, but apparently there are merge
conflicts between master and sunxi/next. Even worse, it fails
to boot properly.

Please try to understand why the code is what it is. The reason we
started out with not calling it for PHY 0 is because in the past
USB0 did not have EHCI/OHCI host pairs, which in turn means there's
no "pmu" (whatever that is) register associated with that PHY.

In time, the check for the pointer to that register was moved into
sun4i_usb_phy_passby() itself, which directly returns if that pointer
is not set. And the PHY driver only sets that pointer if a "pmuX"
resource for X PHY is provided.

ChenYu

> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/phy/allwinner/phy-sun4i-usb.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> index 2b3cf48025..01f585a283 100644
> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> @@ -300,8 +300,7 @@ static int sun4i_usb_phy_init(struct phy *phy)
>                                     data->cfg->disc_thresh, PHY_DISCON_TH_LEN);
>         }
>
> -       if (usb_phy->id != 0)
> -               sun4i_usb_phy_passby(phy, true);
> +       sun4i_usb_phy_passby(phy, true);
>
>         sun4i_usb_phy0_reroute(data, true);
>
> --
> 2.17.1
>

[U-Boot] [PATCH 4/8] phy: sun4i-usb: Remove usb_clk_cfg set in probe

[Jagan Teki]

usb_clk_cfg is setting CTRL_PHYGATE bit value in probe
which is BIT 0 for sun4i, 6i and 8 for a83t but all
these were handling in phy ops init exit calls.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/phy/allwinner/phy-sun4i-usb.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
index 01f585a283..3096f12c1c 100644
--- a/drivers/phy/allwinner/phy-sun4i-usb.c
+++ b/drivers/phy/allwinner/phy-sun4i-usb.c
@@ -462,8 +462,6 @@ static int sun4i_usb_phy_probe(struct udevice *dev)
 		phy->rst_mask = info->rst_mask;
 	};
 
-	setbits_le32(&data->ccm->usb_clk_cfg, CCM_USB_CTRL_PHYGATE);
-
 	debug("Allwinner Sun4I USB PHY driver loaded\n");
 	return 0;
 }
-- 
2.17.1

[U-Boot] [PATCH 5/8] phy: sun4i-usb: Update PHY#3 rst_mask only for H3_H5

[Jagan Teki]

Only H3 and H5 have 4 PHYS so restrict rst_mask only for them.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/phy/allwinner/phy-sun4i-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
index 3096f12c1c..81fcd1f910 100644
--- a/drivers/phy/allwinner/phy-sun4i-usb.c
+++ b/drivers/phy/allwinner/phy-sun4i-usb.c
@@ -117,7 +117,7 @@ struct sun4i_usb_phy_info {
 		.gpio_vbus = CONFIG_USB3_VBUS_PIN,
 		.gpio_vbus_det = NULL,
 		.gpio_id_det = NULL,
-#ifdef CONFIG_MACH_SUN6I
+#ifdef CONFIG_MACH_SUNXI_H3_H5
 		.rst_mask = (CCM_USB_CTRL_PHY3_RST | CCM_USB_CTRL_PHY3_CLK),
 #endif
 	},
-- 
2.17.1

[U-Boot] [PATCH 5/8] phy: sun4i-usb: Update PHY#3 rst_mask only for H3_H5

[Vasily Khoruzhick]

On Mon, Jul 9, 2018 at 12:17 PM, Jagan Teki <jagan@amarulasolutions.com> wrote:
> Only H3 and H5 have 4 PHYS so restrict rst_mask only for them.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/phy/allwinner/phy-sun4i-usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> index 3096f12c1c..81fcd1f910 100644
> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> @@ -117,7 +117,7 @@ struct sun4i_usb_phy_info {
>                 .gpio_vbus = CONFIG_USB3_VBUS_PIN,
>                 .gpio_vbus_det = NULL,
>                 .gpio_id_det = NULL,
> -#ifdef CONFIG_MACH_SUN6I
> +#ifdef CONFIG_MACH_SUNXI_H3_H5

It should be handled by compatibles, not ifdefs. Phy driver is proper
DM driver, so you can use .data in device ids array.

>                 .rst_mask = (CCM_USB_CTRL_PHY3_RST | CCM_USB_CTRL_PHY3_CLK),
>  #endif
>         },
> --
> 2.17.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 5/8] phy: sun4i-usb: Update PHY#3 rst_mask only for H3_H5

[Jagan Teki]

On Wed, Jul 11, 2018 at 12:23 PM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> On Mon, Jul 9, 2018 at 12:17 PM, Jagan Teki <jagan@amarulasolutions.com> wrote:
>> Only H3 and H5 have 4 PHYS so restrict rst_mask only for them.
>>
>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>> ---
>>  drivers/phy/allwinner/phy-sun4i-usb.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
>> index 3096f12c1c..81fcd1f910 100644
>> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
>> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
>> @@ -117,7 +117,7 @@ struct sun4i_usb_phy_info {
>>                 .gpio_vbus = CONFIG_USB3_VBUS_PIN,
>>                 .gpio_vbus_det = NULL,
>>                 .gpio_id_det = NULL,
>> -#ifdef CONFIG_MACH_SUN6I
>> +#ifdef CONFIG_MACH_SUNXI_H3_H5
>
> It should be handled by compatibles, not ifdefs. Phy driver is proper
> DM driver, so you can use .data in device ids array.

True, but since it' already have ifdef and more over this code will
drop very soon, we are planning to support CLK and RST[1] (max of
v2018.11).

[1] https://github.com/amarula/u-boot-amarula/commits/sun-dev

[U-Boot] [PATCH 5/8] phy: sun4i-usb: Update PHY#3 rst_mask only for H3_H5

[Maxime Ripard]

On Wed, Jul 11, 2018 at 12:37:04PM +0530, Jagan Teki wrote:
> On Wed, Jul 11, 2018 at 12:23 PM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> > On Mon, Jul 9, 2018 at 12:17 PM, Jagan Teki <jagan@amarulasolutions.com> wrote:
> >> Only H3 and H5 have 4 PHYS so restrict rst_mask only for them.
> >>
> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >> ---
> >>  drivers/phy/allwinner/phy-sun4i-usb.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> >> index 3096f12c1c..81fcd1f910 100644
> >> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> >> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> >> @@ -117,7 +117,7 @@ struct sun4i_usb_phy_info {
> >>                 .gpio_vbus = CONFIG_USB3_VBUS_PIN,
> >>                 .gpio_vbus_det = NULL,
> >>                 .gpio_id_det = NULL,
> >> -#ifdef CONFIG_MACH_SUN6I
> >> +#ifdef CONFIG_MACH_SUNXI_H3_H5
> >
> > It should be handled by compatibles, not ifdefs. Phy driver is proper
> > DM driver, so you can use .data in device ids array.
> 
> True, but since it' already have ifdef and more over this code will
> drop very soon, we are planning to support CLK and RST[1] (max of
> v2018.11).

Says who?

I'd really prefer to have something when it's ready, rather than
trying to push to meet a deadline that we enforced on ourselves, and
end up with a tons of bugs to fix. The latest PHY patches should be a
pretty good example on why we *shouldn't* do that.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180711/05fbe719/attachment.sig>

[U-Boot] [PATCH 5/8] phy: sun4i-usb: Update PHY#3 rst_mask only for H3_H5

[Jagan Teki]

On Wed, Jul 11, 2018 at 12:44 PM, Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
> On Wed, Jul 11, 2018 at 12:37:04PM +0530, Jagan Teki wrote:
>> On Wed, Jul 11, 2018 at 12:23 PM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>> > On Mon, Jul 9, 2018 at 12:17 PM, Jagan Teki <jagan@amarulasolutions.com> wrote:
>> >> Only H3 and H5 have 4 PHYS so restrict rst_mask only for them.
>> >>
>> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>> >> ---
>> >>  drivers/phy/allwinner/phy-sun4i-usb.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
>> >> index 3096f12c1c..81fcd1f910 100644
>> >> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
>> >> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
>> >> @@ -117,7 +117,7 @@ struct sun4i_usb_phy_info {
>> >>                 .gpio_vbus = CONFIG_USB3_VBUS_PIN,
>> >>                 .gpio_vbus_det = NULL,
>> >>                 .gpio_id_det = NULL,
>> >> -#ifdef CONFIG_MACH_SUN6I
>> >> +#ifdef CONFIG_MACH_SUNXI_H3_H5
>> >
>> > It should be handled by compatibles, not ifdefs. Phy driver is proper
>> > DM driver, so you can use .data in device ids array.
>>
>> True, but since it' already have ifdef and more over this code will
>> drop very soon, we are planning to support CLK and RST[1] (max of
>> v2018.11).
>
> Says who?

Me with USB clocks and resets.

>
> I'd really prefer to have something when it's ready, rather than
> trying to push to meet a deadline that we enforced on ourselves, and
> end up with a tons of bugs to fix. The latest PHY patches should be a
> pretty good example on why we *shouldn't* do that.

These series fixes not certainly related to sunxi because of recent
changes, were there in musb shutdown code and PHY changes were there
before with legacy.

[U-Boot] [PATCH 5/8] phy: sun4i-usb: Update PHY#3 rst_mask only for H3_H5

[Maxime Ripard]

On Wed, Jul 11, 2018 at 12:52:07PM +0530, Jagan Teki wrote:
> On Wed, Jul 11, 2018 at 12:44 PM, Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> > On Wed, Jul 11, 2018 at 12:37:04PM +0530, Jagan Teki wrote:
> >> On Wed, Jul 11, 2018 at 12:23 PM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> >> > On Mon, Jul 9, 2018 at 12:17 PM, Jagan Teki <jagan@amarulasolutions.com> wrote:
> >> >> Only H3 and H5 have 4 PHYS so restrict rst_mask only for them.
> >> >>
> >> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >> >> ---
> >> >>  drivers/phy/allwinner/phy-sun4i-usb.c | 2 +-
> >> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> >> >> index 3096f12c1c..81fcd1f910 100644
> >> >> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> >> >> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> >> >> @@ -117,7 +117,7 @@ struct sun4i_usb_phy_info {
> >> >>                 .gpio_vbus = CONFIG_USB3_VBUS_PIN,
> >> >>                 .gpio_vbus_det = NULL,
> >> >>                 .gpio_id_det = NULL,
> >> >> -#ifdef CONFIG_MACH_SUN6I
> >> >> +#ifdef CONFIG_MACH_SUNXI_H3_H5
> >> >
> >> > It should be handled by compatibles, not ifdefs. Phy driver is proper
> >> > DM driver, so you can use .data in device ids array.
> >>
> >> True, but since it' already have ifdef and more over this code will
> >> drop very soon, we are planning to support CLK and RST[1] (max of
> >> v2018.11).
> >
> > Says who?
> 
> Me with USB clocks and resets.
> 
> > I'd really prefer to have something when it's ready, rather than
> > trying to push to meet a deadline that we enforced on ourselves, and
> > end up with a tons of bugs to fix. The latest PHY patches should be a
> > pretty good example on why we *shouldn't* do that.
> 
> These series fixes not certainly related to sunxi because of recent
> changes, were there in musb shutdown code and PHY changes were there
> before with legacy.

Well, all the fallout with EHCI being broken surely wasn't there
before. It took us a year or so to have support for all the SoCs we
support in Linux and have a somewhat robust, complete clock
support. Thinking that it can be done and merged within 3 monthes is
unreasonable.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180711/484a8e98/attachment.sig>

[U-Boot] [PATCH RFC 6/8] dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown

[Jagan Teki]

Some OTG controllers which operates on Peripheral
mode are registered as UCLASS_USB_DEV_GENERIC.

So add support to shutdown them as well. shutdown
happened during 'usb reset' and U-Boot handoff code
for Linux boot.

controller restarting in 'usb reset' like probing
again is still missing for this type of UCLASS.

Cc: Simon Glass <sjg@chromium.org>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Note:
I tried of traversing for multiple UCLASS in removal
code in usb_stop, but couldn't come with proper solutions.
I don't think any other area of code need a requirement like
this, or may be I'm missing. any help would appreciate. 

 drivers/usb/host/usb-uclass.c | 43 +++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 611ea97a72..99cf3d2b49 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -158,6 +158,45 @@ int usb_get_max_xfer_size(struct usb_device *udev, size_t *size)
 	return ops->get_max_xfer_size(bus, size);
 }
 
+int __usb_stop(void)
+{
+	struct udevice *bus;
+	struct udevice *rh;
+	struct uclass *uc;
+	struct usb_uclass_priv *uc_priv;
+	int err = 0, ret;
+
+	/* De-activate any devices that have been activated */
+	ret = uclass_get(UCLASS_USB_DEV_GENERIC, &uc);
+	if (ret)
+		return ret;
+
+	uc_priv = uc->priv;
+
+	uclass_foreach_dev(bus, uc) {
+		ret = device_remove(bus, DM_REMOVE_NORMAL);
+		if (ret && !err)
+			err = ret;
+
+		/* Locate root hub device */
+		device_find_first_child(bus, &rh);
+		if (rh) {
+			/*
+			 * All USB devices are children of root hub.
+			 * Unbinding root hub will unbind all of its children.
+			 */
+			ret = device_unbind(rh);
+			if (ret && !err)
+				err = ret;
+		}
+	}
+
+	uc_priv->companion_device_count = 0;
+	usb_started = 0;
+
+	return err;
+}
+
 int usb_stop(void)
 {
 	struct udevice *bus;
@@ -166,6 +205,10 @@ int usb_stop(void)
 	struct usb_uclass_priv *uc_priv;
 	int err = 0, ret;
 
+	ret = __usb_stop();
+	if (ret)
+		return ret;
+
 	/* De-activate any devices that have been activated */
 	ret = uclass_get(UCLASS_USB, &uc);
 	if (ret)
-- 
2.17.1

[U-Boot] [PATCH 7/8] usb: musb-new: sunxi: Add proper musb exit support

[Jagan Teki]

musb have platform ops to do proper graceful exit,
so add the exit call and move musb platform exit code
instead of keeping it in driver remove.
This make proper shutdown of musb where .remove will
call disable, exit serially via musb_stop.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/usb/musb-new/sunxi.c | 48 +++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index aa2880eeb9..9f71b84fd1 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -326,6 +326,33 @@ static int sunxi_musb_init(struct musb *musb)
 	return 0;
 }
 
+static int sunxi_musb_exit(struct musb *musb)
+{
+	struct sunxi_glue *glue = to_sunxi_glue(musb->controller);
+	int ret = 0;
+
+	if (generic_phy_valid(&glue->phy)) {
+		ret = generic_phy_exit(&glue->phy);
+		if (ret) {
+			dev_err(dev, "failed to power off usb phy\n");
+			return ret;
+		}
+	}
+
+#ifdef CONFIG_SUNXI_GEN_SUN6I
+	clrbits_le32(&glue->ccm->ahb_reset0_cfg, BIT(AHB_GATE_OFFSET_USB0));
+	if (glue->cfg->rst_bit)
+		clrbits_le32(&glue->ccm->ahb_reset0_cfg,
+			     BIT(glue->cfg->rst_bit));
+#endif
+	clrbits_le32(&glue->ccm->ahb_gate0, BIT(AHB_GATE_OFFSET_USB0));
+	if (glue->cfg->clkgate_bit)
+		clrbits_le32(&glue->ccm->ahb_gate0,
+			     BIT(glue->cfg->clkgate_bit));
+
+	return 0;
+}
+
 static void sunxi_musb_pre_root_reset_end(struct musb *musb)
 {
 	struct sunxi_glue *glue = to_sunxi_glue(musb->controller);
@@ -342,6 +369,7 @@ static void sunxi_musb_post_root_reset_end(struct musb *musb)
 
 static const struct musb_platform_ops sunxi_musb_ops = {
 	.init		= sunxi_musb_init,
+	.exit		= sunxi_musb_exit,
 	.enable		= sunxi_musb_enable,
 	.disable	= sunxi_musb_disable,
 	.pre_root_reset_end = sunxi_musb_pre_root_reset_end,
@@ -456,29 +484,9 @@ static int musb_usb_remove(struct udevice *dev)
 {
 	struct sunxi_glue *glue = dev_get_priv(dev);
 	struct musb_host_data *host = &glue->mdata;
-	int ret;
-
-	if (generic_phy_valid(&glue->phy)) {
-		ret = generic_phy_exit(&glue->phy);
-		if (ret) {
-			pr_err("failed to exit %s USB PHY\n", dev->name);
-			return ret;
-		}
-	}
 
 	musb_stop(host->host);
 
-#ifdef CONFIG_SUNXI_GEN_SUN6I
-	clrbits_le32(&glue->ccm->ahb_reset0_cfg, BIT(AHB_GATE_OFFSET_USB0));
-	if (glue->cfg->rst_bit)
-		clrbits_le32(&glue->ccm->ahb_reset0_cfg,
-			     BIT(glue->cfg->rst_bit));
-#endif
-	clrbits_le32(&glue->ccm->ahb_gate0, BIT(AHB_GATE_OFFSET_USB0));
-	if (glue->cfg->clkgate_bit)
-		clrbits_le32(&glue->ccm->ahb_gate0,
-			     BIT(glue->cfg->clkgate_bit));
-
 	free(host->host);
 	host->host = NULL;
 
-- 
2.17.1

[U-Boot] [PATCH 7/8] usb: musb-new: sunxi: Add proper musb exit support

[Vasily Khoruzhick]

On Mon, Jul 9, 2018 at 12:17 PM, Jagan Teki <jagan@amarulasolutions.com> wrote:
> musb have platform ops to do proper graceful exit,
> so add the exit call and move musb platform exit code
> instead of keeping it in driver remove.
> This make proper shutdown of musb where .remove will
> call disable, exit serially via musb_stop.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/usb/musb-new/sunxi.c | 48 +++++++++++++++++++++---------------
>  1 file changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
> index aa2880eeb9..9f71b84fd1 100644
> --- a/drivers/usb/musb-new/sunxi.c
> +++ b/drivers/usb/musb-new/sunxi.c
> @@ -326,6 +326,33 @@ static int sunxi_musb_init(struct musb *musb)
>         return 0;
>  }
>
> +static int sunxi_musb_exit(struct musb *musb)
> +{
> +       struct sunxi_glue *glue = to_sunxi_glue(musb->controller);
> +       int ret = 0;
> +
> +       if (generic_phy_valid(&glue->phy)) {
> +               ret = generic_phy_exit(&glue->phy);
> +               if (ret) {
> +                       dev_err(dev, "failed to power off usb phy\n");
> +                       return ret;
> +               }
> +       }
> +
> +#ifdef CONFIG_SUNXI_GEN_SUN6I

Same here, since you're refactoring this code, it would be appropriate
to use compatibles and get rid of this ifdef.

> +       clrbits_le32(&glue->ccm->ahb_reset0_cfg, BIT(AHB_GATE_OFFSET_USB0));
> +       if (glue->cfg->rst_bit)
> +               clrbits_le32(&glue->ccm->ahb_reset0_cfg,
> +                            BIT(glue->cfg->rst_bit));
> +#endif
> +       clrbits_le32(&glue->ccm->ahb_gate0, BIT(AHB_GATE_OFFSET_USB0));
> +       if (glue->cfg->clkgate_bit)
> +               clrbits_le32(&glue->ccm->ahb_gate0,
> +                            BIT(glue->cfg->clkgate_bit));
> +
> +       return 0;
> +}
> +
>  static void sunxi_musb_pre_root_reset_end(struct musb *musb)
>  {
>         struct sunxi_glue *glue = to_sunxi_glue(musb->controller);
> @@ -342,6 +369,7 @@ static void sunxi_musb_post_root_reset_end(struct musb *musb)
>
>  static const struct musb_platform_ops sunxi_musb_ops = {
>         .init           = sunxi_musb_init,
> +       .exit           = sunxi_musb_exit,
>         .enable         = sunxi_musb_enable,
>         .disable        = sunxi_musb_disable,
>         .pre_root_reset_end = sunxi_musb_pre_root_reset_end,
> @@ -456,29 +484,9 @@ static int musb_usb_remove(struct udevice *dev)
>  {
>         struct sunxi_glue *glue = dev_get_priv(dev);
>         struct musb_host_data *host = &glue->mdata;
> -       int ret;
> -
> -       if (generic_phy_valid(&glue->phy)) {
> -               ret = generic_phy_exit(&glue->phy);
> -               if (ret) {
> -                       pr_err("failed to exit %s USB PHY\n", dev->name);
> -                       return ret;
> -               }
> -       }
>
>         musb_stop(host->host);
>
> -#ifdef CONFIG_SUNXI_GEN_SUN6I
> -       clrbits_le32(&glue->ccm->ahb_reset0_cfg, BIT(AHB_GATE_OFFSET_USB0));
> -       if (glue->cfg->rst_bit)
> -               clrbits_le32(&glue->ccm->ahb_reset0_cfg,
> -                            BIT(glue->cfg->rst_bit));
> -#endif
> -       clrbits_le32(&glue->ccm->ahb_gate0, BIT(AHB_GATE_OFFSET_USB0));
> -       if (glue->cfg->clkgate_bit)
> -               clrbits_le32(&glue->ccm->ahb_gate0,
> -                            BIT(glue->cfg->clkgate_bit));
> -
>         free(host->host);
>         host->host = NULL;
>
> --
> 2.17.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 8/8] usb: musb-new: core: Call musb_platform_exit from musb_stop

[Jagan Teki]

musb_stop is shutdown code for musb where it call platform and
generic disable. So add platform exit, for proper graceful shutdown.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/usb/musb-new/musb_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/musb-new/musb_core.c b/drivers/usb/musb-new/musb_core.c
index 8fec6f38ad..afea9fbcef 100644
--- a/drivers/usb/musb-new/musb_core.c
+++ b/drivers/usb/musb-new/musb_core.c
@@ -1007,6 +1007,7 @@ void musb_stop(struct musb *musb)
 	 *  - ...
 	 */
 	musb_platform_try_idle(musb, 0);
+	musb_platform_exit(musb);
 }
 
 #ifndef __UBOOT__
-- 
2.17.1