[PATCH 0/2] max3421: add devicetree support

[PATCH 0/2] max3421: add devicetree support

[Jules Maselbas]

Hi,

This patchset adds devicetree support to the max3421 driver.
Thoses modification are based on a previous (unapplied) patch
series by Alexander Amelkin [1].

Since I am not a kernel expert I would a like to have your feedbacks on:
1. Under which condition pdata must be freed (in max3421_probe) ?
2. If it neccessary to free platform_data in max3421_remove function ?

Thank you,

[1]: [https://lkml.org/lkml/2017/5/26/285]

---

Jules Maselbas (2):
  usb: max3421: Add devicetree support
  dt-bindings: max3421: Add bindings documentation

 .../devicetree/bindings/usb/maxim,max3421.txt      | 24 ++++++
 drivers/usb/host/max3421-hcd.c                     | 87 +++++++++++++++++++++-
 2 files changed, 110 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421.txt

-- 
2.14.1

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

[PATCH 1/2] usb: max3421: Add devicetree support

[Jules Maselbas]

Adds support for devicetree to the max3421 driver.

Signed-off-by: Jules Maselbas <jules.maselbas-l+/54qjP3hgoQXW8ST3Jvw@public.gmane.org>
---
 drivers/usb/host/max3421-hcd.c | 87 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 86 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index 0ece9a9341e5..928a5aabee02 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -60,6 +60,7 @@
 #include <linux/spi/spi.h>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
+#include <linux/of.h>
 
 #include <linux/platform_data/max3421-hcd.h>
 
@@ -85,6 +86,8 @@
 			  USB_PORT_STAT_C_OVERCURRENT | \
 			  USB_PORT_STAT_C_RESET) << 16)
 
