[PATCH 00/13] Device tree support for OMAP HS USB Host

[PATCH 00/13] Device tree support for OMAP HS USB Host

[Roger Quadros]

Hi,

This patchset adds device tree support for OMAP's High Speed USB Host
subsystem. Board adaptation for Panda and Beagleboard is also provided.

Tested on Beagleboard.

Will only work with Panda if we provide a reference to the PHY clock
generator in the device tree in PATCH 11. I do not know how to do that
as there is no way to provide a phandle to any of the OMAP generated clocks
in the device tree. Suggestions welcome :).

Based on linux-next:next-20130204

Depends on "USB: omap-ehci: Move PHY management to PHY driver"
	git-9UaJU3cA/F/QT0dZR+AlfA@public.gmane.org:rogerq/linux.git next-usbhost16

The following changes since commit 8c00470e1308d08df1f2b2c7e9a561d868ec0526:

  USB: ehci-omap: Select NOP USB transceiver driver (2013-02-04 16:36:06 +0200)

are available in the git repository at:
  git-9UaJU3cA/F/QT0dZR+AlfA@public.gmane.org:rogerq/linux.git next-usbhost16-dt

Roger Quadros (13):
  usb: phy: nop: Add device tree support and binding information
  USB: phy: nop: Defer probe if device needs VCC/RESET
  mfd: omap-usb-tll: move configuration code to omap_tll_init()
  mfd: omap-usb-tll: Add device tree support
  USB: ehci-omap: Get platform resources by index rather than by name
  USB: ohci-omap3: Get platform resources by index rather than by name
  USB: ohci-omap3: Add device tree support and binding information
  USB: ehci-omap: Add device tree support and binding information
  mfd: omap-usb-host: Add device tree support and binding information
  ARM: dts: OMAP4: Add HS USB Host IP nodes
  ARM: dts: omap4-panda: Add USB Host support
  ARM: dts: OMAP3: Add HS USB Host IP nodes
  ARM: dts: omap3-beagle: Add USB Host support

 .../devicetree/bindings/mfd/omap-usb-host.txt      |   68 +++++++
 .../devicetree/bindings/mfd/omap-usb-tll.txt       |   17 ++
 .../devicetree/bindings/usb/omap-ehci.txt          |   34 +++
 .../devicetree/bindings/usb/omap3-ohci.txt         |   17 ++
 .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 +++
 arch/arm/boot/dts/omap3-beagle.dts                 |   71 +++++++
 arch/arm/boot/dts/omap3.dtsi                       |   31 +++
 arch/arm/boot/dts/omap4-panda.dts                  |   55 +++++
 arch/arm/boot/dts/omap4.dtsi                       |   30 +++
 drivers/mfd/omap-usb-host.c                        |   90 ++++++++-
 drivers/mfd/omap-usb-tll.c                         |  213 ++++++++++----------
 drivers/mfd/omap-usb.h                             |    5 +-
 drivers/usb/host/ehci-omap.c                       |   41 ++++-
 drivers/usb/host/ohci-omap3.c                      |   24 ++-
 drivers/usb/otg/nop-usb-xceiv.c                    |   39 ++++
 include/linux/usb/nop-usb-xceiv.h                  |    4 +
 16 files changed, 651 insertions(+), 122 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-host.txt
 create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-tll.txt
 create mode 100644 Documentation/devicetree/bindings/usb/omap-ehci.txt
 create mode 100644 Documentation/devicetree/bindings/usb/omap3-ohci.txt
 create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt

-- 
1.7.4.1

--
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

[PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Roger Quadros]

The PHY clock, clock rate, VCC regulator and RESET regulator
can now be provided via device tree.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 ++++++++++++++++++++
 drivers/usb/otg/nop-usb-xceiv.c                    |   31 ++++++++++++++++++
 2 files changed, 65 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt

diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
new file mode 100644
index 0000000..d7e2726
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
@@ -0,0 +1,34 @@
+USB NOP PHY
+
+Required properties:
+- compatible: should be usb-nop-xceiv
+
+Optional properties:
+- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
+  /bindings/clock/clock-bindings.txt
+  This property is required if clock-frequency is specified.
+
+- clock-names: Should be "main_clk"
+
+- clock-frequency: the clock frequency (in Hz) that the PHY clock must
+  be configured to.
+
+- vcc-supply: phandle to the regulator that provides RESET to the PHY.
+
+- reset-supply: phandle to the regulator that provides power to the PHY.
+
+Example:
+
+	hsusb1_phy {
+		compatible = "usb-nop-xceiv";
+		clock-frequency = <19200000>;
+		clocks = <&osc 0>;
+		clock-names = "main_clk";
+		vcc-supply = <&hsusb1_vcc_regulator>;
+		reset-supply = <&hsusb1_reset_regulator>;
+	};
+
+hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator
+and expects that clock to be configured to 19.2MHz by the NOP PHY driver.
+hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator
+controls RESET.
diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
index ac027a1..adbb7ab 100644
--- a/drivers/usb/otg/nop-usb-xceiv.c
+++ b/drivers/usb/otg/nop-usb-xceiv.c
@@ -34,6 +34,7 @@
 #include <linux/slab.h>
 #include <linux/clk.h>
 #include <linux/regulator/consumer.h>
+#include <linux/of.h>
 
 struct nop_usb_xceiv {
 	struct usb_phy		phy;
@@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
 	return 0;
 }
 
+static void nop_xeiv_get_dt_pdata(struct device *dev,
+				struct nop_usb_xceiv_platform_data *pdata)
+{
+	struct device_node *node = dev->of_node;
+	u32 clk_rate;
+
+	if (!of_property_read_u32(node, "clock-frequency", &clk_rate))
+		pdata->clk_rate = clk_rate;
+}
+
 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;
 	struct nop_usb_xceiv	*nop;
 	enum usb_phy_type	type = USB_PHY_TYPE_USB2;
@@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	if (!nop->phy.otg)
 		return -ENOMEM;
 
+	if (dev->of_node) {
+		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+		if (!pdata) {
+			dev_err(dev, "Memory allocation failure\n");
+			return -ENOMEM;
+		}
+		nop_xeiv_get_dt_pdata(dev, pdata);
+	} else {
+		pdata = dev->platform_data;
+	}
+
 	if (pdata)
 		type = pdata->type;
 
@@ -236,12 +259,20 @@ 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);
+
 static struct platform_driver nop_usb_xceiv_driver = {
 	.probe		= nop_usb_xceiv_probe,
 	.remove		= nop_usb_xceiv_remove,
 	.driver		= {
 		.name	= "nop_usb_xceiv",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(nop_xceiv_dt_ids),
 	},
 };
 
-- 
1.7.4.1

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Felipe Balbi]

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

Hi,

On Mon, Feb 04, 2013 at 05:58:48PM +0200, Roger Quadros wrote:
> The PHY clock, clock rate, VCC regulator and RESET regulator
> can now be provided via device tree.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 ++++++++++++++++++++
>  drivers/usb/otg/nop-usb-xceiv.c                    |   31 ++++++++++++++++++
>  2 files changed, 65 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> new file mode 100644
> index 0000000..d7e2726
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> @@ -0,0 +1,34 @@
> +USB NOP PHY
> +
> +Required properties:
> +- compatible: should be usb-nop-xceiv
> +
> +Optional properties:
> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
> +  /bindings/clock/clock-bindings.txt
> +  This property is required if clock-frequency is specified.
> +
> +- clock-names: Should be "main_clk"
> +
> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
> +  be configured to.
> +
> +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
> +
> +- reset-supply: phandle to the regulator that provides power to the PHY.
> +
> +Example:
> +
> +	hsusb1_phy {
> +		compatible = "usb-nop-xceiv";
> +		clock-frequency = <19200000>;
> +		clocks = <&osc 0>;
> +		clock-names = "main_clk";
> +		vcc-supply = <&hsusb1_vcc_regulator>;
> +		reset-supply = <&hsusb1_reset_regulator>;
> +	};
> +
> +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator
> +and expects that clock to be configured to 19.2MHz by the NOP PHY driver.
> +hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator
> +controls RESET.
> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
> index ac027a1..adbb7ab 100644
> --- a/drivers/usb/otg/nop-usb-xceiv.c
> +++ b/drivers/usb/otg/nop-usb-xceiv.c
> @@ -34,6 +34,7 @@
>  #include <linux/slab.h>
>  #include <linux/clk.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/of.h>
>  
>  struct nop_usb_xceiv {
>  	struct usb_phy		phy;
> @@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
>  	return 0;
>  }
>  
> +static void nop_xeiv_get_dt_pdata(struct device *dev,

asking to remove, but xeiv != xceiv :-)

> +				struct nop_usb_xceiv_platform_data *pdata)
> +{
> +	struct device_node *node = dev->of_node;
> +	u32 clk_rate;
> +
> +	if (!of_property_read_u32(node, "clock-frequency", &clk_rate))
> +		pdata->clk_rate = clk_rate;
> +}
> +
>  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;
>  	struct nop_usb_xceiv	*nop;
>  	enum usb_phy_type	type = USB_PHY_TYPE_USB2;
> @@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
>  	if (!nop->phy.otg)
>  		return -ENOMEM;
>  
> +	if (dev->of_node) {
> +		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +		if (!pdata) {
> +			dev_err(dev, "Memory allocation failure\n");
> +			return -ENOMEM;
> +		}
> +		nop_xeiv_get_dt_pdata(dev, pdata);

actually, I would prefer to not create pdata at all. I mean, ideally
pdata would be used to initialize fields in your own structure, so first
move clk_rate to your own private structure, copy pdata's clk_rate value
to that, then you don't need this hackery when using DT.

-- 
balbi

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

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Roger Quadros]

On 02/05/2013 09:26 AM, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Feb 04, 2013 at 05:58:48PM +0200, Roger Quadros wrote:
>> The PHY clock, clock rate, VCC regulator and RESET regulator
>> can now be provided via device tree.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 ++++++++++++++++++++
>>  drivers/usb/otg/nop-usb-xceiv.c                    |   31 ++++++++++++++++++
>>  2 files changed, 65 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> new file mode 100644
>> index 0000000..d7e2726
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> @@ -0,0 +1,34 @@
>> +USB NOP PHY
>> +
>> +Required properties:
>> +- compatible: should be usb-nop-xceiv
>> +
>> +Optional properties:
>> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
>> +  /bindings/clock/clock-bindings.txt
>> +  This property is required if clock-frequency is specified.
>> +
>> +- clock-names: Should be "main_clk"
>> +
>> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
>> +  be configured to.
>> +
>> +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
>> +
>> +- reset-supply: phandle to the regulator that provides power to the PHY.
>> +
>> +Example:
>> +
>> +	hsusb1_phy {
>> +		compatible = "usb-nop-xceiv";
>> +		clock-frequency = <19200000>;
>> +		clocks = <&osc 0>;
>> +		clock-names = "main_clk";
>> +		vcc-supply = <&hsusb1_vcc_regulator>;
>> +		reset-supply = <&hsusb1_reset_regulator>;
>> +	};
>> +
>> +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator
>> +and expects that clock to be configured to 19.2MHz by the NOP PHY driver.
>> +hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator
>> +controls RESET.
>> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
>> index ac027a1..adbb7ab 100644
>> --- a/drivers/usb/otg/nop-usb-xceiv.c
>> +++ b/drivers/usb/otg/nop-usb-xceiv.c
>> @@ -34,6 +34,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/clk.h>
>>  #include <linux/regulator/consumer.h>
>> +#include <linux/of.h>
>>  
>>  struct nop_usb_xceiv {
>>  	struct usb_phy		phy;
>> @@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
>>  	return 0;
>>  }
>>  
>> +static void nop_xeiv_get_dt_pdata(struct device *dev,
> 
> asking to remove, but xeiv != xceiv :-)
> 
>> +				struct nop_usb_xceiv_platform_data *pdata)
>> +{
>> +	struct device_node *node = dev->of_node;
>> +	u32 clk_rate;
>> +
>> +	if (!of_property_read_u32(node, "clock-frequency", &clk_rate))
>> +		pdata->clk_rate = clk_rate;
>> +}
>> +
>>  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;
>>  	struct nop_usb_xceiv	*nop;
>>  	enum usb_phy_type	type = USB_PHY_TYPE_USB2;
>> @@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
>>  	if (!nop->phy.otg)
>>  		return -ENOMEM;
>>  
>> +	if (dev->of_node) {
>> +		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +		if (!pdata) {
>> +			dev_err(dev, "Memory allocation failure\n");
>> +			return -ENOMEM;
>> +		}
>> +		nop_xeiv_get_dt_pdata(dev, pdata);
> 
> actually, I would prefer to not create pdata at all. I mean, ideally
> pdata would be used to initialize fields in your own structure, so first
> move clk_rate to your own private structure, copy pdata's clk_rate value
> to that, then you don't need this hackery when using DT.
> 
OK, got it. Will revise.

cheers,
-roger

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Felipe Balbi]

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

On Tue, Feb 05, 2013 at 10:30:49AM +0200, Roger Quadros wrote:
> On 02/05/2013 09:26 AM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Mon, Feb 04, 2013 at 05:58:48PM +0200, Roger Quadros wrote:
> >> The PHY clock, clock rate, VCC regulator and RESET regulator
> >> can now be provided via device tree.
> >>
> >> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
> >> ---
> >>  .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 ++++++++++++++++++++
> >>  drivers/usb/otg/nop-usb-xceiv.c                    |   31 ++++++++++++++++++
> >>  2 files changed, 65 insertions(+), 0 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> >> new file mode 100644
> >> index 0000000..d7e2726
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> >> @@ -0,0 +1,34 @@
> >> +USB NOP PHY
> >> +
> >> +Required properties:
> >> +- compatible: should be usb-nop-xceiv
> >> +
> >> +Optional properties:
> >> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
> >> +  /bindings/clock/clock-bindings.txt
> >> +  This property is required if clock-frequency is specified.
> >> +
> >> +- clock-names: Should be "main_clk"
> >> +
> >> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
> >> +  be configured to.
> >> +
> >> +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
> >> +
> >> +- reset-supply: phandle to the regulator that provides power to the PHY.
> >> +
> >> +Example:
> >> +
> >> +	hsusb1_phy {
> >> +		compatible = "usb-nop-xceiv";
> >> +		clock-frequency = <19200000>;
> >> +		clocks = <&osc 0>;
> >> +		clock-names = "main_clk";
> >> +		vcc-supply = <&hsusb1_vcc_regulator>;
> >> +		reset-supply = <&hsusb1_reset_regulator>;
> >> +	};
> >> +
> >> +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator
> >> +and expects that clock to be configured to 19.2MHz by the NOP PHY driver.
> >> +hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator
> >> +controls RESET.
> >> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
> >> index ac027a1..adbb7ab 100644
> >> --- a/drivers/usb/otg/nop-usb-xceiv.c
> >> +++ b/drivers/usb/otg/nop-usb-xceiv.c
> >> @@ -34,6 +34,7 @@
> >>  #include <linux/slab.h>
> >>  #include <linux/clk.h>
> >>  #include <linux/regulator/consumer.h>
> >> +#include <linux/of.h>
> >>  
> >>  struct nop_usb_xceiv {
> >>  	struct usb_phy		phy;
> >> @@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
> >>  	return 0;
> >>  }
> >>  
> >> +static void nop_xeiv_get_dt_pdata(struct device *dev,
> > 
> > asking to remove, but xeiv != xceiv :-)
> > 
> >> +				struct nop_usb_xceiv_platform_data *pdata)
> >> +{
> >> +	struct device_node *node = dev->of_node;
> >> +	u32 clk_rate;
> >> +
> >> +	if (!of_property_read_u32(node, "clock-frequency", &clk_rate))
> >> +		pdata->clk_rate = clk_rate;
> >> +}
> >> +
> >>  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;
> >>  	struct nop_usb_xceiv	*nop;
> >>  	enum usb_phy_type	type = USB_PHY_TYPE_USB2;
> >> @@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
> >>  	if (!nop->phy.otg)
> >>  		return -ENOMEM;
> >>  
> >> +	if (dev->of_node) {
> >> +		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> >> +		if (!pdata) {
> >> +			dev_err(dev, "Memory allocation failure\n");
> >> +			return -ENOMEM;
> >> +		}
> >> +		nop_xeiv_get_dt_pdata(dev, pdata);
> > 
> > actually, I would prefer to not create pdata at all. I mean, ideally
> > pdata would be used to initialize fields in your own structure, so first
> > move clk_rate to your own private structure, copy pdata's clk_rate value
> > to that, then you don't need this hackery when using DT.
> > 
> OK, got it. Will revise.

Cool, after that you can add my:

Acked-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>

-- 
balbi

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

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 4328 bytes --]

On 02/05/2013 08:26 AM, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Feb 04, 2013 at 05:58:48PM +0200, Roger Quadros wrote:
>> The PHY clock, clock rate, VCC regulator and RESET regulator
>> can now be provided via device tree.
>>
>> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>> ---
>>  .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 ++++++++++++++++++++
>>  drivers/usb/otg/nop-usb-xceiv.c                    |   31 ++++++++++++++++++
>>  2 files changed, 65 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> new file mode 100644
>> index 0000000..d7e2726
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> @@ -0,0 +1,34 @@
>> +USB NOP PHY
>> +
>> +Required properties:
>> +- compatible: should be usb-nop-xceiv
>> +
>> +Optional properties:
>> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
>> +  /bindings/clock/clock-bindings.txt
>> +  This property is required if clock-frequency is specified.
>> +
>> +- clock-names: Should be "main_clk"
>> +
>> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
>> +  be configured to.
>> +
>> +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
>> +
>> +- reset-supply: phandle to the regulator that provides power to the PHY.
>> +
>> +Example:
>> +
>> +	hsusb1_phy {
>> +		compatible = "usb-nop-xceiv";
>> +		clock-frequency = <19200000>;
>> +		clocks = <&osc 0>;
>> +		clock-names = "main_clk";
>> +		vcc-supply = <&hsusb1_vcc_regulator>;
>> +		reset-supply = <&hsusb1_reset_regulator>;
>> +	};
>> +
>> +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator
>> +and expects that clock to be configured to 19.2MHz by the NOP PHY driver.
>> +hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator
>> +controls RESET.
>> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
>> index ac027a1..adbb7ab 100644
>> --- a/drivers/usb/otg/nop-usb-xceiv.c
>> +++ b/drivers/usb/otg/nop-usb-xceiv.c
>> @@ -34,6 +34,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/clk.h>
>>  #include <linux/regulator/consumer.h>
>> +#include <linux/of.h>
>>  
>>  struct nop_usb_xceiv {
>>  	struct usb_phy		phy;
>> @@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
>>  	return 0;
>>  }
>>  
>> +static void nop_xeiv_get_dt_pdata(struct device *dev,
> 
> asking to remove, but xeiv != xceiv :-)
> 
>> +				struct nop_usb_xceiv_platform_data *pdata)
>> +{
>> +	struct device_node *node = dev->of_node;
>> +	u32 clk_rate;
>> +
>> +	if (!of_property_read_u32(node, "clock-frequency", &clk_rate))
>> +		pdata->clk_rate = clk_rate;
>> +}
>> +
>>  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;
>>  	struct nop_usb_xceiv	*nop;
>>  	enum usb_phy_type	type = USB_PHY_TYPE_USB2;
>> @@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
>>  	if (!nop->phy.otg)
>>  		return -ENOMEM;
>>  
>> +	if (dev->of_node) {
>> +		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +		if (!pdata) {
>> +			dev_err(dev, "Memory allocation failure\n");
>> +			return -ENOMEM;
>> +		}
>> +		nop_xeiv_get_dt_pdata(dev, pdata);
> 
> actually, I would prefer to not create pdata at all. I mean, ideally
> pdata would be used to initialize fields in your own structure, so first
> move clk_rate to your own private structure, copy pdata's clk_rate value
> to that, then you don't need this hackery when using DT.

As far as I can see, clk_rate is never used, but in the probe function.
Why should it be saved into the private data structure at all?

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Roger Quadros]

On 03/11/2013 05:52 PM, Marc Kleine-Budde wrote:
> On 02/05/2013 08:26 AM, Felipe Balbi wrote:
>> Hi,
>>
>> On Mon, Feb 04, 2013 at 05:58:48PM +0200, Roger Quadros wrote:
>>> The PHY clock, clock rate, VCC regulator and RESET regulator
>>> can now be provided via device tree.
>>>
>>> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 ++++++++++++++++++++
>>>  drivers/usb/otg/nop-usb-xceiv.c                    |   31 ++++++++++++++++++
>>>  2 files changed, 65 insertions(+), 0 deletions(-)
>>>  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>> new file mode 100644
>>> index 0000000..d7e2726
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>> @@ -0,0 +1,34 @@
>>> +USB NOP PHY
>>> +
>>> +Required properties:
>>> +- compatible: should be usb-nop-xceiv
>>> +
>>> +Optional properties:
>>> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
>>> +  /bindings/clock/clock-bindings.txt
>>> +  This property is required if clock-frequency is specified.
>>> +
>>> +- clock-names: Should be "main_clk"
>>> +
>>> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
>>> +  be configured to.
>>> +
>>> +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
>>> +
>>> +- reset-supply: phandle to the regulator that provides power to the PHY.
>>> +
>>> +Example:
>>> +
>>> +	hsusb1_phy {
>>> +		compatible = "usb-nop-xceiv";
>>> +		clock-frequency = <19200000>;
>>> +		clocks = <&osc 0>;
>>> +		clock-names = "main_clk";
>>> +		vcc-supply = <&hsusb1_vcc_regulator>;
>>> +		reset-supply = <&hsusb1_reset_regulator>;
>>> +	};
>>> +
>>> +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator
>>> +and expects that clock to be configured to 19.2MHz by the NOP PHY driver.
>>> +hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator
>>> +controls RESET.
>>> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
>>> index ac027a1..adbb7ab 100644
>>> --- a/drivers/usb/otg/nop-usb-xceiv.c
>>> +++ b/drivers/usb/otg/nop-usb-xceiv.c
>>> @@ -34,6 +34,7 @@
>>>  #include <linux/slab.h>
>>>  #include <linux/clk.h>
>>>  #include <linux/regulator/consumer.h>
>>> +#include <linux/of.h>
>>>  
>>>  struct nop_usb_xceiv {
>>>  	struct usb_phy		phy;
>>> @@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
>>>  	return 0;
>>>  }
>>>  
>>> +static void nop_xeiv_get_dt_pdata(struct device *dev,
>>
>> asking to remove, but xeiv != xceiv :-)
>>
>>> +				struct nop_usb_xceiv_platform_data *pdata)
>>> +{
>>> +	struct device_node *node = dev->of_node;
>>> +	u32 clk_rate;
>>> +
>>> +	if (!of_property_read_u32(node, "clock-frequency", &clk_rate))
>>> +		pdata->clk_rate = clk_rate;
>>> +}
>>> +
>>>  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;
>>>  	struct nop_usb_xceiv	*nop;
>>>  	enum usb_phy_type	type = USB_PHY_TYPE_USB2;
>>> @@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
>>>  	if (!nop->phy.otg)
>>>  		return -ENOMEM;
>>>  
>>> +	if (dev->of_node) {
>>> +		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>>> +		if (!pdata) {
>>> +			dev_err(dev, "Memory allocation failure\n");
>>> +			return -ENOMEM;
>>> +		}
>>> +		nop_xeiv_get_dt_pdata(dev, pdata);
>>
>> actually, I would prefer to not create pdata at all. I mean, ideally
>> pdata would be used to initialize fields in your own structure, so first
>> move clk_rate to your own private structure, copy pdata's clk_rate value
>> to that, then you don't need this hackery when using DT.
> 
> As far as I can see, clk_rate is never used, but in the probe function.
> Why should it be saved into the private data structure at all?
> 
Yes you are right. I'll fix it up.
Thanks.

cheers,
-roger

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Marc Kleine-Budde]

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

