musb: dsps: make it work with two instances

musb: dsps: make it work with two instances

[Sebastian Andrzej Siewior]

This enables the two musb instances on am335x to work.
Could someone explain what
	ti,hwmods = "usb_otg_hs";
doing? I would want to have something like
| musb {
|	 /* glue /*
|	    {
|	    	musb child node
|            }
| }

and this twice. This would put the glue layer into the right position and
remove this nonsense and the += 0x400 chicken dance (look for
musb_core_offset).

Sebastian

[PATCH 2/5] arm: dts: am33xx: add USB phy nodes

[Sebastian Andrzej Siewior]

The memory address contains three pieces that is the reset module which
is currently the only one used and two other pices which seem
interresting based on what the register.
The phy id (0 or 1) can be obtained via of_alias_get_id(). However once
we need this, we should used something for common register access.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/arm/boot/dts/am33xx.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 0d4df90..a24f1cb 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -333,6 +333,22 @@
 			status = "disabled";
 		};
 
+		musb0_phy: phy0@47401000 {
+			compatible = "ti,am335x-usb-phy";
+			reg = <0x44e10620 0x2c		/* reset module */
+				0x47401000 0x200
+				0x47401300 0x100>;
+			status = "disabled";
+		};
+
+		musb1_phy: phy1@47401800 {
+			compatible = "ti,am335x-usb-phy";
+			reg = <0x44e10628 0x24		/* reset module */
+				0x47401800 0x200
+				0x47401b00 0x100>;
+			status = "disabled";
+		};
+
 		usb@47400000 {
 			compatible = "ti,musb-am33xx";
 			reg = <0x47400000 0x1000	/* usbss */
-- 
1.8.3.2

Re: [PATCH 2/5] arm: dts: am33xx: add USB phy nodes

[Ruchika Kharwar]

On 07/05/2013 08:32 AM, Sebastian Andrzej Siewior wrote:
> The memory address contains three pieces that is the reset module which
> is currently the only one used and two other pices which seem
> interresting based on what the register.
Please fix typos "pices.. interresting".. also the description is not clear.
> The phy id (0 or 1) can be obtained via of_alias_get_id(). However once
> we need this, we should used something for common register access.

>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> ---
>   arch/arm/boot/dts/am33xx.dtsi | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 0d4df90..a24f1cb 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -333,6 +333,22 @@
>   			status = "disabled";
>   		};
>   
> +		musb0_phy: phy0@47401000 {
> +			compatible = "ti,am335x-usb-phy";
> +			reg = <0x44e10620 0x2c		/* reset module */
> +				0x47401000 0x200
> +				0x47401300 0x100>;
> +			status = "disabled";
> +		};
> +
> +		musb1_phy: phy1@47401800 {
> +			compatible = "ti,am335x-usb-phy";
> +			reg = <0x44e10628 0x24		/* reset module */
> +				0x47401800 0x200
> +				0x47401b00 0x100>;
> +			status = "disabled";
> +		};
> +
>   		usb@47400000 {
>   			compatible = "ti,musb-am33xx";
>   			reg = <0x47400000 0x1000	/* usbss */

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] arm: dts: am33xx: add USB phy nodes

[Sebastian Andrzej Siewior]

On 07/05/2013 04:41 PM, Ruchika Kharwar wrote:
> 
> On 07/05/2013 08:32 AM, Sebastian Andrzej Siewior wrote:
>> The memory address contains three pieces that is the reset module which
>> is currently the only one used and two other pices which seem
>> interresting based on what the register.
> Please fix typos "pices.. interresting".. also the description is not
> clear.

Okay. What part is not clear?


Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] arm: dts: am33xx: add USB phy nodes

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 699 bytes --]

On Fri, Jul 05, 2013 at 04:56:17PM +0200, Sebastian Andrzej Siewior wrote:
> On 07/05/2013 04:41 PM, Ruchika Kharwar wrote:
> > 
> > On 07/05/2013 08:32 AM, Sebastian Andrzej Siewior wrote:
> >> The memory address contains three pieces that is the reset module which
> >> is currently the only one used and two other pices which seem
> >> interresting based on what the register.
> > Please fix typos "pices.. interresting".. also the description is not
> > clear.
> 
> Okay. What part is not clear?

Well, this statement:

"... which seem interresting based on what the register."

seems like it misses something. based on what register
\(does\|is\|implements\) ???

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 3/5] usb: musb: dsps: remove the hardcoded phy pieces

[Sebastian Andrzej Siewior]

dsps uses a nop driver which is added in dsps itself and does the PHY
on/off calls within dsps. Since those calls are now moved the nop driver
itself, we can now request the phy proper phy and remove those calls.
Currently only the first musb interface is used so we only add one phy
node for now.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/arm/boot/dts/am335x-bone.dts  |  4 ++
 arch/arm/boot/dts/am335x-evm.dts   |  4 ++
 arch/arm/boot/dts/am335x-evmsk.dts |  4 ++
 arch/arm/boot/dts/am33xx.dtsi      |  1 +
 drivers/usb/musb/musb_dsps.c       | 97 +-------------------------------------
 5 files changed, 14 insertions(+), 96 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
index 04feaf8..83184e5 100644
--- a/arch/arm/boot/dts/am335x-bone.dts
+++ b/arch/arm/boot/dts/am335x-bone.dts
@@ -120,6 +120,10 @@
 			status = "okay";
 		};
 
+		musb0_phy: phy0@47401000 {
+			status = "okay";
+		};
+
 		i2c0: i2c@44e0b000 {
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2c0_pins>;
diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index a16bb96..317637a 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -171,6 +171,10 @@
 			};
 		};
 
+		musb0_phy: phy0@47401000 {
+			status = "okay";
+		};
+
 		i2c1: i2c@4802a000 {
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2c1_pins>;
diff --git a/arch/arm/boot/dts/am335x-evmsk.dts b/arch/arm/boot/dts/am335x-evmsk.dts
index 9e00eef..99d9444 100644
--- a/arch/arm/boot/dts/am335x-evmsk.dts
+++ b/arch/arm/boot/dts/am335x-evmsk.dts
@@ -207,6 +207,10 @@
 			};
 		};
 