+#define MAX3421_GPOUT_COUNT	8
+
 enum max3421_rh_state {
 	MAX3421_RH_RESET,
 	MAX3421_RH_SUSPENDED,
@@ -1672,7 +1675,7 @@ max3421_gpout_set_value(struct usb_hcd *hcd, u8 pin_number, u8 value)
 	u8 mask, idx;
 
 	--pin_number;
-	if (pin_number > 7)
+	if (pin_number >= MAX3421_GPOUT_COUNT)
 		return;
 
 	mask = 1u << (pin_number % 4);
@@ -1699,6 +1702,10 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
 	spin_lock_irqsave(&max3421_hcd->lock, flags);
 
 	pdata = spi->dev.platform_data;
+	if (!pdata) {
+		dev_err(&spi->dev, "Device platform data is missing\n");
+		return -EFAULT;
+	}
 
 	switch (type_req) {
 	case ClearHubFeature:
@@ -1831,11 +1838,35 @@ static const struct hc_driver max3421_hcd_desc = {
 	.bus_resume =		max3421_bus_resume,
 };
 
+static int
+max3421_of_vbus_en_pin(struct device *dev, struct max3421_hcd_platform_data *pdata)
+{
+	int retval;
+	uint32_t value[2];
+
+	if (!pdata)
+		return -EINVAL;
+
+	retval = of_property_read_u32_array(dev->of_node, "maxim,vbus-en-pin", value, 2);
+	if (retval) {
+		dev_err(dev, "device tree node property 'maxim,vbus-en-pin' is missing\n");
+		return retval;
+	}
+	dev_info(dev, "property 'maxim,vbus-en-pin' value is <%d %d>\n", value[0], value[1]);
+
+	pdata->vbus_gpout = value[0];
+	pdata->vbus_active_level = value[1];
+
+	return 0;
+}
+
 static int
 max3421_probe(struct spi_device *spi)
 {
+	struct device *dev = &spi->dev;
 	struct max3421_hcd *max3421_hcd;
 	struct usb_hcd *hcd = NULL;
+	struct max3421_hcd_platform_data *pdata = NULL;
 	int retval = -ENOMEM;
 
 	if (spi_setup(spi) < 0) {
@@ -1843,6 +1874,42 @@ max3421_probe(struct spi_device *spi)
 		return -EFAULT;
 	}
 
+	if (!spi->irq) {
+		dev_err(dev, "Failed to get SPI IRQ");
+		return -EFAULT;
+	}
+
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+		pdata = devm_kzalloc(&spi->dev, sizeof(*pdata), GFP_KERNEL);
+		if (!pdata) {
+			dev_err(&spi->dev, "failed to allocate memory for private data\n");
+			retval = -ENOMEM;
+			goto error;
+		}
+		retval = max3421_of_vbus_en_pin(dev, pdata);
+		if (retval)
+			goto error;
+
+		spi->dev.platform_data = pdata;
+	}
+
+	pdata = spi->dev.platform_data;
+	if (!pdata) {
+		dev_err(&spi->dev, "driver configuration data is not provided\n");
+		retval = -EFAULT;
+		goto error;
+	}
+	if (pdata->vbus_active_level > 1) {
+		dev_err(&spi->dev, "vbus active level value %d is out of range (0/1)\n", pdata->vbus_active_level);
+		retval = -EINVAL;
+		goto error;
+	}
+	if (pdata->vbus_gpout < 1 || pdata->vbus_gpout > MAX3421_GPOUT_COUNT) {
+		dev_err(&spi->dev, "vbus gpout value %d is out of range (1..8)\n", pdata->vbus_gpout);
+		retval = -EINVAL;
+		goto error;
+	}
+
 	hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
 			     dev_name(&spi->dev));
 	if (!hcd) {
@@ -1885,6 +1952,11 @@ max3421_probe(struct spi_device *spi)
 	return 0;
 
 error:
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node && pdata) {
+		devm_kfree(&spi->dev, pdata);
+		spi->dev.platform_data = NULL;
+	}
+
 	if (hcd) {
 		kfree(max3421_hcd->tx);
 		kfree(max3421_hcd->rx);
@@ -1923,17 +1995,30 @@ max3421_remove(struct spi_device *spi)
 
 	spin_unlock_irqrestore(&max3421_hcd->lock, flags);
 
+	if (IS_ENABLED(CONFIG_OF) && spi->dev.platform_data) {
+		dev_dbg(&spi->dev, "Freeing platform data structure\n");
+		devm_kfree(&spi->dev, spi->dev.platform_data);
+		spi->dev.platform_data = NULL;
+	}
+
 	free_irq(spi->irq, hcd);
 
 	usb_put_hcd(hcd);
 	return 0;
 }
 
+static const struct of_device_id max3421_of_match_table[] = {
+	{ .compatible = "maxim,max3421", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, max3421_of_match_table);
+
 static struct spi_driver max3421_driver = {
 	.probe		= max3421_probe,
 	.remove		= max3421_remove,
 	.driver		= {
 		.name	= "max3421-hcd",
+		.of_match_table = of_match_ptr(max3421_of_match_table),
 	},
 };
 
-- 
2.14.1

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

[PATCH 2/2] dt-bindings: max3421: Add bindings documentation

[Jules Maselbas]

Adds bindings documentation for the max3421 driver.

Signed-off-by: Jules Maselbas <jules.maselbas-l+/54qjP3hgoQXW8ST3Jvw@public.gmane.org>
---
 .../devicetree/bindings/usb/maxim,max3421.txt      | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421.txt

diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421.txt b/Documentation/devicetree/bindings/usb/maxim,max3421.txt
new file mode 100644
index 000000000000..7536c3ab3e5a
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/maxim,max3421.txt
@@ -0,0 +1,24 @@
+Maxim Integrated SPI-based USB 2.0 host controller MAX3421E
+
+Required properties:
+ - compatible: "maxim,max3421"
+ - spi-max-frequency: maximum frequency for this device must not exceed 26 MHz.
+ - reg: chip select number to which this device is connected.
+ - maxim,vbus-en-pin: <GPOUTx ACTIVE_LEVEL>
+   GPOUTx is the number (1-8) of the GPOUT pin of MAX3421E to drive Vbus.
+   ACTIVE_LEVEL is 0 or 1.
+ - interrupt-parent: the phandle of the associated interrupt controller.
+ - interrupts: the interuption line description for the interrupt controller.
+   The driver configures MAX3421E for active low level triggered interrupts,
+   configure your interrupt line accordingly.
+
+Example:
+
+	usb@0 {
+		compatible = "maxim,max3421";
+		reg = <0>;
+		maxim,vbus-en-pin = <3 1>;
+		spi-max-frequency = <26000000>;
+		interrupt-parent = <&PIC>;
+		interrupts = <42>;
+	};
-- 
2.14.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 2/2] dt-bindings: max3421: Add bindings documentation

[Rob Herring]

On Fri, Sep 15, 2017 at 06:58:46PM +0200, Jules Maselbas wrote:
> Adds bindings documentation for the max3421 driver.
> 
> Signed-off-by: Jules Maselbas <jules.maselbas-l+/54qjP3hgoQXW8ST3Jvw@public.gmane.org>
> ---
>  .../devicetree/bindings/usb/maxim,max3421.txt      | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421.txt

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

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

[PATCH v2 0/2] max3421: add devicetree support

[Jules Maselbas]

Hi,

This patchset adds devicetree support to the max3421 driver.
Theses modification are based on a previous (unapplied) patch
series by Alexander Amelkin [1].

Changes since v2:
 * The platform_data is no longer freed in max3421_remove() as it is
   allocated with devm_kzalloc.
 * Removed the dev_err print if devm_kzalloc fail.
 * Removed a test for platform_data null pointer in max3421_hub_control()
   as the driver probe will fail if no platform_data is found.

Thank you.

[1]: [https://lkml.org/lkml/2017/5/26/285]

---

Jules Maselbas (2):
  usb: max3421: Add devicetree support
  dt-bindings: max3421: Add bindings documentation

 .../devicetree/bindings/usb/maxim,max3421.txt      | 24 +++++++
 drivers/usb/host/max3421-hcd.c                     | 75 +++++++++++++++++++++-
 2 files changed, 98 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421.txt

-- 
2.14.2

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

[PATCH v2 1/2] usb: max3421: Add devicetree support

[Jules Maselbas]

Adds support for devicetree to the max3421 driver.

Signed-off-by: Jules Maselbas <jules.maselbas-l+/54qjP3hgoQXW8ST3Jvw@public.gmane.org>
---
 drivers/usb/host/max3421-hcd.c | 75 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index 0ece9a9341e5..d77b3f27c6e7 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -60,6 +60,7 @@
 #include <linux/spi/spi.h>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
+#include <linux/of.h>
 
 #include <linux/platform_data/max3421-hcd.h>
 
@@ -85,6 +86,8 @@
 			  USB_PORT_STAT_C_OVERCURRENT | \
 			  USB_PORT_STAT_C_RESET) << 16)
 