On 02/04/2013 04:58 PM, Roger Quadros wrote:
> The PHY clock, clock rate, VCC regulator and RESET regulator
> can now be provided via device tree.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 ++++++++++++++++++++
>  drivers/usb/otg/nop-usb-xceiv.c                    |   31 ++++++++++++++++++
>  2 files changed, 65 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> new file mode 100644
> index 0000000..d7e2726
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> @@ -0,0 +1,34 @@
> +USB NOP PHY
> +
> +Required properties:
> +- compatible: should be usb-nop-xceiv
> +
> +Optional properties:
> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
> +  /bindings/clock/clock-bindings.txt
> +  This property is required if clock-frequency is specified.
> +
> +- clock-names: Should be "main_clk"
> +
> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
> +  be configured to.
> +
> +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
> +
> +- reset-supply: phandle to the regulator that provides power to the PHY.
> +
> +Example:
> +
> +	hsusb1_phy {
> +		compatible = "usb-nop-xceiv";
> +		clock-frequency = <19200000>;

Why do you hardcode the clock frequency here? You should use
clk_get_rate() to get the frequency from the clock tree.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Roger Quadros]

On 03/08/2013 12:46 PM, Marc Kleine-Budde wrote:
> On 02/04/2013 04:58 PM, Roger Quadros wrote:
>> The PHY clock, clock rate, VCC regulator and RESET regulator
>> can now be provided via device tree.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 ++++++++++++++++++++
>>  drivers/usb/otg/nop-usb-xceiv.c                    |   31 ++++++++++++++++++
>>  2 files changed, 65 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> new file mode 100644
>> index 0000000..d7e2726
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> @@ -0,0 +1,34 @@
>> +USB NOP PHY
>> +
>> +Required properties:
>> +- compatible: should be usb-nop-xceiv
>> +
>> +Optional properties:
>> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
>> +  /bindings/clock/clock-bindings.txt
>> +  This property is required if clock-frequency is specified.
>> +
>> +- clock-names: Should be "main_clk"
>> +
>> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
>> +  be configured to.
>> +
>> +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
>> +
>> +- reset-supply: phandle to the regulator that provides power to the PHY.
>> +
>> +Example:
>> +
>> +	hsusb1_phy {
>> +		compatible = "usb-nop-xceiv";
>> +		clock-frequency = <19200000>;
> 
> Why do you hardcode the clock frequency here? You should use
> clk_get_rate() to get the frequency from the clock tree.

That would work only if the clock was programmed to the correct frequency
by someone.

e.g. In the OMAP case nobody programs the auxiliary clock on Panda which clocks
the USB PHY.

The usb-nop-xceiv device driver must program the clock rate using clk_set_rate(),
but it needs to know what frequency it must program it to. Different boards/PHYs
might use a different clock frequency. The 'clock-frequency' property
is used to pass on this information to the driver.

cheers,
-roger

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 2183 bytes --]

On 03/08/2013 11:46 AM, Marc Kleine-Budde wrote:
> On 02/04/2013 04:58 PM, Roger Quadros wrote:
>> The PHY clock, clock rate, VCC regulator and RESET regulator
>> can now be provided via device tree.
>>
>> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>> ---
>>  .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 ++++++++++++++++++++
>>  drivers/usb/otg/nop-usb-xceiv.c                    |   31 ++++++++++++++++++
>>  2 files changed, 65 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> new file mode 100644
>> index 0000000..d7e2726
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> @@ -0,0 +1,34 @@
>> +USB NOP PHY
>> +
>> +Required properties:
>> +- compatible: should be usb-nop-xceiv
>> +
>> +Optional properties:
>> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
>> +  /bindings/clock/clock-bindings.txt
>> +  This property is required if clock-frequency is specified.
>> +
>> +- clock-names: Should be "main_clk"
>> +
>> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
>> +  be configured to.
>> +
>> +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
>> +
>> +- reset-supply: phandle to the regulator that provides power to the PHY.
>> +
>> +Example:
>> +
>> +	hsusb1_phy {
>> +		compatible = "usb-nop-xceiv";
>> +		clock-frequency = <19200000>;
> 
> Why do you hardcode the clock frequency here? You should use
> clk_get_rate() to get the frequency from the clock tree.

What about declaring a "fixed-clock" node in the device tree? Then it
should be possible to keep the driver free of any omap specific code.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Roger Quadros]

On 03/08/2013 05:45 PM, Marc Kleine-Budde wrote:
> On 03/08/2013 11:46 AM, Marc Kleine-Budde wrote:
>> On 02/04/2013 04:58 PM, Roger Quadros wrote:
>>> The PHY clock, clock rate, VCC regulator and RESET regulator
>>> can now be provided via device tree.
>>>
>>> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 ++++++++++++++++++++
>>>  drivers/usb/otg/nop-usb-xceiv.c                    |   31 ++++++++++++++++++
>>>  2 files changed, 65 insertions(+), 0 deletions(-)
>>>  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>> new file mode 100644
>>> index 0000000..d7e2726
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>> @@ -0,0 +1,34 @@
>>> +USB NOP PHY
>>> +
>>> +Required properties:
>>> +- compatible: should be usb-nop-xceiv
>>> +
>>> +Optional properties:
>>> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
>>> +  /bindings/clock/clock-bindings.txt
>>> +  This property is required if clock-frequency is specified.
>>> +
>>> +- clock-names: Should be "main_clk"
>>> +
>>> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
>>> +  be configured to.
>>> +
>>> +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
>>> +
>>> +- reset-supply: phandle to the regulator that provides power to the PHY.
>>> +
>>> +Example:
>>> +
>>> +	hsusb1_phy {
>>> +		compatible = "usb-nop-xceiv";
>>> +		clock-frequency = <19200000>;
>>
>> Why do you hardcode the clock frequency here? You should use
>> clk_get_rate() to get the frequency from the clock tree.
> 
> What about declaring a "fixed-clock" node in the device tree? Then it
> should be possible to keep the driver free of any omap specific code.
> 

The current implementation is not OMAP specific and is not limited to a
fixed frequency clock. The PHY driver is using standard clock APIs to set
the clock rate 'only' if the 'clock-frequency' property is present in the
device tree node.

What is the benefit of declaring it as a "fixed-clock"?
FYI. The clock may not necessarily be a fixed frequency clock and someone
needs to program the rate.

cheers,
-roger

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Marc Kleine-Budde]

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

On 03/11/2013 09:40 AM, Roger Quadros wrote:
> On 03/08/2013 05:45 PM, Marc Kleine-Budde wrote:
>> On 03/08/2013 11:46 AM, Marc Kleine-Budde wrote:
>>> On 02/04/2013 04:58 PM, Roger Quadros wrote:
>>>> The PHY clock, clock rate, VCC regulator and RESET regulator
>>>> can now be provided via device tree.
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> ---
>>>>  .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 ++++++++++++++++++++
>>>>  drivers/usb/otg/nop-usb-xceiv.c                    |   31 ++++++++++++++++++
>>>>  2 files changed, 65 insertions(+), 0 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>>> new file mode 100644
>>>> index 0000000..d7e2726
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>>> @@ -0,0 +1,34 @@
>>>> +USB NOP PHY
>>>> +
>>>> +Required properties:
>>>> +- compatible: should be usb-nop-xceiv
>>>> +
>>>> +Optional properties:
>>>> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
>>>> +  /bindings/clock/clock-bindings.txt
>>>> +  This property is required if clock-frequency is specified.
>>>> +
>>>> +- clock-names: Should be "main_clk"
>>>> +
>>>> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
>>>> +  be configured to.
>>>> +
>>>> +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
>>>> +
>>>> +- reset-supply: phandle to the regulator that provides power to the PHY.
>>>> +
>>>> +Example:
>>>> +
>>>> +	hsusb1_phy {
>>>> +		compatible = "usb-nop-xceiv";
>>>> +		clock-frequency = <19200000>;
>>>
>>> Why do you hardcode the clock frequency here? You should use
>>> clk_get_rate() to get the frequency from the clock tree.
>>
>> What about declaring a "fixed-clock" node in the device tree? Then it
>> should be possible to keep the driver free of any omap specific code.
>>
> 
> The current implementation is not OMAP specific and is not limited to a
> fixed frequency clock. The PHY driver is using standard clock APIs to set
> the clock rate 'only' if the 'clock-frequency' property is present in the
> device tree node.
> 
> What is the benefit of declaring it as a "fixed-clock"?
> FYI. The clock may not necessarily be a fixed frequency clock and someone
> needs to program the rate.

Okay, now I got it.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[PATCH 05/13] USB: ehci-omap: Get platform resources by index rather than by name

[Roger Quadros]

Since there is only one resource per type we don't really need
to use resource name to obtain it. This also also makes it easier
for device tree adaptation.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/host/ehci-omap.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 60c4a2d..5a831aef 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -148,14 +148,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	irq = platform_get_irq_byname(pdev, "ehci-irq");
+	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		dev_err(dev, "EHCI irq failed\n");
 		return -ENODEV;
 	}
 
-	res =  platform_get_resource_byname(pdev,
-				IORESOURCE_MEM, "ehci");
+	res =  platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	regs = devm_ioremap_resource(dev, res);
 	if (IS_ERR(regs)) {
 		dev_err(dev, "Resource request/ioremap failed\n");
-- 
1.7.4.1

Re: [PATCH 05/13] USB: ehci-omap: Get platform resources by index rather than by name

[Alan Stern]

On Mon, 4 Feb 2013, Roger Quadros wrote:

> Since there is only one resource per type we don't really need
> to use resource name to obtain it. This also also makes it easier
> for device tree adaptation.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>

Acked-by: Alan Stern <stern@rowland.harvard.edu>

[PATCH 06/13] USB: ohci-omap3: Get platform resources by index rather than by name

[Roger Quadros]

Since there is only one resource per type we don't really need
to use resource name to obtain it. This also also makes it easier
for device tree adaptation.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/host/ohci-omap3.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ohci-omap3.c b/drivers/usb/host/ohci-omap3.c
index eb35d96..5ed28c5 100644
--- a/drivers/usb/host/ohci-omap3.c
+++ b/drivers/usb/host/ohci-omap3.c
@@ -141,14 +141,13 @@ static int ohci_hcd_omap3_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	irq = platform_get_irq_byname(pdev, "ohci-irq");
+	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		dev_err(dev, "OHCI irq failed\n");
 		return -ENODEV;
 	}
 
-	res = platform_get_resource_byname(pdev,
-				IORESOURCE_MEM, "ohci");
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(dev, "UHH OHCI get resource failed\n");
 		return -ENOMEM;
-- 
1.7.4.1

Re: [PATCH 06/13] USB: ohci-omap3: Get platform resources by index rather than by name

[Alan Stern]

On Mon, 4 Feb 2013, Roger Quadros wrote:

> Since there is only one resource per type we don't really need
> to use resource name to obtain it. This also also makes it easier
> for device tree adaptation.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>

Acked-by: Alan Stern <stern@rowland.harvard.edu>

[PATCH 07/13] USB: ohci-omap3: Add device tree support and binding information

[Roger Quadros]

Allows the OHCI controller found in OMAP3 and later chips to
be specified via device tree.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 .../devicetree/bindings/usb/omap3-ohci.txt         |   17 +++++++++++++++++
 drivers/usb/host/ohci-omap3.c                      |   19 +++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/omap3-ohci.txt

diff --git a/Documentation/devicetree/bindings/usb/omap3-ohci.txt b/Documentation/devicetree/bindings/usb/omap3-ohci.txt
new file mode 100644
index 0000000..aabbfdc
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/omap3-ohci.txt
@@ -0,0 +1,17 @@
+OMAP HS USB OHCI controller (OMAP3 and later)
+
+Required properties:
+
+- compatible: should be "ti,omap3-ohci"
+- reg: should contain one register range i.e. start and length
+- interrupt-parent: phandle to the interrupt controller
+- interrupts: description of the interrupt line
+
+Example for OMAP4:
+
+usbhsohci: ohci@0x4a064800 {
+	compatible = "ti,omap3-ohci", "usb-ohci";
+	reg = <0x4a064800 0x400>;
+	interrupt-parent = <&gic>;
+	interrupts = <0 76 0x4>;
+};
diff --git a/drivers/usb/host/ohci-omap3.c b/drivers/usb/host/ohci-omap3.c
index 5ed28c5..b0bfab1 100644
--- a/drivers/usb/host/ohci-omap3.c
+++ b/drivers/usb/host/ohci-omap3.c
@@ -31,6 +31,8 @@
 
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/of.h>
+#include <linux/dma-mapping.h>
 
 /*-------------------------------------------------------------------------*/
 
@@ -112,6 +114,8 @@ static const struct hc_driver ohci_omap3_hc_driver = {
 
 /*-------------------------------------------------------------------------*/
 
+static u64 omap_ohci_dma_mask = DMA_BIT_MASK(32);
+
 /*
  * configure so an HC device and id are always provided
  * always called with process context; sleeping is OK
@@ -159,6 +163,13 @@ static int ohci_hcd_omap3_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	/*
+	 * Right now device-tree probed devices don't get dma_mask set.
+	 * Since shared usb code relies on it, set it here for now.
+	 * Once we have dma capability bindings this can go away.
+	 */
+	if (!pdev->dev.dma_mask)
+		pdev->dev.dma_mask = &omap_ohci_dma_mask;
 
 	hcd = usb_create_hcd(&ohci_omap3_hc_driver, dev,
 			dev_name(dev));
@@ -228,12 +239,20 @@ static void ohci_hcd_omap3_shutdown(struct platform_device *pdev)
 		hcd->driver->shutdown(hcd);
 }
 
+static const struct of_device_id omap_ohci_dt_ids[] = {
+	{ .compatible = "ti,omap3-ohci" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, omap_ohci_dt_ids);
+
 static struct platform_driver ohci_hcd_omap3_driver = {
 	.probe		= ohci_hcd_omap3_probe,
 	.remove		= ohci_hcd_omap3_remove,
 	.shutdown	= ohci_hcd_omap3_shutdown,
 	.driver		= {
 		.name	= "ohci-omap3",
+		.of_match_table = of_match_ptr(omap_ohci_dt_ids),
 	},
 };
 
-- 
1.7.4.1

Re: [PATCH 07/13] USB: ohci-omap3: Add device tree support and binding information

[Alan Stern]

On Mon, 4 Feb 2013, Roger Quadros wrote:

> Allows the OHCI controller found in OMAP3 and later chips to
> be specified via device tree.
> 
> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>

For the ohci-omap3 part:

Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>


--
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

[PATCH 08/13] USB: ehci-omap: Add device tree support and binding information

[Roger Quadros]

Allows the OMAP EHCI controller to be specified via device tree.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 .../devicetree/bindings/usb/omap-ehci.txt          |   34 ++++++++++++++++++
 drivers/usb/host/ehci-omap.c                       |   36 +++++++++++++++++++-
 2 files changed, 69 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/omap-ehci.txt

diff --git a/Documentation/devicetree/bindings/usb/omap-ehci.txt b/Documentation/devicetree/bindings/usb/omap-ehci.txt
new file mode 100644
index 0000000..90e6e3a
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/omap-ehci.txt
@@ -0,0 +1,34 @@
+OMAP HS USB EHCI controller
+
+This device is usually the child of the omap-usb-host
+Documentation/devicetree/bindings/mfd/omap-usb-host.txt
+
+Required properties:
+
+- compatible: should be "ti,omap-ehci"
+- reg: should contain one register range i.e. start and length
+- interrupt-parent: phandle to the interrupt controller
+- interrupts: description of the interrupt line
+
+Optional properties:
+
+- phy: list of phandles to PHY nodes.
+  This property is required if at least one of the ports are in
+  PHY mode i.e. OMAP_EHCI_PORT_MODE_PHY
+
+To specify the port mode, see
+Documentation/devicetree/bindings/mfd/omap-usb-host.txt
+
+Example for OMAP4:
+
+usbhsehci: ehci@0x4a064c00 {
+	compatible = "ti,omap-ehci", "usb-ehci";
+	reg = <0x4a064c00 0x400>;
+	interrupt-parent = <&gic>;
+	interrupts = <0 77 0x4>;
+};
+
+&usbhsehci {
+	phy = <&hsusb1_phy 0 &hsusb3_phy>;
+};
+
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 5a831aef..15cc419 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -48,6 +48,8 @@
 #include <linux/clk.h>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
+#include <linux/of.h>
+#include <linux/dma-mapping.h>
 
 #include "ehci.h"
 
@@ -121,6 +123,8 @@ static const struct ehci_driver_overrides ehci_omap_overrides __initdata = {
 	.extra_priv_size = sizeof(struct omap_hcd),
 };
 
+static u64 omap_ehci_dma_mask = DMA_BIT_MASK(32);
+
 /**
  * ehci_hcd_omap_probe - initialize TI-based HCDs
  *
@@ -148,6 +152,17 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	/* For DT boot, get platform data from parent. i.e. usbhshost */
+	if (dev->of_node) {
+		pdata = dev->parent->platform_data;
+		dev->platform_data = pdata;
+	}
+
+	if (!pdata) {
+		dev_err(dev, "Missing platform data\n");
+		return -ENODEV;
+	}
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		dev_err(dev, "EHCI irq failed\n");
@@ -161,6 +176,14 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		return PTR_ERR(regs);
 	}
 