+		musb0_phy: phy0@47401000 {
+			status = "okay";
+		};
+
 		epwmss2: epwmss@48304000 {
 			status = "okay";
 
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index a24f1cb..66bb420 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -364,6 +364,7 @@
 			port1-mode = <3>;
 			power = <250>;
 			ti,hwmods = "usb_otg_hs";
+			phys = <&musb1_phy>;
 		};
 
 		epwmss0: epwmss@48300000 {
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index e1b661d..22ad532 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -123,49 +123,8 @@ struct dsps_glue {
 	const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
 	struct timer_list timer[2];	/* otg_workaround timer */
 	unsigned long last_timer[2];    /* last timer data for each instance */
-	u32 __iomem *usb_ctrl[2];
 };
 
-#define	DSPS_AM33XX_CONTROL_MODULE_PHYS_0	0x44e10620
-#define	DSPS_AM33XX_CONTROL_MODULE_PHYS_1	0x44e10628
-
-static const resource_size_t dsps_control_module_phys[] = {
-	DSPS_AM33XX_CONTROL_MODULE_PHYS_0,
-	DSPS_AM33XX_CONTROL_MODULE_PHYS_1,
-};
-
-#define USBPHY_CM_PWRDN		(1 << 0)
-#define USBPHY_OTG_PWRDN	(1 << 1)
-#define USBPHY_OTGVDET_EN	(1 << 19)
-#define USBPHY_OTGSESSEND_EN	(1 << 20)
-
-/**
- * musb_dsps_phy_control - phy on/off
- * @glue: struct dsps_glue *
- * @id: musb instance
- * @on: flag for phy to be switched on or off
- *
- * This is to enable the PHY using usb_ctrl register in system control
- * module space.
- *
- * XXX: This function will be removed once we have a seperate driver for
- * control module
- */
-static void musb_dsps_phy_control(struct dsps_glue *glue, u8 id, u8 on)
-{
-	u32 usbphycfg;
-
-	usbphycfg = readl(glue->usb_ctrl[id]);
-
-	if (on) {
-		usbphycfg &= ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN);
-		usbphycfg |= USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN;
-	} else {
-		usbphycfg |= USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN;
-	}
-
-	writel(usbphycfg, glue->usb_ctrl[id]);
-}
 /**
  * dsps_musb_enable - enable interrupts
  */
@@ -416,8 +375,7 @@ static int dsps_musb_init(struct musb *musb)
 	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);
+	musb->xceiv = devm_usb_get_phy_by_phandle(glue->dev, "phys", 0);
 	if (IS_ERR_OR_NULL(musb->xceiv))
 		return -EPROBE_DEFER;
 
@@ -433,9 +391,6 @@ static int dsps_musb_init(struct musb *musb)
 	/* Reset the musb */
 	dsps_writel(reg_base, wrp->control, (1 << wrp->reset));
 
-	/* Start the on-chip PHY and its PLL. */
-	musb_dsps_phy_control(glue, pdev->id, 1);
-
 	musb->isr = dsps_interrupt;
 
 	/* reset the otgdisable bit, needed for host mode to work */
@@ -448,8 +403,6 @@ static int dsps_musb_init(struct musb *musb)
 
 	return 0;
 err0:
-	usb_put_phy(musb->xceiv);
-	usb_nop_xceiv_unregister();
 	return status;
 }
 
@@ -461,13 +414,6 @@ static int dsps_musb_exit(struct musb *musb)
 
 	del_timer_sync(&glue->timer[pdev->id]);
 
-	/* Shutdown the on-chip PHY and its PLL. */
-	musb_dsps_phy_control(glue, pdev->id, 0);
-
-	/* NOP driver needs change if supporting dual instance */
-	usb_put_phy(musb->xceiv);
-	usb_nop_xceiv_unregister();
-
 	return 0;
 }
 
@@ -496,16 +442,6 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, u8 id)
 	char res_name[11];
 	int ret;
 