+#define MAX3421_GPOUT_COUNT	8
+
 enum max3421_rh_state {
 	MAX3421_RH_RESET,
 	MAX3421_RH_SUSPENDED,
@@ -1672,7 +1675,7 @@ max3421_gpout_set_value(struct usb_hcd *hcd, u8 pin_number, u8 value)
 	u8 mask, idx;
 
 	--pin_number;
-	if (pin_number > 7)
+	if (pin_number >= MAX3421_GPOUT_COUNT)
 		return;
 
 	mask = 1u << (pin_number % 4);
@@ -1831,11 +1834,35 @@ static const struct hc_driver max3421_hcd_desc = {
 	.bus_resume =		max3421_bus_resume,
 };
 
+static int
+max3421_of_vbus_en_pin(struct device *dev, struct max3421_hcd_platform_data *pdata)
+{
+	int retval;
+	uint32_t value[2];
+
+	if (!pdata)
+		return -EINVAL;
+
+	retval = of_property_read_u32_array(dev->of_node, "maxim,vbus-en-pin", value, 2);
+	if (retval) {
+		dev_err(dev, "device tree node property 'maxim,vbus-en-pin' is missing\n");
+		return retval;
+	}
+	dev_info(dev, "property 'maxim,vbus-en-pin' value is <%d %d>\n", value[0], value[1]);
+
+	pdata->vbus_gpout = value[0];
+	pdata->vbus_active_level = value[1];
+
+	return 0;
+}
+
 static int
 max3421_probe(struct spi_device *spi)
 {
+	struct device *dev = &spi->dev;
 	struct max3421_hcd *max3421_hcd;
 	struct usb_hcd *hcd = NULL;
+	struct max3421_hcd_platform_data *pdata = NULL;
 	int retval = -ENOMEM;
 
 	if (spi_setup(spi) < 0) {
@@ -1843,6 +1870,40 @@ max3421_probe(struct spi_device *spi)
 		return -EFAULT;
 	}
 
+	if (!spi->irq) {
+		dev_err(dev, "Failed to get SPI IRQ");
+		return -EFAULT;
+	}
+
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+		if (!pdata) {
+			retval = -ENOMEM;
+			goto error;
+		}
+		retval = max3421_of_vbus_en_pin(dev, pdata);
+		if (retval)
+			goto error;
+		spi->dev.platform_data = pdata;
+	}
+
+	pdata = dev->platform_data;
+	if (!pdata) {
+		dev_err(dev, "driver configuration data is not provided\n");
+		retval = -EFAULT;
+		goto error;
+	}
+	if (pdata->vbus_active_level > 1) {
+		dev_err(dev, "vbus active level value %d is out of range (0/1)\n", pdata->vbus_active_level);
+		retval = -EINVAL;
+		goto error;
+	}
+	if (pdata->vbus_gpout < 1 || pdata->vbus_gpout > MAX3421_GPOUT_COUNT) {
+		dev_err(dev, "vbus gpout value %d is out of range (1..8)\n", pdata->vbus_gpout);
+		retval = -EINVAL;
+		goto error;
+	}
+
 	hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
 			     dev_name(&spi->dev));
 	if (!hcd) {
@@ -1885,6 +1946,11 @@ max3421_probe(struct spi_device *spi)
 	return 0;
 
 error:
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node && pdata) {
+		devm_kfree(&spi->dev, pdata);
+		spi->dev.platform_data = NULL;
+	}
+
 	if (hcd) {
 		kfree(max3421_hcd->tx);
 		kfree(max3421_hcd->rx);
@@ -1929,11 +1995,18 @@ max3421_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id max3421_of_match_table[] = {
+	{ .compatible = "maxim,max3421", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, max3421_of_match_table);
+
 static struct spi_driver max3421_driver = {
 	.probe		= max3421_probe,
 	.remove		= max3421_remove,
 	.driver		= {
 		.name	= "max3421-hcd",
+		.of_match_table = of_match_ptr(max3421_of_match_table),
 	},
 };
 
-- 
2.14.2

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

[PATCH v2 2/2] dt-bindings: max3421: Add bindings documentation

[Jules Maselbas]

Adds bindings documentation for the max3421 driver.

Signed-off-by: Jules Maselbas <jules.maselbas-l+/54qjP3hgoQXW8ST3Jvw@public.gmane.org>
---
 .../devicetree/bindings/usb/maxim,max3421.txt      | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421.txt

diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421.txt b/Documentation/devicetree/bindings/usb/maxim,max3421.txt
new file mode 100644
index 000000000000..7536c3ab3e5a
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/maxim,max3421.txt
@@ -0,0 +1,24 @@
+Maxim Integrated SPI-based USB 2.0 host controller MAX3421E
+
+Required properties:
+ - compatible: "maxim,max3421"
+ - spi-max-frequency: maximum frequency for this device must not exceed 26 MHz.
+ - reg: chip select number to which this device is connected.
+ - maxim,vbus-en-pin: <GPOUTx ACTIVE_LEVEL>
+   GPOUTx is the number (1-8) of the GPOUT pin of MAX3421E to drive Vbus.
+   ACTIVE_LEVEL is 0 or 1.
+ - interrupt-parent: the phandle of the associated interrupt controller.
+ - interrupts: the interuption line description for the interrupt controller.
+   The driver configures MAX3421E for active low level triggered interrupts,
+   configure your interrupt line accordingly.
+
+Example:
+
+	usb@0 {
+		compatible = "maxim,max3421";
+		reg = <0>;
+		maxim,vbus-en-pin = <3 1>;
+		spi-max-frequency = <26000000>;
+		interrupt-parent = <&PIC>;
+		interrupts = <42>;
+	};
-- 
2.14.2

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

Re: [PATCH v2 2/2] dt-bindings: max3421: Add bindings documentation

[Sergei Shtylyov]

Hello!

On 10/16/2017 2:43 AM, Jules Maselbas wrote:

> Adds bindings documentation for the max3421 driver.
> 
> Signed-off-by: Jules Maselbas <jules.maselbas-l+/54qjP3hgoQXW8ST3Jvw@public.gmane.org>
> ---
>   .../devicetree/bindings/usb/maxim,max3421.txt      | 24 ++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421.txt b/Documentation/devicetree/bindings/usb/maxim,max3421.txt
> new file mode 100644
> index 000000000000..7536c3ab3e5a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/maxim,max3421.txt
> @@ -0,0 +1,24 @@
> +Maxim Integrated SPI-based USB 2.0 host controller MAX3421E
> +
> +Required properties:
> + - compatible: "maxim,max3421"
> + - spi-max-frequency: maximum frequency for this device must not exceed 26 MHz.
> + - reg: chip select number to which this device is connected.
> + - maxim,vbus-en-pin: <GPOUTx ACTIVE_LEVEL>
> +   GPOUTx is the number (1-8) of the GPOUT pin of MAX3421E to drive Vbus.
> +   ACTIVE_LEVEL is 0 or 1.
> + - interrupt-parent: the phandle of the associated interrupt controller.

    This is never a required prob, it may be inherited from the parent nodes.

> + - interrupts: the interuption line description for the interrupt controller.

    Interrupt.

[...]

MBR, Sergei
--
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 v2 2/2] dt-bindings: max3421: Add bindings documentation

[Sergei Shtylyov]

On 10/16/2017 12:04 PM, Sergei Shtylyov wrote:

>> Adds bindings documentation for the max3421 driver.
>>
>> Signed-off-by: Jules Maselbas <jules.maselbas-l+/54qjP3hgoQXW8ST3Jvw@public.gmane.org>
>> ---
>>   .../devicetree/bindings/usb/maxim,max3421.txt      | 24 
>> ++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421.txt 
>> b/Documentation/devicetree/bindings/usb/maxim,max3421.txt
>> new file mode 100644
>> index 000000000000..7536c3ab3e5a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/maxim,max3421.txt
>> @@ -0,0 +1,24 @@
>> +Maxim Integrated SPI-based USB 2.0 host controller MAX3421E
>> +
>> +Required properties:
>> + - compatible: "maxim,max3421"
>> + - spi-max-frequency: maximum frequency for this device must not exceed 26 
>> MHz.
>> + - reg: chip select number to which this device is connected.
>> + - maxim,vbus-en-pin: <GPOUTx ACTIVE_LEVEL>
>> +   GPOUTx is the number (1-8) of the GPOUT pin of MAX3421E to drive Vbus.
>> +   ACTIVE_LEVEL is 0 or 1.
>> + - interrupt-parent: the phandle of the associated interrupt controller.
> 
>     This is never a required prob, it may be inherited from the parent nodes.

   Prop, sorry. :-)