+	/*
+	 * Right now device-tree probed devices don't get dma_mask set.
+	 * Since shared usb code relies on it, set it here for now.
+	 * Once we have dma capability bindings this can go away.
+	 */
+	if (!pdev->dev.dma_mask)
+		pdev->dev.dma_mask = &omap_ehci_dma_mask;
+
 	hcd = usb_create_hcd(&ehci_omap_hc_driver, dev,
 			dev_name(dev));
 	if (!hcd) {
@@ -185,7 +208,10 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 			continue;
 
 		/* get the PHY device */
-		phy = devm_usb_get_phy_dev(dev, i);
+		if (dev->of_node)
+			phy = devm_usb_get_phy_by_phandle(dev, "phy", i);
+		else
+			phy = devm_usb_get_phy_dev(dev, i);
 		if (IS_ERR(phy) || !phy) {
 			ret = IS_ERR(phy) ? PTR_ERR(phy) : -ENODEV;
 			dev_err(dev, "Can't get PHY device for port %d: %d\n",
@@ -275,6 +301,13 @@ static void ehci_hcd_omap_shutdown(struct platform_device *pdev)
 		hcd->driver->shutdown(hcd);
 }
 
+static const struct of_device_id omap_ehci_dt_ids[] = {
+	{ .compatible = "ti,omap-ehci" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, omap_ehci_dt_ids);
+
 static struct platform_driver ehci_hcd_omap_driver = {
 	.probe			= ehci_hcd_omap_probe,
 	.remove			= ehci_hcd_omap_remove,
@@ -283,6 +316,7 @@ static struct platform_driver ehci_hcd_omap_driver = {
 	/*.resume		= ehci_hcd_omap_resume, */
 	.driver = {
 		.name		= hcd_name,
+		.of_match_table = of_match_ptr(omap_ehci_dt_ids),
 	}
 };
 
-- 
1.7.4.1

Re: [PATCH 08/13] USB: ehci-omap: Add device tree support and binding information

[Alan Stern]

On Mon, 4 Feb 2013, Roger Quadros wrote:

> Allows the OMAP EHCI controller to be specified via device tree.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>

For the ehci-omap part:

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Re: [PATCH 08/13] USB: ehci-omap: Add device tree support and binding information

[Mark Rutland]

On Mon, Feb 04, 2013 at 03:58:55PM +0000, Roger Quadros wrote:
> Allows the OMAP EHCI controller to be specified via device tree.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  .../devicetree/bindings/usb/omap-ehci.txt          |   34 ++++++++++++++++++
>  drivers/usb/host/ehci-omap.c                       |   36 +++++++++++++++++++-
>  2 files changed, 69 insertions(+), 1 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/omap-ehci.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/omap-ehci.txt b/Documentation/devicetree/bindings/usb/omap-ehci.txt
> new file mode 100644
> index 0000000..90e6e3a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/omap-ehci.txt
> @@ -0,0 +1,34 @@
> +OMAP HS USB EHCI controller
> +
> +This device is usually the child of the omap-usb-host
> +Documentation/devicetree/bindings/mfd/omap-usb-host.txt
> +
> +Required properties:
> +
> +- compatible: should be "ti,omap-ehci"
> +- reg: should contain one register range i.e. start and length
> +- interrupt-parent: phandle to the interrupt controller
> +- interrupts: description of the interrupt line
> +
> +Optional properties:
> +
> +- phy: list of phandles to PHY nodes.
> +  This property is required if at least one of the ports are in
> +  PHY mode i.e. OMAP_EHCI_PORT_MODE_PHY

Any reason for not calling this "phys", given it's a list?

[...]

Thanks,
Mark.

Re: [PATCH 08/13] USB: ehci-omap: Add device tree support and binding information

[Roger Quadros]

On 02/05/2013 02:33 PM, Mark Rutland wrote:
> On Mon, Feb 04, 2013 at 03:58:55PM +0000, Roger Quadros wrote:
>> Allows the OMAP EHCI controller to be specified via device tree.
>>
>> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>> ---
>>  .../devicetree/bindings/usb/omap-ehci.txt          |   34 ++++++++++++++++++
>>  drivers/usb/host/ehci-omap.c                       |   36 +++++++++++++++++++-
>>  2 files changed, 69 insertions(+), 1 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/usb/omap-ehci.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/omap-ehci.txt b/Documentation/devicetree/bindings/usb/omap-ehci.txt
>> new file mode 100644
>> index 0000000..90e6e3a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/omap-ehci.txt
>> @@ -0,0 +1,34 @@
>> +OMAP HS USB EHCI controller
>> +
>> +This device is usually the child of the omap-usb-host
>> +Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> +
>> +Required properties:
>> +
>> +- compatible: should be "ti,omap-ehci"
>> +- reg: should contain one register range i.e. start and length
>> +- interrupt-parent: phandle to the interrupt controller
>> +- interrupts: description of the interrupt line
>> +
>> +Optional properties:
>> +
>> +- phy: list of phandles to PHY nodes.
>> +  This property is required if at least one of the ports are in
>> +  PHY mode i.e. OMAP_EHCI_PORT_MODE_PHY
> 
> Any reason for not calling this "phys", given it's a list?
>

No good reason. "phys" seems more appropriate. Thanks.

cheers,
-roger
--
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

[PATCH 02/13] USB: phy: nop: Defer probe if device needs VCC/RESET

[Roger Quadros]

Add 2 flags, needs_vcc and needs_reset to platform data.
If the flag is set and the regulator couldn't be found
then we bail out with -EPROBE_DEFER.

For device tree boot we depend on presensce of vcc-supply/
reset-supply properties to decide if we should bail out
with -EPROBE_DEFER or just continue in case the regulator
can't be found.

This is required for proper functionality in cases where the
regulator is needed but is probed later than the PHY device.

Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/otg/nop-usb-xceiv.c   |    8 ++++++++
 include/linux/usb/nop-usb-xceiv.h |    4 ++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
index adbb7ab..7860e7569 100644
--- a/drivers/usb/otg/nop-usb-xceiv.c
+++ b/drivers/usb/otg/nop-usb-xceiv.c
@@ -147,6 +147,10 @@ static void nop_xeiv_get_dt_pdata(struct device *dev,
 
 	if (!of_property_read_u32(node, "clock-frequency", &clk_rate))
 		pdata->clk_rate = clk_rate;
+	if (of_property_read_bool(node, "vcc-supply"))
+		pdata->needs_vcc = true;
+	if (of_property_read_bool(node, "reset-supply"))
+		pdata->needs_reset = true;
 }
 
 static int nop_usb_xceiv_probe(struct platform_device *pdev)
@@ -205,12 +209,16 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	if (IS_ERR(nop->vcc)) {
 		dev_dbg(&pdev->dev, "Error getting vcc regulator: %ld\n",
 					PTR_ERR(nop->vcc));
+		if (pdata->needs_vcc)
+			return -EPROBE_DEFER;
 	}
 
 	nop->reset = devm_regulator_get(&pdev->dev, "reset");
 	if (IS_ERR(nop->reset)) {
 		dev_dbg(&pdev->dev, "Error getting reset regulator: %ld\n",
 					PTR_ERR(nop->reset));
+		if (pdata->needs_reset)
+			return -EPROBE_DEFER;
 	}
 
 	nop->dev		= &pdev->dev;
diff --git a/include/linux/usb/nop-usb-xceiv.h b/include/linux/usb/nop-usb-xceiv.h
index 3265b61..148d351 100644
--- a/include/linux/usb/nop-usb-xceiv.h
+++ b/include/linux/usb/nop-usb-xceiv.h
@@ -6,6 +6,10 @@
 struct nop_usb_xceiv_platform_data {
 	enum usb_phy_type type;
 	unsigned long clk_rate;
+
+	/* if set fails with -EPROBE_DEFER if can't get regulator */
+	unsigned int needs_vcc:1;
+	unsigned int needs_reset:1;
 };
 
 #if defined(CONFIG_NOP_USB_XCEIV) || (defined(CONFIG_NOP_USB_XCEIV_MODULE) && defined(MODULE))
-- 
1.7.4.1

--
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 02/13] USB: phy: nop: Defer probe if device needs VCC/RESET

[kishon]

On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
> Add 2 flags, needs_vcc and needs_reset to platform data.
> If the flag is set and the regulator couldn't be found
> then we bail out with -EPROBE_DEFER.
>
> For device tree boot we depend on presensce of vcc-supply/
> reset-supply properties to decide if we should bail out
> with -EPROBE_DEFER or just continue in case the regulator
> can't be found.
>
> This is required for proper functionality in cases where the
> regulator is needed but is probed later than the PHY device.
>
> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
> ---
>   drivers/usb/otg/nop-usb-xceiv.c   |    8 ++++++++
>   include/linux/usb/nop-usb-xceiv.h |    4 ++++
>   2 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
> index adbb7ab..7860e7569 100644
> --- a/drivers/usb/otg/nop-usb-xceiv.c
> +++ b/drivers/usb/otg/nop-usb-xceiv.c
> @@ -147,6 +147,10 @@ static void nop_xeiv_get_dt_pdata(struct device *dev,
>
>   	if (!of_property_read_u32(node, "clock-frequency", &clk_rate))
>   		pdata->clk_rate = clk_rate;
> +	if (of_property_read_bool(node, "vcc-supply"))
> +		pdata->needs_vcc = true;
This can be written as..
pdata->needs_vcc = of_property_read_bool(node, "vcc-supply");

> +	if (of_property_read_bool(node, "reset-supply"))
> +		pdata->needs_reset = true;
same here..
>   }
>
>   static int nop_usb_xceiv_probe(struct platform_device *pdev)
> @@ -205,12 +209,16 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
>   	if (IS_ERR(nop->vcc)) {
>   		dev_dbg(&pdev->dev, "Error getting vcc regulator: %ld\n",
>   					PTR_ERR(nop->vcc));
> +		if (pdata->needs_vcc)
> +			return -EPROBE_DEFER;
>   	}
>
>   	nop->reset = devm_regulator_get(&pdev->dev, "reset");
>   	if (IS_ERR(nop->reset)) {
>   		dev_dbg(&pdev->dev, "Error getting reset regulator: %ld\n",
>   					PTR_ERR(nop->reset));
> +		if (pdata->needs_reset)
> +			return -EPROBE_DEFER;
>   	}
>
>   	nop->dev		= &pdev->dev;
> diff --git a/include/linux/usb/nop-usb-xceiv.h b/include/linux/usb/nop-usb-xceiv.h
> index 3265b61..148d351 100644
> --- a/include/linux/usb/nop-usb-xceiv.h
> +++ b/include/linux/usb/nop-usb-xceiv.h
> @@ -6,6 +6,10 @@
>   struct nop_usb_xceiv_platform_data {
>   	enum usb_phy_type type;
>   	unsigned long clk_rate;
> +
> +	/* if set fails with -EPROBE_DEFER if can't get regulator */
> +	unsigned int needs_vcc:1;
> +	unsigned int needs_reset:1;

how about u8 here?

Thanks
Kishon

Re: [PATCH 02/13] USB: phy: nop: Defer probe if device needs VCC/RESET

[Roger Quadros]

On 02/05/2013 07:54 AM, kishon wrote:
> On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
>> Add 2 flags, needs_vcc and needs_reset to platform data.
>> If the flag is set and the regulator couldn't be found
>> then we bail out with -EPROBE_DEFER.
>>
>> For device tree boot we depend on presensce of vcc-supply/
>> reset-supply properties to decide if we should bail out
>> with -EPROBE_DEFER or just continue in case the regulator
>> can't be found.
>>
>> This is required for proper functionality in cases where the
>> regulator is needed but is probed later than the PHY device.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>   drivers/usb/otg/nop-usb-xceiv.c   |    8 ++++++++
>>   include/linux/usb/nop-usb-xceiv.h |    4 ++++
>>   2 files changed, 12 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
>> index adbb7ab..7860e7569 100644
>> --- a/drivers/usb/otg/nop-usb-xceiv.c
>> +++ b/drivers/usb/otg/nop-usb-xceiv.c
>> @@ -147,6 +147,10 @@ static void nop_xeiv_get_dt_pdata(struct device *dev,
>>
>>       if (!of_property_read_u32(node, "clock-frequency", &clk_rate))
>>           pdata->clk_rate = clk_rate;
>> +    if (of_property_read_bool(node, "vcc-supply"))
>> +        pdata->needs_vcc = true;
> This can be written as..
> pdata->needs_vcc = of_property_read_bool(node, "vcc-supply");

OK.

> 
>> +    if (of_property_read_bool(node, "reset-supply"))
>> +        pdata->needs_reset = true;
> same here..
>>   }
>>
>>   static int nop_usb_xceiv_probe(struct platform_device *pdev)
>> @@ -205,12 +209,16 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
>>       if (IS_ERR(nop->vcc)) {
>>           dev_dbg(&pdev->dev, "Error getting vcc regulator: %ld\n",
>>                       PTR_ERR(nop->vcc));
>> +        if (pdata->needs_vcc)
>> +            return -EPROBE_DEFER;
>>       }
>>
>>       nop->reset = devm_regulator_get(&pdev->dev, "reset");
>>       if (IS_ERR(nop->reset)) {
>>           dev_dbg(&pdev->dev, "Error getting reset regulator: %ld\n",
>>                       PTR_ERR(nop->reset));
>> +        if (pdata->needs_reset)
>> +            return -EPROBE_DEFER;
>>       }
>>
>>       nop->dev        = &pdev->dev;
>> diff --git a/include/linux/usb/nop-usb-xceiv.h b/include/linux/usb/nop-usb-xceiv.h
>> index 3265b61..148d351 100644
>> --- a/include/linux/usb/nop-usb-xceiv.h
>> +++ b/include/linux/usb/nop-usb-xceiv.h
>> @@ -6,6 +6,10 @@
>>   struct nop_usb_xceiv_platform_data {
>>       enum usb_phy_type type;
>>       unsigned long clk_rate;
>> +
>> +    /* if set fails with -EPROBE_DEFER if can't get regulator */
>> +    unsigned int needs_vcc:1;
>> +    unsigned int needs_reset:1;
> 
> how about u8 here?

Not sure. Bitfields are usually defined as unsigned int.

cheers,
-roger

Re: [PATCH 02/13] USB: phy: nop: Defer probe if device needs VCC/RESET

[Felipe Balbi]

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

On Tue, Feb 05, 2013 at 10:44:05AM +0200, Roger Quadros wrote:
> >> diff --git a/include/linux/usb/nop-usb-xceiv.h b/include/linux/usb/nop-usb-xceiv.h
> >> index 3265b61..148d351 100644
> >> --- a/include/linux/usb/nop-usb-xceiv.h
> >> +++ b/include/linux/usb/nop-usb-xceiv.h
> >> @@ -6,6 +6,10 @@
> >>   struct nop_usb_xceiv_platform_data {
> >>       enum usb_phy_type type;
> >>       unsigned long clk_rate;
> >> +
> >> +    /* if set fails with -EPROBE_DEFER if can't get regulator */
> >> +    unsigned int needs_vcc:1;
> >> +    unsigned int needs_reset:1;
> > 
> > how about u8 here?
> 
> Not sure. Bitfields are usually defined as unsigned int.

IIRC the benefit is that compiler can try to optimize those flags. I
mean, if you have 32 1-bit flags, compiler will combine those in a
single u32. Someone correct me if I'm wrong.

after you fix other comments from kishon (about of_read_bool()), you can
add my:

Acked-by: Felipe Balbi <balbi@ti.com>

-- 
balbi

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

Re: [PATCH 02/13] USB: phy: nop: Defer probe if device needs VCC/RESET

[Roger Quadros]

On 02/05/2013 11:09 AM, Felipe Balbi wrote:
> On Tue, Feb 05, 2013 at 10:44:05AM +0200, Roger Quadros wrote:
>>>> diff --git a/include/linux/usb/nop-usb-xceiv.h b/include/linux/usb/nop-usb-xceiv.h
>>>> index 3265b61..148d351 100644
>>>> --- a/include/linux/usb/nop-usb-xceiv.h
>>>> +++ b/include/linux/usb/nop-usb-xceiv.h
>>>> @@ -6,6 +6,10 @@
>>>>   struct nop_usb_xceiv_platform_data {
>>>>       enum usb_phy_type type;
>>>>       unsigned long clk_rate;
>>>> +
>>>> +    /* if set fails with -EPROBE_DEFER if can't get regulator */
>>>> +    unsigned int needs_vcc:1;
>>>> +    unsigned int needs_reset:1;
>>>
>>> how about u8 here?
>>
>> Not sure. Bitfields are usually defined as unsigned int.
> 
> IIRC the benefit is that compiler can try to optimize those flags. I
> mean, if you have 32 1-bit flags, compiler will combine those in a
> single u32. Someone correct me if I'm wrong.
> 

Yes you are right. Kishon was asking me to use u8 instead of unsigned int, which
I don't think is necessary. AFAIK, it is a norm to use unsigned int when defining
a bitfield. Compilers are known to behave funny with bitfields. I don't mind
using bool for each.

cheers,
-roger

Re: [PATCH 02/13] USB: phy: nop: Defer probe if device needs VCC/RESET

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1739 bytes --]

On 02/05/2013 10:43 AM, Roger Quadros wrote:
> On 02/05/2013 11:09 AM, Felipe Balbi wrote:
>> On Tue, Feb 05, 2013 at 10:44:05AM +0200, Roger Quadros wrote:
>>>>> diff --git a/include/linux/usb/nop-usb-xceiv.h b/include/linux/usb/nop-usb-xceiv.h
>>>>> index 3265b61..148d351 100644
>>>>> --- a/include/linux/usb/nop-usb-xceiv.h
>>>>> +++ b/include/linux/usb/nop-usb-xceiv.h
>>>>> @@ -6,6 +6,10 @@
>>>>>   struct nop_usb_xceiv_platform_data {
>>>>>       enum usb_phy_type type;
>>>>>       unsigned long clk_rate;
>>>>> +
>>>>> +    /* if set fails with -EPROBE_DEFER if can't get regulator */
>>>>> +    unsigned int needs_vcc:1;
>>>>> +    unsigned int needs_reset:1;
>>>>
>>>> how about u8 here?
>>>
>>> Not sure. Bitfields are usually defined as unsigned int.
>>
>> IIRC the benefit is that compiler can try to optimize those flags. I
>> mean, if you have 32 1-bit flags, compiler will combine those in a
>> single u32. Someone correct me if I'm wrong.
>>
> 
> Yes you are right. Kishon was asking me to use u8 instead of unsigned int, which
> I don't think is necessary. AFAIK, it is a norm to use unsigned int when defining
> a bitfield. Compilers are known to behave funny with bitfields. I don't mind
> using bool for each.

In the current version (rogerq/v3.8-usbhost17-dt) of this patch you've
put both needs_* into the private data struct and the pdata, but it's
only used inside the probe function.

regards,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 02/13] USB: phy: nop: Defer probe if device needs VCC/RESET

[Roger Quadros]

On 03/11/2013 05:58 PM, Marc Kleine-Budde wrote:
> On 02/05/2013 10:43 AM, Roger Quadros wrote:
>> On 02/05/2013 11:09 AM, Felipe Balbi wrote:
>>> On Tue, Feb 05, 2013 at 10:44:05AM +0200, Roger Quadros wrote:
>>>>>> diff --git a/include/linux/usb/nop-usb-xceiv.h b/include/linux/usb/nop-usb-xceiv.h
>>>>>> index 3265b61..148d351 100644
>>>>>> --- a/include/linux/usb/nop-usb-xceiv.h
>>>>>> +++ b/include/linux/usb/nop-usb-xceiv.h
>>>>>> @@ -6,6 +6,10 @@
>>>>>>   struct nop_usb_xceiv_platform_data {
>>>>>>       enum usb_phy_type type;
>>>>>>       unsigned long clk_rate;
>>>>>> +
>>>>>> +    /* if set fails with -EPROBE_DEFER if can't get regulator */
>>>>>> +    unsigned int needs_vcc:1;
>>>>>> +    unsigned int needs_reset:1;
>>>>>
>>>>> how about u8 here?
>>>>
>>>> Not sure. Bitfields are usually defined as unsigned int.
>>>
>>> IIRC the benefit is that compiler can try to optimize those flags. I
>>> mean, if you have 32 1-bit flags, compiler will combine those in a
>>> single u32. Someone correct me if I'm wrong.
>>>
>>
>> Yes you are right. Kishon was asking me to use u8 instead of unsigned int, which
>> I don't think is necessary. AFAIK, it is a norm to use unsigned int when defining
>> a bitfield. Compilers are known to behave funny with bitfields. I don't mind
>> using bool for each.
> 
> In the current version (rogerq/v3.8-usbhost17-dt) of this patch you've
> put both needs_* into the private data struct and the pdata, but it's
> only used inside the probe function.
> 

Good catch! Will fix.

cheers,
-roger

[PATCH 03/13] mfd: omap-usb-tll: move configuration code to omap_tll_init()

[Roger Quadros]

This is because we want to get rid of platform_data usage from probe().
The only information we need is PORT_MODE, and this can be supplied
to us by the user (i.e. omap-usb-host.c).

We also move channel clock management from runtime PM handlers into
omap_tll_enable/disable().

CC: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
---
 drivers/mfd/omap-usb-host.c |    7 +-
 drivers/mfd/omap-usb-tll.c  |  204 +++++++++++++++++++++----------------------
 drivers/mfd/omap-usb.h      |    5 +-
 3 files changed, 107 insertions(+), 109 deletions(-)

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 502a779..f8ed08e 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -278,7 +278,7 @@ static int usbhs_runtime_resume(struct device *dev)
 
 	dev_dbg(dev, "usbhs_runtime_resume\n");
 
-	omap_tll_enable();
+	omap_tll_enable(pdata);
 
 	if (!IS_ERR(omap->ehci_logic_fck))
 		clk_enable(omap->ehci_logic_fck);
@@ -353,7 +353,7 @@ static int usbhs_runtime_suspend(struct device *dev)
 	if (!IS_ERR(omap->ehci_logic_fck))
 		clk_disable(omap->ehci_logic_fck);
 
-	omap_tll_disable();
+	omap_tll_disable(pdata);
 
 	return 0;
 }
@@ -499,6 +499,9 @@ static int usbhs_omap_probe(struct platform_device *pdev)
 
 	omap->pdata = pdata;
 
+	/* Initialize the TLL subsystem */
+	omap_tll_init(pdata);
+
 	pm_runtime_enable(dev);
 
 	platform_set_drvdata(pdev, omap);
diff --git a/drivers/mfd/omap-usb-tll.c b/drivers/mfd/omap-usb-tll.c
index 0aef1a7..f7d2568 100644
--- a/drivers/mfd/omap-usb-tll.c
+++ b/drivers/mfd/omap-usb-tll.c
@@ -1,8 +1,9 @@
 /**
  * omap-usb-tll.c - The USB TLL driver for OMAP EHCI & OHCI
  *
- * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2012-2013 Texas Instruments Incorporated - http://www.ti.com
  * Author: Keshava Munegowda <keshava_mgowda-l0cyMroinI0@public.gmane.org>
+ * Author: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2  of
@@ -105,8 +106,8 @@
 
 struct usbtll_omap {
 	int					nch;	/* num. of channels */
-	struct usbhs_omap_platform_data		*pdata;
 	struct clk				**ch_clk;
+	void __iomem				*base;
 };
 
 /*-------------------------------------------------------------------------*/
@@ -210,14 +211,10 @@ static unsigned ohci_omap3_fslsmode(enum usbhs_omap_port_mode mode)
 static int usbtll_omap_probe(struct platform_device *pdev)
 {
 	struct device				*dev =  &pdev->dev;
-	struct usbhs_omap_platform_data		*pdata = dev->platform_data;
-	void __iomem				*base;
 	struct resource				*res;
 	struct usbtll_omap			*tll;
-	unsigned				reg;
 	int					ret = 0;
 	int					i, ver;
-	bool needs_tll;
 
 	dev_dbg(dev, "starting TI HSUSB TLL Controller\n");
 
@@ -227,16 +224,9 @@ static int usbtll_omap_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	if (!pdata) {
-		dev_err(dev, "Platform data missing\n");
-		return -ENODEV;
-	}
-
-	tll->pdata = pdata;
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_request_and_ioremap(dev, res);
-	if (!base) {
+	tll->base = devm_request_and_ioremap(dev, res);
+	if (!tll->base) {
 		ret = -EADDRNOTAVAIL;
 		dev_err(dev, "Resource request/ioremap failed:%d\n", ret);
 		return ret;
@@ -246,7 +236,7 @@ static int usbtll_omap_probe(struct platform_device *pdev)
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
 
-	ver =  usbtll_read(base, OMAP_USBTLL_REVISION);
+	ver =  usbtll_read(tll->base, OMAP_USBTLL_REVISION);
 	switch (ver) {
 	case OMAP_USBTLL_REV1:
 	case OMAP_USBTLL_REV4:
@@ -283,11 +273,77 @@ static int usbtll_omap_probe(struct platform_device *pdev)
 			dev_dbg(dev, "can't get clock : %s\n", clkname);
 	}
 
+	pm_runtime_put_sync(dev);
+	/* only after this can omap_tll_enable/disable work */
+	spin_lock(&tll_lock);
+	tll_dev = dev;
+	spin_unlock(&tll_lock);
+
+	return 0;
+
+err_clk_alloc:
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
+
+	return ret;
+}
+
+/**
+ * usbtll_omap_remove - shutdown processing for UHH & TLL HCDs
+ * @pdev: USB Host Controller being removed
+ *
+ * Reverses the effect of usbtll_omap_probe().
+ */
+static int usbtll_omap_remove(struct platform_device *pdev)
+{
+	struct usbtll_omap *tll = platform_get_drvdata(pdev);
+	int i;
+
+	spin_lock(&tll_lock);
+	tll_dev = NULL;
+	spin_unlock(&tll_lock);
+
+	for (i = 0; i < tll->nch; i++)
+		if (!IS_ERR(tll->ch_clk[i]))
+			clk_put(tll->ch_clk[i]);
+
+	pm_runtime_disable(&pdev->dev);
+	return 0;
+}
+
+static struct platform_driver usbtll_omap_driver = {
+	.driver = {
+		.name		= (char *)usbtll_driver_name,
+		.owner		= THIS_MODULE,
+	},
+	.probe		= usbtll_omap_probe,
+	.remove		= usbtll_omap_remove,
+};
+
+int omap_tll_init(struct usbhs_omap_platform_data *pdata)
+{
+	int i;
+	bool needs_tll;
+	unsigned reg;
+	struct usbtll_omap *tll;
+
+	spin_lock(&tll_lock);
+
+	if (!tll_dev) {
+		spin_unlock(&tll_lock);
+		return -ENODEV;
+	}
+
+	tll = dev_get_drvdata(tll_dev);
+
 	needs_tll = false;
 	for (i = 0; i < tll->nch; i++)
 		needs_tll |= omap_usb_mode_needs_tll(pdata->port_mode[i]);
 
+	pm_runtime_get_sync(tll_dev);
+
 	if (needs_tll) {
+		void __iomem *base = tll->base;
 
 		/* Program Common TLL register */
 		reg = usbtll_read(base, OMAP_TLL_SHARED_CONF);
@@ -336,51 +392,29 @@ static int usbtll_omap_probe(struct platform_device *pdev)
 		}
 	}
 
-	pm_runtime_put_sync(dev);
-	/* only after this can omap_tll_enable/disable work */
-	spin_lock(&tll_lock);
-	tll_dev = dev;
+	pm_runtime_put_sync(tll_dev);
+
 	spin_unlock(&tll_lock);
 
 	return 0;
-
-err_clk_alloc:
-	pm_runtime_put_sync(dev);
-	pm_runtime_disable(dev);
-
-	return ret;
 }
+EXPORT_SYMBOL_GPL(omap_tll_init);
 
-/**
- * usbtll_omap_remove - shutdown processing for UHH & TLL HCDs
- * @pdev: USB Host Controller being removed
- *
- * Reverses the effect of usbtll_omap_probe().
- */
-static int usbtll_omap_remove(struct platform_device *pdev)
+int omap_tll_enable(struct usbhs_omap_platform_data *pdata)
 {
-	struct usbtll_omap *tll = platform_get_drvdata(pdev);
 	int i;
+	struct usbtll_omap *tll;
 
 	spin_lock(&tll_lock);
-	tll_dev = NULL;
-	spin_unlock(&tll_lock);
-
-	for (i = 0; i < tll->nch; i++)
-		if (!IS_ERR(tll->ch_clk[i]))
-			clk_put(tll->ch_clk[i]);
 
-	pm_runtime_disable(&pdev->dev);
-	return 0;
-}
+	if (!tll_dev) {
+		spin_unlock(&tll_lock);
+		return -ENODEV;
+	}
 
-static int usbtll_runtime_resume(struct device *dev)
-{
-	struct usbtll_omap			*tll = dev_get_drvdata(dev);
-	struct usbhs_omap_platform_data		*pdata = tll->pdata;
-	int i;
+	tll = dev_get_drvdata(tll_dev);
 
-	dev_dbg(dev, "usbtll_runtime_resume\n");
+	pm_runtime_get_sync(tll_dev);
 
 	for (i = 0; i < tll->nch; i++) {
 		if (omap_usb_mode_needs_tll(pdata->port_mode[i])) {
@@ -391,22 +425,31 @@ static int usbtll_runtime_resume(struct device *dev)
 
 			r = clk_enable(tll->ch_clk[i]);
 			if (r) {
-				dev_err(dev,
+				dev_err(tll_dev,
 				 "Error enabling ch %d clock: %d\n", i, r);
 			}
 		}
 	}
 
+	spin_unlock(&tll_lock);
+
 	return 0;
 }
+EXPORT_SYMBOL_GPL(omap_tll_enable);
 
-static int usbtll_runtime_suspend(struct device *dev)
+int omap_tll_disable(struct usbhs_omap_platform_data *pdata)
 {
-	struct usbtll_omap			*tll = dev_get_drvdata(dev);
-	struct usbhs_omap_platform_data		*pdata = tll->pdata;
 	int i;
+	struct usbtll_omap *tll;
+
+	spin_lock(&tll_lock);
+
+	if (!tll_dev) {
+		spin_unlock(&tll_lock);
+		return -ENODEV;
+	}
 
-	dev_dbg(dev, "usbtll_runtime_suspend\n");
+	tll = dev_get_drvdata(tll_dev);
 
 	for (i = 0; i < tll->nch; i++) {
 		if (omap_usb_mode_needs_tll(pdata->port_mode[i])) {
@@ -415,60 +458,11 @@ static int usbtll_runtime_suspend(struct device *dev)
 		}
 	}
 
-	return 0;
-}
-
-static const struct dev_pm_ops usbtllomap_dev_pm_ops = {
-	SET_RUNTIME_PM_OPS(usbtll_runtime_suspend,
-			   usbtll_runtime_resume,
-			   NULL)
-};
-
-static struct platform_driver usbtll_omap_driver = {
-	.driver = {
-		.name		= (char *)usbtll_driver_name,
-		.owner		= THIS_MODULE,
-		.pm		= &usbtllomap_dev_pm_ops,
-	},
-	.probe		= usbtll_omap_probe,
-	.remove		= usbtll_omap_remove,
-};
-
-int omap_tll_enable(void)
-{
-	int ret;
-
-	spin_lock(&tll_lock);
-
-	if (!tll_dev) {
-		pr_err("%s: OMAP USB TLL not initialized\n", __func__);
-		ret = -ENODEV;
-	} else {
-		ret = pm_runtime_get_sync(tll_dev);
-	}
+	pm_runtime_put_sync(tll_dev);
 
 	spin_unlock(&tll_lock);
 
-	return ret;
-}
-EXPORT_SYMBOL_GPL(omap_tll_enable);
-
-int omap_tll_disable(void)
-{
-	int ret;
-
-	spin_lock(&tll_lock);
-
-	if (!tll_dev) {
-		pr_err("%s: OMAP USB TLL not initialized\n", __func__);
-		ret = -ENODEV;
-	} else {
-		ret = pm_runtime_put_sync(tll_dev);
-	}
-
-	spin_unlock(&tll_lock);

[PATCH 04/13] mfd: omap-usb-tll: Add device tree support

[Roger Quadros]

Enable this driver to probe in device tree boot.

CC: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
---
 .../devicetree/bindings/mfd/omap-usb-tll.txt       |   17 +++++++++++++++++
 drivers/mfd/omap-usb-tll.c                         |    9 +++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-tll.txt

diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-tll.txt b/Documentation/devicetree/bindings/mfd/omap-usb-tll.txt
new file mode 100644
index 0000000..835cf4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/omap-usb-tll.txt
@@ -0,0 +1,17 @@
+OMAP HS USB Host TLL (Transceiver-Less Interface)
+
+Required properties:
+
+- compatible : should be "ti,usbhs-tll"
+- reg : should contain one register range i.e. start and length
+- interrupts : should contain the TLL module's interrupt
+- ti,hwmod : must contain "usb_tll_hs"
+
+Example:
+
+	usbhstll: usbhstll@0x4a062000 {
+		compatible = "ti,usbhs-tll";
+		reg = <0x4a062000 0x1000>;
+		interrupts = <78>;
+		ti,hwmods = "usb_tll_hs";
+	  };
diff --git a/drivers/mfd/omap-usb-tll.c b/drivers/mfd/omap-usb-tll.c
index f7d2568..f7afb22 100644
--- a/drivers/mfd/omap-usb-tll.c
+++ b/drivers/mfd/omap-usb-tll.c
@@ -28,6 +28,7 @@
 #include <linux/err.h>
 #include <linux/pm_runtime.h>
 #include <linux/platform_data/usb-omap.h>
+#include <linux/of.h>
 
 #define USBTLL_DRIVER_NAME	"usbhs_tll"
 
@@ -311,10 +312,18 @@ static int usbtll_omap_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id usbtll_omap_dt_ids[] = {
+	{ .compatible = "ti,usbhs-tll" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, usbtll_omap_dt_ids);
+
 static struct platform_driver usbtll_omap_driver = {
 	.driver = {
 		.name		= (char *)usbtll_driver_name,
 		.owner		= THIS_MODULE,
+		.of_match_table = of_match_ptr(usbtll_omap_dt_ids),
 	},
 	.probe		= usbtll_omap_probe,
 	.remove		= usbtll_omap_remove,
-- 
1.7.4.1

--
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 04/13] mfd: omap-usb-tll: Add device tree support

[kishon]

On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
> Enable this driver to probe in device tree boot.
>
> CC: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>   .../devicetree/bindings/mfd/omap-usb-tll.txt       |   17 +++++++++++++++++
>   drivers/mfd/omap-usb-tll.c                         |    9 +++++++++
>   2 files changed, 26 insertions(+), 0 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-tll.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-tll.txt b/Documentation/devicetree/bindings/mfd/omap-usb-tll.txt
> new file mode 100644
> index 0000000..835cf4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-tll.txt
> @@ -0,0 +1,17 @@
> +OMAP HS USB Host TLL (Transceiver-Less Interface)
> +
> +Required properties:
> +
> +- compatible : should be "ti,usbhs-tll"
> +- reg : should contain one register range i.e. start and length
> +- interrupts : should contain the TLL module's interrupt
> +- ti,hwmod : must contain "usb_tll_hs"
> +
> +Example:
> +
> +	usbhstll: usbhstll@0x4a062000 {
The node name shouldn't have 0x. This comment applies to all your 
patches adding device tree support.

Thanks
Kishon

Re: [PATCH 04/13] mfd: omap-usb-tll: Add device tree support

[Roger Quadros]

On 02/05/2013 08:04 AM, kishon wrote:
> On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
>> Enable this driver to probe in device tree boot.
>>
>> CC: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>> ---
>>   .../devicetree/bindings/mfd/omap-usb-tll.txt       |   17 +++++++++++++++++
>>   drivers/mfd/omap-usb-tll.c                         |    9 +++++++++
>>   2 files changed, 26 insertions(+), 0 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-tll.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-tll.txt b/Documentation/devicetree/bindings/mfd/omap-usb-tll.txt
>> new file mode 100644
>> index 0000000..835cf4f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-tll.txt
>> @@ -0,0 +1,17 @@
>> +OMAP HS USB Host TLL (Transceiver-Less Interface)
>> +
>> +Required properties:
>> +
>> +- compatible : should be "ti,usbhs-tll"
>> +- reg : should contain one register range i.e. start and length
>> +- interrupts : should contain the TLL module's interrupt
>> +- ti,hwmod : must contain "usb_tll_hs"
>> +
>> +Example:
>> +
>> +    usbhstll: usbhstll@0x4a062000 {
> The node name shouldn't have 0x. This comment applies to all your patches adding device tree support.

OK, will fix.

cheers,
-roger

[PATCH 09/13] mfd: omap-usb-host: Add device tree support and binding information

[Roger Quadros]

Allows the OMAP HS USB host controller to be specified
via device tree.

CC: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
---
 .../devicetree/bindings/mfd/omap-usb-host.txt      |   68 ++++++++++++++++
 drivers/mfd/omap-usb-host.c                        |   83 ++++++++++++++++++--
 2 files changed, 145 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-host.txt

diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
new file mode 100644
index 0000000..2196893
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
@@ -0,0 +1,68 @@
+OMAP HS USB Host
+
+Required properties:
+
+- compatible: should be "ti,usbhs-host"
+- reg: should contain one register range i.e. start and length
+- ti,hwmods: must contain "usb_host_hs"
+
+Optional properties:
+
+- nports: number of USB ports. Usually this is automatically detected
+  from the IP's revision register but can be overridden by specifying
+  this property.
+
+- portN_mode: Integer specifying the port mode for port N, where N can be
+  from 1 to nports. The port mode must be as per enum usbhs_omap_port_mode
+  in include/linux/platform_data/usb-omap.h
+  If the port mode is not specified, that port is treated as unused.
+
+- single_ulpi_bypass: Must be present if the controller contains a single
+  ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
+
+Required properties if child node exists:
+
+- #address-cells: Must be 1
+- #size-cells: Must be 1
+- ranges: must be present
+
+Properties for children:
+
+The OMAP HS USB Host subsystem contains EHCI and OHCI controllers.
+See Documentation/devicetree/bindings/usb/omap-ehci.txt and
+omap3-ohci.txt
+
+Example for OMAP4:
+
+usbhshost: usbhshost@0x4a064000 {
+	compatible = "ti,usbhs-host";
+	reg = <0x4a064000 0x800>;
+	ti,hwmods = "usb_host_hs";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+
+	usbhsohci: ohci@0x4a064800 {
+		compatible = "ti,omap3-ohci", "usb-ohci";
+		reg = <0x4a064800 0x400>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 76 0x4>;
+	};
+
+	usbhsehci: ehci@0x4a064c00 {
+		compatible = "ti,omap-ehci", "usb-ehci";
+		reg = <0x4a064c00 0x400>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 77 0x4>;
+	};
+};
+
+&usbhshost {
+	port1_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
+	port2_mode = <2>; /* OMAP_EHCI_PORT_MODE_TLL */
+	port3_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
+};
+
+&usbhsehci {
+	phy = <&hsusb1_phy 0 &hsusb3_phy>;
+};
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index f8ed08e..0f67856 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -1,8 +1,9 @@
 /**
  * omap-usb-host.c - The USBHS core driver for OMAP EHCI & OHCI
  *
- * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2011-2013 Texas Instruments Incorporated - http://www.ti.com
  * Author: Keshava Munegowda <keshava_mgowda-l0cyMroinI0@public.gmane.org>
+ * Author: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2  of
@@ -27,6 +28,8 @@
 #include <linux/platform_device.h>
 #include <linux/platform_data/usb-omap.h>
 #include <linux/pm_runtime.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
 
 #include "omap-usb.h"
 
@@ -464,6 +467,37 @@ static void omap_usbhs_init(struct device *dev)
 	pm_runtime_put_sync(dev);
 }
 
+static int usbhs_omap_get_dt_pdata(struct device_node *node,
+					struct usbhs_omap_platform_data *pdata)
+{
+	int ret, i;
+
+	ret = of_property_read_u32(node, "nports", &pdata->nports);
+	if (ret)
+		pdata->nports = 0;
+
+	/* get port modes */
+	for (i = 0; i < OMAP3_HS_USB_PORTS; i++) {
+		char prop[11];
+
+		snprintf(prop, sizeof(prop), "port%d_mode", i + 1);
+		ret = of_property_read_u32(node, prop, &pdata->port_mode[i]);
+		if (ret)
+			pdata->port_mode[i] = OMAP_USBHS_PORT_MODE_UNUSED;
+	}
+
+	/* get flags */
+	pdata->single_ulpi_bypass = of_property_read_bool(node,
+						"single_ulpi_bypass");
+	return 0;
+}
+
+static struct of_device_id usbhs_child_match_table[] __initdata = {
+	{ .compatible = "ti,omap-ehci", },
+	{ .compatible = "ti,omap-ohci", },
+	{ }
+};
+
 /**
  * usbhs_omap_probe - initialize TI-based HCDs
  *
@@ -479,6 +513,21 @@ static int usbhs_omap_probe(struct platform_device *pdev)
 	int				i;
 	bool				need_logic_fck;
 
+	if (dev->of_node) {
+		/* For DT boot we populate platform data from OF node */
+		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+		if (!pdata)
+			return -ENOMEM;
+
+		if (usbhs_omap_get_dt_pdata(dev->of_node, pdata)) {
+			dev_err(dev,
+				"Error getting platform data from DT node\n");
+			return -ENODEV;
+		}
+
+		dev->platform_data = pdata;
+	}
+
 	if (!pdata) {
 		dev_err(dev, "Missing platform data\n");
 		return -ENODEV;
@@ -490,7 +539,7 @@ static int usbhs_omap_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "uhh");
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	omap->uhh_base = devm_request_and_ioremap(dev, res);
 	if (!omap->uhh_base) {
 		dev_err(dev, "Resource request/ioremap failed\n");
@@ -661,10 +710,23 @@ static int usbhs_omap_probe(struct platform_device *pdev)
 	}
 
 	omap_usbhs_init(dev);
-	ret = omap_usbhs_alloc_children(pdev);
-	if (ret) {
-		dev_err(dev, "omap_usbhs_alloc_children failed\n");
-		goto err_alloc;
+
+	if (dev->of_node) {
+		ret = of_platform_populate(dev->of_node,
+				usbhs_child_match_table, NULL, dev);
+
+		if (ret) {
+			dev_err(dev, "Failed to create DT children: %d\n", ret);
+			goto err_alloc;
+		}
+
+	} else {
+		ret = omap_usbhs_alloc_children(pdev);
+		if (ret) {
+			dev_err(dev, "omap_usbhs_alloc_children failed: %d\n",
+						ret);
+			goto err_alloc;
+		}
 	}
 
 	return 0;
@@ -742,11 +804,20 @@ static const struct dev_pm_ops usbhsomap_dev_pm_ops = {
 	.runtime_resume		= usbhs_runtime_resume,
 };
 
+static const struct of_device_id usbhs_omap_dt_ids[] = {
+	{ .compatible = "ti,usbhs-host" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, usbhs_omap_dt_ids);
+
+
 static struct platform_driver usbhs_omap_driver = {
 	.driver = {
 		.name		= (char *)usbhs_driver_name,
 		.owner		= THIS_MODULE,
 		.pm		= &usbhsomap_dev_pm_ops,
+		.of_match_table = of_match_ptr(usbhs_omap_dt_ids),
 	},
 	.remove		= usbhs_omap_remove,
 };
-- 
1.7.4.1

--
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 09/13] mfd: omap-usb-host: Add device tree support and binding information

[kishon]

On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
> Allows the OMAP HS USB host controller to be specified
> via device tree.
>
> CC: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
> ---
>   .../devicetree/bindings/mfd/omap-usb-host.txt      |   68 ++++++++++++++++
>   drivers/mfd/omap-usb-host.c                        |   83 ++++++++++++++++++--
>   2 files changed, 145 insertions(+), 6 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
> new file mode 100644
> index 0000000..2196893
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
> @@ -0,0 +1,68 @@
> +OMAP HS USB Host
> +
> +Required properties:
> +
> +- compatible: should be "ti,usbhs-host"
> +- reg: should contain one register range i.e. start and length
> +- ti,hwmods: must contain "usb_host_hs"
> +
> +Optional properties:
> +
> +- nports: number of USB ports. Usually this is automatically detected
> +  from the IP's revision register but can be overridden by specifying
> +  this property.
> +
> +- portN_mode: Integer specifying the port mode for port N, where N can be
> +  from 1 to nports. The port mode must be as per enum usbhs_omap_port_mode
> +  in include/linux/platform_data/usb-omap.h
> +  If the port mode is not specified, that port is treated as unused.
> +
> +- single_ulpi_bypass: Must be present if the controller contains a single
> +  ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
> +
> +Required properties if child node exists:
> +
> +- #address-cells: Must be 1
> +- #size-cells: Must be 1
> +- ranges: must be present
> +
> +Properties for children:
> +
> +The OMAP HS USB Host subsystem contains EHCI and OHCI controllers.
> +See Documentation/devicetree/bindings/usb/omap-ehci.txt and
> +omap3-ohci.txt
> +
> +Example for OMAP4:
> +
> +usbhshost: usbhshost@0x4a064000 {
> +	compatible = "ti,usbhs-host";
> +	reg = <0x4a064000 0x800>;
> +	ti,hwmods = "usb_host_hs";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	ranges;
> +
> +	usbhsohci: ohci@0x4a064800 {
> +		compatible = "ti,omap3-ohci", "usb-ohci";
> +		reg = <0x4a064800 0x400>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 76 0x4>;
> +	};
> +
> +	usbhsehci: ehci@0x4a064c00 {
> +		compatible = "ti,omap-ehci", "usb-ehci";
> +		reg = <0x4a064c00 0x400>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 77 0x4>;
> +	};
> +};
> +
> +&usbhshost {
> +	port1_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
> +	port2_mode = <2>; /* OMAP_EHCI_PORT_MODE_TLL */
> +	port3_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
> +};
> +
> +&usbhsehci {
> +	phy = <&hsusb1_phy 0 &hsusb3_phy>;
> +};
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index f8ed08e..0f67856 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -1,8 +1,9 @@
>   /**
>    * omap-usb-host.c - The USBHS core driver for OMAP EHCI & OHCI
>    *
> - * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com
> + * Copyright (C) 2011-2013 Texas Instruments Incorporated - http://www.ti.com
>    * Author: Keshava Munegowda <keshava_mgowda-l0cyMroinI0@public.gmane.org>
> + * Author: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>    *
>    * This program is free software: you can redistribute it and/or modify
>    * it under the terms of the GNU General Public License version 2  of
> @@ -27,6 +28,8 @@
>   #include <linux/platform_device.h>
>   #include <linux/platform_data/usb-omap.h>
>   #include <linux/pm_runtime.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
>
>   #include "omap-usb.h"
>
> @@ -464,6 +467,37 @@ static void omap_usbhs_init(struct device *dev)
>   	pm_runtime_put_sync(dev);
>   }
>
> +static int usbhs_omap_get_dt_pdata(struct device_node *node,
> +					struct usbhs_omap_platform_data *pdata)
> +{
> +	int ret, i;
> +
> +	ret = of_property_read_u32(node, "nports", &pdata->nports);
> +	if (ret)
> +		pdata->nports = 0;
> +
> +	/* get port modes */
> +	for (i = 0; i < OMAP3_HS_USB_PORTS; i++) {
> +		char prop[11];
> +
> +		snprintf(prop, sizeof(prop), "port%d_mode", i + 1);
> +		ret = of_property_read_u32(node, prop, &pdata->port_mode[i]);
> +		if (ret)
> +			pdata->port_mode[i] = OMAP_USBHS_PORT_MODE_UNUSED;
> +	}
> +
> +	/* get flags */
> +	pdata->single_ulpi_bypass = of_property_read_bool(node,
> +						"single_ulpi_bypass");
> +	return 0;
> +}
> +
> +static struct of_device_id usbhs_child_match_table[] __initdata = {
> +	{ .compatible = "ti,omap-ehci", },
> +	{ .compatible = "ti,omap-ohci", },
> +	{ }
> +};
> +
>   /**
>    * usbhs_omap_probe - initialize TI-based HCDs
>    *
> @@ -479,6 +513,21 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>   	int				i;
>   	bool				need_logic_fck;
>
> +	if (dev->of_node) {
> +		/* For DT boot we populate platform data from OF node */
> +		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +		if (!pdata)
> +			return -ENOMEM;
> +
> +		if (usbhs_omap_get_dt_pdata(dev->of_node, pdata)) {
> +			dev_err(dev,
> +				"Error getting platform data from DT node\n");
> +			return -ENODEV;
> +		}
> +
> +		dev->platform_data = pdata;
> +	}
> +
>   	if (!pdata) {
>   		dev_err(dev, "Missing platform data\n");
>   		return -ENODEV;
> @@ -490,7 +539,7 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>   	}
>
> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "uhh");
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	omap->uhh_base = devm_request_and_ioremap(dev, res);
>   	if (!omap->uhh_base) {
>   		dev_err(dev, "Resource request/ioremap failed\n");
> @@ -661,10 +710,23 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>   	}
>
>   	omap_usbhs_init(dev);
> -	ret = omap_usbhs_alloc_children(pdev);
> -	if (ret) {
> -		dev_err(dev, "omap_usbhs_alloc_children failed\n");
> -		goto err_alloc;
> +
> +	if (dev->of_node) {
> +		ret = of_platform_populate(dev->of_node,
> +				usbhs_child_match_table, NULL, dev);
> +
> +		if (ret) {
> +			dev_err(dev, "Failed to create DT children: %d\n", ret);
> +			goto err_alloc;
> +		}
> +
> +	} else {
> +		ret = omap_usbhs_alloc_children(pdev);
> +		if (ret) {
> +			dev_err(dev, "omap_usbhs_alloc_children failed: %d\n",
> +						ret);
> +			goto err_alloc;
> +		}
These child devices should be destroyed on driver remove..no?

Thanks
Kishon

Re: [PATCH 09/13] mfd: omap-usb-host: Add device tree support and binding information

[Roger Quadros]

On 02/05/2013 08:16 AM, kishon wrote:
> On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
>> Allows the OMAP HS USB host controller to be specified
>> via device tree.
>>
>> CC: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>> ---
>>   .../devicetree/bindings/mfd/omap-usb-host.txt      |   68 ++++++++++++++++
>>   drivers/mfd/omap-usb-host.c                        |   83 ++++++++++++++++++--
>>   2 files changed, 145 insertions(+), 6 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> new file mode 100644
>> index 0000000..2196893
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> @@ -0,0 +1,68 @@
>> +OMAP HS USB Host
>> +
>> +Required properties:
>> +
>> +- compatible: should be "ti,usbhs-host"
>> +- reg: should contain one register range i.e. start and length
>> +- ti,hwmods: must contain "usb_host_hs"
>> +
>> +Optional properties:
>> +
>> +- nports: number of USB ports. Usually this is automatically detected
>> +  from the IP's revision register but can be overridden by specifying
>> +  this property.
>> +
>> +- portN_mode: Integer specifying the port mode for port N, where N can be
>> +  from 1 to nports. The port mode must be as per enum usbhs_omap_port_mode
>> +  in include/linux/platform_data/usb-omap.h
>> +  If the port mode is not specified, that port is treated as unused.
>> +
>> +- single_ulpi_bypass: Must be present if the controller contains a single
>> +  ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
>> +
>> +Required properties if child node exists:
>> +
>> +- #address-cells: Must be 1
>> +- #size-cells: Must be 1
>> +- ranges: must be present
>> +
>> +Properties for children:
>> +
>> +The OMAP HS USB Host subsystem contains EHCI and OHCI controllers.
>> +See Documentation/devicetree/bindings/usb/omap-ehci.txt and
>> +omap3-ohci.txt
>> +
>> +Example for OMAP4:
>> +
>> +usbhshost: usbhshost@0x4a064000 {
>> +    compatible = "ti,usbhs-host";
>> +    reg = <0x4a064000 0x800>;
>> +    ti,hwmods = "usb_host_hs";
>> +    #address-cells = <1>;
>> +    #size-cells = <1>;
>> +    ranges;
>> +
>> +    usbhsohci: ohci@0x4a064800 {
>> +        compatible = "ti,omap3-ohci", "usb-ohci";
>> +        reg = <0x4a064800 0x400>;
>> +        interrupt-parent = <&gic>;
>> +        interrupts = <0 76 0x4>;
>> +    };
>> +
>> +    usbhsehci: ehci@0x4a064c00 {
>> +        compatible = "ti,omap-ehci", "usb-ehci";
>> +        reg = <0x4a064c00 0x400>;
>> +        interrupt-parent = <&gic>;
>> +        interrupts = <0 77 0x4>;
>> +    };
>> +};
>> +
>> +&usbhshost {
>> +    port1_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
>> +    port2_mode = <2>; /* OMAP_EHCI_PORT_MODE_TLL */
>> +    port3_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
>> +};
>> +
>> +&usbhsehci {
>> +    phy = <&hsusb1_phy 0 &hsusb3_phy>;
>> +};
>> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
>> index f8ed08e..0f67856 100644
>> --- a/drivers/mfd/omap-usb-host.c
>> +++ b/drivers/mfd/omap-usb-host.c
>> @@ -1,8 +1,9 @@
>>   /**
>>    * omap-usb-host.c - The USBHS core driver for OMAP EHCI & OHCI
>>    *
>> - * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com
>> + * Copyright (C) 2011-2013 Texas Instruments Incorporated - http://www.ti.com
>>    * Author: Keshava Munegowda <keshava_mgowda-l0cyMroinI0@public.gmane.org>
>> + * Author: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>>    *
>>    * This program is free software: you can redistribute it and/or modify
>>    * it under the terms of the GNU General Public License version 2  of
>> @@ -27,6 +28,8 @@
>>   #include <linux/platform_device.h>
>>   #include <linux/platform_data/usb-omap.h>
>>   #include <linux/pm_runtime.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>>
>>   #include "omap-usb.h"
>>
>> @@ -464,6 +467,37 @@ static void omap_usbhs_init(struct device *dev)
>>       pm_runtime_put_sync(dev);
>>   }
>>
>> +static int usbhs_omap_get_dt_pdata(struct device_node *node,
>> +                    struct usbhs_omap_platform_data *pdata)
>> +{
>> +    int ret, i;
>> +
>> +    ret = of_property_read_u32(node, "nports", &pdata->nports);
>> +    if (ret)
>> +        pdata->nports = 0;
>> +
>> +    /* get port modes */
>> +    for (i = 0; i < OMAP3_HS_USB_PORTS; i++) {
>> +        char prop[11];
>> +
>> +        snprintf(prop, sizeof(prop), "port%d_mode", i + 1);
>> +        ret = of_property_read_u32(node, prop, &pdata->port_mode[i]);
>> +        if (ret)
>> +            pdata->port_mode[i] = OMAP_USBHS_PORT_MODE_UNUSED;
>> +    }
>> +
>> +    /* get flags */
>> +    pdata->single_ulpi_bypass = of_property_read_bool(node,
>> +                        "single_ulpi_bypass");
>> +    return 0;
>> +}
>> +
>> +static struct of_device_id usbhs_child_match_table[] __initdata = {
>> +    { .compatible = "ti,omap-ehci", },
>> +    { .compatible = "ti,omap-ohci", },
>> +    { }
>> +};
>> +
>>   /**
>>    * usbhs_omap_probe - initialize TI-based HCDs
>>    *
>> @@ -479,6 +513,21 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>       int                i;
>>       bool                need_logic_fck;
>>
>> +    if (dev->of_node) {
>> +        /* For DT boot we populate platform data from OF node */
>> +        pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +        if (!pdata)
>> +            return -ENOMEM;
>> +
>> +        if (usbhs_omap_get_dt_pdata(dev->of_node, pdata)) {
>> +            dev_err(dev,
>> +                "Error getting platform data from DT node\n");
>> +            return -ENODEV;
>> +        }
>> +
>> +        dev->platform_data = pdata;
>> +    }
>> +
>>       if (!pdata) {
>>           dev_err(dev, "Missing platform data\n");
>>           return -ENODEV;
>> @@ -490,7 +539,7 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>           return -ENOMEM;
>>       }
>>
>> -    res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "uhh");
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>       omap->uhh_base = devm_request_and_ioremap(dev, res);
>>       if (!omap->uhh_base) {
>>           dev_err(dev, "Resource request/ioremap failed\n");
>> @@ -661,10 +710,23 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>       }
>>
>>       omap_usbhs_init(dev);
>> -    ret = omap_usbhs_alloc_children(pdev);
>> -    if (ret) {
>> -        dev_err(dev, "omap_usbhs_alloc_children failed\n");
>> -        goto err_alloc;
>> +
>> +    if (dev->of_node) {
>> +        ret = of_platform_populate(dev->of_node,
>> +                usbhs_child_match_table, NULL, dev);
>> +
>> +        if (ret) {
>> +            dev_err(dev, "Failed to create DT children: %d\n", ret);
>> +            goto err_alloc;
>> +        }
>> +
>> +    } else {
>> +        ret = omap_usbhs_alloc_children(pdev);
>> +        if (ret) {
>> +            dev_err(dev, "omap_usbhs_alloc_children failed: %d\n",
>> +                        ret);
>> +            goto err_alloc;
>> +        }
> These child devices should be destroyed on driver remove..no?
> 
Indeed. good catch :).

cheers,
-roger

--
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 09/13] mfd: omap-usb-host: Add device tree support and binding information

[Roger Quadros]

On 02/05/2013 08:16 AM, kishon wrote:
> On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
>> Allows the OMAP HS USB host controller to be specified
>> via device tree.
>>
>> CC: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>> ---
>>   .../devicetree/bindings/mfd/omap-usb-host.txt      |   68 ++++++++++++++++
>>   drivers/mfd/omap-usb-host.c                        |   83 ++++++++++++++++++--
>>   2 files changed, 145 insertions(+), 6 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> new file mode 100644
>> index 0000000..2196893
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> @@ -0,0 +1,68 @@
>> +OMAP HS USB Host
>> +
>> +Required properties:
>> +
>> +- compatible: should be "ti,usbhs-host"
>> +- reg: should contain one register range i.e. start and length
>> +- ti,hwmods: must contain "usb_host_hs"
>> +
>> +Optional properties:
>> +
>> +- nports: number of USB ports. Usually this is automatically detected
>> +  from the IP's revision register but can be overridden by specifying
>> +  this property.
>> +
>> +- portN_mode: Integer specifying the port mode for port N, where N can be
>> +  from 1 to nports. The port mode must be as per enum usbhs_omap_port_mode
>> +  in include/linux/platform_data/usb-omap.h
>> +  If the port mode is not specified, that port is treated as unused.
>> +
>> +- single_ulpi_bypass: Must be present if the controller contains a single
>> +  ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
>> +
>> +Required properties if child node exists:
>> +
>> +- #address-cells: Must be 1
>> +- #size-cells: Must be 1
>> +- ranges: must be present
>> +
>> +Properties for children:
>> +
>> +The OMAP HS USB Host subsystem contains EHCI and OHCI controllers.
>> +See Documentation/devicetree/bindings/usb/omap-ehci.txt and
>> +omap3-ohci.txt
>> +
>> +Example for OMAP4:
>> +
>> +usbhshost: usbhshost@0x4a064000 {
>> +    compatible = "ti,usbhs-host";
>> +    reg = <0x4a064000 0x800>;
>> +    ti,hwmods = "usb_host_hs";
>> +    #address-cells = <1>;
>> +    #size-cells = <1>;
>> +    ranges;
>> +
>> +    usbhsohci: ohci@0x4a064800 {
>> +        compatible = "ti,omap3-ohci", "usb-ohci";
>> +        reg = <0x4a064800 0x400>;
>> +        interrupt-parent = <&gic>;
>> +        interrupts = <0 76 0x4>;
>> +    };
>> +
>> +    usbhsehci: ehci@0x4a064c00 {
>> +        compatible = "ti,omap-ehci", "usb-ehci";
>> +        reg = <0x4a064c00 0x400>;
>> +        interrupt-parent = <&gic>;
>> +        interrupts = <0 77 0x4>;
>> +    };
>> +};
>> +
>> +&usbhshost {
>> +    port1_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
>> +    port2_mode = <2>; /* OMAP_EHCI_PORT_MODE_TLL */
>> +    port3_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
>> +};
>> +
>> +&usbhsehci {
>> +    phy = <&hsusb1_phy 0 &hsusb3_phy>;
>> +};
>> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
>> index f8ed08e..0f67856 100644
>> --- a/drivers/mfd/omap-usb-host.c
>> +++ b/drivers/mfd/omap-usb-host.c
>> @@ -1,8 +1,9 @@
>>   /**
>>    * omap-usb-host.c - The USBHS core driver for OMAP EHCI & OHCI
>>    *
>> - * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com
>> + * Copyright (C) 2011-2013 Texas Instruments Incorporated - http://www.ti.com
>>    * Author: Keshava Munegowda <keshava_mgowda-l0cyMroinI0@public.gmane.org>
>> + * Author: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>>    *
>>    * This program is free software: you can redistribute it and/or modify
>>    * it under the terms of the GNU General Public License version 2  of
>> @@ -27,6 +28,8 @@
>>   #include <linux/platform_device.h>
>>   #include <linux/platform_data/usb-omap.h>
>>   #include <linux/pm_runtime.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>>
>>   #include "omap-usb.h"
>>
>> @@ -464,6 +467,37 @@ static void omap_usbhs_init(struct device *dev)
>>       pm_runtime_put_sync(dev);
>>   }
>>
>> +static int usbhs_omap_get_dt_pdata(struct device_node *node,
>> +                    struct usbhs_omap_platform_data *pdata)
>> +{
>> +    int ret, i;
>> +
>> +    ret = of_property_read_u32(node, "nports", &pdata->nports);
>> +    if (ret)
>> +        pdata->nports = 0;
>> +
>> +    /* get port modes */
>> +    for (i = 0; i < OMAP3_HS_USB_PORTS; i++) {
>> +        char prop[11];
>> +
>> +        snprintf(prop, sizeof(prop), "port%d_mode", i + 1);
>> +        ret = of_property_read_u32(node, prop, &pdata->port_mode[i]);
>> +        if (ret)
>> +            pdata->port_mode[i] = OMAP_USBHS_PORT_MODE_UNUSED;
>> +    }
>> +
>> +    /* get flags */
>> +    pdata->single_ulpi_bypass = of_property_read_bool(node,
>> +                        "single_ulpi_bypass");
>> +    return 0;
>> +}
>> +
>> +static struct of_device_id usbhs_child_match_table[] __initdata = {
>> +    { .compatible = "ti,omap-ehci", },
>> +    { .compatible = "ti,omap-ohci", },
>> +    { }
>> +};
>> +
>>   /**
>>    * usbhs_omap_probe - initialize TI-based HCDs
>>    *
>> @@ -479,6 +513,21 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>       int                i;
>>       bool                need_logic_fck;
>>
>> +    if (dev->of_node) {
>> +        /* For DT boot we populate platform data from OF node */
>> +        pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +        if (!pdata)
>> +            return -ENOMEM;
>> +
>> +        if (usbhs_omap_get_dt_pdata(dev->of_node, pdata)) {
>> +            dev_err(dev,
>> +                "Error getting platform data from DT node\n");
>> +            return -ENODEV;
>> +        }
>> +
>> +        dev->platform_data = pdata;
>> +    }
>> +
>>       if (!pdata) {
>>           dev_err(dev, "Missing platform data\n");
>>           return -ENODEV;
>> @@ -490,7 +539,7 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>           return -ENOMEM;
>>       }
>>
>> -    res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "uhh");
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>       omap->uhh_base = devm_request_and_ioremap(dev, res);
>>       if (!omap->uhh_base) {
>>           dev_err(dev, "Resource request/ioremap failed\n");
>> @@ -661,10 +710,23 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>       }
>>
>>       omap_usbhs_init(dev);
>> -    ret = omap_usbhs_alloc_children(pdev);
>> -    if (ret) {
>> -        dev_err(dev, "omap_usbhs_alloc_children failed\n");
>> -        goto err_alloc;
>> +
>> +    if (dev->of_node) {
>> +        ret = of_platform_populate(dev->of_node,
>> +                usbhs_child_match_table, NULL, dev);
>> +
>> +        if (ret) {
>> +            dev_err(dev, "Failed to create DT children: %d\n", ret);
>> +            goto err_alloc;
>> +        }
>> +
>> +    } else {
>> +        ret = omap_usbhs_alloc_children(pdev);
>> +        if (ret) {
>> +            dev_err(dev, "omap_usbhs_alloc_children failed: %d\n",
>> +                        ret);
>> +            goto err_alloc;
>> +        }
> These child devices should be destroyed on driver remove..no?
> 
I could not find a function that does the opposite of of_platform_populate() or
of_platform_device_create_pdata(). It seems that platform devices created via
device tree are never meant to be destroyed.

It kind of makes sense for EHCI/OHCI, cause the devices are always present
on the SoC. Also, this driver can't be built as a module so it can never be removed.

So let's leave this patch the way it is except removing 0x from the device name
in the example.

cheers,
-roger

Re: [PATCH 09/13] mfd: omap-usb-host: Add device tree support and binding information

[kishon]

Hi,

On Tuesday 05 February 2013 04:28 PM, Roger Quadros wrote:
> On 02/05/2013 08:16 AM, kishon wrote:
>> On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
>>> Allows the OMAP HS USB host controller to be specified
>>> via device tree.
>>>
>>> CC: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>>> ---
>>>    .../devicetree/bindings/mfd/omap-usb-host.txt      |   68 ++++++++++++++++
>>>    drivers/mfd/omap-usb-host.c                        |   83 ++++++++++++++++++--
>>>    2 files changed, 145 insertions(+), 6 deletions(-)
>>>    create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>>> new file mode 100644
>>> index 0000000..2196893
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>>> @@ -0,0 +1,68 @@
>>> +OMAP HS USB Host
>>> +
>>> +Required properties:
>>> +
>>> +- compatible: should be "ti,usbhs-host"
>>> +- reg: should contain one register range i.e. start and length
>>> +- ti,hwmods: must contain "usb_host_hs"
>>> +
>>> +Optional properties:
>>> +
>>> +- nports: number of USB ports. Usually this is automatically detected
>>> +  from the IP's revision register but can be overridden by specifying
>>> +  this property.
>>> +
>>> +- portN_mode: Integer specifying the port mode for port N, where N can be
>>> +  from 1 to nports. The port mode must be as per enum usbhs_omap_port_mode
>>> +  in include/linux/platform_data/usb-omap.h
>>> +  If the port mode is not specified, that port is treated as unused.
>>> +
>>> +- single_ulpi_bypass: Must be present if the controller contains a single
>>> +  ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
>>> +
>>> +Required properties if child node exists:
>>> +
>>> +- #address-cells: Must be 1
>>> +- #size-cells: Must be 1
>>> +- ranges: must be present
>>> +
>>> +Properties for children:
>>> +
>>> +The OMAP HS USB Host subsystem contains EHCI and OHCI controllers.
>>> +See Documentation/devicetree/bindings/usb/omap-ehci.txt and
>>> +omap3-ohci.txt
>>> +
>>> +Example for OMAP4:
>>> +
>>> +usbhshost: usbhshost@0x4a064000 {
>>> +    compatible = "ti,usbhs-host";
>>> +    reg = <0x4a064000 0x800>;
>>> +    ti,hwmods = "usb_host_hs";
>>> +    #address-cells = <1>;
>>> +    #size-cells = <1>;
>>> +    ranges;
>>> +
>>> +    usbhsohci: ohci@0x4a064800 {
>>> +        compatible = "ti,omap3-ohci", "usb-ohci";
>>> +        reg = <0x4a064800 0x400>;
>>> +        interrupt-parent = <&gic>;
>>> +        interrupts = <0 76 0x4>;
>>> +    };
>>> +
>>> +    usbhsehci: ehci@0x4a064c00 {
>>> +        compatible = "ti,omap-ehci", "usb-ehci";
>>> +        reg = <0x4a064c00 0x400>;
>>> +        interrupt-parent = <&gic>;
>>> +        interrupts = <0 77 0x4>;
>>> +    };
>>> +};
>>> +
>>> +&usbhshost {
>>> +    port1_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
>>> +    port2_mode = <2>; /* OMAP_EHCI_PORT_MODE_TLL */
>>> +    port3_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
>>> +};
>>> +
>>> +&usbhsehci {
>>> +    phy = <&hsusb1_phy 0 &hsusb3_phy>;
>>> +};
>>> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
>>> index f8ed08e..0f67856 100644
>>> --- a/drivers/mfd/omap-usb-host.c
>>> +++ b/drivers/mfd/omap-usb-host.c
>>> @@ -1,8 +1,9 @@
>>>    /**
>>>     * omap-usb-host.c - The USBHS core driver for OMAP EHCI & OHCI
>>>     *
>>> - * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com
>>> + * Copyright (C) 2011-2013 Texas Instruments Incorporated - http://www.ti.com
>>>     * Author: Keshava Munegowda <keshava_mgowda-l0cyMroinI0@public.gmane.org>
>>> + * Author: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>>>     *
>>>     * This program is free software: you can redistribute it and/or modify
>>>     * it under the terms of the GNU General Public License version 2  of
>>> @@ -27,6 +28,8 @@
>>>    #include <linux/platform_device.h>
>>>    #include <linux/platform_data/usb-omap.h>
>>>    #include <linux/pm_runtime.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_platform.h>
>>>
>>>    #include "omap-usb.h"
>>>
>>> @@ -464,6 +467,37 @@ static void omap_usbhs_init(struct device *dev)
>>>        pm_runtime_put_sync(dev);
>>>    }
>>>
>>> +static int usbhs_omap_get_dt_pdata(struct device_node *node,
>>> +                    struct usbhs_omap_platform_data *pdata)
>>> +{
>>> +    int ret, i;
>>> +
>>> +    ret = of_property_read_u32(node, "nports", &pdata->nports);
>>> +    if (ret)
>>> +        pdata->nports = 0;
>>> +
>>> +    /* get port modes */
>>> +    for (i = 0; i < OMAP3_HS_USB_PORTS; i++) {
>>> +        char prop[11];
>>> +
>>> +        snprintf(prop, sizeof(prop), "port%d_mode", i + 1);
>>> +        ret = of_property_read_u32(node, prop, &pdata->port_mode[i]);
>>> +        if (ret)
>>> +            pdata->port_mode[i] = OMAP_USBHS_PORT_MODE_UNUSED;
>>> +    }
>>> +
>>> +    /* get flags */
>>> +    pdata->single_ulpi_bypass = of_property_read_bool(node,
>>> +                        "single_ulpi_bypass");
>>> +    return 0;
>>> +}
>>> +
>>> +static struct of_device_id usbhs_child_match_table[] __initdata = {
>>> +    { .compatible = "ti,omap-ehci", },
>>> +    { .compatible = "ti,omap-ohci", },
>>> +    { }
>>> +};
>>> +
>>>    /**
>>>     * usbhs_omap_probe - initialize TI-based HCDs
>>>     *
>>> @@ -479,6 +513,21 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>>        int                i;
>>>        bool                need_logic_fck;
>>>
>>> +    if (dev->of_node) {
>>> +        /* For DT boot we populate platform data from OF node */
>>> +        pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>>> +        if (!pdata)
>>> +            return -ENOMEM;
>>> +
>>> +        if (usbhs_omap_get_dt_pdata(dev->of_node, pdata)) {
>>> +            dev_err(dev,
>>> +                "Error getting platform data from DT node\n");
>>> +            return -ENODEV;
>>> +        }
>>> +
>>> +        dev->platform_data = pdata;
>>> +    }
>>> +
>>>        if (!pdata) {
>>>            dev_err(dev, "Missing platform data\n");
>>>            return -ENODEV;
>>> @@ -490,7 +539,7 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>>            return -ENOMEM;
>>>        }
>>>
>>> -    res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "uhh");
>>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>        omap->uhh_base = devm_request_and_ioremap(dev, res);
>>>        if (!omap->uhh_base) {
>>>            dev_err(dev, "Resource request/ioremap failed\n");
>>> @@ -661,10 +710,23 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>>        }
>>>
>>>        omap_usbhs_init(dev);
>>> -    ret = omap_usbhs_alloc_children(pdev);
>>> -    if (ret) {
>>> -        dev_err(dev, "omap_usbhs_alloc_children failed\n");
>>> -        goto err_alloc;
>>> +
>>> +    if (dev->of_node) {
>>> +        ret = of_platform_populate(dev->of_node,
>>> +                usbhs_child_match_table, NULL, dev);
>>> +
>>> +        if (ret) {
>>> +            dev_err(dev, "Failed to create DT children: %d\n", ret);
>>> +            goto err_alloc;
>>> +        }
>>> +
>>> +    } else {
>>> +        ret = omap_usbhs_alloc_children(pdev);
>>> +        if (ret) {
>>> +            dev_err(dev, "omap_usbhs_alloc_children failed: %d\n",
>>> +                        ret);
>>> +            goto err_alloc;
>>> +        }
>> These child devices should be destroyed on driver remove..no?
>>
> I could not find a function that does the opposite of of_platform_populate() or
> of_platform_device_create_pdata(). It seems that platform devices created via
> device tree are never meant to be destroyed.

No. I've done it for dwc3 in usb/dwc3/dwc3-omap.c. (you can check usb-next)
>
> It kind of makes sense for EHCI/OHCI, cause the devices are always present
> on the SoC.
Not true for devices created in drivers/ IMHO. It makes sense only if 
you create the device in some platform specific initialization file.

> Also, this driver can't be built as a module so it can never be removed.
Why is this restriction btw?

Thanks
Kishon
--
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 09/13] mfd: omap-usb-host: Add device tree support and binding information

[Roger Quadros]

On 02/05/2013 02:11 PM, kishon wrote:
> Hi,
> 
> On Tuesday 05 February 2013 04:28 PM, Roger Quadros wrote:
>> On 02/05/2013 08:16 AM, kishon wrote:
>>> On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
>>>> Allows the OMAP HS USB host controller to be specified
>>>> via device tree.
>>>>
>>>> CC: Samuel Ortiz <sameo@linux.intel.com>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> ---
>>>>    .../devicetree/bindings/mfd/omap-usb-host.txt      |   68 ++++++++++++++++
>>>>    drivers/mfd/omap-usb-host.c                        |   83 ++++++++++++++++++--
>>>>    2 files changed, 145 insertions(+), 6 deletions(-)
>>>>    create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>>>> new file mode 100644
>>>> index 0000000..2196893
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>>>> @@ -0,0 +1,68 @@
>>>> +OMAP HS USB Host
>>>> +
>>>> +Required properties:
>>>> +
>>>> +- compatible: should be "ti,usbhs-host"
>>>> +- reg: should contain one register range i.e. start and length
>>>> +- ti,hwmods: must contain "usb_host_hs"
>>>> +
>>>> +Optional properties:
>>>> +
>>>> +- nports: number of USB ports. Usually this is automatically detected
>>>> +  from the IP's revision register but can be overridden by specifying
>>>> +  this property.
>>>> +
>>>> +- portN_mode: Integer specifying the port mode for port N, where N can be
>>>> +  from 1 to nports. The port mode must be as per enum usbhs_omap_port_mode
>>>> +  in include/linux/platform_data/usb-omap.h
>>>> +  If the port mode is not specified, that port is treated as unused.
>>>> +
>>>> +- single_ulpi_bypass: Must be present if the controller contains a single
>>>> +  ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
>>>> +
>>>> +Required properties if child node exists:
>>>> +
>>>> +- #address-cells: Must be 1
>>>> +- #size-cells: Must be 1
>>>> +- ranges: must be present
>>>> +
>>>> +Properties for children:
>>>> +
>>>> +The OMAP HS USB Host subsystem contains EHCI and OHCI controllers.
>>>> +See Documentation/devicetree/bindings/usb/omap-ehci.txt and
>>>> +omap3-ohci.txt
>>>> +
>>>> +Example for OMAP4:
>>>> +
>>>> +usbhshost: usbhshost@0x4a064000 {
>>>> +    compatible = "ti,usbhs-host";
>>>> +    reg = <0x4a064000 0x800>;
>>>> +    ti,hwmods = "usb_host_hs";
>>>> +    #address-cells = <1>;
>>>> +    #size-cells = <1>;
>>>> +    ranges;
>>>> +
>>>> +    usbhsohci: ohci@0x4a064800 {
>>>> +        compatible = "ti,omap3-ohci", "usb-ohci";
>>>> +        reg = <0x4a064800 0x400>;
>>>> +        interrupt-parent = <&gic>;
>>>> +        interrupts = <0 76 0x4>;
>>>> +    };
>>>> +
>>>> +    usbhsehci: ehci@0x4a064c00 {
>>>> +        compatible = "ti,omap-ehci", "usb-ehci";
>>>> +        reg = <0x4a064c00 0x400>;
>>>> +        interrupt-parent = <&gic>;
>>>> +        interrupts = <0 77 0x4>;
>>>> +    };
>>>> +};
>>>> +
>>>> +&usbhshost {
>>>> +    port1_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
>>>> +    port2_mode = <2>; /* OMAP_EHCI_PORT_MODE_TLL */
>>>> +    port3_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
>>>> +};
>>>> +
>>>> +&usbhsehci {
>>>> +    phy = <&hsusb1_phy 0 &hsusb3_phy>;
>>>> +};
>>>> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
>>>> index f8ed08e..0f67856 100644
>>>> --- a/drivers/mfd/omap-usb-host.c
>>>> +++ b/drivers/mfd/omap-usb-host.c
>>>> @@ -1,8 +1,9 @@
>>>>    /**
>>>>     * omap-usb-host.c - The USBHS core driver for OMAP EHCI & OHCI
>>>>     *
>>>> - * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com
>>>> + * Copyright (C) 2011-2013 Texas Instruments Incorporated - http://www.ti.com
>>>>     * Author: Keshava Munegowda <keshava_mgowda@ti.com>
>>>> + * Author: Roger Quadros <rogerq@ti.com>
>>>>     *
>>>>     * This program is free software: you can redistribute it and/or modify
>>>>     * it under the terms of the GNU General Public License version 2  of
>>>> @@ -27,6 +28,8 @@
>>>>    #include <linux/platform_device.h>
>>>>    #include <linux/platform_data/usb-omap.h>
>>>>    #include <linux/pm_runtime.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/of_platform.h>
>>>>
>>>>    #include "omap-usb.h"
>>>>
>>>> @@ -464,6 +467,37 @@ static void omap_usbhs_init(struct device *dev)
>>>>        pm_runtime_put_sync(dev);
>>>>    }
>>>>
>>>> +static int usbhs_omap_get_dt_pdata(struct device_node *node,
>>>> +                    struct usbhs_omap_platform_data *pdata)
>>>> +{
>>>> +    int ret, i;
>>>> +
>>>> +    ret = of_property_read_u32(node, "nports", &pdata->nports);
>>>> +    if (ret)
>>>> +        pdata->nports = 0;
>>>> +
>>>> +    /* get port modes */
>>>> +    for (i = 0; i < OMAP3_HS_USB_PORTS; i++) {
>>>> +        char prop[11];
>>>> +
>>>> +        snprintf(prop, sizeof(prop), "port%d_mode", i + 1);
>>>> +        ret = of_property_read_u32(node, prop, &pdata->port_mode[i]);
>>>> +        if (ret)
>>>> +            pdata->port_mode[i] = OMAP_USBHS_PORT_MODE_UNUSED;
>>>> +    }
>>>> +
>>>> +    /* get flags */
>>>> +    pdata->single_ulpi_bypass = of_property_read_bool(node,
>>>> +                        "single_ulpi_bypass");
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static struct of_device_id usbhs_child_match_table[] __initdata = {
>>>> +    { .compatible = "ti,omap-ehci", },
>>>> +    { .compatible = "ti,omap-ohci", },
>>>> +    { }
>>>> +};
>>>> +
>>>>    /**
>>>>     * usbhs_omap_probe - initialize TI-based HCDs
>>>>     *
>>>> @@ -479,6 +513,21 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>>>        int                i;
>>>>        bool                need_logic_fck;
>>>>
>>>> +    if (dev->of_node) {
>>>> +        /* For DT boot we populate platform data from OF node */
>>>> +        pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>>>> +        if (!pdata)
>>>> +            return -ENOMEM;
>>>> +
>>>> +        if (usbhs_omap_get_dt_pdata(dev->of_node, pdata)) {
>>>> +            dev_err(dev,
>>>> +                "Error getting platform data from DT node\n");
>>>> +            return -ENODEV;
>>>> +        }
>>>> +
>>>> +        dev->platform_data = pdata;
>>>> +    }
>>>> +
>>>>        if (!pdata) {
>>>>            dev_err(dev, "Missing platform data\n");
>>>>            return -ENODEV;
>>>> @@ -490,7 +539,7 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>>>            return -ENOMEM;
>>>>        }
>>>>
>>>> -    res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "uhh");
>>>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>        omap->uhh_base = devm_request_and_ioremap(dev, res);
>>>>        if (!omap->uhh_base) {
>>>>            dev_err(dev, "Resource request/ioremap failed\n");
>>>> @@ -661,10 +710,23 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>>>        }
>>>>
>>>>        omap_usbhs_init(dev);
>>>> -    ret = omap_usbhs_alloc_children(pdev);
>>>> -    if (ret) {
>>>> -        dev_err(dev, "omap_usbhs_alloc_children failed\n");
>>>> -        goto err_alloc;
>>>> +
>>>> +    if (dev->of_node) {
>>>> +        ret = of_platform_populate(dev->of_node,
>>>> +                usbhs_child_match_table, NULL, dev);
>>>> +
>>>> +        if (ret) {
>>>> +            dev_err(dev, "Failed to create DT children: %d\n", ret);
>>>> +            goto err_alloc;
>>>> +        }
>>>> +
>>>> +    } else {
>>>> +        ret = omap_usbhs_alloc_children(pdev);
>>>> +        if (ret) {
>>>> +            dev_err(dev, "omap_usbhs_alloc_children failed: %d\n",
>>>> +                        ret);
>>>> +            goto err_alloc;
>>>> +        }
>>> These child devices should be destroyed on driver remove..no?
>>>
>> I could not find a function that does the opposite of of_platform_populate() or
>> of_platform_device_create_pdata(). It seems that platform devices created via
>> device tree are never meant to be destroyed.
> 
> No. I've done it for dwc3 in usb/dwc3/dwc3-omap.c. (you can check usb-next)

OK, so platform_device_unregister() is sufficient it seems. Thanks for the hint.

>>
>> It kind of makes sense for EHCI/OHCI, cause the devices are always present
>> on the SoC.
> Not true for devices created in drivers/ IMHO. It makes sense only if you create the device in some platform specific initialization file.
> 
>> Also, this driver can't be built as a module so it can never be removed.
> Why is this restriction btw?

I think it is because of the interdependency to load the omap-usb-tll driver before the omap-usb-host driver.
Now that we have deferred probing mechanism, I don't think it should be a problem any more.

cheers,
-roger

Re: [PATCH 09/13] mfd: omap-usb-host: Add device tree support and binding information

[Mark Rutland]

Hi,

I have a few comments on the binding and the way it's parsed.

On Mon, Feb 04, 2013 at 03:58:56PM +0000, Roger Quadros wrote:
> Allows the OMAP HS USB host controller to be specified
> via device tree.
> 
> CC: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
> ---
>  .../devicetree/bindings/mfd/omap-usb-host.txt      |   68 ++++++++++++++++
>  drivers/mfd/omap-usb-host.c                        |   83 ++++++++++++++++++--
>  2 files changed, 145 insertions(+), 6 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-host.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
> new file mode 100644
> index 0000000..2196893
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
> @@ -0,0 +1,68 @@
> +OMAP HS USB Host
> +
> +Required properties:
> +
> +- compatible: should be "ti,usbhs-host"
> +- reg: should contain one register range i.e. start and length
> +- ti,hwmods: must contain "usb_host_hs"
> +
> +Optional properties:
> +
> +- nports: number of USB ports. Usually this is automatically detected
> +  from the IP's revision register but can be overridden by specifying
> +  this property.

It would be nice if this were "num-ports", as atmel-usb is already using that,
and it's clear that it's a number of ports rather than some other meaning of
'n'.

>From a quick grep of binding documents, out of "nTHING(s)", "nr-THINGs", and
num-THINGs, num-THINGs seems to be the most common. It would be nice if new
bindings could standardise this.

> +
> +- portN_mode: Integer specifying the port mode for port N, where N can be
> +  from 1 to nports. The port mode must be as per enum usbhs_omap_port_mode
> +  in include/linux/platform_data/usb-omap.h
> +  If the port mode is not specified, that port is treated as unused.

I'm against devicetree bindings refering to Linux internals. It makes a poorly
documented ABI that someone might change in future without realising the
implications, and it makes it stupidly difficult to read a dts.

Everything required should be specified in the binding document (or another
linked binding document). It might be better to describe this with a string
property that gets mapped by your dt parsing code to whatever internal
representation you need. That way it's far easier for a human to verify the dts
is correct, and you know by construction that the parsed value is something you
can handle in the driver.

It would be nicer is you used '-' rather than '_' for consistency with
devicetree bindings in general.

> +
> +- single_ulpi_bypass: Must be present if the controller contains a single
> +  ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1

Again it would be nicer to have '-' rather than '_' here. It might be worth
prefixing this "ti,".

> +
> +Required properties if child node exists:
> +
> +- #address-cells: Must be 1
> +- #size-cells: Must be 1
> +- ranges: must be present
> +
> +Properties for children:
> +
> +The OMAP HS USB Host subsystem contains EHCI and OHCI controllers.
> +See Documentation/devicetree/bindings/usb/omap-ehci.txt and
> +omap3-ohci.txt
> +
> +Example for OMAP4:
> +
> +usbhshost: usbhshost@0x4a064000 {
> +	compatible = "ti,usbhs-host";
> +	reg = <0x4a064000 0x800>;
> +	ti,hwmods = "usb_host_hs";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	ranges;
> +
> +	usbhsohci: ohci@0x4a064800 {
> +		compatible = "ti,omap3-ohci", "usb-ohci";
> +		reg = <0x4a064800 0x400>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 76 0x4>;
> +	};
> +
> +	usbhsehci: ehci@0x4a064c00 {
> +		compatible = "ti,omap-ehci", "usb-ehci";
> +		reg = <0x4a064c00 0x400>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 77 0x4>;
> +	};
> +};
> +
> +&usbhshost {
> +	port1_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
> +	port2_mode = <2>; /* OMAP_EHCI_PORT_MODE_TLL */
> +	port3_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */

With a string property, these values would be self-documenting.

> +};
> +
> +&usbhsehci {
> +	phy = <&hsusb1_phy 0 &hsusb3_phy>;
> +};
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index f8ed08e..0f67856 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -1,8 +1,9 @@
>  /**
>   * omap-usb-host.c - The USBHS core driver for OMAP EHCI & OHCI
>   *
> - * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com
> + * Copyright (C) 2011-2013 Texas Instruments Incorporated - http://www.ti.com
>   * Author: Keshava Munegowda <keshava_mgowda-l0cyMroinI0@public.gmane.org>
> + * Author: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>   *
>   * This program is free software: you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2  of
> @@ -27,6 +28,8 @@
>  #include <linux/platform_device.h>
>  #include <linux/platform_data/usb-omap.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
>  
>  #include "omap-usb.h"
>  
> @@ -464,6 +467,37 @@ static void omap_usbhs_init(struct device *dev)
>  	pm_runtime_put_sync(dev);
>  }
>  
> +static int usbhs_omap_get_dt_pdata(struct device_node *node,
> +					struct usbhs_omap_platform_data *pdata)
> +{
> +	int ret, i;
> +
> +	ret = of_property_read_u32(node, "nports", &pdata->nports);
> +	if (ret)
> +		pdata->nports = 0;

Is there no upper bound on how many ports the controller can have lower than
4294967295?

I see there are several places in the driver that assume you can only have at
most OMAP3_HS_USB_PORTS (i.e. 3) ports. Is this expected to grow, or is the
hardware design capped at 3?

I don't seem to have usbhs_omap_platform_data::nports in my tree, and I
couldn't see it addded in any of this series so far. Where can I find a tree
with it present? 

Is it a u32? If not, you'll need to use a temporary when reading the dt.

> +
> +	/* get port modes */
> +	for (i = 0; i < OMAP3_HS_USB_PORTS; i++) {
> +		char prop[11];
> +
> +		snprintf(prop, sizeof(prop), "port%d_mode", i + 1);
> +		ret = of_property_read_u32(node, prop, &pdata->port_mode[i]);
> +		if (ret)
> +			pdata->port_mode[i] = OMAP_USBHS_PORT_MODE_UNUSED;

What if the port has an invalid mode value? What if something needs to be added
to or removed from the enum in future?

In my tree, pdata->port_mode[i] is an enum usbhs_omap_port_mode, not a u32.
Assuming it's the same in your tree. depending on what size the compiler
allocates the enum, you may clobber the other entries in the array (or data
immediately beyond it).

It'd at least be worth warning the user if there's a value the driver doesn't
understand.

> +	}
> +
> +	/* get flags */
> +	pdata->single_ulpi_bypass = of_property_read_bool(node,
> +						"single_ulpi_bypass");
> +	return 0;
> +}
> +
> +static struct of_device_id usbhs_child_match_table[] __initdata = {
> +	{ .compatible = "ti,omap-ehci", },
> +	{ .compatible = "ti,omap-ohci", },
> +	{ }
> +};
> +
>  /**
>   * usbhs_omap_probe - initialize TI-based HCDs
>   *
> @@ -479,6 +513,21 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>  	int				i;
>  	bool				need_logic_fck;
>  
> +	if (dev->of_node) {
> +		/* For DT boot we populate platform data from OF node */
> +		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +		if (!pdata)
> +			return -ENOMEM;
> +
> +		if (usbhs_omap_get_dt_pdata(dev->of_node, pdata)) {
> +			dev_err(dev,
> +				"Error getting platform data from DT node\n");
> +			return -ENODEV;

This is currently unnecessary, as usbhs_omap_get_dt_pdata always returns 0.

It would be nicer if it error'd out on an invalid dt.

> +		}
> +
> +		dev->platform_data = pdata;
> +	}
> +
>  	if (!pdata) {
>  		dev_err(dev, "Missing platform data\n");
>  		return -ENODEV;

[...]

Thanks,
Mark.

--
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 09/13] mfd: omap-usb-host: Add device tree support and binding information

[Roger Quadros]

On 02/05/2013 04:20 PM, Mark Rutland wrote:
> Hi,
> 
> I have a few comments on the binding and the way it's parsed.
> 
> On Mon, Feb 04, 2013 at 03:58:56PM +0000, Roger Quadros wrote:
>> Allows the OMAP HS USB host controller to be specified
>> via device tree.
>>
>> CC: Samuel Ortiz <sameo@linux.intel.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  .../devicetree/bindings/mfd/omap-usb-host.txt      |   68 ++++++++++++++++
>>  drivers/mfd/omap-usb-host.c                        |   83 ++++++++++++++++++--
>>  2 files changed, 145 insertions(+), 6 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> new file mode 100644
>> index 0000000..2196893
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> @@ -0,0 +1,68 @@
>> +OMAP HS USB Host
>> +
>> +Required properties:
>> +
>> +- compatible: should be "ti,usbhs-host"
>> +- reg: should contain one register range i.e. start and length
>> +- ti,hwmods: must contain "usb_host_hs"
>> +
>> +Optional properties:
>> +
>> +- nports: number of USB ports. Usually this is automatically detected
>> +  from the IP's revision register but can be overridden by specifying
>> +  this property.
> 
> It would be nice if this were "num-ports", as atmel-usb is already using that,
> and it's clear that it's a number of ports rather than some other meaning of
> 'n'.
> 
> From a quick grep of binding documents, out of "nTHING(s)", "nr-THINGs", and
> num-THINGs, num-THINGs seems to be the most common. It would be nice if new
> bindings could standardise this.

Agreed.
> 
>> +
>> +- portN_mode: Integer specifying the port mode for port N, where N can be
>> +  from 1 to nports. The port mode must be as per enum usbhs_omap_port_mode
>> +  in include/linux/platform_data/usb-omap.h
>> +  If the port mode is not specified, that port is treated as unused.
> 
> I'm against devicetree bindings refering to Linux internals. It makes a poorly
> documented ABI that someone might change in future without realising the
> implications, and it makes it stupidly difficult to read a dts.
> 
> Everything required should be specified in the binding document (or another
> linked binding document). It might be better to describe this with a string
> property that gets mapped by your dt parsing code to whatever internal
> representation you need. That way it's far easier for a human to verify the dts
> is correct, and you know by construction that the parsed value is something you
> can handle in the driver.

As string makes it self documenting, I'll convert it to a string and update the
binding document.

> 
> It would be nicer is you used '-' rather than '_' for consistency with
> devicetree bindings in general.

OK.

> 
>> +
>> +- single_ulpi_bypass: Must be present if the controller contains a single
>> +  ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
> 
> Again it would be nicer to have '-' rather than '_' here. It might be worth
> prefixing this "ti,".

Is prefixing with "ti" really required? how does it better?

> 
>> +
>> +Required properties if child node exists:
>> +
>> +- #address-cells: Must be 1
>> +- #size-cells: Must be 1
>> +- ranges: must be present
>> +
>> +Properties for children:
>> +
>> +The OMAP HS USB Host subsystem contains EHCI and OHCI controllers.
>> +See Documentation/devicetree/bindings/usb/omap-ehci.txt and
>> +omap3-ohci.txt
>> +
>> +Example for OMAP4:
>> +
>> +usbhshost: usbhshost@0x4a064000 {
>> +	compatible = "ti,usbhs-host";
>> +	reg = <0x4a064000 0x800>;
>> +	ti,hwmods = "usb_host_hs";
>> +	#address-cells = <1>;
>> +	#size-cells = <1>;
>> +	ranges;
>> +
>> +	usbhsohci: ohci@0x4a064800 {
>> +		compatible = "ti,omap3-ohci", "usb-ohci";
>> +		reg = <0x4a064800 0x400>;
>> +		interrupt-parent = <&gic>;
>> +		interrupts = <0 76 0x4>;
>> +	};
>> +
>> +	usbhsehci: ehci@0x4a064c00 {
>> +		compatible = "ti,omap-ehci", "usb-ehci";
>> +		reg = <0x4a064c00 0x400>;
>> +		interrupt-parent = <&gic>;
>> +		interrupts = <0 77 0x4>;
>> +	};
>> +};
>> +
>> +&usbhshost {
>> +	port1_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
>> +	port2_mode = <2>; /* OMAP_EHCI_PORT_MODE_TLL */
>> +	port3_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
> 
> With a string property, these values would be self-documenting.
> 
>> +};
>> +
>> +&usbhsehci {
>> +	phy = <&hsusb1_phy 0 &hsusb3_phy>;
>> +};
>> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
>> index f8ed08e..0f67856 100644
>> --- a/drivers/mfd/omap-usb-host.c
>> +++ b/drivers/mfd/omap-usb-host.c
>> @@ -1,8 +1,9 @@
>>  /**
>>   * omap-usb-host.c - The USBHS core driver for OMAP EHCI & OHCI
>>   *
>> - * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com
>> + * Copyright (C) 2011-2013 Texas Instruments Incorporated - http://www.ti.com
>>   * Author: Keshava Munegowda <keshava_mgowda@ti.com>
>> + * Author: Roger Quadros <rogerq@ti.com>
>>   *
>>   * This program is free software: you can redistribute it and/or modify
>>   * it under the terms of the GNU General Public License version 2  of
>> @@ -27,6 +28,8 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/platform_data/usb-omap.h>
>>  #include <linux/pm_runtime.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>>  
>>  #include "omap-usb.h"
>>  
>> @@ -464,6 +467,37 @@ static void omap_usbhs_init(struct device *dev)
>>  	pm_runtime_put_sync(dev);
>>  }
>>  
>> +static int usbhs_omap_get_dt_pdata(struct device_node *node,
>> +					struct usbhs_omap_platform_data *pdata)
>> +{
>> +	int ret, i;
>> +
>> +	ret = of_property_read_u32(node, "nports", &pdata->nports);
>> +	if (ret)
>> +		pdata->nports = 0;
> 
> Is there no upper bound on how many ports the controller can have lower than
> 4294967295?
> 
> I see there are several places in the driver that assume you can only have at
> most OMAP3_HS_USB_PORTS (i.e. 3) ports. Is this expected to grow, or is the
> hardware design capped at 3?

AFAIK it is capped at 3.
> 
> I don't seem to have usbhs_omap_platform_data::nports in my tree, and I
> couldn't see it addded in any of this series so far. Where can I find a tree
> with it present? 

It should be in linux-next. Alternatively you can pull the patchset from 
	git://github.com/rogerq/linux.git linux-usbhost14-part

> 
> Is it a u32? If not, you'll need to use a temporary when reading the dt.

nports is int.
> 
>> +
>> +	/* get port modes */
>> +	for (i = 0; i < OMAP3_HS_USB_PORTS; i++) {
>> +		char prop[11];
>> +
>> +		snprintf(prop, sizeof(prop), "port%d_mode", i + 1);
>> +		ret = of_property_read_u32(node, prop, &pdata->port_mode[i]);
>> +		if (ret)
>> +			pdata->port_mode[i] = OMAP_USBHS_PORT_MODE_UNUSED;
> 
> What if the port has an invalid mode value? What if something needs to be added
> to or removed from the enum in future?

Right. I'll add checks for invalid modes and print a warning.

> 
> In my tree, pdata->port_mode[i] is an enum usbhs_omap_port_mode, not a u32.
> Assuming it's the same in your tree. depending on what size the compiler
> allocates the enum, you may clobber the other entries in the array (or data
> immediately beyond it).

it pdata->port_mod[i] is still enum usbhs_omap_port_mode. So i'll have to fix this.

> 
> It'd at least be worth warning the user if there's a value the driver doesn't
> understand.
> 
>> +	}
>> +
>> +	/* get flags */
>> +	pdata->single_ulpi_bypass = of_property_read_bool(node,
>> +						"single_ulpi_bypass");
>> +	return 0;
>> +}
>> +
>> +static struct of_device_id usbhs_child_match_table[] __initdata = {
>> +	{ .compatible = "ti,omap-ehci", },
>> +	{ .compatible = "ti,omap-ohci", },
>> +	{ }
>> +};
>> +
>>  /**
>>   * usbhs_omap_probe - initialize TI-based HCDs
>>   *
>> @@ -479,6 +513,21 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>>  	int				i;
>>  	bool				need_logic_fck;
>>  
>> +	if (dev->of_node) {
>> +		/* For DT boot we populate platform data from OF node */
>> +		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +		if (!pdata)
>> +			return -ENOMEM;
>> +
>> +		if (usbhs_omap_get_dt_pdata(dev->of_node, pdata)) {
>> +			dev_err(dev,
>> +				"Error getting platform data from DT node\n");
>> +			return -ENODEV;
> 
> This is currently unnecessary, as usbhs_omap_get_dt_pdata always returns 0.
> 
> It would be nicer if it error'd out on an invalid dt.

yes.

> 
>> +		}
>> +
>> +		dev->platform_data = pdata;
>> +	}
>> +
>>  	if (!pdata) {
>>  		dev_err(dev, "Missing platform data\n");
>>  		return -ENODEV;
> 

Thanks for the in-depth review :).