-	resources[0].start = dsps_control_module_phys[id];
-	resources[0].end = resources[0].start + SZ_4 - 1;
-	resources[0].flags = IORESOURCE_MEM;
-
-	glue->usb_ctrl[id] = devm_ioremap_resource(&pdev->dev, resources);
-	if (IS_ERR(glue->usb_ctrl[id])) {
-		ret = PTR_ERR(glue->usb_ctrl[id]);
-		goto err0;
-	}
-
 	/* first resource is for usbss, so start index from 1 */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, id + 1);
 	if (!res) {
@@ -689,36 +625,6 @@ static int dsps_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int dsps_suspend(struct device *dev)
-{
-	struct platform_device *pdev = to_platform_device(dev->parent);
-	struct dsps_glue *glue = platform_get_drvdata(pdev);
-	const struct dsps_musb_wrapper *wrp = glue->wrp;
-	int i;
-
-	for (i = 0; i < wrp->instances; i++)
-		musb_dsps_phy_control(glue, i, 0);
-
-	return 0;
-}
-
-static int dsps_resume(struct device *dev)
-{
-	struct platform_device *pdev = to_platform_device(dev->parent);
-	struct dsps_glue *glue = platform_get_drvdata(pdev);
-	const struct dsps_musb_wrapper *wrp = glue->wrp;
-	int i;
-
-	for (i = 0; i < wrp->instances; i++)
-		musb_dsps_phy_control(glue, i, 1);
-
-	return 0;
-}
-#endif
-
-static SIMPLE_DEV_PM_OPS(dsps_pm_ops, dsps_suspend, dsps_resume);
-
 static const struct dsps_musb_wrapper ti81xx_driver_data = {
 	.revision		= 0x00,
 	.control		= 0x14,
@@ -773,7 +679,6 @@ static struct platform_driver dsps_usbss_driver = {
 	.remove         = dsps_remove,
 	.driver         = {
 		.name   = "musb-dsps",
-		.pm	= &dsps_pm_ops,
 		.of_match_table	= of_match_ptr(musb_dsps_of_match),
 	},
 	.id_table	= musb_dsps_id_table,
-- 
1.8.3.2

Re: [PATCH 3/5] usb: musb: dsps: remove the hardcoded phy pieces

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

Hi,

On Fri, Jul 05, 2013 at 03:32:56PM +0200, Sebastian Andrzej Siewior wrote:
> dsps uses a nop driver which is added in dsps itself and does the PHY
> on/off calls within dsps. Since those calls are now moved the nop driver
> itself, we can now request the phy proper phy and remove those calls.
> Currently only the first musb interface is used so we only add one phy
> node for now.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

looks good, just wondering if this won't pose a regression since you're
not substituting PHY handling with proper usb_phy_set_suspend() calls.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/5] usb: phy: phy-nop: add support for am335x PHY

[Sebastian Andrzej Siewior]

The am335x PHY code is well hidden in multiple places. The glue layer
uses the nop and does up/down in the background. This patch copies the
power on / power off part out of dsps so it can be removed later in
dsps. At this point I am not sure if it is better to write a new phy
driver for am335x or just add the missing pieces to this one.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
 drivers/usb/phy/phy-nop.c | 113 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 101 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/phy/phy-nop.c b/drivers/usb/phy/phy-nop.c
index 55445e5d..ad85090 100644
--- a/drivers/usb/phy/phy-nop.c
+++ b/drivers/usb/phy/phy-nop.c
@@ -35,6 +35,7 @@
 #include <linux/clk.h>
 #include <linux/regulator/consumer.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 
 struct nop_usb_xceiv {
 	struct usb_phy phy;
@@ -42,6 +43,14 @@ struct nop_usb_xceiv {
 	struct clk *clk;
 	struct regulator *vcc;
 	struct regulator *reset;
+
+	void __iomem *priv_reg;
+};
+
+struct phy_data {
+	int	(*phy_init)(struct usb_phy *x);
+	void	(*phy_shutdown)(struct usb_phy *x);
+	int	(*reg_init)(struct platform_device *pdev);
 };
 
 static struct platform_device *pd;
@@ -139,10 +148,84 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
 	return 0;
 }
 
+static int am335x_reg_init(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct nop_usb_xceiv *nop;
+	struct resource res;
+	int ret;
+
+	nop = platform_get_drvdata(pdev);
+	ret = of_address_to_resource(node, 0, &res);
+		return ret;
+
+	nop->priv_reg = devm_request_and_ioremap(&pdev->dev, 0);
+	if (!nop->priv_reg)
+		return -EINVAL;
+	return 0;
+}
+
+#define AM335X_USB_CTRL	0x00
+#define AM335x_USB_STS	0x04
+
+#define USBPHY_CM_PWRDN		(1 << 0)
+#define USBPHY_OTG_PWRDN	(1 << 1)
+#define USBPHY_OTGVDET_EN	(1 << 19)
+#define USBPHY_OTGSESSEND_EN	(1 << 20)
+
+static void am335x_phy_power(struct nop_usb_xceiv *nop, bool on)
+{
+	u32 val;
+
+	val = readl(nop->priv_reg);
+	if (on) {
+		val &= ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN);
+		val |= USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN;
+	} else {
+		val |= USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN;
+	}
+
+	writel(val, nop->priv_reg);
+}
+
+static int am335x_phy_init(struct usb_phy *phy)
+{
+	struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
+	int ret;
+
+	ret = nop_init(phy);
+	if (ret)
+		return ret;
+	am335x_phy_power(nop, true);
+	return 0;
+}
+
+static void am335x_phy_shutdown(struct usb_phy *phy)
+{
+	struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
+
+	am335x_phy_power(nop, false);
+	nop_shutdown(phy);
+}
+
+struct phy_data am335x_phy_data = {
+	.reg_init	= am335x_reg_init,
+	.phy_init	= am335x_phy_init,
+	.phy_shutdown	= am335x_phy_shutdown,
+};
+
+static const struct of_device_id nop_xceiv_dt_ids[] = {
+	{ .compatible = "usb-nop-xceiv" },
+	{ .compatible = "ti,am335x-usb-phy", .data = &am335x_phy_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, nop_xceiv_dt_ids);
+
 static int nop_usb_xceiv_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct nop_usb_xceiv_platform_data *pdata = pdev->dev.platform_data;
+	const struct phy_data	*phy_data = NULL;
 	struct nop_usb_xceiv	*nop;
 	enum usb_phy_type	type = USB_PHY_TYPE_USB2;
 	int err;
@@ -154,6 +237,7 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	if (!nop)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, nop);
 	nop->phy.otg = devm_kzalloc(&pdev->dev, sizeof(*nop->phy.otg),
 							GFP_KERNEL);
 	if (!nop->phy.otg)
@@ -161,13 +245,20 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 
 	if (dev->of_node) {
 		struct device_node *node = dev->of_node;
+		const struct of_device_id *of_id;
 
 		if (of_property_read_u32(node, "clock-frequency", &clk_rate))
 			clk_rate = 0;
 
 		needs_vcc = of_property_read_bool(node, "vcc-supply");
 		needs_reset = of_property_read_bool(node, "reset-supply");
-
+		of_id = of_match_node(nop_xceiv_dt_ids, node);
+		if (of_id) {
+			phy_data = of_id->data;
+			err = phy_data->reg_init(pdev);
+			if (err)
+				return err;
+		}
 	} else if (pdata) {
 		type = pdata->type;
 		clk_rate = pdata->clk_rate;
@@ -217,8 +308,15 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	nop->phy.dev		= nop->dev;
 	nop->phy.label		= "nop-xceiv";
 	nop->phy.set_suspend	= nop_set_suspend;
-	nop->phy.init		= nop_init;
-	nop->phy.shutdown	= nop_shutdown;
+	if (phy_data && phy_data->phy_init)
+		nop->phy.init	= phy_data->phy_init;
+	else
+		nop->phy.init	= nop_init;
+
+	if (phy_data && phy_data->phy_shutdown)
+		nop->phy.shutdown = phy_data->phy_shutdown;
+	else
+		nop->phy.shutdown	= nop_shutdown;
 	nop->phy.state		= OTG_STATE_UNDEFINED;
 	nop->phy.type		= type;
 
@@ -233,8 +331,6 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 		goto err_add;
 	}
 
-	platform_set_drvdata(pdev, nop);
-
 	ATOMIC_INIT_NOTIFIER_HEAD(&nop->phy.notifier);
 
 	return 0;
@@ -257,13 +353,6 @@ static int nop_usb_xceiv_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id nop_xceiv_dt_ids[] = {
-	{ .compatible = "usb-nop-xceiv" },
-	{ }
-};
-
-MODULE_DEVICE_TABLE(of, nop_xceiv_dt_ids);

Re: [PATCH 1/5] usb: phy: phy-nop: add support for am335x PHY

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 747 bytes --]