[...]

MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 v2 2/2] dt-bindings: max3421: Add bindings documentation

[Jules Maselbas]

Thanks Sergei,

Is it better if the documentation say:
"
Optional property:
 - interrupt-parent: the phandle of the associated interrupt controller.
"
or should I also add that "it may be inherited from the parent nodes."?

Jules Maselbas.

----- Mail original -----
De: "Sergei Shtylyov" <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
À: "Jules Maselbas" <jules.maselbas-l+/54qjP3hgoQXW8ST3Jvw@public.gmane.org>, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org
Cc: "julia lawall" <julia.lawall-L2FTfq7BK8M@public.gmane.org>, robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, "mark rutland" <mark.rutland-5wv7dgnIgG8@public.gmane.org>, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Envoyé: Lundi 16 Octobre 2017 22:21:53
Objet: Re: [PATCH v2 2/2] dt-bindings: max3421: Add bindings documentation

On 10/16/2017 12:04 PM, Sergei Shtylyov wrote:

>> Adds bindings documentation for the max3421 driver.
>>
>> Signed-off-by: Jules Maselbas <jules.maselbas-l+/54qjP3hgoQXW8ST3Jvw@public.gmane.org>
>> ---
>>   .../devicetree/bindings/usb/maxim,max3421.txt      | 24 
>> ++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421.txt 
>> b/Documentation/devicetree/bindings/usb/maxim,max3421.txt
>> new file mode 100644
>> index 000000000000..7536c3ab3e5a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/maxim,max3421.txt
>> @@ -0,0 +1,24 @@
>> +Maxim Integrated SPI-based USB 2.0 host controller MAX3421E
>> +
>> +Required properties:
>> + - compatible: "maxim,max3421"
>> + - spi-max-frequency: maximum frequency for this device must not exceed 26 
>> MHz.
>> + - reg: chip select number to which this device is connected.
>> + - maxim,vbus-en-pin: <GPOUTx ACTIVE_LEVEL>
>> +   GPOUTx is the number (1-8) of the GPOUT pin of MAX3421E to drive Vbus.
>> +   ACTIVE_LEVEL is 0 or 1.
>> + - interrupt-parent: the phandle of the associated interrupt controller.
> 
>     This is never a required prob, it may be inherited from the parent nodes.

   Prop, sorry. :-)