cheers,
-roger

Re: [PATCH 09/13] mfd: omap-usb-host: Add device tree support and binding information

[Mark Rutland]

[...]

> >> +
> >> +- single_ulpi_bypass: Must be present if the controller contains a single
> >> +  ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
> > 
> > Again it would be nicer to have '-' rather than '_' here. It might be worth
> > prefixing this "ti,".
> 
> Is prefixing with "ti" really required? how does it better?

I thought single-ulpi-bypass sounded rather generic, but it probably is
specific enough as-is. I don't know enough about USB hardware to have strong
feelings either way.

[...]

> Thanks for the in-depth review :).

You're welcome.

Thanks,
Mark.

Re: [PATCH 09/13] mfd: omap-usb-host: Add device tree support and binding information

[Roger Quadros]

On 02/05/2013 06:11 PM, Mark Rutland wrote:
> [...]
> 
>>>> +
>>>> +- single_ulpi_bypass: Must be present if the controller contains a single
>>>> +  ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
>>>
>>> Again it would be nicer to have '-' rather than '_' here. It might be worth
>>> prefixing this "ti,".
>>
>> Is prefixing with "ti" really required? how does it better?
> 
> I thought single-ulpi-bypass sounded rather generic, but it probably is
> specific enough as-is. I don't know enough about USB hardware to have strong
> feelings either way.
> 