Hi,

On Fri, Jul 05, 2013 at 03:32:54PM +0200, Sebastian Andrzej Siewior wrote:
> The am335x PHY code is well hidden in multiple places. The glue layer
> uses the nop and does up/down in the background. This patch copies the
> power on / power off part out of dsps so it can be removed later in
> dsps. At this point I am not sure if it is better to write a new phy
> driver for am335x or just add the missing pieces to this one.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>

hmm, if you do all of this then phy-nop doesn't make a lot of sense
anymore. Can you add a few patches to start calling file and functions
phy-generic/usb_phy_gen. Other than that looks good.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 4/5] usb: musb: dsps: use proper child nodes

[Sebastian Andrzej Siewior]

This moves the two instances from the big node into two child nodes. The
glue layer ontop does almost nothing.
This could be two indepentant child nodes but I have no idea how
'ti,hwmods = "usb_otg_hs";' affects the two musb controler.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
 arch/arm/boot/dts/am335x-bone.dts  |  8 ++++++
 arch/arm/boot/dts/am335x-evm.dts   | 16 ++++++++++++
 arch/arm/boot/dts/am335x-evmsk.dts | 16 ++++++++++++
 arch/arm/boot/dts/am33xx.dtsi      | 47 ++++++++++++++++++++++++-----------
 drivers/usb/musb/musb_dsps.c       | 51 ++++++++++++++++++--------------------
 5 files changed, 97 insertions(+), 41 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
index 83184e5..ae50fd7 100644
--- a/arch/arm/boot/dts/am335x-bone.dts
+++ b/arch/arm/boot/dts/am335x-bone.dts
@@ -124,6 +124,14 @@
 			status = "okay";
 		};
 
+		musb: usb@47400000 {
+			status = "okay";
+
+			usb0@47401000 {
+				status = "okay";
+			};
+		};
+
 		i2c0: i2c@44e0b000 {
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2c0_pins>;
diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 317637a..dc236f4 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -175,6 +175,22 @@
 			status = "okay";
 		};
 
+		musb1_phy: phy1@47401800 {
+			status = "okay";
+		};
+
+		musb: usb@47400000 {
+			status = "okay";
+
+			usb0@47401000 {
+				status = "okay";
+			};
+
+			usb1@47401800 {
+				status = "okay";
+			};
+		};
+
 		i2c1: i2c@4802a000 {
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2c1_pins>;
diff --git a/arch/arm/boot/dts/am335x-evmsk.dts b/arch/arm/boot/dts/am335x-evmsk.dts
index 99d9444..74ce579 100644
--- a/arch/arm/boot/dts/am335x-evmsk.dts
+++ b/arch/arm/boot/dts/am335x-evmsk.dts
@@ -211,6 +211,22 @@
 			status = "okay";
 		};
 
+		musb1_phy: phy1@47401800 {
+			status = "okay";
+		};
+
+		musb: usb@47400000 {
+			status = "okay";
+
+			usb0@47401000 {
+				status = "okay";
+			};
+
+			usb1@47401800 {
+				status = "okay";
+			};
+		};
+
 		epwmss2: epwmss@48304000 {
 			status = "okay";
 
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 66bb420..bb2298c 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -349,22 +349,41 @@
 			status = "disabled";
 		};
 