[...]

MBR, Sergei
--
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 v2 2/2] dt-bindings: max3421: Add bindings documentation

[Rob Herring]

On Mon, Oct 16, 2017 at 01:43:06AM +0200, Jules Maselbas wrote:
> Adds bindings documentation for the max3421 driver.
> 
> Signed-off-by: Jules Maselbas <jules.maselbas-l+/54qjP3hgoQXW8ST3Jvw@public.gmane.org>
> ---
>  .../devicetree/bindings/usb/maxim,max3421.txt      | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421.txt

Please add acks when posting new versions.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 v2 2/2] dt-bindings: max3421: Add bindings documentation

[Sergei Shtylyov]

On 10/17/2017 12:22 AM, Jules Maselbas wrote:

> Thanks Sergei,
> 
> Is it better if the documentation say:
> "
> Optional property:
>   - interrupt-parent: the phandle of the associated interrupt controller.
> "

    That should be enough.

> or should I also add that "it may be inherited from the parent nodes."?
> 
> Jules Maselbas.

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

[PATCH v3 0/2] max3421: add devicetree support

[Jules Maselbas]

Hi,

This patchset adds devicetree support to the max3421 driver.
Theses modification are based on a previous (unapplied) patch
series by Alexander Amelkin [1].

Changes in v3:
 * Documentation modification, interrupt-parent is optional.