Yes, it is specific to the TI silicon. I'll leave it as it is then.

cheers,
-roger
--
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

[PATCH 13/13] ARM: dts: omap3-beagle: Add USB Host support

[Roger Quadros]

Provide RESET and Power regulators for the USB PHY,
the USB Host port mode and the PHY device.

Also provide pin multiplexer information for USB host
pins.

Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/omap3-beagle.dts |   71 ++++++++++++++++++++++++++++++++++++
 1 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
index f624dc8..2c4a6d6 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -38,6 +38,57 @@
 		};
 	};
 
+	/* HS USB Port 2 RESET */
+	hsusb2_reset: hsusb2_reset_reg {
+		compatible = "regulator-fixed";
+		regulator-name = "hsusb2_reset";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&gpio5 19 0>;	/* gpio_147 */
+		startup-delay-us = <70000>;
+		enable-active-high;
+	};
+
+	/* HS USB Port 2 Power */
+	hsusb2_power: hsusb2_power_reg {
+		compatible = "regulator-fixed";
+		regulator-name = "hsusb2_vbus";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&twl_gpio 18 0>;	/* GPIO LEDA */
+		startup-delay-us = <70000>;
+	};
+
+	/* HS USB Host PHY on PORT 2 */
+	hsusb2_phy: hsusb2_phy {
+		compatible = "usb-nop-xceiv";
+		reset-supply = <&hsusb2_reset>;
+		vcc-supply = <&hsusb2_power>;
+	};
+};
+
+&omap3_pmx_core {
+	pinctrl-names = "default";
+	pinctrl-0 = <
+			&hsusbb2_pins
+	>;
+
+	hsusbb2_pins: pinmux_hsusbb2_pins {
+		pinctrl-single,pins = <
+			0x5c0 0x3  /* USBB2_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_clk OUTPUT */
+			0x5c2 0x3  /* USBB2_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_stp OUTPUT */
+			0x5c4 0x10b  /* USBB2_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dir INPUT | PULLDOWN */
+			0x5c6 0x10b  /* USBB2_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_nxt INPUT | PULLDOWN */
+			0x5c8 0x10b  /* USBB2_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat0 INPUT | PULLDOWN */
+			0x5cA 0x10b  /* USBB2_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat1 INPUT | PULLDOWN */
+			0x1a4 0x10b  /* USBB2_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat2 INPUT | PULLDOWN */
+			0x1a6 0x10b  /* USBB2_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat3 INPUT | PULLDOWN */
+			0x1a8 0x10b  /* USBB2_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat4 INPUT | PULLDOWN */
+			0x1aa 0x10b  /* USBB2_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat5 INPUT | PULLDOWN */
+			0x1ac 0x10b  /* USBB2_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat6 INPUT | PULLDOWN */
+			0x1ae 0x10b  /* USBB2_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat7 INPUT | PULLDOWN */
+		>;
+	};
 };
 
 &i2c1 {
@@ -65,3 +116,23 @@
 &mmc3 {
 	status = "disabled";
 };
+
+&usbhshost {
+	port2_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
+};
+
+&usbhsehci {
+	phy = <0 &hsusb2_phy>;
+};
+
+&twl_gpio {
+	ti,use-leds;
+	/* pullups: BIT(1) */
+	ti,pullups = <0x000002>;
+	/*
+	 * pulldowns:
+	 * BIT(2), BIT(6), BIT(7), BIT(8), BIT(13)
+	 * BIT(15), BIT(16), BIT(17)
+	 */
+	ti,pulldowns = <0x03a1c4>;
+};
-- 
1.7.4.1

--
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

[PATCH 10/13] ARM: dts: OMAP4: Add HS USB Host IP nodes

[Roger Quadros]

Adds device nodes for HS USB Host module, TLL module,
OHCI and EHCI controllers.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/omap4.dtsi |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 739bb79..3429280 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -529,5 +529,35 @@
 			ti,hwmods = "timer11";
 			ti,timer-pwm;
 		};