-		usb@47400000 {
+		musb: usb@47400000 {
 			compatible = "ti,musb-am33xx";
-			reg = <0x47400000 0x1000	/* usbss */
-			       0x47401000 0x800		/* musb instance 0 */
-			       0x47401800 0x800>;	/* musb instance 1 */
-			interrupts = <17		/* usbss */
-				      18		/* musb instance 0 */
-				      19>;		/* musb instance 1 */
-			multipoint = <1>;
-			num-eps = <16>;
-			ram-bits = <12>;
-			port0-mode = <3>;
-			port1-mode = <3>;
-			power = <250>;
+			reg = <0x47400000 0x1000>;
+			ranges;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			interrupts = <17>;
 			ti,hwmods = "usb_otg_hs";
-			phys = <&musb1_phy>;
+			status = "disabled";
+
+			usb0@47401000 {
+				reg = <0x47401000 0x800>;
+				interrupts = <18>;
+				interrupt-names = "mc";
+				multipoint = <1>;
+				num-eps = <16>;
+				ram-bits = <12>;
+				port-mode = <3>;
+				power = <250>;
+				phys = <&musb0_phy>;
+				status = "disabled";
+			};
+
+			usb1@47401800 {
+				reg = <0x47401800 0x800>;
+				interrupts = <19>;
+				interrupt-names = "mc";
+				multipoint = <1>;
+				num-eps = <16>;
+				ram-bits = <12>;
+				port-mode = <3>;
+				power = <250>;
+				phys = <&musb1_phy>;
+				status = "disabled";
+			};
 		};
 
 		epwmss0: epwmss@48300000 {
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 22ad532..0e01cc1 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -43,6 +43,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_address.h>
+#include <linux/of_irq.h>
 
 #include "musb_core.h"
 
@@ -375,9 +376,9 @@ static int dsps_musb_init(struct musb *musb)
 	musb->mregs += wrp->musb_core_offset;
 
 	/* NOP driver needs change if supporting dual instance */
-	musb->xceiv = devm_usb_get_phy_by_phandle(glue->dev, "phys", 0);
-	if (IS_ERR_OR_NULL(musb->xceiv))
-		return -EPROBE_DEFER;
+	musb->xceiv = devm_usb_get_phy_by_phandle(dev, "phys", 0);
+	if (IS_ERR(musb->xceiv))
+		return PTR_ERR(musb->xceiv);
 
 	/* Returns zero if e.g. not clocked */
 	rev = dsps_readl(reg_base, wrp->revision);
@@ -429,39 +430,32 @@ static struct musb_platform_ops dsps_ops = {
 
 static u64 musb_dmamask = DMA_BIT_MASK(32);
 
-static int dsps_create_musb_pdev(struct dsps_glue *glue, u8 id)
+static int dsps_create_musb_pdev(struct dsps_glue *glue, u8 id,
+		struct device_node *np)
 {
 	struct device *dev = glue->dev;
 	struct platform_device *pdev = to_platform_device(dev);
 	struct musb_hdrc_platform_data  *pdata = dev->platform_data;
-	struct device_node *np = pdev->dev.of_node;
 	struct musb_hdrc_config	*config;
 	struct platform_device	*musb;
-	struct resource *res;
 	struct resource	resources[2];
-	char res_name[11];
 	int ret;
 
-	/* first resource is for usbss, so start index from 1 */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, id + 1);
-	if (!res) {
+	ret = of_address_to_resource(np, 0, &resources[0]);
+	if (ret) {
 		dev_err(dev, "failed to get memory for instance %d\n", id);
-		ret = -ENODEV;
 		goto err0;
 	}
-	res->parent = NULL;
-	resources[0] = *res;
 
 	/* first resource is for usbss, so start index from 1 */
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, id + 1);
-	if (!res) {
+	ret = of_irq_to_resource(np, 0, &resources[1]);
+	if (ret == 0) {
 		dev_err(dev, "failed to get irq for instance %d\n", id);
-		ret = -ENODEV;
+		ret = -EINVAL;
 		goto err0;
 	}
-	res->parent = NULL;
-	resources[1] = *res;
-	resources[1].name = "mc";
+	resources[0].parent = NULL;
+	resources[1].parent = NULL;
 
 	/* allocate the child platform device */
 	musb = platform_device_alloc("musb-hdrc", PLATFORM_DEVID_AUTO);
@@ -474,6 +468,7 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, u8 id)
 	musb->dev.parent		= dev;
 	musb->dev.dma_mask		= &musb_dmamask;
 	musb->dev.coherent_dma_mask	= musb_dmamask;
+	musb->dev.of_node		= of_node_get(np);
 
 	glue->musb[id]			= musb;
 
@@ -502,8 +497,7 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, u8 id)
 
 		of_property_read_u32(np, "num-eps", (u32 *)&config->num_eps);
 		of_property_read_u32(np, "ram-bits", (u32 *)&config->ram_bits);
-		snprintf(res_name, sizeof(res_name), "port%d-mode", id);
-		of_property_read_u32(np, res_name, (u32 *)&pdata->mode);
+		of_property_read_u32(np, "port-mode", (u32 *)&pdata->mode);
 		of_property_read_u32(np, "power", (u32 *)&pdata->power);
 		config->multipoint = of_property_read_bool(np, "multipoint");
 
@@ -537,6 +531,7 @@ static int dsps_probe(struct platform_device *pdev)
 	const struct of_device_id *match;
 	const struct dsps_musb_wrapper *wrp;
 	struct dsps_glue *glue;
+	struct device_node *child;
 	struct resource *iomem;
 	int ret, i;
 
@@ -583,9 +578,9 @@ static int dsps_probe(struct platform_device *pdev)
 		goto err2;
 	}
 
-	/* create the child platform device for all instances of musb */
-	for (i = 0; i < wrp->instances ; i++) {
-		ret = dsps_create_musb_pdev(glue, i);
+	i = 0;
+	for_each_available_child_of_node(pdev->dev.of_node, child) {
+		ret = dsps_create_musb_pdev(glue, i, child);
 		if (ret != 0) {
 			dev_err(&pdev->dev, "failed to create child pdev\n");
 			/* release resources of previously created instances */
@@ -593,6 +588,7 @@ static int dsps_probe(struct platform_device *pdev)
 				platform_device_unregister(glue->musb[i]);
 			goto err3;
 		}
+		i++;
 	}
 
 	return 0;
@@ -610,12 +606,13 @@ static int dsps_probe(struct platform_device *pdev)
 static int dsps_remove(struct platform_device *pdev)
 {
 	struct dsps_glue *glue = platform_get_drvdata(pdev);
-	const struct dsps_musb_wrapper *wrp = glue->wrp;
 	int i;
 
 	/* delete the child platform device */
-	for (i = 0; i < wrp->instances ; i++)
-		platform_device_unregister(glue->musb[i]);
+	for (i = 0; i < ARRAY_SIZE(glue->musb); i++) {
+		if (glue->musb[i])
+			platform_device_unregister(glue->musb[i]);
+	}
 
 	/* disable usbss clocks */
 	pm_runtime_put(&pdev->dev);
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] usb: musb: dsps: use proper child nodes

[Enric Balletbo Serra]

Hi Sebastian,

2013/7/5 Sebastian Andrzej Siewior <bigeasy@linutronix.de>:
> This moves the two instances from the big node into two child nodes. The
> glue layer ontop does almost nothing.
> This could be two indepentant child nodes but I have no idea how
> 'ti,hwmods = "usb_otg_hs";' affects the two musb controler.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  arch/arm/boot/dts/am335x-bone.dts  |  8 ++++++
>  arch/arm/boot/dts/am335x-evm.dts   | 16 ++++++++++++
>  arch/arm/boot/dts/am335x-evmsk.dts | 16 ++++++++++++
>  arch/arm/boot/dts/am33xx.dtsi      | 47 ++++++++++++++++++++++++-----------
>  drivers/usb/musb/musb_dsps.c       | 51 ++++++++++++++++++--------------------
>  5 files changed, 97 insertions(+), 41 deletions(-)
>
> diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
> index 83184e5..ae50fd7 100644
> --- a/arch/arm/boot/dts/am335x-bone.dts
> +++ b/arch/arm/boot/dts/am335x-bone.dts
> @@ -124,6 +124,14 @@
>                         status = "okay";
>                 };
>
> +               musb: usb@47400000 {
> +                       status = "okay";
> +
> +                       usb0@47401000 {
> +                               status = "okay";
> +                       };
> +               };
> +
>                 i2c0: i2c@44e0b000 {
>                         pinctrl-names = "default";
>                         pinctrl-0 = <&i2c0_pins>;
> diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
> index 317637a..dc236f4 100644
> --- a/arch/arm/boot/dts/am335x-evm.dts
> +++ b/arch/arm/boot/dts/am335x-evm.dts
> @@ -175,6 +175,22 @@
>                         status = "okay";
>                 };
>
> +               musb1_phy: phy1@47401800 {
> +                       status = "okay";
> +               };
> +
> +               musb: usb@47400000 {
> +                       status = "okay";
> +
> +                       usb0@47401000 {
> +                               status = "okay";
> +                       };
> +
> +                       usb1@47401800 {
> +                               status = "okay";
> +                       };
> +               };
> +
>                 i2c1: i2c@4802a000 {
>                         pinctrl-names = "default";
>                         pinctrl-0 = <&i2c1_pins>;
> diff --git a/arch/arm/boot/dts/am335x-evmsk.dts b/arch/arm/boot/dts/am335x-evmsk.dts
> index 99d9444..74ce579 100644
> --- a/arch/arm/boot/dts/am335x-evmsk.dts
> +++ b/arch/arm/boot/dts/am335x-evmsk.dts
> @@ -211,6 +211,22 @@
>                         status = "okay";
>                 };
>
> +               musb1_phy: phy1@47401800 {
> +                       status = "okay";
> +               };
> +
> +               musb: usb@47400000 {
> +                       status = "okay";
> +
> +                       usb0@47401000 {
> +                               status = "okay";
> +                       };
> +
> +                       usb1@47401800 {
> +                               status = "okay";
> +                       };
> +               };
> +
>                 epwmss2: epwmss@48304000 {
>                         status = "okay";
>
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 66bb420..bb2298c 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -349,22 +349,41 @@
>                         status = "disabled";
>                 };
>
> -               usb@47400000 {
> +               musb: usb@47400000 {
>                         compatible = "ti,musb-am33xx";
> -                       reg = <0x47400000 0x1000        /* usbss */
> -                              0x47401000 0x800         /* musb instance 0 */
> -                              0x47401800 0x800>;       /* musb instance 1 */
> -                       interrupts = <17                /* usbss */
> -                                     18                /* musb instance 0 */
> -                                     19>;              /* musb instance 1 */
> -                       multipoint = <1>;
> -                       num-eps = <16>;
> -                       ram-bits = <12>;
> -                       port0-mode = <3>;
> -                       port1-mode = <3>;
> -                       power = <250>;
> +                       reg = <0x47400000 0x1000>;
> +                       ranges;
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       interrupts = <17>;
>                         ti,hwmods = "usb_otg_hs";
> -                       phys = <&musb1_phy>;
> +                       status = "disabled";
> +
> +                       usb0@47401000 {
> +                               reg = <0x47401000 0x800>;
> +                               interrupts = <18>;
> +                               interrupt-names = "mc";
> +                               multipoint = <1>;
> +                               num-eps = <16>;
> +                               ram-bits = <12>;
> +                               port-mode = <3>;
> +                               power = <250>;
> +                               phys = <&musb0_phy>;
> +                               status = "disabled";
> +                       };
> +
> +                       usb1@47401800 {
> +                               reg = <0x47401800 0x800>;
> +                               interrupts = <19>;
> +                               interrupt-names = "mc";
> +                               multipoint = <1>;
> +                               num-eps = <16>;
> +                               ram-bits = <12>;
> +                               port-mode = <3>;
> +                               power = <250>;
> +                               phys = <&musb1_phy>;
> +                               status = "disabled";
> +                       };
>                 };
>
>                 epwmss0: epwmss@48300000 {
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index 22ad532..0e01cc1 100644
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -43,6 +43,7 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/of_address.h>
> +#include <linux/of_irq.h>
>
>  #include "musb_core.h"
>
> @@ -375,9 +376,9 @@ static int dsps_musb_init(struct musb *musb)
>         musb->mregs += wrp->musb_core_offset;
>
>         /* NOP driver needs change if supporting dual instance */
> -       musb->xceiv = devm_usb_get_phy_by_phandle(glue->dev, "phys", 0);
> -       if (IS_ERR_OR_NULL(musb->xceiv))
> -               return -EPROBE_DEFER;
> +       musb->xceiv = devm_usb_get_phy_by_phandle(dev, "phys", 0);
> +       if (IS_ERR(musb->xceiv))
> +               return PTR_ERR(musb->xceiv);
>
>         /* Returns zero if e.g. not clocked */
>         rev = dsps_readl(reg_base, wrp->revision);
> @@ -429,39 +430,32 @@ static struct musb_platform_ops dsps_ops = {
>
>  static u64 musb_dmamask = DMA_BIT_MASK(32);
>
> -static int dsps_create_musb_pdev(struct dsps_glue *glue, u8 id)
> +static int dsps_create_musb_pdev(struct dsps_glue *glue, u8 id,
> +               struct device_node *np)
>  {
>         struct device *dev = glue->dev;
>         struct platform_device *pdev = to_platform_device(dev);
>         struct musb_hdrc_platform_data  *pdata = dev->platform_data;
> -       struct device_node *np = pdev->dev.of_node;
>         struct musb_hdrc_config *config;
>         struct platform_device  *musb;
> -       struct resource *res;
>         struct resource resources[2];
> -       char res_name[11];
>         int ret;
>
> -       /* first resource is for usbss, so start index from 1 */
> -       res = platform_get_resource(pdev, IORESOURCE_MEM, id + 1);
> -       if (!res) {
> +       ret = of_address_to_resource(np, 0, &resources[0]);
> +       if (ret) {
>                 dev_err(dev, "failed to get memory for instance %d\n", id);
> -               ret = -ENODEV;
>                 goto err0;
>         }
> -       res->parent = NULL;
> -       resources[0] = *res;
>
>         /* first resource is for usbss, so start index from 1 */
> -       res = platform_get_resource(pdev, IORESOURCE_IRQ, id + 1);
> -       if (!res) {
> +       ret = of_irq_to_resource(np, 0, &resources[1]);
> +       if (ret == 0) {
>                 dev_err(dev, "failed to get irq for instance %d\n", id);
> -               ret = -ENODEV;
> +               ret = -EINVAL;
>                 goto err0;
>         }
> -       res->parent = NULL;
> -       resources[1] = *res;
> -       resources[1].name = "mc";
> +       resources[0].parent = NULL;
> +       resources[1].parent = NULL;
>
>         /* allocate the child platform device */
>         musb = platform_device_alloc("musb-hdrc", PLATFORM_DEVID_AUTO);
> @@ -474,6 +468,7 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, u8 id)
>         musb->dev.parent                = dev;
>         musb->dev.dma_mask              = &musb_dmamask;
>         musb->dev.coherent_dma_mask     = musb_dmamask;
> +       musb->dev.of_node               = of_node_get(np);
>
>         glue->musb[id]                  = musb;
>
> @@ -502,8 +497,7 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, u8 id)
>
>                 of_property_read_u32(np, "num-eps", (u32 *)&config->num_eps);
>                 of_property_read_u32(np, "ram-bits", (u32 *)&config->ram_bits);
> -               snprintf(res_name, sizeof(res_name), "port%d-mode", id);
> -               of_property_read_u32(np, res_name, (u32 *)&pdata->mode);
> +               of_property_read_u32(np, "port-mode", (u32 *)&pdata->mode);
>                 of_property_read_u32(np, "power", (u32 *)&pdata->power);
>                 config->multipoint = of_property_read_bool(np, "multipoint");
>
> @@ -537,6 +531,7 @@ static int dsps_probe(struct platform_device *pdev)
>         const struct of_device_id *match;
>         const struct dsps_musb_wrapper *wrp;
>         struct dsps_glue *glue;
> +       struct device_node *child;
>         struct resource *iomem;
>         int ret, i;
>
> @@ -583,9 +578,9 @@ static int dsps_probe(struct platform_device *pdev)
>                 goto err2;
>         }
>
> -       /* create the child platform device for all instances of musb */
> -       for (i = 0; i < wrp->instances ; i++) {
> -               ret = dsps_create_musb_pdev(glue, i);
> +       i = 0;
> +       for_each_available_child_of_node(pdev->dev.of_node, child) {
> +               ret = dsps_create_musb_pdev(glue, i, child);
>                 if (ret != 0) {
>                         dev_err(&pdev->dev, "failed to create child pdev\n");
>                         /* release resources of previously created instances */
> @@ -593,6 +588,7 @@ static int dsps_probe(struct platform_device *pdev)
>                                 platform_device_unregister(glue->musb[i]);
>                         goto err3;
>                 }
> +               i++;
>         }
>
>         return 0;
> @@ -610,12 +606,13 @@ static int dsps_probe(struct platform_device *pdev)
>  static int dsps_remove(struct platform_device *pdev)
>  {
>         struct dsps_glue *glue = platform_get_drvdata(pdev);
> -       const struct dsps_musb_wrapper *wrp = glue->wrp;
>         int i;
>
>         /* delete the child platform device */
> -       for (i = 0; i < wrp->instances ; i++)
> -               platform_device_unregister(glue->musb[i]);
> +       for (i = 0; i < ARRAY_SIZE(glue->musb); i++) {
> +               if (glue->musb[i])
> +                       platform_device_unregister(glue->musb[i]);
> +       }
>
>         /* disable usbss clocks */
>         pm_runtime_put(&pdev->dev);
> --
> 1.8.3.2
>
> --
> 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


I'm trying to test these patches but I've problems, I've following error,

[   10.096492] musb-dsps 47400000.usb: failed to get memory for instance 0
[   10.103481] musb-dsps 47400000.usb: failed to create child pdev

I'm missing something ?

Thanks in advance,
     Enric

Re: [PATCH 4/5] usb: musb: dsps: use proper child nodes

[Sebastian Andrzej Siewior]

On 07/05/2013 05:08 PM, Enric Balletbo Serra wrote:
> Hi Sebastian,

Hi Enric,

> I'm trying to test these patches but I've problems, I've following error,
> 
> [   10.096492] musb-dsps 47400000.usb: failed to get memory for instance 0
> [   10.103481] musb-dsps 47400000.usb: failed to create child pdev
> 
> I'm missing something ?

This is printed if you don't have the memory for the child device. Did
you update your device tree?

> 
> Thanks in advance,
>      Enric

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] usb: musb: dsps: use proper child nodes

[Enric Balletbo Serra]

2013/7/5 Sebastian Andrzej Siewior <bigeasy@linutronix.de>:
> On 07/05/2013 05:08 PM, Enric Balletbo Serra wrote:
>> Hi Sebastian,
>
> Hi Enric,
>
>> I'm trying to test these patches but I've problems, I've following error,
>>
>> [   10.096492] musb-dsps 47400000.usb: failed to get memory for instance 0
>> [   10.103481] musb-dsps 47400000.usb: failed to create child pdev
>>
>> I'm missing something ?
>
> This is printed if you don't have the memory for the child device. Did
> you update your device tree?
>
Fixed, I didn't update my DT for my custom board properly. Thanks.

>>
>> Thanks in advance,
>>      Enric
>
> Sebastian

Re: [PATCH 4/5] usb: musb: dsps: use proper child nodes

[Sebastian Andrzej Siewior]

On 07/05/2013 05:22 PM, Enric Balletbo Serra wrote:
> Fixed, I didn't update my DT for my custom board properly. Thanks.

Good. I hate breaking the compatibility for DT but unfortunately it is
beyond repair.

Sebastian

Re: [PATCH 4/5] usb: musb: dsps: use proper child nodes

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 429 bytes --]

On Fri, Jul 05, 2013 at 03:32:57PM +0200, Sebastian Andrzej Siewior wrote:
> This moves the two instances from the big node into two child nodes. The
> glue layer ontop does almost nothing.
> This could be two indepentant child nodes but I have no idea how
> 'ti,hwmods = "usb_otg_hs";' affects the two musb controler.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

pretty good :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 5/5] musb: musb: dsps: remove instances variable

[Sebastian Andrzej Siewior]

Remove the instances variable, there is no single use of it.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
 drivers/usb/musb/musb_dsps.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 0e01cc1..edeb2ad 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -111,8 +111,6 @@ struct dsps_musb_wrapper {
 	/* miscellaneous stuff */
 	u32		musb_core_offset;
 	u8		poll_seconds;
-	/* number of musb instances */
-	u8		instances;
 };
 
 /**
@@ -650,7 +648,6 @@ static const struct dsps_musb_wrapper ti81xx_driver_data = {
 	.rxep_bitmap		= (0xfffe << 16),
 	.musb_core_offset	= 0x400,
 	.poll_seconds		= 2,
-	.instances		= 1,
 };
 
 static const struct platform_device_id musb_dsps_id_table[] = {
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] musb: musb: dsps: remove instances variable

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 238 bytes --]

On Fri, Jul 05, 2013 at 03:32:58PM +0200, Sebastian Andrzej Siewior wrote:
> Remove the instances variable, there is no single use of it.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

nice :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: musb: dsps: make it work with two instances

[Ezequiel Garcia]

Hi Sebastian,

On Fri, Jul 5, 2013 at 10:32 AM, Sebastian Andrzej Siewior
<bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:
> This enables the two musb instances on am335x to work.

I like a lot the idea of splitting the DT representation of the two
USB instances. The DT binding
looks much better this way.

After some minor DT tweaking on the current patchset,
I've managed to detect an USB mass storage device in the
second instance (host / usb1) using a Beaglebone black board.

However, after I unplug the device, it's not recognized
when I replug it. Maybe you can take a look at this;
i'll do some more testings and see what I can come up with.

Also, FWIW, I think that having a separate USB phy for am35xx would be
much better.

Nice job!
-- 
    Ezequiel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: musb: dsps: make it work with two instances

[Sebastian Andrzej Siewior]

On 07/06/2013 11:39 PM, Ezequiel Garcia wrote:
> Hi Sebastian,

Hello Ezequiel,

> On Fri, Jul 5, 2013 at 10:32 AM, Sebastian Andrzej Siewior
> <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:
>> This enables the two musb instances on am335x to work.
> 
> I like a lot the idea of splitting the DT representation of the two
> USB instances. The DT binding
> looks much better this way.
> 
> After some minor DT tweaking on the current patchset,
> I've managed to detect an USB mass storage device in the
> second instance (host / usb1) using a Beaglebone black board.

Forgot that. Will update the .dts

> However, after I unplug the device, it's not recognized
> when I replug it. Maybe you can take a look at this;
> i'll do some more testings and see what I can come up with.

I've seen that. If I remember correctly this isn't happening if
runtime-pm is enabled. However if I reload the driver it works again.
I will look at this.

> Also, FWIW, I think that having a separate USB phy for am35xx would be
> much better.

George Cherian said that he is writing a phy driver. So my phy is just
temporary but I think he keep the device nodes just remove the nop-phy
code.

> 
> Nice job!


Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: musb: dsps: make it work with two instances

[Sebastian Andrzej Siewior]

* Ezequiel Garcia | 2013-07-06 18:39:50 [-0300]:

>Hi Sebastian,
Hi Ezequiel,

>After some minor DT tweaking on the current patchset,
>I've managed to detect an USB mass storage device in the
>second instance (host / usb1) using a Beaglebone black board.

Beaglebone black, that one has a different device tree which is not
mainline, right?

>However, after I unplug the device, it's not recognized
>when I replug it. Maybe you can take a look at this;
>i'll do some more testings and see what I can come up with.

I figured out why my Host is not recognized on the second plug:
At module load time, musb_start() is executed and it sets the
MUSB_DEVCTL_SESSION in devctl.
After the device is unplugged dsps_musb_try_idle() schedules a timer
which executes the local otg_timer() function. Since the phy is in
OTG_STATE_A_WAIT_BCON state, the MUSB_DEVCTL_SESSION bit gets removed.
If the removal of the bit is ignored, the device is recognized after a
re-plug.

Now a question: I see that am35x and anothers also remove that flag in
thsi case. How is the flag supposed to come back?
I see that blackfin removes that bit and stuffs it back in. Any idea
what should be done here?

>Also, FWIW, I think that having a separate USB phy for am35xx would be
>much better.
So you would prefer a new file with 90% copy of what we already have in
the nop_phy?

Sebastian

Re: musb: dsps: make it work with two instances

[Ezequiel Garcia]

Hi Sebastian,

On Wed, Jul 17, 2013 at 07:12:29PM +0200, Sebastian Andrzej Siewior wrote:
> 
> >After some minor DT tweaking on the current patchset,
> >I've managed to detect an USB mass storage device in the
> >second instance (host / usb1) using a Beaglebone black board.
> 
> Beaglebone black, that one has a different device tree which is not
> mainline, right?
> 

Beaglebone black it is. But I'm almost sure I just used the
am....bone.dts file that's mainlined.

I just changed the mode or something like that, very minor tweaking
indeed, altough right now I don't remember exactly which changes.

> >However, after I unplug the device, it's not recognized
> >when I replug it. Maybe you can take a look at this;
> >i'll do some more testings and see what I can come up with.
> 
> I figured out why my Host is not recognized on the second plug:
> At module load time, musb_start() is executed and it sets the
> MUSB_DEVCTL_SESSION in devctl.
> After the device is unplugged dsps_musb_try_idle() schedules a timer
> which executes the local otg_timer() function. Since the phy is in
> OTG_STATE_A_WAIT_BCON state, the MUSB_DEVCTL_SESSION bit gets removed.
> If the removal of the bit is ignored, the device is recognized after a
> re-plug.
> 

Mmmm... okey. Interesting insight, thanks!

> >Also, FWIW, I think that having a separate USB phy for am35xx would be
> >much better.
> So you would prefer a new file with 90% copy of what we already have in
> the nop_phy?
> 

No, of course not.

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html