Changes in v2:
 * The platform_data is no longer freed in max3421_remove() as it is
   allocated with devm_kzalloc.
 * Removed the dev_err print if devm_kzalloc fail.
 * Removed a test for platform_data null pointer in max3421_hub_control()
   as the driver probe will fail if no platform_data is found.

Thank you.

[1]: [https://lkml.org/lkml/2017/5/26/285]

---

Jules Maselbas (2):
  usb: max3421: Add devicetree support
  dt-bindings: max3421: Add bindings documentation

 .../devicetree/bindings/usb/maxim,max3421.txt      | 25 ++++++++
 drivers/usb/host/max3421-hcd.c                     | 75 +++++++++++++++++++++-
 2 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421.txt

-- 
2.14.2

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

[PATCH v3 1/2] usb: max3421: Add devicetree support

[Jules Maselbas]

Adds support for devicetree to the max3421 driver.

Signed-off-by: Jules Maselbas <jules.maselbas-l+/54qjP3hgoQXW8ST3Jvw@public.gmane.org>
---
 drivers/usb/host/max3421-hcd.c | 75 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index 0ece9a9341e5..d77b3f27c6e7 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -60,6 +60,7 @@
 #include <linux/spi/spi.h>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
+#include <linux/of.h>
 
 #include <linux/platform_data/max3421-hcd.h>
 
@@ -85,6 +86,8 @@
 			  USB_PORT_STAT_C_OVERCURRENT | \
 			  USB_PORT_STAT_C_RESET) << 16)
 