+
+		usbhstll: usbhstll@0x4a062000 {
+			compatible = "ti,usbhs-tll";
+			reg = <0x4a062000 0x1000>;
+			interrupts = <0 78 0x4>;
+			ti,hwmods = "usb_tll_hs";
+		};
+
+		usbhshost: usbhshost@0x4a064000 {
+			compatible = "ti,usbhs-host";
+			reg = <0x4a064000 0x800>;
+			ti,hwmods = "usb_host_hs";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			usbhsohci: ohci@0x4a064800 {
+				compatible = "ti,omap3-ohci", "usb-ohci";
+				reg = <0x4a064800 0x400>;
+				interrupt-parent = <&gic>;
+				interrupts = <0 76 0x4>;
+			};
+
+			usbhsehci: ehci@0x4a064c00 {
+				compatible = "ti,omap-ehci", "usb-ehci";
+				reg = <0x4a064c00 0x400>;
+				interrupt-parent = <&gic>;
+				interrupts = <0 77 0x4>;
+			};
+		};
 	};
 };
-- 
1.7.4.1

Re: [PATCH 10/13] ARM: dts: OMAP4: Add HS USB Host IP nodes

[kishon]

On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
> Adds device nodes for HS USB Host module, TLL module,
> OHCI and EHCI controllers.
>
> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
> ---
>   arch/arm/boot/dts/omap4.dtsi |   30 ++++++++++++++++++++++++++++++
>   1 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 739bb79..3429280 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -529,5 +529,35 @@
>   			ti,hwmods = "timer11";
>   			ti,timer-pwm;
>   		};
> +
> +		usbhstll: usbhstll@0x4a062000 {
> +			compatible = "ti,usbhs-tll";
> +			reg = <0x4a062000 0x1000>;
> +			interrupts = <0 78 0x4>;
> +			ti,hwmods = "usb_tll_hs";
> +		};
> +
> +		usbhshost: usbhshost@0x4a064000 {
> +			compatible = "ti,usbhs-host";
> +			reg = <0x4a064000 0x800>;
> +			ti,hwmods = "usb_host_hs";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			usbhsohci: ohci@0x4a064800 {
> +				compatible = "ti,omap3-ohci", "usb-ohci";
> +				reg = <0x4a064800 0x400>;
> +				interrupt-parent = <&gic>;

Just curious.. Were you facing issues if you are not having 
interrupt-parent here? It's also missing in your dt node usbhstll.

Thanks
Kishon

Re: [PATCH 10/13] ARM: dts: OMAP4: Add HS USB Host IP nodes

[Roger Quadros]

On 02/05/2013 08:24 AM, kishon wrote:
> On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
>> Adds device nodes for HS USB Host module, TLL module,
>> OHCI and EHCI controllers.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>   arch/arm/boot/dts/omap4.dtsi |   30 ++++++++++++++++++++++++++++++
>>   1 files changed, 30 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>> index 739bb79..3429280 100644
>> --- a/arch/arm/boot/dts/omap4.dtsi
>> +++ b/arch/arm/boot/dts/omap4.dtsi
>> @@ -529,5 +529,35 @@
>>               ti,hwmods = "timer11";
>>               ti,timer-pwm;
>>           };
>> +
>> +        usbhstll: usbhstll@0x4a062000 {
>> +            compatible = "ti,usbhs-tll";
>> +            reg = <0x4a062000 0x1000>;
>> +            interrupts = <0 78 0x4>;
>> +            ti,hwmods = "usb_tll_hs";
>> +        };
>> +
>> +        usbhshost: usbhshost@0x4a064000 {
>> +            compatible = "ti,usbhs-host";
>> +            reg = <0x4a064000 0x800>;
>> +            ti,hwmods = "usb_host_hs";
>> +            #address-cells = <1>;
>> +            #size-cells = <1>;
>> +            ranges;
>> +
>> +            usbhsohci: ohci@0x4a064800 {
>> +                compatible = "ti,omap3-ohci", "usb-ohci";
>> +                reg = <0x4a064800 0x400>;
>> +                interrupt-parent = <&gic>;
> 
> Just curious.. Were you facing issues if you are not having interrupt-parent here? It's also missing in your dt node usbhstll.

Yes I was. Interrupt-parent is not there in any of the children which are at the same level as usbhstll.

cheers,
-roger

Re: [PATCH 10/13] ARM: dts: OMAP4: Add HS USB Host IP nodes

[kishon]

On Tuesday 05 February 2013 02:24 PM, Roger Quadros wrote:
> On 02/05/2013 08:24 AM, kishon wrote:
>> On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
>>> Adds device nodes for HS USB Host module, TLL module,
>>> OHCI and EHCI controllers.
>>>
>>> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>>> ---
>>>    arch/arm/boot/dts/omap4.dtsi |   30 ++++++++++++++++++++++++++++++
>>>    1 files changed, 30 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>>> index 739bb79..3429280 100644
>>> --- a/arch/arm/boot/dts/omap4.dtsi
>>> +++ b/arch/arm/boot/dts/omap4.dtsi
>>> @@ -529,5 +529,35 @@
>>>                ti,hwmods = "timer11";
>>>                ti,timer-pwm;
>>>            };
>>> +
>>> +        usbhstll: usbhstll@0x4a062000 {
>>> +            compatible = "ti,usbhs-tll";
>>> +            reg = <0x4a062000 0x1000>;
>>> +            interrupts = <0 78 0x4>;
>>> +            ti,hwmods = "usb_tll_hs";
>>> +        };
>>> +
>>> +        usbhshost: usbhshost@0x4a064000 {
>>> +            compatible = "ti,usbhs-host";
>>> +            reg = <0x4a064000 0x800>;
>>> +            ti,hwmods = "usb_host_hs";
>>> +            #address-cells = <1>;
>>> +            #size-cells = <1>;
>>> +            ranges;
>>> +
>>> +            usbhsohci: ohci@0x4a064800 {
>>> +                compatible = "ti,omap3-ohci", "usb-ohci";
>>> +                reg = <0x4a064800 0x400>;
>>> +                interrupt-parent = <&gic>;
>>
>> Just curious.. Were you facing issues if you are not having interrupt-parent here? It's also missing in your dt node usbhstll.
>
> Yes I was. Interrupt-parent is not there in any of the children which are at the same level as usbhstll.

Cool. Thought so :-)

Thanks
Kishon
--
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 10/13] ARM: dts: OMAP4: Add HS USB Host IP nodes

[Felipe Balbi]

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

On Mon, Feb 04, 2013 at 05:58:57PM +0200, Roger Quadros wrote:
> Adds device nodes for HS USB Host module, TLL module,
> OHCI and EHCI controllers.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  arch/arm/boot/dts/omap4.dtsi |   30 ++++++++++++++++++++++++++++++
>  1 files changed, 30 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 739bb79..3429280 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -529,5 +529,35 @@
>  			ti,hwmods = "timer11";
>  			ti,timer-pwm;
>  		};
> +
> +		usbhstll: usbhstll@0x4a062000 {
> +			compatible = "ti,usbhs-tll";
> +			reg = <0x4a062000 0x1000>;
> +			interrupts = <0 78 0x4>;
> +			ti,hwmods = "usb_tll_hs";
> +		};
> +
> +		usbhshost: usbhshost@0x4a064000 {
> +			compatible = "ti,usbhs-host";
> +			reg = <0x4a064000 0x800>;
> +			ti,hwmods = "usb_host_hs";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			usbhsohci: ohci@0x4a064800 {

usbhsohci is a bit misleading :-)

How about we stick to ohci and ehci for these nodes ? :-)

-- 
balbi

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

Re: [PATCH 10/13] ARM: dts: OMAP4: Add HS USB Host IP nodes

[Roger Quadros]

On 02/05/2013 09:41 AM, Felipe Balbi wrote:
> On Mon, Feb 04, 2013 at 05:58:57PM +0200, Roger Quadros wrote:
>> Adds device nodes for HS USB Host module, TLL module,
>> OHCI and EHCI controllers.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  arch/arm/boot/dts/omap4.dtsi |   30 ++++++++++++++++++++++++++++++
>>  1 files changed, 30 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>> index 739bb79..3429280 100644
>> --- a/arch/arm/boot/dts/omap4.dtsi
>> +++ b/arch/arm/boot/dts/omap4.dtsi
>> @@ -529,5 +529,35 @@
>>  			ti,hwmods = "timer11";
>>  			ti,timer-pwm;
>>  		};
>> +
>> +		usbhstll: usbhstll@0x4a062000 {
>> +			compatible = "ti,usbhs-tll";
>> +			reg = <0x4a062000 0x1000>;
>> +			interrupts = <0 78 0x4>;
>> +			ti,hwmods = "usb_tll_hs";
>> +		};
>> +
>> +		usbhshost: usbhshost@0x4a064000 {
>> +			compatible = "ti,usbhs-host";
>> +			reg = <0x4a064000 0x800>;
>> +			ti,hwmods = "usb_host_hs";
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			ranges;
>> +
>> +			usbhsohci: ohci@0x4a064800 {
> 
> usbhsohci is a bit misleading :-)
> 
> How about we stick to ohci and ehci for these nodes ? :-)
> 
Was just thinking of a unique label that will point to the OHCI/EHCI
controller in the HS USB subsystem. We need the label to provide
PHY information in the board DT.