+#define MAX3421_GPOUT_COUNT	8
+
 enum max3421_rh_state {
 	MAX3421_RH_RESET,
 	MAX3421_RH_SUSPENDED,
@@ -1672,7 +1675,7 @@ max3421_gpout_set_value(struct usb_hcd *hcd, u8 pin_number, u8 value)
 	u8 mask, idx;
 
 	--pin_number;
-	if (pin_number > 7)
+	if (pin_number >= MAX3421_GPOUT_COUNT)
 		return;
 
 	mask = 1u << (pin_number % 4);
@@ -1831,11 +1834,35 @@ static const struct hc_driver max3421_hcd_desc = {
 	.bus_resume =		max3421_bus_resume,
 };
 
+static int
+max3421_of_vbus_en_pin(struct device *dev, struct max3421_hcd_platform_data *pdata)
+{
+	int retval;
+	uint32_t value[2];
+
+	if (!pdata)
+		return -EINVAL;
+
+	retval = of_property_read_u32_array(dev->of_node, "maxim,vbus-en-pin", value, 2);
+	if (retval) {
+		dev_err(dev, "device tree node property 'maxim,vbus-en-pin' is missing\n");
+		return retval;
+	}
+	dev_info(dev, "property 'maxim,vbus-en-pin' value is <%d %d>\n", value[0], value[1]);
+
+	pdata->vbus_gpout = value[0];
+	pdata->vbus_active_level = value[1];
+
+	return 0;
+}
+
 static int
 max3421_probe(struct spi_device *spi)
 {
+	struct device *dev = &spi->dev;
 	struct max3421_hcd *max3421_hcd;
 	struct usb_hcd *hcd = NULL;
+	struct max3421_hcd_platform_data *pdata = NULL;
 	int retval = -ENOMEM;
 
 	if (spi_setup(spi) < 0) {
@@ -1843,6 +1870,40 @@ max3421_probe(struct spi_device *spi)
 		return -EFAULT;
 	}
 
+	if (!spi->irq) {
+		dev_err(dev, "Failed to get SPI IRQ");
+		return -EFAULT;
+	}
+
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+		if (!pdata) {
+			retval = -ENOMEM;
+			goto error;
+		}
+		retval = max3421_of_vbus_en_pin(dev, pdata);
+		if (retval)
+			goto error;
+		spi->dev.platform_data = pdata;
+	}
+
+	pdata = dev->platform_data;
+	if (!pdata) {
+		dev_err(dev, "driver configuration data is not provided\n");
+		retval = -EFAULT;
+		goto error;
+	}
+	if (pdata->vbus_active_level > 1) {
+		dev_err(dev, "vbus active level value %d is out of range (0/1)\n", pdata->vbus_active_level);
+		retval = -EINVAL;
+		goto error;
+	}
+	if (pdata->vbus_gpout < 1 || pdata->vbus_gpout > MAX3421_GPOUT_COUNT) {
+		dev_err(dev, "vbus gpout value %d is out of range (1..8)\n", pdata->vbus_gpout);
+		retval = -EINVAL;
+		goto error;
+	}
+
 	hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
 			     dev_name(&spi->dev));
 	if (!hcd) {
@@ -1885,6 +1946,11 @@ max3421_probe(struct spi_device *spi)
 	return 0;
 
 error:
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node && pdata) {
+		devm_kfree(&spi->dev, pdata);
+		spi->dev.platform_data = NULL;
+	}
+
 	if (hcd) {
 		kfree(max3421_hcd->tx);
 		kfree(max3421_hcd->rx);
@@ -1929,11 +1995,18 @@ max3421_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id max3421_of_match_table[] = {
+	{ .compatible = "maxim,max3421", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, max3421_of_match_table);
+
 static struct spi_driver max3421_driver = {
 	.probe		= max3421_probe,
 	.remove		= max3421_remove,
 	.driver		= {
 		.name	= "max3421-hcd",
+		.of_match_table = of_match_ptr(max3421_of_match_table),
 	},
 };
 
-- 
2.14.2

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

[PATCH v3 2/2] dt-bindings: max3421: Add bindings documentation

[Jules Maselbas]

Adds bindings documentation for the max3421 driver.

Signed-off-by: Jules Maselbas <jules.maselbas-l+/54qjP3hgoQXW8ST3Jvw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 .../devicetree/bindings/usb/maxim,max3421.txt      | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421.txt

diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421.txt b/Documentation/devicetree/bindings/usb/maxim,max3421.txt
new file mode 100644
index 000000000000..5c699e52ce3f
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/maxim,max3421.txt
@@ -0,0 +1,25 @@
+Maxim Integrated SPI-based USB 2.0 host controller MAX3421E
+
+Required properties:
+ - compatible: Should be "maxim,max3421"
+ - spi-max-frequency: maximum frequency for this device must not exceed 26 MHz.
+ - reg: chip select number to which this device is connected.
+ - maxim,vbus-en-pin: <GPOUTx ACTIVE_LEVEL>
+   GPOUTx is the number (1-8) of the GPOUT pin of MAX3421E to drive Vbus.
+   ACTIVE_LEVEL is 0 or 1.
+ - interrupts: the interrupt line description for the interrupt controller.
+   The driver configures MAX3421E for active low level triggered interrupts,
+   configure your interrupt line accordingly.
+
+Optional property:
+ - interrupt-parent: the phandle to the associated interrupt controller.
+
+Example:
+	usb@0 {
+		compatible = "maxim,max3421";
+		reg = <0>;
+		maxim,vbus-en-pin = <3 1>;
+		spi-max-frequency = <26000000>;
+		interrupt-parent = <&PIC>;
+		interrupts = <42>;
+	};
-- 
2.14.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 v3 0/2] max3421: add devicetree support

[Greg KH]

On Tue, Oct 24, 2017 at 01:08:41AM +0200, Jules Maselbas wrote:
> Hi,
> 
> This patchset adds devicetree support to the max3421 driver.
> Theses modification are based on a previous (unapplied) patch
> series by Alexander Amelkin [1].

I've already applied this series, right?  If there were any changes from
the older patch that I applied, can you send a fix-up patch?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 v3 0/2] max3421: add devicetree support