If we are sure we won't have another OHCI/EHCI controller then I can
just use ehci/ohci.

cheers,
-roger

[PATCH 11/13] ARM: dts: omap4-panda: Add USB Host support

[Roger Quadros]

Provide the RESET and Power regulators for the USB PHY,
the USB Host port mode and the PHY device.

Also provide pin multiplexer information for the USB host
pins.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/omap4-panda.dts |   55 +++++++++++++++++++++++++++++++++++++
 1 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
index 4122efe..fe2d3d4 100644
--- a/arch/arm/boot/dts/omap4-panda.dts
+++ b/arch/arm/boot/dts/omap4-panda.dts
@@ -57,6 +57,35 @@
 			"AFML", "Line In",
 			"AFMR", "Line In";
 	};
+
+	/* HS USB Port 1 RESET */
+	hsusb1_reset: hsusb1_reset_reg {
+		compatible = "regulator-fixed";
+		regulator-name = "hsusb1_reset";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&gpio2 30 0>;	/* gpio_62 */
+		startup-delay-us = <70000>;
+		enable-active-high;
+	};
+
+	/* HS USB Port 1 Power */
+	hsusb1_power: hsusb1_power_reg {
+		compatible = "regulator-fixed";
+		regulator-name = "hsusb1_vbus";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&gpio1 1 0>;	/* gpio_1 */
+		startup-delay-us = <70000>;
+		enable-active-high;
+	};
+
+	/* HS USB Host PHY on PORT 1 */
+	hsusb1_phy: hsusb1_phy {
+		compatible = "usb-nop-xceiv";
+		reset-supply = <&hsusb1_reset>;
+		vcc-supply = <&hsusb1_power>;
+	};
 };
 
 &omap4_pmx_core {
@@ -67,6 +96,7 @@
 			&mcbsp1_pins
 			&dss_hdmi_pins
 			&tpd12s015_pins
+			&hsusbb1_pins
 	>;
 
 	twl6040_pins: pinmux_twl6040_pins {
@@ -110,6 +140,23 @@
 			0x58 0x10b	/* hdmi_hpd.gpio_63 INPUT PULLDOWN | MODE3 */
 		>;
 	};
+
+	hsusbb1_pins: pinmux_hsusbb1_pins {
+		pinctrl-single,pins = <
+			0x82 0x10C	/* USBB1_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_clk INPUT | PULLDOWN */
+			0x84 0x4	/* USBB1_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_stp OUTPUT */
+			0x86 0x104	/* USBB1_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dir INPUT | PULLDOWN */
+			0x88 0x104	/* USBB1_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_nxt INPUT | PULLDOWN */
+			0x8a 0x104	/* USBB1_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat0 INPUT | PULLDOWN */
+			0x8c 0x104	/* USBB1_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat1 INPUT | PULLDOWN */
+			0x8e 0x104	/* USBB1_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat2 INPUT | PULLDOWN */
+			0x90 0x104	/* USBB1_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat3 INPUT | PULLDOWN */
+			0x92 0x104	/* USBB1_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat4 INPUT | PULLDOWN */
+			0x94 0x104	/* USBB1_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat5 INPUT | PULLDOWN */
+			0x96 0x104	/* USBB1_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat6 INPUT | PULLDOWN */
+			0x98 0x104	/* USBB1_ULPITLL_CLK_MUXMODE.usbb1_ulpiphy_dat7 INPUT | PULLDOWN */
+		>;
+	    };
 };
 
 &i2c1 {
@@ -206,3 +253,11 @@
 &twl_usb_comparator {
 	usb-supply = <&vusb>;
 };
+
+&usbhshost {
+	port1_mode = <1>; /* OMAP_EHCI_PORT_MODE_PHY */
+};
+
+&usbhsehci {
+	phy = <&hsusb1_phy>;
+};
-- 
1.7.4.1

how to specify an OMAP clock in device tree?

[Roger Quadros]

Hi Rajendra,

On 02/04/2013 05:58 PM, Roger Quadros wrote:
> Provide the RESET and Power regulators for the USB PHY,
> the USB Host port mode and the PHY device.
> 
> Also provide pin multiplexer information for the USB host
> pins.
> 
> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
> ---
>  arch/arm/boot/dts/omap4-panda.dts |   55 +++++++++++++++++++++++++++++++++++++
>  1 files changed, 55 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
> index 4122efe..fe2d3d4 100644
> --- a/arch/arm/boot/dts/omap4-panda.dts
> +++ b/arch/arm/boot/dts/omap4-panda.dts
> @@ -57,6 +57,35 @@
>  			"AFML", "Line In",
>  			"AFMR", "Line In";
>  	};
> +
> +	/* HS USB Port 1 RESET */
> +	hsusb1_reset: hsusb1_reset_reg {
> +		compatible = "regulator-fixed";
> +		regulator-name = "hsusb1_reset";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&gpio2 30 0>;	/* gpio_62 */
> +		startup-delay-us = <70000>;
> +		enable-active-high;
> +	};
> +
> +	/* HS USB Port 1 Power */
> +	hsusb1_power: hsusb1_power_reg {
> +		compatible = "regulator-fixed";
> +		regulator-name = "hsusb1_vbus";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&gpio1 1 0>;	/* gpio_1 */
> +		startup-delay-us = <70000>;
> +		enable-active-high;
> +	};
> +
> +	/* HS USB Host PHY on PORT 1 */
> +	hsusb1_phy: hsusb1_phy {
> +		compatible = "usb-nop-xceiv";
> +		reset-supply = <&hsusb1_reset>;
> +		vcc-supply = <&hsusb1_power>;
> +	};

This is the patch I was discussing with you about before.

Let me explain the problem again.

The Pandaboard has a USB PHY whose reference clock is provided by
FREF_CLK3 pin which is a clock generated by the OMAP.
The PHY driver expects a reference to this clock in the PHY device node.
See the above node hsusb1_phy. we would need something like
	hsusb1_phy {
		...
		clocks = <&fref_clk3>;
		clock-names = "main_clk";
		...
	};

Currently on OMAP, there is no way to provide a phandle to this clock.

Is it practical to provide device tree based implementation of at least
the externally accessible OMAP clocks?

cheers,
-roger

>  };
>  
>  &omap4_pmx_core {
> @@ -67,6 +96,7 @@
>  			&mcbsp1_pins
>  			&dss_hdmi_pins
>  			&tpd12s015_pins
> +			&hsusbb1_pins
>  	>;
>  
>  	twl6040_pins: pinmux_twl6040_pins {
> @@ -110,6 +140,23 @@
>  			0x58 0x10b	/* hdmi_hpd.gpio_63 INPUT PULLDOWN | MODE3 */
>  		>;
>  	};

Re: how to specify an OMAP clock in device tree?

[Rajendra Nayak]

On Tuesday 05 February 2013 03:04 PM, Roger Quadros wrote:
> Hi Rajendra,
>
> On 02/04/2013 05:58 PM, Roger Quadros wrote:
>> Provide the RESET and Power regulators for the USB PHY,
>> the USB Host port mode and the PHY device.
>>
>> Also provide pin multiplexer information for the USB host
>> pins.
>>
>> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
>> ---
>>   arch/arm/boot/dts/omap4-panda.dts |   55 +++++++++++++++++++++++++++++++++++++
>>   1 files changed, 55 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
>> index 4122efe..fe2d3d4 100644
>> --- a/arch/arm/boot/dts/omap4-panda.dts
>> +++ b/arch/arm/boot/dts/omap4-panda.dts
>> @@ -57,6 +57,35 @@
>>   			"AFML", "Line In",
>>   			"AFMR", "Line In";
>>   	};
>> +
>> +	/* HS USB Port 1 RESET */
>> +	hsusb1_reset: hsusb1_reset_reg {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "hsusb1_reset";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		gpio = <&gpio2 30 0>;	/* gpio_62 */
>> +		startup-delay-us = <70000>;
>> +		enable-active-high;
>> +	};
>> +
>> +	/* HS USB Port 1 Power */
>> +	hsusb1_power: hsusb1_power_reg {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "hsusb1_vbus";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		gpio = <&gpio1 1 0>;	/* gpio_1 */
>> +		startup-delay-us = <70000>;
>> +		enable-active-high;
>> +	};
>> +
>> +	/* HS USB Host PHY on PORT 1 */
>> +	hsusb1_phy: hsusb1_phy {
>> +		compatible = "usb-nop-xceiv";
>> +		reset-supply = <&hsusb1_reset>;
>> +		vcc-supply = <&hsusb1_power>;
>> +	};
>
> This is the patch I was discussing with you about before.
>
> Let me explain the problem again.
>
> The Pandaboard has a USB PHY whose reference clock is provided by
> FREF_CLK3 pin which is a clock generated by the OMAP.
> The PHY driver expects a reference to this clock in the PHY device node.

Well, the driver just does a clk_get(dev, "main_clk"); and clk_get() is
then able to either pick the reference from the PHY dt node or from a
clkdev entry.

The problem here seems to be that you are not able to add a clkdev entry
because the device name wouldn't be fixed, since you have a node in
the form of 'node: node {'. All other onchip OMAP devices don't have
this issue because they have an entry in the form of 'node: node@addr'
and hence have a fixed device name and hence can add a clkdev entry for
the clocks they want to control.

I don't know if there is any good way to define the DT node for the
on board PHY in such a way that the device name is always fixed, in
which case you can then add a clkdev entry for 'dev, main_clk' mapping
to the onchip clock that provides the clock, but if there is one, then
that should fix your problem.

> See the above node hsusb1_phy. we would need something like
> 	hsusb1_phy {
> 		...
> 		clocks = <&fref_clk3>;
> 		clock-names = "main_clk";
> 		...
> 	};
>
> Currently on OMAP, there is no way to provide a phandle to this clock.
>
> Is it practical to provide device tree based implementation of at least
> the externally accessible OMAP clocks?
>
> cheers,
> -roger
>
>>   };
>>
>>   &omap4_pmx_core {
>> @@ -67,6 +96,7 @@
>>   			&mcbsp1_pins
>>   			&dss_hdmi_pins
>>   			&tpd12s015_pins
>> +			&hsusbb1_pins
>>   	>;
>>
>>   	twl6040_pins: pinmux_twl6040_pins {
>> @@ -110,6 +140,23 @@
>>   			0x58 0x10b	/* hdmi_hpd.gpio_63 INPUT PULLDOWN | MODE3 */
>>   		>;
>>   	};
>
>

Re: how to specify an OMAP clock in device tree?

[Roger Quadros]

On 02/05/2013 01:15 PM, Rajendra Nayak wrote:
> On Tuesday 05 February 2013 03:04 PM, Roger Quadros wrote:
>> Hi Rajendra,
>>
>> On 02/04/2013 05:58 PM, Roger Quadros wrote:
>>> Provide the RESET and Power regulators for the USB PHY,
>>> the USB Host port mode and the PHY device.
>>>
>>> Also provide pin multiplexer information for the USB host
>>> pins.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>   arch/arm/boot/dts/omap4-panda.dts |   55 +++++++++++++++++++++++++++++++++++++
>>>   1 files changed, 55 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
>>> index 4122efe..fe2d3d4 100644
>>> --- a/arch/arm/boot/dts/omap4-panda.dts
>>> +++ b/arch/arm/boot/dts/omap4-panda.dts
>>> @@ -57,6 +57,35 @@
>>>               "AFML", "Line In",
>>>               "AFMR", "Line In";
>>>       };
>>> +
>>> +    /* HS USB Port 1 RESET */
>>> +    hsusb1_reset: hsusb1_reset_reg {
>>> +        compatible = "regulator-fixed";
>>> +        regulator-name = "hsusb1_reset";
>>> +        regulator-min-microvolt = <3300000>;
>>> +        regulator-max-microvolt = <3300000>;
>>> +        gpio = <&gpio2 30 0>;    /* gpio_62 */
>>> +        startup-delay-us = <70000>;
>>> +        enable-active-high;
>>> +    };
>>> +
>>> +    /* HS USB Port 1 Power */
>>> +    hsusb1_power: hsusb1_power_reg {
>>> +        compatible = "regulator-fixed";
>>> +        regulator-name = "hsusb1_vbus";
>>> +        regulator-min-microvolt = <3300000>;
>>> +        regulator-max-microvolt = <3300000>;
>>> +        gpio = <&gpio1 1 0>;    /* gpio_1 */
>>> +        startup-delay-us = <70000>;
>>> +        enable-active-high;
>>> +    };
>>> +
>>> +    /* HS USB Host PHY on PORT 1 */
>>> +    hsusb1_phy: hsusb1_phy {
>>> +        compatible = "usb-nop-xceiv";
>>> +        reset-supply = <&hsusb1_reset>;
>>> +        vcc-supply = <&hsusb1_power>;
>>> +    };
>>
>> This is the patch I was discussing with you about before.
>>
>> Let me explain the problem again.
>>
>> The Pandaboard has a USB PHY whose reference clock is provided by
>> FREF_CLK3 pin which is a clock generated by the OMAP.
>> The PHY driver expects a reference to this clock in the PHY device node.
> 
> Well, the driver just does a clk_get(dev, "main_clk"); and clk_get() is
> then able to either pick the reference from the PHY dt node or from a
> clkdev entry.
> 
> The problem here seems to be that you are not able to add a clkdev entry
> because the device name wouldn't be fixed, since you have a node in
> the form of 'node: node {'. All other onchip OMAP devices don't have
> this issue because they have an entry in the form of 'node: node@addr'
> and hence have a fixed device name and hence can add a clkdev entry for
> the clocks they want to control.
> 
> I don't know if there is any good way to define the DT node for the
> on board PHY in such a way that the device name is always fixed, in
> which case you can then add a clkdev entry for 'dev, main_clk' mapping
> to the onchip clock that provides the clock, but if there is one, then
> that should fix your problem.

Fixing the device name doesn't really solve the problem.
Not all OMAP boards will use the same clock for the external device.
So you can't provide this information in some common clock data file.

The data has to come from a board specific location, which in the DT boot
case is the board's dts file.

I think all we need to do is register a clock provider using of_clk_add_provider()
and provide a clk_src_get() hook that can get the right clock.

usage e.g.

/* provider */
clks: omapclocks {
        compatible = "ti,omapclocks";
        #clock-cells = <1>;
};

/* consumer */
hsusb1_phy: hsusb1_phy {
	compatible = "usb-nop-xceiv";
	clocks = <&clks "auxclk3_ck">;	/* FREF_CLK3 */
	clock-names = "main-clk";
};

The only problem I see is that the argument to the clks phandle
cannot be a string. It needs to be u32.

In that case we need to map all clocks into a u32 index.

If we can do that only for auxclks, my problem is solved for panda.

The usage e.g then changes to

/* provider */
aux_clks: omap_aux_clocks {
        compatible = "ti,omap_aux_clocks";
        #clock-cells = <1>;
};

/* consumer */
hsusb1_phy: hsusb1_phy {
	compatible = "usb-nop-xceiv";
	clocks = <&aux_clks 3>;	/* FREF_CLK3 */
	clock-names = "main-clk";
};

Does this idea sound reasonable?

regards,
-roger

Re: how to specify an OMAP clock in device tree?

[Rajendra Nayak]

On Tuesday 05 February 2013 07:16 PM, Roger Quadros wrote:
> Fixing the device name doesn't really solve the problem.
> Not all OMAP boards will use the same clock for the external device.

Are you saying different OMAP boards will use different Internal clocks?
Or different OMAP boards will use a single Internal clock or an
external one.

Re: how to specify an OMAP clock in device tree?

[Roger Quadros]

On 02/05/2013 04:13 PM, Rajendra Nayak wrote:
> On Tuesday 05 February 2013 07:16 PM, Roger Quadros wrote:
>> Fixing the device name doesn't really solve the problem.
>> Not all OMAP boards will use the same clock for the external device.
> 
> Are you saying different OMAP boards will use different Internal clocks?
> Or different OMAP boards will use a single Internal clock or an
> external one.
> 
All I was saying is that one board can use for example auxclk1 whereas another
one can use auxclk3, both generated by OMAP for the same PHY configuration.

cheers,
-roger

Re: how to specify an OMAP clock in device tree?

[Rajendra Nayak]

On Tuesday 05 February 2013 07:48 PM, Roger Quadros wrote:
> On 02/05/2013 04:13 PM, Rajendra Nayak wrote:
>> On Tuesday 05 February 2013 07:16 PM, Roger Quadros wrote:
>>> Fixing the device name doesn't really solve the problem.
>>> Not all OMAP boards will use the same clock for the external device.
>>
>> Are you saying different OMAP boards will use different Internal clocks?
>> Or different OMAP boards will use a single Internal clock or an
>> external one.
>>
> All I was saying is that one board can use for example auxclk1 whereas another
> one can use auxclk3, both generated by OMAP for the same PHY configuration.

Ok, so lets keep DT aside for a while. How would something like this
work in a non-DT world? Would the driver then be able to do a
clk_get(dev, "main_clk"); and get say auxclk1 on one board and
auxclk3 on another?

>
> cheers,
> -roger
>

Re: how to specify an OMAP clock in device tree?

[Roger Quadros]

On 02/05/2013 04:21 PM, Rajendra Nayak wrote:
> On Tuesday 05 February 2013 07:48 PM, Roger Quadros wrote:
>> On 02/05/2013 04:13 PM, Rajendra Nayak wrote:
>>> On Tuesday 05 February 2013 07:16 PM, Roger Quadros wrote:
>>>> Fixing the device name doesn't really solve the problem.
>>>> Not all OMAP boards will use the same clock for the external device.
>>>
>>> Are you saying different OMAP boards will use different Internal clocks?
>>> Or different OMAP boards will use a single Internal clock or an
>>> external one.
>>>
>> All I was saying is that one board can use for example auxclk1 whereas another
>> one can use auxclk3, both generated by OMAP for the same PHY configuration.
> 
> Ok, so lets keep DT aside for a while. How would something like this
> work in a non-DT world? Would the driver then be able to do a
> clk_get(dev, "main_clk"); and get say auxclk1 on one board and
> auxclk3 on another?
> 

Yes, all you need to do is specify an alias to the clock in the board file.

e.g. in board 1 file
	clk_add_alias("main_clk", "phy.1", "auxclk1_ck", NULL);

in board 2 file
	clk_add_alias("main_clk", "phy.1", "auxclk3_ck", NULL);

cheers,
-roger

Re: how to specify an OMAP clock in device tree?

[Rajendra Nayak]

On Tuesday 05 February 2013 07:59 PM, Roger Quadros wrote:
> On 02/05/2013 04:21 PM, Rajendra Nayak wrote:
>> On Tuesday 05 February 2013 07:48 PM, Roger Quadros wrote:
>>> On 02/05/2013 04:13 PM, Rajendra Nayak wrote:
>>>> On Tuesday 05 February 2013 07:16 PM, Roger Quadros wrote:
>>>>> Fixing the device name doesn't really solve the problem.
>>>>> Not all OMAP boards will use the same clock for the external device.
>>>>
>>>> Are you saying different OMAP boards will use different Internal clocks?
>>>> Or different OMAP boards will use a single Internal clock or an
>>>> external one.
>>>>
>>> All I was saying is that one board can use for example auxclk1 whereas another
>>> one can use auxclk3, both generated by OMAP for the same PHY configuration.
>>
>> Ok, so lets keep DT aside for a while. How would something like this
>> work in a non-DT world? Would the driver then be able to do a
>> clk_get(dev, "main_clk"); and get say auxclk1 on one board and
>> auxclk3 on another?
>>
>
> Yes, all you need to do is specify an alias to the clock in the board file.

Can we then create a special board specific node for panda and do
similar things from DT?
See a similar discussion below on how to handle the gpio_request()
we had as part of board files
http://www.spinics.net/lists/linux-omap/msg85248.html

>
> e.g. in board 1 file
> 	clk_add_alias("main_clk", "phy.1", "auxclk1_ck", NULL);
>
> in board 2 file
> 	clk_add_alias("main_clk", "phy.1", "auxclk3_ck", NULL);
>
> cheers,
> -roger
>

--
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: how to specify an OMAP clock in device tree?

[Roger Quadros]

On 02/05/2013 04:36 PM, Rajendra Nayak wrote:
> On Tuesday 05 February 2013 07:59 PM, Roger Quadros wrote:
>> On 02/05/2013 04:21 PM, Rajendra Nayak wrote:
>>> On Tuesday 05 February 2013 07:48 PM, Roger Quadros wrote:
>>>> On 02/05/2013 04:13 PM, Rajendra Nayak wrote:
>>>>> On Tuesday 05 February 2013 07:16 PM, Roger Quadros wrote:
>>>>>> Fixing the device name doesn't really solve the problem.
>>>>>> Not all OMAP boards will use the same clock for the external device.
>>>>>
>>>>> Are you saying different OMAP boards will use different Internal clocks?
>>>>> Or different OMAP boards will use a single Internal clock or an
>>>>> external one.
>>>>>
>>>> All I was saying is that one board can use for example auxclk1 whereas another
>>>> one can use auxclk3, both generated by OMAP for the same PHY configuration.
>>>
>>> Ok, so lets keep DT aside for a while. How would something like this
>>> work in a non-DT world? Would the driver then be able to do a
>>> clk_get(dev, "main_clk"); and get say auxclk1 on one board and
>>> auxclk3 on another?
>>>
>>
>> Yes, all you need to do is specify an alias to the clock in the board file.
> 
> Can we then create a special board specific node for panda and do
> similar things from DT?
> See a similar discussion below on how to handle the gpio_request()
> we had as part of board files
> http://www.spinics.net/lists/linux-omap/msg85248.html

Doesn't look very elegant to me, but I wouldn't mind if there is no better option.
Even then, we can't rely on the device name as its index can change based on where it is
located in the dts file. e.g. in the beginning it may be named phy.8, and if a device
node is added before it, it will get changed to phy.9

> 
>>
>> e.g. in board 1 file
>>     clk_add_alias("main_clk", "phy.1", "auxclk1_ck", NULL);
>>
>> in board 2 file
>>     clk_add_alias("main_clk", "phy.1", "auxclk3_ck", NULL);
>>

cheers,
-roger

Re: how to specify an OMAP clock in device tree?

[Rajendra Nayak]

On Tuesday 05 February 2013 08:22 PM, Roger Quadros wrote:
> Doesn't look very elegant to me, but I wouldn't mind if there is no better option.
> Even then, we can't rely on the device name as its index can change based on where it is

Well, thats what I said in the first mail, that *if* you are able to
fix the device name, *then* we could use clkdev the way its used in
non-DT case. But then you came back saying 'Fixing the device name
doesn't really solve the problem.' :)

> located in the dts file. e.g. in the beginning it may be named phy.8, and if a device
> node is added before it, it will get changed to phy.9

If you provide a phandle to the PHY node in the board node, for which
you need to add the clk alias, you can always extract the device (using
of_find_device_by_node() ) and hence its name, so it doesn't matter if
its phy.8 or phy.9.

Re: how to specify an OMAP clock in device tree?

[Roger Quadros]

On 02/06/2013 12:21 PM, Rajendra Nayak wrote:
> On Tuesday 05 February 2013 08:22 PM, Roger Quadros wrote:
>> Doesn't look very elegant to me, but I wouldn't mind if there is no better option.
>> Even then, we can't rely on the device name as its index can change based on where it is
> 
> Well, thats what I said in the first mail, that *if* you are able to
> fix the device name, *then* we could use clkdev the way its used in
> non-DT case. But then you came back saying 'Fixing the device name
> doesn't really solve the problem.' :)

It does, yes, but depending on fixed device names is not so great for DT, is it?
Doesn't solve the problem in the right sense :)

> 
>> located in the dts file. e.g. in the beginning it may be named phy.8, and if a device
>> node is added before it, it will get changed to phy.9
> 
> If you provide a phandle to the PHY node in the board node, for which
> you need to add the clk alias, you can always extract the device (using
> of_find_device_by_node() ) and hence its name, so it doesn't matter if
> its phy.8 or phy.9.

Right, I'll come up with something on these lines.

Thanks for the suggestions :).

cheers,
-roger

--
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

[PATCH 12/13] ARM: dts: OMAP3: Add HS USB Host IP nodes

[Roger Quadros]

Adds device nodes for HS USB Host module, TLL module,
OHCI and EHCI controllers.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/omap3.dtsi |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 1acc261..39442b4 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -397,5 +397,36 @@
 			ti,timer-alwon;
 			ti,timer-secure;
 		};
+
+		usbhstll: usbhstll@0x48062000 {
+			compatible = "ti,usbhs-tll";
+			reg = <0x48062000 0x1000>;
+			interrupts = <78>;
+			ti,hwmods = "usb_tll_hs";
+		};
+
+		usbhshost: usbhshost@0x48064000 {
+			compatible = "ti,usbhs-host";
+			reg = <0x48064000 0x400>;
+			ti,hwmods = "usb_host_hs";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			usbhsohci: ohci@0x48064400 {
+				compatible = "ti,omap3-ohci", "usb-ohci";
+				reg = <0x48064400 0x400>;
+				interrupt-parent = <&intc>;
+				interrupts = <76>;
+			};
+
+			usbhsehci: ehci@0x48064800 {
+				compatible = "ti,omap-ehci", "usb-ehci";
+				reg = <0x48064800 0x400>;
+				interrupt-parent = <&intc>;
+				interrupts = <77>;
+			};
+		};
+
 	};
 };
-- 
1.7.4.1

Re: [PATCH 00/13] Device tree support for OMAP HS USB Host

[Rajendra Nayak]

On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
> This patchset adds device tree support for OMAP's High Speed USB Host
> subsystem. Board adaptation for Panda and Beagleboard is also provided.
>
> Tested on Beagleboard.
>
> Will only work with Panda if we provide a reference to the PHY clock

But there is no reference provided even for Beagle. Does it work because
the default clk speed is 192Mhz?

> generator in the device tree in PATCH 11. I do not know how to do that
> as there is no way to provide a phandle to any of the OMAP generated clocks
> in the device tree. Suggestions welcome:).

Re: [PATCH 00/13] Device tree support for OMAP HS USB Host

[Roger Quadros]

On 02/05/2013 01:25 PM, Rajendra Nayak wrote:
> On Monday 04 February 2013 09:28 PM, Roger Quadros wrote:
>> This patchset adds device tree support for OMAP's High Speed USB Host
>> subsystem. Board adaptation for Panda and Beagleboard is also provided.
>>
>> Tested on Beagleboard.
>>
>> Will only work with Panda if we provide a reference to the PHY clock
> 
> But there is no reference provided even for Beagle. Does it work because
> the default clk speed is 192Mhz?

On beagle board the PHY is connected differently, i.e. as the clock receptor.
The PHY clock comes directly from the USB_CLK pin.

On Panda the PHY is the clock provider to the USB_CLK pin. For that it needs
a reference clock at the REFCLK pin which comes from FREF_CLK3.

> 
>> generator in the device tree in PATCH 11. I do not know how to do that
>> as there is no way to provide a phandle to any of the OMAP generated clocks
>> in the device tree. Suggestions welcome:).
> 

cheers,
-roger
--
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