[Jules Maselbas]

Yes,

The first version of this patch series was already applied (on usb-next).
I made new versions because I forgot that there are no rebase. So this is why I sent new patch.

The newest (v3) version has been split in two patch and also applied (on usb-testing).
 9b796ffcd16dfaf820ecc6a9dc287e6d758a556e dt-bindings: usb: max3421: Interrupt-parent is optional
 892f6ebc53ac1a12a26c21eb26d18064461a7007 usb: host: max3421-hcd: Remove pdata test in max3421_hub_control()

I am not sure, are theses patch fix-up?

Thanks,
Jules.

----- Le 1 Nov 17, à 17:12, gregkh gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org a écrit :

> On Tue, Oct 24, 2017 at 01:08:41AM +0200, Jules Maselbas wrote:
>> Hi,
>> 
>> This patchset adds devicetree support to the max3421 driver.
>> Theses modification are based on a previous (unapplied) patch
>> series by Alexander Amelkin [1].
> 
> I've already applied this series, right?  If there were any changes from
> the older patch that I applied, can you send a fix-up patch?
> 
> thanks,
> 
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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 v3 0/2] max3421: add devicetree support

[gregkh]

On Wed, Nov 01, 2017 at 05:50:56PM +0100, Jules Maselbas wrote:
> Yes,
> 
> The first version of this patch series was already applied (on usb-next).
> I made new versions because I forgot that there are no rebase. So this is why I sent new patch.
> 
> The newest (v3) version has been split in two patch and also applied (on usb-testing).
>  9b796ffcd16dfaf820ecc6a9dc287e6d758a556e dt-bindings: usb: max3421: Interrupt-parent is optional
>  892f6ebc53ac1a12a26c21eb26d18064461a7007 usb: host: max3421-hcd: Remove pdata test in max3421_hub_control()
> 
> I am not sure, are theses patch fix-up?

If you are happy with all of the patches I've now taken, all is good :)

If not, please send me what needs to be applied.

thanks,

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