* [GIT][PULL] Improvements to max3421-hcd driver
@ 2017-05-26 11:22 ` Alexander Amelkin
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Amelkin @ 2017-05-26 11:22 UTC (permalink / raw)
To: linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland
The following changes since commit
08332893e37af6ae779367e78e444f8f9571511d:
Linux 4.12-rc2 (2017-05-21 19:30:23 -0700)
are available in the git repository at:
https://github.com/AlexanderAmelkin/linux-wandboard.git
tags/max3421-improvements-1
for you to fetch changes up to b9a7559d24c0b2cb6e69124d861a943f79272681:
usb: max3421-hcd: Remove function name from dev_dbg calls (2017-05-25
11:52:13 +0300)
The separate patches for these changes will be sent as a follow up.
----------------------------------------------------------------
usb: max3421-hcd: Make the driver more up-to-date and flexible
This set of patches updates the max3421-hcd driver and fixes
some bugs in it. Specifically, it:
- Adds support for devicetree
- Makes platform related parameters configurable
- Adds sanity checks for platform related parameters
- Fixes crash on driver removal
- Tidies up use of memory allocation/deallocation functions
- Removes superfluous debug text
----------------------------------------------------------------
Alexander Amelkin (3):
usb: max3421-hcd: Add devicetree support to the driver
usb: max3421-hcd: Fix crash on the driver removal
usb: max3421-hcd: Remove function name from dev_dbg calls
.../devicetree/bindings/usb/maxim,max3421-hcd.txt | 19 +++
drivers/usb/host/max3421-hcd.c | 131
+++++++++++++++++----
2 files changed, 127 insertions(+), 23 deletions(-)
create mode 100644
Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
--
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
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/3] usb: max3421-hcd: Add devicetree support to the driver
@ 2017-05-26 11:28 ` Alexander Amelkin
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Amelkin @ 2017-05-26 11:28 UTC (permalink / raw)
To: linux-usb, devicetree, linux-kernel
Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland
[-- Attachment #1: Type: text/plain, Size: 7166 bytes --]
NOTE:
Please don't use the plain text here as a patch because it most probably
is corrupted by my webmail client.
Attached is a copy of the following text guaranteed to have correct
tabs/spaces.
-------------------------
Before this patch the max3421-hcd driver could only use
statically linked platform data to initialize its parameters.
This prevented it from being used on systems using device
tree.
The data taken from the device tree is put into dev->platform_data
when CONFIG_OF is enabled and the device is an OpenFirmware node.
The driver's 'compatible' string is 'maxim,max3421'
Binding documentation is also added with this patch.
Signed-off-by: Alexander Amelkin <alexander@amelkin.msk.ru>
---
.../devicetree/bindings/usb/maxim,max3421-hcd.txt | 19 +++++
drivers/usb/host/max3421-hcd.c | 96
++++++++++++++++++++--
2 files changed, 110 insertions(+), 5 deletions(-)
create mode 100644
Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
new file mode 100644
index 0000000..a8b9faa
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
@@ -0,0 +1,19 @@
+* SPI-based USB host controller Maxim Integrated max3421e
+
+Required properties:
+- compatible: must be "maxim,max3421"
+- reg: chip select number to which the max3421 chip is connected
+ (depends on master SPI controller)
+- spi-max-frequency: the operational frequency, must not exceed
<26000000>
+- interrupt-parent: phandle of the associated GPIO controller to which
the INT line
+ of max3421e chip is connected
+- interrupts: specification of the GPIO pin (in terms of the
`interrupt-parent`)
+ to which INT line of max3421e chip is connected.
+ The driver configures MAX3421E for active low level triggered
interrupts.
+ Configure your 'interrupt-parent' gpio controller accordingly.
+- vbus: <GPOUTx ACTIVE_LEVEL>
+ GPOUTx is the number (1-8) of GPOUT pin of max3421e used to drive
Vbus.
+ ACTIVE_LEVEL is 1 or 0.
+
+Don't forget to add pinctrl properties if you need some GPIO pins
reconfigured
+for use as INT. See binding information for the pinctrl nodes.
diff --git a/drivers/usb/host/max3421-hcd.c
b/drivers/usb/host/max3421-hcd.c
index 369869a..f600052 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -61,6 +61,12 @@
#include <linux/usb.h>
#include <linux/usb/hcd.h>
+#if defined(CONFIG_OF)
+#include <linux/of_device.h>
+
+#define MAX3421_GPOUT_COUNT 8
+#endif
+
#include <linux/platform_data/max3421-hcd.h>
#define DRIVER_DESC "MAX3421 USB Host-Controller Driver"
@@ -1699,6 +1705,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,20 +1841,80 @@ static struct hc_driver max3421_hcd_desc = {
.bus_resume = max3421_bus_resume,
};
+#if defined(CONFIG_OF)
+static struct max3421_hcd_platform_data max3421_data;
+
+static const struct of_device_id max3421_dt_ids[] = {
+ { .compatible = "maxim,max3421", .data = &max3421_data, },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max3421_dt_ids);
+#endif
+
static int
max3421_probe(struct spi_device *spi)
{
struct max3421_hcd *max3421_hcd;
struct usb_hcd *hcd = NULL;
int retval = -ENOMEM;
+#if defined(CONFIG_OF)
+ struct max3421_hcd_platform_data *pdata = NULL;
+#endif
if (spi_setup(spi) < 0) {
dev_err(&spi->dev, "Unable to setup SPI bus");
return -EFAULT;
}
- hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
- dev_name(&spi->dev));
+ if (!spi->irq) {
+ dev_err(&spi->dev, "Failed to get SPI IRQ for node '%s'\n",
spi->dev.of_node->full_name);
+ return -EFAULT;
+ }
+
+#if defined(CONFIG_OF)
+ if (spi->dev.of_node) {
+ /* A temporary alias structure */
+ union {
+ uint32_t value[2];
+ struct {
+ uint32_t gpout;
+ uint32_t active_level;
+ };
+ } vbus;
+
+ if(!(pdata = devm_kzalloc(&spi->dev, sizeof(*pdata),
GFP_KERNEL))) {
+ dev_err(&spi->dev, "failed to allocate memory for private
data\n");
+ retval = -ENOMEM;
+ goto error;
+ }
+ spi->dev.platform_data = pdata;
+
+ if ((retval = of_property_read_u32_array(spi->dev.of_node,
"vbus", vbus.value, 2))) {
+ dev_err(&spi->dev, "device tree node property 'vbus' is
missing\n");
+ goto error;
+ }
+ pdata->vbus_gpout = vbus.gpout;
+ pdata->vbus_active_level = vbus.active_level;
+ }
+ else
+#endif
+ 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) {
dev_err(&spi->dev, "failed to create HCD structure\n");
goto error;
@@ -1892,6 +1962,12 @@ max3421_probe(struct spi_device *spi)
kthread_stop(max3421_hcd->spi_thread);
usb_put_hcd(hcd);
}
+#if defined(CONFIG_OF)
+ if (pdata && spi->dev.platform_data == pdata) {
+ devm_kfree(&spi->dev, pdata);
+ spi->dev.platform_data = NULL;
+ }
+#endif
return retval;
}
@@ -1908,14 +1984,12 @@ max3421_remove(struct spi_device *spi)
if (hcd->self.controller == &spi->dev)
break;
}
+
if (!max3421_hcd) {
dev_err(&spi->dev, "no MAX3421 HCD found for SPI device %p\n",
spi);
return -ENODEV;
}
-
- usb_remove_hcd(hcd);
-
spin_lock_irqsave(&max3421_hcd->lock, flags);
kthread_stop(max3421_hcd->spi_thread);
@@ -1923,8 +1997,19 @@ max3421_remove(struct spi_device *spi)
spin_unlock_irqrestore(&max3421_hcd->lock, flags);
+#if defined(CONFIG_OF)
+ if (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;
+ }
+#endif
+
free_irq(spi->irq, hcd);
+ usb_remove_hcd(hcd);
+
+
usb_put_hcd(hcd);
return 0;
}
@@ -1934,6 +2019,7 @@ static struct spi_driver max3421_driver = {
.remove = max3421_remove,
.driver = {
.name = "max3421-hcd",
+ .of_match_table = of_match_ptr(max3421_dt_ids),
},
};
--
2.7.4
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-usb-max3421-hcd-Add-devicetree-support-to-the-driver.patch --]
[-- Type: text/x-diff; name=0001-usb-max3421-hcd-Add-devicetree-support-to-the-driver.patch, Size: 6691 bytes --]
From 0eb4464398c8b0a336aa0305724b4b84ef2be097 Mon Sep 17 00:00:00 2001
From: Alexander Amelkin <amelkin@fastwel.ru>
Date: Tue, 28 Mar 2017 20:59:06 +0300
Subject: [PATCH 1/3] usb: max3421-hcd: Add devicetree support to the driver
Before this patch the max3421-hcd driver could only use
statically linked platform data to initialize its parameters.
This prevented it from being used on systems using device
tree.
The data taken from the device tree is put into dev->platform_data
when CONFIG_OF is enabled and the device is an OpenFirmware node.
The driver's 'compatible' string is 'maxim,max3421'
Binding documentation is also added with this patch.
Signed-off-by: Alexander Amelkin <alexander@amelkin.msk.ru>
---
.../devicetree/bindings/usb/maxim,max3421-hcd.txt | 19 +++++
drivers/usb/host/max3421-hcd.c | 96 ++++++++++++++++++++--
2 files changed, 110 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
new file mode 100644
index 0000000..a8b9faa
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
@@ -0,0 +1,19 @@
+* SPI-based USB host controller Maxim Integrated max3421e
+
+Required properties:
+- compatible: must be "maxim,max3421"
+- reg: chip select number to which the max3421 chip is connected
+ (depends on master SPI controller)
+- spi-max-frequency: the operational frequency, must not exceed <26000000>
+- interrupt-parent: phandle of the associated GPIO controller to which the INT line
+ of max3421e chip is connected
+- interrupts: specification of the GPIO pin (in terms of the `interrupt-parent`)
+ to which INT line of max3421e chip is connected.
+ The driver configures MAX3421E for active low level triggered interrupts.
+ Configure your 'interrupt-parent' gpio controller accordingly.
+- vbus: <GPOUTx ACTIVE_LEVEL>
+ GPOUTx is the number (1-8) of GPOUT pin of max3421e used to drive Vbus.
+ ACTIVE_LEVEL is 1 or 0.
+
+Don't forget to add pinctrl properties if you need some GPIO pins reconfigured
+for use as INT. See binding information for the pinctrl nodes.
diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index 369869a..f600052 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -61,6 +61,12 @@
#include <linux/usb.h>
#include <linux/usb/hcd.h>
+#if defined(CONFIG_OF)
+#include <linux/of_device.h>
+
+#define MAX3421_GPOUT_COUNT 8
+#endif
+
#include <linux/platform_data/max3421-hcd.h>
#define DRIVER_DESC "MAX3421 USB Host-Controller Driver"
@@ -1699,6 +1705,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,20 +1841,80 @@ static struct hc_driver max3421_hcd_desc = {
.bus_resume = max3421_bus_resume,
};
+#if defined(CONFIG_OF)
+static struct max3421_hcd_platform_data max3421_data;
+
+static const struct of_device_id max3421_dt_ids[] = {
+ { .compatible = "maxim,max3421", .data = &max3421_data, },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max3421_dt_ids);
+#endif
+
static int
max3421_probe(struct spi_device *spi)
{
struct max3421_hcd *max3421_hcd;
struct usb_hcd *hcd = NULL;
int retval = -ENOMEM;
+#if defined(CONFIG_OF)
+ struct max3421_hcd_platform_data *pdata = NULL;
+#endif
if (spi_setup(spi) < 0) {
dev_err(&spi->dev, "Unable to setup SPI bus");
return -EFAULT;
}
- hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
- dev_name(&spi->dev));
+ if (!spi->irq) {
+ dev_err(&spi->dev, "Failed to get SPI IRQ for node '%s'\n", spi->dev.of_node->full_name);
+ return -EFAULT;
+ }
+
+#if defined(CONFIG_OF)
+ if (spi->dev.of_node) {
+ /* A temporary alias structure */
+ union {
+ uint32_t value[2];
+ struct {
+ uint32_t gpout;
+ uint32_t active_level;
+ };
+ } vbus;
+
+ if(!(pdata = devm_kzalloc(&spi->dev, sizeof(*pdata), GFP_KERNEL))) {
+ dev_err(&spi->dev, "failed to allocate memory for private data\n");
+ retval = -ENOMEM;
+ goto error;
+ }
+ spi->dev.platform_data = pdata;
+
+ if ((retval = of_property_read_u32_array(spi->dev.of_node, "vbus", vbus.value, 2))) {
+ dev_err(&spi->dev, "device tree node property 'vbus' is missing\n");
+ goto error;
+ }
+ pdata->vbus_gpout = vbus.gpout;
+ pdata->vbus_active_level = vbus.active_level;
+ }
+ else
+#endif
+ 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) {
dev_err(&spi->dev, "failed to create HCD structure\n");
goto error;
@@ -1892,6 +1962,12 @@ max3421_probe(struct spi_device *spi)
kthread_stop(max3421_hcd->spi_thread);
usb_put_hcd(hcd);
}
+#if defined(CONFIG_OF)
+ if (pdata && spi->dev.platform_data == pdata) {
+ devm_kfree(&spi->dev, pdata);
+ spi->dev.platform_data = NULL;
+ }
+#endif
return retval;
}
@@ -1908,14 +1984,12 @@ max3421_remove(struct spi_device *spi)
if (hcd->self.controller == &spi->dev)
break;
}
+
if (!max3421_hcd) {
dev_err(&spi->dev, "no MAX3421 HCD found for SPI device %p\n",
spi);
return -ENODEV;
}
-
- usb_remove_hcd(hcd);
-
spin_lock_irqsave(&max3421_hcd->lock, flags);
kthread_stop(max3421_hcd->spi_thread);
@@ -1923,8 +1997,19 @@ max3421_remove(struct spi_device *spi)
spin_unlock_irqrestore(&max3421_hcd->lock, flags);
+#if defined(CONFIG_OF)
+ if (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;
+ }
+#endif
+
free_irq(spi->irq, hcd);
+ usb_remove_hcd(hcd);
+
+
usb_put_hcd(hcd);
return 0;
}
@@ -1934,6 +2019,7 @@ static struct spi_driver max3421_driver = {
.remove = max3421_remove,
.driver = {
.name = "max3421-hcd",
+ .of_match_table = of_match_ptr(max3421_dt_ids),
},
};
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 1/3] usb: max3421-hcd: Add devicetree support to the driver
@ 2017-05-26 11:28 ` Alexander Amelkin
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Amelkin @ 2017-05-26 11:28 UTC (permalink / raw)
To: linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland
[-- Attachment #1: Type: text/plain, Size: 7191 bytes --]
NOTE:
Please don't use the plain text here as a patch because it most probably
is corrupted by my webmail client.
Attached is a copy of the following text guaranteed to have correct
tabs/spaces.
-------------------------
Before this patch the max3421-hcd driver could only use
statically linked platform data to initialize its parameters.
This prevented it from being used on systems using device
tree.
The data taken from the device tree is put into dev->platform_data
when CONFIG_OF is enabled and the device is an OpenFirmware node.
The driver's 'compatible' string is 'maxim,max3421'
Binding documentation is also added with this patch.
Signed-off-by: Alexander Amelkin <alexander-RgVAGUil9pwRDxlzMovjVg@public.gmane.org>
---
.../devicetree/bindings/usb/maxim,max3421-hcd.txt | 19 +++++
drivers/usb/host/max3421-hcd.c | 96
++++++++++++++++++++--
2 files changed, 110 insertions(+), 5 deletions(-)
create mode 100644
Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
new file mode 100644
index 0000000..a8b9faa
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
@@ -0,0 +1,19 @@
+* SPI-based USB host controller Maxim Integrated max3421e
+
+Required properties:
+- compatible: must be "maxim,max3421"
+- reg: chip select number to which the max3421 chip is connected
+ (depends on master SPI controller)
+- spi-max-frequency: the operational frequency, must not exceed
<26000000>
+- interrupt-parent: phandle of the associated GPIO controller to which
the INT line
+ of max3421e chip is connected
+- interrupts: specification of the GPIO pin (in terms of the
`interrupt-parent`)
+ to which INT line of max3421e chip is connected.
+ The driver configures MAX3421E for active low level triggered
interrupts.
+ Configure your 'interrupt-parent' gpio controller accordingly.
+- vbus: <GPOUTx ACTIVE_LEVEL>
+ GPOUTx is the number (1-8) of GPOUT pin of max3421e used to drive
Vbus.
+ ACTIVE_LEVEL is 1 or 0.
+
+Don't forget to add pinctrl properties if you need some GPIO pins
reconfigured
+for use as INT. See binding information for the pinctrl nodes.
diff --git a/drivers/usb/host/max3421-hcd.c
b/drivers/usb/host/max3421-hcd.c
index 369869a..f600052 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -61,6 +61,12 @@
#include <linux/usb.h>
#include <linux/usb/hcd.h>
+#if defined(CONFIG_OF)
+#include <linux/of_device.h>
+
+#define MAX3421_GPOUT_COUNT 8
+#endif
+
#include <linux/platform_data/max3421-hcd.h>
#define DRIVER_DESC "MAX3421 USB Host-Controller Driver"
@@ -1699,6 +1705,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,20 +1841,80 @@ static struct hc_driver max3421_hcd_desc = {
.bus_resume = max3421_bus_resume,
};
+#if defined(CONFIG_OF)
+static struct max3421_hcd_platform_data max3421_data;
+
+static const struct of_device_id max3421_dt_ids[] = {
+ { .compatible = "maxim,max3421", .data = &max3421_data, },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max3421_dt_ids);
+#endif
+
static int
max3421_probe(struct spi_device *spi)
{
struct max3421_hcd *max3421_hcd;
struct usb_hcd *hcd = NULL;
int retval = -ENOMEM;
+#if defined(CONFIG_OF)
+ struct max3421_hcd_platform_data *pdata = NULL;
+#endif
if (spi_setup(spi) < 0) {
dev_err(&spi->dev, "Unable to setup SPI bus");
return -EFAULT;
}
- hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
- dev_name(&spi->dev));
+ if (!spi->irq) {
+ dev_err(&spi->dev, "Failed to get SPI IRQ for node '%s'\n",
spi->dev.of_node->full_name);
+ return -EFAULT;
+ }
+
+#if defined(CONFIG_OF)
+ if (spi->dev.of_node) {
+ /* A temporary alias structure */
+ union {
+ uint32_t value[2];
+ struct {
+ uint32_t gpout;
+ uint32_t active_level;
+ };
+ } vbus;
+
+ if(!(pdata = devm_kzalloc(&spi->dev, sizeof(*pdata),
GFP_KERNEL))) {
+ dev_err(&spi->dev, "failed to allocate memory for private
data\n");
+ retval = -ENOMEM;
+ goto error;
+ }
+ spi->dev.platform_data = pdata;
+
+ if ((retval = of_property_read_u32_array(spi->dev.of_node,
"vbus", vbus.value, 2))) {
+ dev_err(&spi->dev, "device tree node property 'vbus' is
missing\n");
+ goto error;
+ }
+ pdata->vbus_gpout = vbus.gpout;
+ pdata->vbus_active_level = vbus.active_level;
+ }
+ else
+#endif
+ 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) {
dev_err(&spi->dev, "failed to create HCD structure\n");
goto error;
@@ -1892,6 +1962,12 @@ max3421_probe(struct spi_device *spi)
kthread_stop(max3421_hcd->spi_thread);
usb_put_hcd(hcd);
}
+#if defined(CONFIG_OF)
+ if (pdata && spi->dev.platform_data == pdata) {
+ devm_kfree(&spi->dev, pdata);
+ spi->dev.platform_data = NULL;
+ }
+#endif
return retval;
}
@@ -1908,14 +1984,12 @@ max3421_remove(struct spi_device *spi)
if (hcd->self.controller == &spi->dev)
break;
}
+
if (!max3421_hcd) {
dev_err(&spi->dev, "no MAX3421 HCD found for SPI device %p\n",
spi);
return -ENODEV;
}
-
- usb_remove_hcd(hcd);
-
spin_lock_irqsave(&max3421_hcd->lock, flags);
kthread_stop(max3421_hcd->spi_thread);
@@ -1923,8 +1997,19 @@ max3421_remove(struct spi_device *spi)
spin_unlock_irqrestore(&max3421_hcd->lock, flags);
+#if defined(CONFIG_OF)
+ if (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;
+ }
+#endif
+
free_irq(spi->irq, hcd);
+ usb_remove_hcd(hcd);
+
+
usb_put_hcd(hcd);
return 0;
}
@@ -1934,6 +2019,7 @@ static struct spi_driver max3421_driver = {
.remove = max3421_remove,
.driver = {
.name = "max3421-hcd",
+ .of_match_table = of_match_ptr(max3421_dt_ids),
},
};
--
2.7.4
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-usb-max3421-hcd-Add-devicetree-support-to-the-driver.patch --]
[-- Type: text/x-diff; name=0001-usb-max3421-hcd-Add-devicetree-support-to-the-driver.patch, Size: 6691 bytes --]
From 0eb4464398c8b0a336aa0305724b4b84ef2be097 Mon Sep 17 00:00:00 2001
From: Alexander Amelkin <amelkin@fastwel.ru>
Date: Tue, 28 Mar 2017 20:59:06 +0300
Subject: [PATCH 1/3] usb: max3421-hcd: Add devicetree support to the driver
Before this patch the max3421-hcd driver could only use
statically linked platform data to initialize its parameters.
This prevented it from being used on systems using device
tree.
The data taken from the device tree is put into dev->platform_data
when CONFIG_OF is enabled and the device is an OpenFirmware node.
The driver's 'compatible' string is 'maxim,max3421'
Binding documentation is also added with this patch.
Signed-off-by: Alexander Amelkin <alexander@amelkin.msk.ru>
---
.../devicetree/bindings/usb/maxim,max3421-hcd.txt | 19 +++++
drivers/usb/host/max3421-hcd.c | 96 ++++++++++++++++++++--
2 files changed, 110 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
new file mode 100644
index 0000000..a8b9faa
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
@@ -0,0 +1,19 @@
+* SPI-based USB host controller Maxim Integrated max3421e
+
+Required properties:
+- compatible: must be "maxim,max3421"
+- reg: chip select number to which the max3421 chip is connected
+ (depends on master SPI controller)
+- spi-max-frequency: the operational frequency, must not exceed <26000000>
+- interrupt-parent: phandle of the associated GPIO controller to which the INT line
+ of max3421e chip is connected
+- interrupts: specification of the GPIO pin (in terms of the `interrupt-parent`)
+ to which INT line of max3421e chip is connected.
+ The driver configures MAX3421E for active low level triggered interrupts.
+ Configure your 'interrupt-parent' gpio controller accordingly.
+- vbus: <GPOUTx ACTIVE_LEVEL>
+ GPOUTx is the number (1-8) of GPOUT pin of max3421e used to drive Vbus.
+ ACTIVE_LEVEL is 1 or 0.
+
+Don't forget to add pinctrl properties if you need some GPIO pins reconfigured
+for use as INT. See binding information for the pinctrl nodes.
diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index 369869a..f600052 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -61,6 +61,12 @@
#include <linux/usb.h>
#include <linux/usb/hcd.h>
+#if defined(CONFIG_OF)
+#include <linux/of_device.h>
+
+#define MAX3421_GPOUT_COUNT 8
+#endif
+
#include <linux/platform_data/max3421-hcd.h>
#define DRIVER_DESC "MAX3421 USB Host-Controller Driver"
@@ -1699,6 +1705,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,20 +1841,80 @@ static struct hc_driver max3421_hcd_desc = {
.bus_resume = max3421_bus_resume,
};
+#if defined(CONFIG_OF)
+static struct max3421_hcd_platform_data max3421_data;
+
+static const struct of_device_id max3421_dt_ids[] = {
+ { .compatible = "maxim,max3421", .data = &max3421_data, },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max3421_dt_ids);
+#endif
+
static int
max3421_probe(struct spi_device *spi)
{
struct max3421_hcd *max3421_hcd;
struct usb_hcd *hcd = NULL;
int retval = -ENOMEM;
+#if defined(CONFIG_OF)
+ struct max3421_hcd_platform_data *pdata = NULL;
+#endif
if (spi_setup(spi) < 0) {
dev_err(&spi->dev, "Unable to setup SPI bus");
return -EFAULT;
}
- hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
- dev_name(&spi->dev));
+ if (!spi->irq) {
+ dev_err(&spi->dev, "Failed to get SPI IRQ for node '%s'\n", spi->dev.of_node->full_name);
+ return -EFAULT;
+ }
+
+#if defined(CONFIG_OF)
+ if (spi->dev.of_node) {
+ /* A temporary alias structure */
+ union {
+ uint32_t value[2];
+ struct {
+ uint32_t gpout;
+ uint32_t active_level;
+ };
+ } vbus;
+
+ if(!(pdata = devm_kzalloc(&spi->dev, sizeof(*pdata), GFP_KERNEL))) {
+ dev_err(&spi->dev, "failed to allocate memory for private data\n");
+ retval = -ENOMEM;
+ goto error;
+ }
+ spi->dev.platform_data = pdata;
+
+ if ((retval = of_property_read_u32_array(spi->dev.of_node, "vbus", vbus.value, 2))) {
+ dev_err(&spi->dev, "device tree node property 'vbus' is missing\n");
+ goto error;
+ }
+ pdata->vbus_gpout = vbus.gpout;
+ pdata->vbus_active_level = vbus.active_level;
+ }
+ else
+#endif
+ 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) {
dev_err(&spi->dev, "failed to create HCD structure\n");
goto error;
@@ -1892,6 +1962,12 @@ max3421_probe(struct spi_device *spi)
kthread_stop(max3421_hcd->spi_thread);
usb_put_hcd(hcd);
}
+#if defined(CONFIG_OF)
+ if (pdata && spi->dev.platform_data == pdata) {
+ devm_kfree(&spi->dev, pdata);
+ spi->dev.platform_data = NULL;
+ }
+#endif
return retval;
}
@@ -1908,14 +1984,12 @@ max3421_remove(struct spi_device *spi)
if (hcd->self.controller == &spi->dev)
break;
}
+
if (!max3421_hcd) {
dev_err(&spi->dev, "no MAX3421 HCD found for SPI device %p\n",
spi);
return -ENODEV;
}
-
- usb_remove_hcd(hcd);
-
spin_lock_irqsave(&max3421_hcd->lock, flags);
kthread_stop(max3421_hcd->spi_thread);
@@ -1923,8 +1997,19 @@ max3421_remove(struct spi_device *spi)
spin_unlock_irqrestore(&max3421_hcd->lock, flags);
+#if defined(CONFIG_OF)
+ if (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;
+ }
+#endif
+
free_irq(spi->irq, hcd);
+ usb_remove_hcd(hcd);
+
+
usb_put_hcd(hcd);
return 0;
}
@@ -1934,6 +2019,7 @@ static struct spi_driver max3421_driver = {
.remove = max3421_remove,
.driver = {
.name = "max3421-hcd",
+ .of_match_table = of_match_ptr(max3421_dt_ids),
},
};
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/3] usb: max3421-hcd: Add devicetree support to the driver
@ 2017-05-31 19:03 ` Rob Herring
0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2017-05-31 19:03 UTC (permalink / raw)
To: Alexander Amelkin
Cc: linux-usb, devicetree, linux-kernel, Greg Kroah-Hartman, Mark Rutland
On Fri, May 26, 2017 at 02:28:24PM +0300, Alexander Amelkin wrote:
> NOTE:
> Please don't use the plain text here as a patch because it most probably is
> corrupted by my webmail client.
> Attached is a copy of the following text guaranteed to have correct
> tabs/spaces.
> -------------------------
Huh?
>
> Before this patch the max3421-hcd driver could only use
> statically linked platform data to initialize its parameters.
> This prevented it from being used on systems using device
> tree.
>
> The data taken from the device tree is put into dev->platform_data
> when CONFIG_OF is enabled and the device is an OpenFirmware node.
>
> The driver's 'compatible' string is 'maxim,max3421'
>
> Binding documentation is also added with this patch.
Please split binding to a separate patch.
>
> Signed-off-by: Alexander Amelkin <alexander@amelkin.msk.ru>
> ---
> .../devicetree/bindings/usb/maxim,max3421-hcd.txt | 19 +++++
Drop the "-hcd"
> drivers/usb/host/max3421-hcd.c | 96
> ++++++++++++++++++++--
> 2 files changed, 110 insertions(+), 5 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
> b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
> new file mode 100644
> index 0000000..a8b9faa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
> @@ -0,0 +1,19 @@
> +* SPI-based USB host controller Maxim Integrated max3421e
> +
> +Required properties:
> +- compatible: must be "maxim,max3421"
> +- reg: chip select number to which the max3421 chip is connected
> + (depends on master SPI controller)
> +- spi-max-frequency: the operational frequency, must not exceed <26000000>
> +- interrupt-parent: phandle of the associated GPIO controller to which the
> INT line
Some line wrapping problems...
While typically an interrupt to a board level device is a GPIO, that's
outside the scope of this binding. For this binding, it is just an
interrupt line.
> + of max3421e chip is connected
> +- interrupts: specification of the GPIO pin (in terms of the
> `interrupt-parent`)
> + to which INT line of max3421e chip is connected.
> + The driver configures MAX3421E for active low level triggered interrupts.
> + Configure your 'interrupt-parent' gpio controller accordingly.
This is wrong. The flags cell tells how to configure the interrupt.
> +- vbus: <GPOUTx ACTIVE_LEVEL>
> + GPOUTx is the number (1-8) of GPOUT pin of max3421e used to drive Vbus.
> + ACTIVE_LEVEL is 1 or 0.
Just "vbus" could be a lot of things. Perhaps "maxim,vbus-en-pin".
> +
> +Don't forget to add pinctrl properties if you need some GPIO pins
> reconfigured
> +for use as INT. See binding information for the pinctrl nodes.
List the properties as optional.
> diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
> index 369869a..f600052 100644
> --- a/drivers/usb/host/max3421-hcd.c
> +++ b/drivers/usb/host/max3421-hcd.c
> @@ -61,6 +61,12 @@
> #include <linux/usb.h>
> #include <linux/usb/hcd.h>
>
> +#if defined(CONFIG_OF)
> +#include <linux/of_device.h>
Probably should be of.h instead.
> +
> +#define MAX3421_GPOUT_COUNT 8
> +#endif
Don't need an ifdef here.
> +
> #include <linux/platform_data/max3421-hcd.h>
>
> #define DRIVER_DESC "MAX3421 USB Host-Controller Driver"
> @@ -1699,6 +1705,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,20 +1841,80 @@ static struct hc_driver max3421_hcd_desc = {
> .bus_resume = max3421_bus_resume,
> };
>
> +#if defined(CONFIG_OF)
> +static struct max3421_hcd_platform_data max3421_data;
> +
> +static const struct of_device_id max3421_dt_ids[] = {
> + { .compatible = "maxim,max3421", .data = &max3421_data, },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, max3421_dt_ids);
> +#endif
> +
> static int
> max3421_probe(struct spi_device *spi)
> {
> struct max3421_hcd *max3421_hcd;
> struct usb_hcd *hcd = NULL;
> int retval = -ENOMEM;
> +#if defined(CONFIG_OF)
> + struct max3421_hcd_platform_data *pdata = NULL;
> +#endif
>
> if (spi_setup(spi) < 0) {
> dev_err(&spi->dev, "Unable to setup SPI bus");
> return -EFAULT;
> }
>
> - hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
> - dev_name(&spi->dev));
> + if (!spi->irq) {
> + dev_err(&spi->dev, "Failed to get SPI IRQ for node '%s'\n",
> spi->dev.of_node->full_name);
> + return -EFAULT;
> + }
> +
> +#if defined(CONFIG_OF)
> + if (spi->dev.of_node) {
if (IS_ENABLED(CONFIG_OF) && spi->dev.of_node)
> + /* A temporary alias structure */
> + union {
> + uint32_t value[2];
> + struct {
> + uint32_t gpout;
> + uint32_t active_level;
> + };
> + } vbus;
> +
> + if(!(pdata = devm_kzalloc(&spi->dev, sizeof(*pdata), GFP_KERNEL))) {
> + dev_err(&spi->dev, "failed to allocate memory for private
> data\n");
> + retval = -ENOMEM;
> + goto error;
> + }
> + spi->dev.platform_data = pdata;
> +
> + if ((retval = of_property_read_u32_array(spi->dev.of_node, "vbus",
> vbus.value, 2))) {
> + dev_err(&spi->dev, "device tree node property 'vbus' is
> missing\n");
> + goto error;
> + }
> + pdata->vbus_gpout = vbus.gpout;
> + pdata->vbus_active_level = vbus.active_level;
> + }
> + else
> +#endif
> + 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) {
> dev_err(&spi->dev, "failed to create HCD structure\n");
> goto error;
> @@ -1892,6 +1962,12 @@ max3421_probe(struct spi_device *spi)
> kthread_stop(max3421_hcd->spi_thread);
> usb_put_hcd(hcd);
> }
> +#if defined(CONFIG_OF)
> + if (pdata && spi->dev.platform_data == pdata) {
IS_ENABLED...
> + devm_kfree(&spi->dev, pdata);
> + spi->dev.platform_data = NULL;
> + }
> +#endif
> return retval;
> }
>
> @@ -1908,14 +1984,12 @@ max3421_remove(struct spi_device *spi)
> if (hcd->self.controller == &spi->dev)
> break;
> }
> +
Spurious change.
> if (!max3421_hcd) {
> dev_err(&spi->dev, "no MAX3421 HCD found for SPI device %p\n",
> spi);
> return -ENODEV;
> }
> -
> - usb_remove_hcd(hcd);
> -
> spin_lock_irqsave(&max3421_hcd->lock, flags);
>
> kthread_stop(max3421_hcd->spi_thread);
> @@ -1923,8 +1997,19 @@ max3421_remove(struct spi_device *spi)
>
> spin_unlock_irqrestore(&max3421_hcd->lock, flags);
>
> +#if defined(CONFIG_OF)
> + if (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;
> + }
> +#endif
> +
> free_irq(spi->irq, hcd);
>
> + usb_remove_hcd(hcd);
> +
> +
One blank line.
> usb_put_hcd(hcd);
> return 0;
> }
> @@ -1934,6 +2019,7 @@ static struct spi_driver max3421_driver = {
> .remove = max3421_remove,
> .driver = {
> .name = "max3421-hcd",
> + .of_match_table = of_match_ptr(max3421_dt_ids),
> },
> };
>
> --
> 2.7.4
> From 0eb4464398c8b0a336aa0305724b4b84ef2be097 Mon Sep 17 00:00:00 2001
> From: Alexander Amelkin <amelkin@fastwel.ru>
> Date: Tue, 28 Mar 2017 20:59:06 +0300
> Subject: [PATCH 1/3] usb: max3421-hcd: Add devicetree support to the driver
>
> Before this patch the max3421-hcd driver could only use
> statically linked platform data to initialize its parameters.
> This prevented it from being used on systems using device
> tree.
>
> The data taken from the device tree is put into dev->platform_data
> when CONFIG_OF is enabled and the device is an OpenFirmware node.
>
> The driver's 'compatible' string is 'maxim,max3421'
>
> Binding documentation is also added with this patch.
>
> Signed-off-by: Alexander Amelkin <alexander@amelkin.msk.ru>
> ---
> .../devicetree/bindings/usb/maxim,max3421-hcd.txt | 19 +++++
> drivers/usb/host/max3421-hcd.c | 96 ++++++++++++++++++++--
> 2 files changed, 110 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
> new file mode 100644
> index 0000000..a8b9faa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
> @@ -0,0 +1,19 @@
> +* SPI-based USB host controller Maxim Integrated max3421e
> +
> +Required properties:
> +- compatible: must be "maxim,max3421"
> +- reg: chip select number to which the max3421 chip is connected
> + (depends on master SPI controller)
> +- spi-max-frequency: the operational frequency, must not exceed <26000000>
> +- interrupt-parent: phandle of the associated GPIO controller to which the INT line
> + of max3421e chip is connected
> +- interrupts: specification of the GPIO pin (in terms of the `interrupt-parent`)
> + to which INT line of max3421e chip is connected.
> + The driver configures MAX3421E for active low level triggered interrupts.
> + Configure your 'interrupt-parent' gpio controller accordingly.
> +- vbus: <GPOUTx ACTIVE_LEVEL>
> + GPOUTx is the number (1-8) of GPOUT pin of max3421e used to drive Vbus.
> + ACTIVE_LEVEL is 1 or 0.
> +
> +Don't forget to add pinctrl properties if you need some GPIO pins reconfigured
> +for use as INT. See binding information for the pinctrl nodes.
> diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
> index 369869a..f600052 100644
> --- a/drivers/usb/host/max3421-hcd.c
> +++ b/drivers/usb/host/max3421-hcd.c
> @@ -61,6 +61,12 @@
> #include <linux/usb.h>
> #include <linux/usb/hcd.h>
>
> +#if defined(CONFIG_OF)
> +#include <linux/of_device.h>
> +
> +#define MAX3421_GPOUT_COUNT 8
> +#endif
> +
> #include <linux/platform_data/max3421-hcd.h>
>
> #define DRIVER_DESC "MAX3421 USB Host-Controller Driver"
> @@ -1699,6 +1705,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,20 +1841,80 @@ static struct hc_driver max3421_hcd_desc = {
> .bus_resume = max3421_bus_resume,
> };
>
> +#if defined(CONFIG_OF)
> +static struct max3421_hcd_platform_data max3421_data;
> +
> +static const struct of_device_id max3421_dt_ids[] = {
> + { .compatible = "maxim,max3421", .data = &max3421_data, },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, max3421_dt_ids);
> +#endif
> +
> static int
> max3421_probe(struct spi_device *spi)
> {
> struct max3421_hcd *max3421_hcd;
> struct usb_hcd *hcd = NULL;
> int retval = -ENOMEM;
> +#if defined(CONFIG_OF)
> + struct max3421_hcd_platform_data *pdata = NULL;
> +#endif
>
> if (spi_setup(spi) < 0) {
> dev_err(&spi->dev, "Unable to setup SPI bus");
> return -EFAULT;
> }
>
> - hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
> - dev_name(&spi->dev));
> + if (!spi->irq) {
> + dev_err(&spi->dev, "Failed to get SPI IRQ for node '%s'\n", spi->dev.of_node->full_name);
> + return -EFAULT;
> + }
> +
> +#if defined(CONFIG_OF)
> + if (spi->dev.of_node) {
> + /* A temporary alias structure */
> + union {
> + uint32_t value[2];
> + struct {
> + uint32_t gpout;
> + uint32_t active_level;
> + };
> + } vbus;
> +
> + if(!(pdata = devm_kzalloc(&spi->dev, sizeof(*pdata), GFP_KERNEL))) {
> + dev_err(&spi->dev, "failed to allocate memory for private data\n");
> + retval = -ENOMEM;
> + goto error;
> + }
> + spi->dev.platform_data = pdata;
> +
> + if ((retval = of_property_read_u32_array(spi->dev.of_node, "vbus", vbus.value, 2))) {
> + dev_err(&spi->dev, "device tree node property 'vbus' is missing\n");
> + goto error;
> + }
> + pdata->vbus_gpout = vbus.gpout;
> + pdata->vbus_active_level = vbus.active_level;
> + }
> + else
> +#endif
> + 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) {
> dev_err(&spi->dev, "failed to create HCD structure\n");
> goto error;
> @@ -1892,6 +1962,12 @@ max3421_probe(struct spi_device *spi)
> kthread_stop(max3421_hcd->spi_thread);
> usb_put_hcd(hcd);
> }
> +#if defined(CONFIG_OF)
> + if (pdata && spi->dev.platform_data == pdata) {
> + devm_kfree(&spi->dev, pdata);
> + spi->dev.platform_data = NULL;
> + }
> +#endif
> return retval;
> }
>
> @@ -1908,14 +1984,12 @@ max3421_remove(struct spi_device *spi)
> if (hcd->self.controller == &spi->dev)
> break;
> }
> +
> if (!max3421_hcd) {
> dev_err(&spi->dev, "no MAX3421 HCD found for SPI device %p\n",
> spi);
> return -ENODEV;
> }
> -
> - usb_remove_hcd(hcd);
> -
> spin_lock_irqsave(&max3421_hcd->lock, flags);
>
> kthread_stop(max3421_hcd->spi_thread);
> @@ -1923,8 +1997,19 @@ max3421_remove(struct spi_device *spi)
>
> spin_unlock_irqrestore(&max3421_hcd->lock, flags);
>
> +#if defined(CONFIG_OF)
> + if (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;
> + }
> +#endif
> +
> free_irq(spi->irq, hcd);
>
> + usb_remove_hcd(hcd);
> +
> +
> usb_put_hcd(hcd);
> return 0;
> }
> @@ -1934,6 +2019,7 @@ static struct spi_driver max3421_driver = {
> .remove = max3421_remove,
> .driver = {
> .name = "max3421-hcd",
> + .of_match_table = of_match_ptr(max3421_dt_ids),
> },
> };
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/3] usb: max3421-hcd: Add devicetree support to the driver
@ 2017-05-31 19:03 ` Rob Herring
0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2017-05-31 19:03 UTC (permalink / raw)
To: Alexander Amelkin
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
Mark Rutland
On Fri, May 26, 2017 at 02:28:24PM +0300, Alexander Amelkin wrote:
> NOTE:
> Please don't use the plain text here as a patch because it most probably is
> corrupted by my webmail client.
> Attached is a copy of the following text guaranteed to have correct
> tabs/spaces.
> -------------------------
Huh?
>
> Before this patch the max3421-hcd driver could only use
> statically linked platform data to initialize its parameters.
> This prevented it from being used on systems using device
> tree.
>
> The data taken from the device tree is put into dev->platform_data
> when CONFIG_OF is enabled and the device is an OpenFirmware node.
>
> The driver's 'compatible' string is 'maxim,max3421'
>
> Binding documentation is also added with this patch.
Please split binding to a separate patch.
>
> Signed-off-by: Alexander Amelkin <alexander-RgVAGUil9pwRDxlzMovjVg@public.gmane.org>
> ---
> .../devicetree/bindings/usb/maxim,max3421-hcd.txt | 19 +++++
Drop the "-hcd"
> drivers/usb/host/max3421-hcd.c | 96
> ++++++++++++++++++++--
> 2 files changed, 110 insertions(+), 5 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
> b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
> new file mode 100644
> index 0000000..a8b9faa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
> @@ -0,0 +1,19 @@
> +* SPI-based USB host controller Maxim Integrated max3421e
> +
> +Required properties:
> +- compatible: must be "maxim,max3421"
> +- reg: chip select number to which the max3421 chip is connected
> + (depends on master SPI controller)
> +- spi-max-frequency: the operational frequency, must not exceed <26000000>
> +- interrupt-parent: phandle of the associated GPIO controller to which the
> INT line
Some line wrapping problems...
While typically an interrupt to a board level device is a GPIO, that's
outside the scope of this binding. For this binding, it is just an
interrupt line.
> + of max3421e chip is connected
> +- interrupts: specification of the GPIO pin (in terms of the
> `interrupt-parent`)
> + to which INT line of max3421e chip is connected.
> + The driver configures MAX3421E for active low level triggered interrupts.
> + Configure your 'interrupt-parent' gpio controller accordingly.
This is wrong. The flags cell tells how to configure the interrupt.
> +- vbus: <GPOUTx ACTIVE_LEVEL>
> + GPOUTx is the number (1-8) of GPOUT pin of max3421e used to drive Vbus.
> + ACTIVE_LEVEL is 1 or 0.
Just "vbus" could be a lot of things. Perhaps "maxim,vbus-en-pin".
> +
> +Don't forget to add pinctrl properties if you need some GPIO pins
> reconfigured
> +for use as INT. See binding information for the pinctrl nodes.
List the properties as optional.
> diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
> index 369869a..f600052 100644
> --- a/drivers/usb/host/max3421-hcd.c
> +++ b/drivers/usb/host/max3421-hcd.c
> @@ -61,6 +61,12 @@
> #include <linux/usb.h>
> #include <linux/usb/hcd.h>
>
> +#if defined(CONFIG_OF)
> +#include <linux/of_device.h>
Probably should be of.h instead.
> +
> +#define MAX3421_GPOUT_COUNT 8
> +#endif
Don't need an ifdef here.
> +
> #include <linux/platform_data/max3421-hcd.h>
>
> #define DRIVER_DESC "MAX3421 USB Host-Controller Driver"
> @@ -1699,6 +1705,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,20 +1841,80 @@ static struct hc_driver max3421_hcd_desc = {
> .bus_resume = max3421_bus_resume,
> };
>
> +#if defined(CONFIG_OF)
> +static struct max3421_hcd_platform_data max3421_data;
> +
> +static const struct of_device_id max3421_dt_ids[] = {
> + { .compatible = "maxim,max3421", .data = &max3421_data, },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, max3421_dt_ids);
> +#endif
> +
> static int
> max3421_probe(struct spi_device *spi)
> {
> struct max3421_hcd *max3421_hcd;
> struct usb_hcd *hcd = NULL;
> int retval = -ENOMEM;
> +#if defined(CONFIG_OF)
> + struct max3421_hcd_platform_data *pdata = NULL;
> +#endif
>
> if (spi_setup(spi) < 0) {
> dev_err(&spi->dev, "Unable to setup SPI bus");
> return -EFAULT;
> }
>
> - hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
> - dev_name(&spi->dev));
> + if (!spi->irq) {
> + dev_err(&spi->dev, "Failed to get SPI IRQ for node '%s'\n",
> spi->dev.of_node->full_name);
> + return -EFAULT;
> + }
> +
> +#if defined(CONFIG_OF)
> + if (spi->dev.of_node) {
if (IS_ENABLED(CONFIG_OF) && spi->dev.of_node)
> + /* A temporary alias structure */
> + union {
> + uint32_t value[2];
> + struct {
> + uint32_t gpout;
> + uint32_t active_level;
> + };
> + } vbus;
> +
> + if(!(pdata = devm_kzalloc(&spi->dev, sizeof(*pdata), GFP_KERNEL))) {
> + dev_err(&spi->dev, "failed to allocate memory for private
> data\n");
> + retval = -ENOMEM;
> + goto error;
> + }
> + spi->dev.platform_data = pdata;
> +
> + if ((retval = of_property_read_u32_array(spi->dev.of_node, "vbus",
> vbus.value, 2))) {
> + dev_err(&spi->dev, "device tree node property 'vbus' is
> missing\n");
> + goto error;
> + }
> + pdata->vbus_gpout = vbus.gpout;
> + pdata->vbus_active_level = vbus.active_level;
> + }
> + else
> +#endif
> + 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) {
> dev_err(&spi->dev, "failed to create HCD structure\n");
> goto error;
> @@ -1892,6 +1962,12 @@ max3421_probe(struct spi_device *spi)
> kthread_stop(max3421_hcd->spi_thread);
> usb_put_hcd(hcd);
> }
> +#if defined(CONFIG_OF)
> + if (pdata && spi->dev.platform_data == pdata) {
IS_ENABLED...
> + devm_kfree(&spi->dev, pdata);
> + spi->dev.platform_data = NULL;
> + }
> +#endif
> return retval;
> }
>
> @@ -1908,14 +1984,12 @@ max3421_remove(struct spi_device *spi)
> if (hcd->self.controller == &spi->dev)
> break;
> }
> +
Spurious change.
> if (!max3421_hcd) {
> dev_err(&spi->dev, "no MAX3421 HCD found for SPI device %p\n",
> spi);
> return -ENODEV;
> }
> -
> - usb_remove_hcd(hcd);
> -
> spin_lock_irqsave(&max3421_hcd->lock, flags);
>
> kthread_stop(max3421_hcd->spi_thread);
> @@ -1923,8 +1997,19 @@ max3421_remove(struct spi_device *spi)
>
> spin_unlock_irqrestore(&max3421_hcd->lock, flags);
>
> +#if defined(CONFIG_OF)
> + if (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;
> + }
> +#endif
> +
> free_irq(spi->irq, hcd);
>
> + usb_remove_hcd(hcd);
> +
> +
One blank line.
> usb_put_hcd(hcd);
> return 0;
> }
> @@ -1934,6 +2019,7 @@ static struct spi_driver max3421_driver = {
> .remove = max3421_remove,
> .driver = {
> .name = "max3421-hcd",
> + .of_match_table = of_match_ptr(max3421_dt_ids),
> },
> };
>
> --
> 2.7.4
> From 0eb4464398c8b0a336aa0305724b4b84ef2be097 Mon Sep 17 00:00:00 2001
> From: Alexander Amelkin <amelkin-0pd1rHjVOVEvJsYlp49lxw@public.gmane.org>
> Date: Tue, 28 Mar 2017 20:59:06 +0300
> Subject: [PATCH 1/3] usb: max3421-hcd: Add devicetree support to the driver
>
> Before this patch the max3421-hcd driver could only use
> statically linked platform data to initialize its parameters.
> This prevented it from being used on systems using device
> tree.
>
> The data taken from the device tree is put into dev->platform_data
> when CONFIG_OF is enabled and the device is an OpenFirmware node.
>
> The driver's 'compatible' string is 'maxim,max3421'
>
> Binding documentation is also added with this patch.
>
> Signed-off-by: Alexander Amelkin <alexander-RgVAGUil9pwRDxlzMovjVg@public.gmane.org>
> ---
> .../devicetree/bindings/usb/maxim,max3421-hcd.txt | 19 +++++
> drivers/usb/host/max3421-hcd.c | 96 ++++++++++++++++++++--
> 2 files changed, 110 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
> new file mode 100644
> index 0000000..a8b9faa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/maxim,max3421-hcd.txt
> @@ -0,0 +1,19 @@
> +* SPI-based USB host controller Maxim Integrated max3421e
> +
> +Required properties:
> +- compatible: must be "maxim,max3421"
> +- reg: chip select number to which the max3421 chip is connected
> + (depends on master SPI controller)
> +- spi-max-frequency: the operational frequency, must not exceed <26000000>
> +- interrupt-parent: phandle of the associated GPIO controller to which the INT line
> + of max3421e chip is connected
> +- interrupts: specification of the GPIO pin (in terms of the `interrupt-parent`)
> + to which INT line of max3421e chip is connected.
> + The driver configures MAX3421E for active low level triggered interrupts.
> + Configure your 'interrupt-parent' gpio controller accordingly.
> +- vbus: <GPOUTx ACTIVE_LEVEL>
> + GPOUTx is the number (1-8) of GPOUT pin of max3421e used to drive Vbus.
> + ACTIVE_LEVEL is 1 or 0.
> +
> +Don't forget to add pinctrl properties if you need some GPIO pins reconfigured
> +for use as INT. See binding information for the pinctrl nodes.
> diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
> index 369869a..f600052 100644
> --- a/drivers/usb/host/max3421-hcd.c
> +++ b/drivers/usb/host/max3421-hcd.c
> @@ -61,6 +61,12 @@
> #include <linux/usb.h>
> #include <linux/usb/hcd.h>
>
> +#if defined(CONFIG_OF)
> +#include <linux/of_device.h>
> +
> +#define MAX3421_GPOUT_COUNT 8
> +#endif
> +
> #include <linux/platform_data/max3421-hcd.h>
>
> #define DRIVER_DESC "MAX3421 USB Host-Controller Driver"
> @@ -1699,6 +1705,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,20 +1841,80 @@ static struct hc_driver max3421_hcd_desc = {
> .bus_resume = max3421_bus_resume,
> };
>
> +#if defined(CONFIG_OF)
> +static struct max3421_hcd_platform_data max3421_data;
> +
> +static const struct of_device_id max3421_dt_ids[] = {
> + { .compatible = "maxim,max3421", .data = &max3421_data, },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, max3421_dt_ids);
> +#endif
> +
> static int
> max3421_probe(struct spi_device *spi)
> {
> struct max3421_hcd *max3421_hcd;
> struct usb_hcd *hcd = NULL;
> int retval = -ENOMEM;
> +#if defined(CONFIG_OF)
> + struct max3421_hcd_platform_data *pdata = NULL;
> +#endif
>
> if (spi_setup(spi) < 0) {
> dev_err(&spi->dev, "Unable to setup SPI bus");
> return -EFAULT;
> }
>
> - hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
> - dev_name(&spi->dev));
> + if (!spi->irq) {
> + dev_err(&spi->dev, "Failed to get SPI IRQ for node '%s'\n", spi->dev.of_node->full_name);
> + return -EFAULT;
> + }
> +
> +#if defined(CONFIG_OF)
> + if (spi->dev.of_node) {
> + /* A temporary alias structure */
> + union {
> + uint32_t value[2];
> + struct {
> + uint32_t gpout;
> + uint32_t active_level;
> + };
> + } vbus;
> +
> + if(!(pdata = devm_kzalloc(&spi->dev, sizeof(*pdata), GFP_KERNEL))) {
> + dev_err(&spi->dev, "failed to allocate memory for private data\n");
> + retval = -ENOMEM;
> + goto error;
> + }
> + spi->dev.platform_data = pdata;
> +
> + if ((retval = of_property_read_u32_array(spi->dev.of_node, "vbus", vbus.value, 2))) {
> + dev_err(&spi->dev, "device tree node property 'vbus' is missing\n");
> + goto error;
> + }
> + pdata->vbus_gpout = vbus.gpout;
> + pdata->vbus_active_level = vbus.active_level;
> + }
> + else
> +#endif
> + 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) {
> dev_err(&spi->dev, "failed to create HCD structure\n");
> goto error;
> @@ -1892,6 +1962,12 @@ max3421_probe(struct spi_device *spi)
> kthread_stop(max3421_hcd->spi_thread);
> usb_put_hcd(hcd);
> }
> +#if defined(CONFIG_OF)
> + if (pdata && spi->dev.platform_data == pdata) {
> + devm_kfree(&spi->dev, pdata);
> + spi->dev.platform_data = NULL;
> + }
> +#endif
> return retval;
> }
>
> @@ -1908,14 +1984,12 @@ max3421_remove(struct spi_device *spi)
> if (hcd->self.controller == &spi->dev)
> break;
> }
> +
> if (!max3421_hcd) {
> dev_err(&spi->dev, "no MAX3421 HCD found for SPI device %p\n",
> spi);
> return -ENODEV;
> }
> -
> - usb_remove_hcd(hcd);
> -
> spin_lock_irqsave(&max3421_hcd->lock, flags);
>
> kthread_stop(max3421_hcd->spi_thread);
> @@ -1923,8 +1997,19 @@ max3421_remove(struct spi_device *spi)
>
> spin_unlock_irqrestore(&max3421_hcd->lock, flags);
>
> +#if defined(CONFIG_OF)
> + if (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;
> + }
> +#endif
> +
> free_irq(spi->irq, hcd);
>
> + usb_remove_hcd(hcd);
> +
> +
> usb_put_hcd(hcd);
> return 0;
> }
> @@ -1934,6 +2019,7 @@ static struct spi_driver max3421_driver = {
> .remove = max3421_remove,
> .driver = {
> .name = "max3421-hcd",
> + .of_match_table = of_match_ptr(max3421_dt_ids),
> },
> };
>
> --
> 2.7.4
>
--
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] usb: max3421-hcd: Fix crash on the driver removal
2017-05-26 11:22 ` Alexander Amelkin
(?)
(?)
@ 2017-05-26 11:30 ` Alexander Amelkin
-1 siblings, 0 replies; 11+ messages in thread
From: Alexander Amelkin @ 2017-05-26 11:30 UTC (permalink / raw)
To: linux-usb, devicetree, linux-kernel
Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland
[-- Attachment #1: Type: text/plain, Size: 3584 bytes --]
NOTE:
Please don't use the plain text here as a patch because it most probably
is corrupted by my webmail client.
Attached is a copy of the following text guaranteed to have correct
tabs/spaces.
-------------------------
From 8c4c65d3892df3721474023836216a02e03fb23e Mon Sep 17 00:00:00 2001
From: Alexander Amelkin <amelkin@fastwel.ru>
Date: Fri, 14 Apr 2017 17:58:07 +0300
Subject: [PATCH 2/3] usb: max3421-hcd: Fix crash on the driver removal
The driver was calling kthread_stop inside a spin-locked
region while the thread was calling schedule() on a regular
basis. That resulted in panic due to 'scheduling while atomic'.
There was no need to get the spin-lock before stopping the
thread as thread stopping is asynchronous and the thread
only stops when it detects the stop flag set by kthread_stop(),
at which point it is not accessing any resources anyway.
Hence, this patch removes the spin-locking around the
kthread_stop() call.
Additionally, the allocated resources were not free'd in
the correct order. Some were not free'd at all. This patch
switches all resource allocations to devm_* functions
and also reorders deallocation to properly revert the
allocations (although not actually needed for devm-allocated
resources).
Signed-off-by: Alexander Amelkin <alexander@amelkin.msk.ru>
---
drivers/usb/host/max3421-hcd.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/max3421-hcd.c
b/drivers/usb/host/max3421-hcd.c
index f600052..bd59c16 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -1925,10 +1925,10 @@ max3421_probe(struct spi_device *spi)
max3421_hcd_list = max3421_hcd;
INIT_LIST_HEAD(&max3421_hcd->ep_list);
- max3421_hcd->tx = kmalloc(sizeof(*max3421_hcd->tx), GFP_KERNEL);
+ max3421_hcd->tx = devm_kzalloc(&spi->dev, sizeof(*max3421_hcd->tx),
GFP_KERNEL);
if (!max3421_hcd->tx)
goto error;
- max3421_hcd->rx = kmalloc(sizeof(*max3421_hcd->rx), GFP_KERNEL);
+ max3421_hcd->rx = devm_kzalloc(&spi->dev, sizeof(*max3421_hcd->rx),
GFP_KERNEL);
if (!max3421_hcd->rx)
goto error;
@@ -1946,8 +1946,8 @@ max3421_probe(struct spi_device *spi)
goto error;
}
- retval = request_irq(spi->irq, max3421_irq_handler,
- IRQF_TRIGGER_LOW, "max3421", hcd);
+ retval = devm_request_irq(&spi->dev, spi->irq, max3421_irq_handler,
+ IRQF_TRIGGER_LOW, "max3421", hcd);
if (retval < 0) {
dev_err(&spi->dev, "failed to request irq %d\n", spi->irq);
goto error;
@@ -1976,7 +1976,6 @@ max3421_remove(struct spi_device *spi)
{
struct max3421_hcd *max3421_hcd = NULL, **prev;
struct usb_hcd *hcd = NULL;
- unsigned long flags;
for (prev = &max3421_hcd_list; *prev; prev = &(*prev)->next) {
max3421_hcd = *prev;
@@ -1990,12 +1989,20 @@ max3421_remove(struct spi_device *spi)
spi);
return -ENODEV;
}
- spin_lock_irqsave(&max3421_hcd->lock, flags);
+
+ devm_free_irq(&spi->dev, spi->irq, hcd);
+
+ usb_remove_hcd(hcd);
kthread_stop(max3421_hcd->spi_thread);
- *prev = max3421_hcd->next;
- spin_unlock_irqrestore(&max3421_hcd->lock, flags);
+ devm_kfree(&spi->dev, max3421_hcd->rx);
+ max3421_hcd->rx = NULL;
+ devm_kfree(&spi->dev, max3421_hcd->tx);
+ max3421_hcd->tx = NULL;
+
+ *prev = max3421_hcd->next;
+ usb_put_hcd(hcd);
#if defined(CONFIG_OF)
if (spi->dev.platform_data) {
@@ -2005,12 +2012,6 @@ max3421_remove(struct spi_device *spi)
}
#endif
- free_irq(spi->irq, hcd);
-
- usb_remove_hcd(hcd);
-
-
- usb_put_hcd(hcd);
return 0;
}
--
2.7.4
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-usb-max3421-hcd-Fix-crash-on-the-driver-removal.patch --]
[-- Type: text/x-diff; name=0002-usb-max3421-hcd-Fix-crash-on-the-driver-removal.patch, Size: 3337 bytes --]
From 8c4c65d3892df3721474023836216a02e03fb23e Mon Sep 17 00:00:00 2001
From: Alexander Amelkin <amelkin@fastwel.ru>
Date: Fri, 14 Apr 2017 17:58:07 +0300
Subject: [PATCH 2/3] usb: max3421-hcd: Fix crash on the driver removal
The driver was calling kthread_stop inside a spin-locked
region while the thread was calling schedule() on a regular
basis. That resulted in panic due to 'scheduling while atomic'.
There was no need to get the spin-lock before stopping the
thread as thread stopping is asynchronous and the thread
only stops when it detects the stop flag set by kthread_stop(),
at which point it is not accessing any resources anyway.
Hence, this patch removes the spin-locking around the
kthread_stop() call.
Additionally, the allocated resources were not free'd in
the correct order. Some were not free'd at all. This patch
switches all resource allocations to devm_* functions
and also reorders deallocation to properly revert the
allocations (although not actually needed for devm-allocated
resources).
Signed-off-by: Alexander Amelkin <alexander@amelkin.msk.ru>
---
drivers/usb/host/max3421-hcd.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index f600052..bd59c16 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -1925,10 +1925,10 @@ max3421_probe(struct spi_device *spi)
max3421_hcd_list = max3421_hcd;
INIT_LIST_HEAD(&max3421_hcd->ep_list);
- max3421_hcd->tx = kmalloc(sizeof(*max3421_hcd->tx), GFP_KERNEL);
+ max3421_hcd->tx = devm_kzalloc(&spi->dev, sizeof(*max3421_hcd->tx), GFP_KERNEL);
if (!max3421_hcd->tx)
goto error;
- max3421_hcd->rx = kmalloc(sizeof(*max3421_hcd->rx), GFP_KERNEL);
+ max3421_hcd->rx = devm_kzalloc(&spi->dev, sizeof(*max3421_hcd->rx), GFP_KERNEL);
if (!max3421_hcd->rx)
goto error;
@@ -1946,8 +1946,8 @@ max3421_probe(struct spi_device *spi)
goto error;
}
- retval = request_irq(spi->irq, max3421_irq_handler,
- IRQF_TRIGGER_LOW, "max3421", hcd);
+ retval = devm_request_irq(&spi->dev, spi->irq, max3421_irq_handler,
+ IRQF_TRIGGER_LOW, "max3421", hcd);
if (retval < 0) {
dev_err(&spi->dev, "failed to request irq %d\n", spi->irq);
goto error;
@@ -1976,7 +1976,6 @@ max3421_remove(struct spi_device *spi)
{
struct max3421_hcd *max3421_hcd = NULL, **prev;
struct usb_hcd *hcd = NULL;
- unsigned long flags;
for (prev = &max3421_hcd_list; *prev; prev = &(*prev)->next) {
max3421_hcd = *prev;
@@ -1990,12 +1989,20 @@ max3421_remove(struct spi_device *spi)
spi);
return -ENODEV;
}
- spin_lock_irqsave(&max3421_hcd->lock, flags);
+
+ devm_free_irq(&spi->dev, spi->irq, hcd);
+
+ usb_remove_hcd(hcd);
kthread_stop(max3421_hcd->spi_thread);
- *prev = max3421_hcd->next;
- spin_unlock_irqrestore(&max3421_hcd->lock, flags);
+ devm_kfree(&spi->dev, max3421_hcd->rx);
+ max3421_hcd->rx = NULL;
+ devm_kfree(&spi->dev, max3421_hcd->tx);
+ max3421_hcd->tx = NULL;
+
+ *prev = max3421_hcd->next;
+ usb_put_hcd(hcd);
#if defined(CONFIG_OF)
if (spi->dev.platform_data) {
@@ -2005,12 +2012,6 @@ max3421_remove(struct spi_device *spi)
}
#endif
- free_irq(spi->irq, hcd);
-
- usb_remove_hcd(hcd);
-
-
- usb_put_hcd(hcd);
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/3] usb: max3421-hcd: Remove function name from dev_dbg calls
2017-05-26 11:22 ` Alexander Amelkin
` (2 preceding siblings ...)
(?)
@ 2017-05-26 11:32 ` Alexander Amelkin
2017-05-26 18:41 ` Greg Kroah-Hartman
-1 siblings, 1 reply; 11+ messages in thread
From: Alexander Amelkin @ 2017-05-26 11:32 UTC (permalink / raw)
To: linux-usb, devicetree, linux-kernel
Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland
[-- Attachment #1: Type: text/plain, Size: 2865 bytes --]
NOTE:
Please don't use the plain text here as a patch because it most probably
is corrupted by my webmail client.
Attached is a copy of the following text guaranteed to have correct
tabs/spaces.
-------------------------
From b9a7559d24c0b2cb6e69124d861a943f79272681 Mon Sep 17 00:00:00 2001
From: Alexander Amelkin <amelkin@fastwel.ru>
Date: Fri, 14 Apr 2017 18:01:58 +0300
Subject: [PATCH 3/3] usb: max3421-hcd: Remove function name from dev_dbg
calls
The kernel dynamic debugging facility already has a
means for displaying the function name if the developer
wants to (the 'f' flag).
There is no need to hard-code output of the function
name into dev_dbg calls.
Signed-off-by: Alexander Amelkin <alexander@amelkin.msk.ru>
---
drivers/usb/host/max3421-hcd.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/max3421-hcd.c
b/drivers/usb/host/max3421-hcd.c
index bd59c16..cfca8a2 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -706,8 +706,8 @@ max3421_select_and_start_urb(struct usb_hcd *hcd)
urb = list_first_entry(&ep->urb_list, struct urb,
urb_list);
if (urb->unlinked) {
- dev_dbg(&spi->dev, "%s: URB %p unlinked=%d",
- __func__, urb, urb->unlinked);
+ dev_dbg(&spi->dev, "URB %p unlinked=%d",
+ urb, urb->unlinked);
max3421_hcd->curr_urb = urb;
max3421_hcd->urb_done = 1;
spin_unlock_irqrestore(&max3421_hcd->lock,
@@ -815,8 +815,8 @@ max3421_check_unlink(struct usb_hcd *hcd)
list_for_each_entry_safe(urb, next, &ep->urb_list, urb_list) {
if (urb->unlinked) {
retval = 1;
- dev_dbg(&spi->dev, "%s: URB %p unlinked=%d",
- __func__, urb, urb->unlinked);
+ dev_dbg(&spi->dev, "URB %p unlinked=%d",
+ urb, urb->unlinked);
usb_hcd_unlink_urb_from_ep(hcd, urb);
spin_unlock_irqrestore(&max3421_hcd->lock,
flags);
@@ -912,8 +912,8 @@ max3421_handle_error(struct usb_hcd *hcd, u8 hrsl)
* from; report error
*/
max3421_hcd->urb_done = hrsl_to_error[result_code];
- dev_dbg(&spi->dev, "%s: unexpected error HRSL=0x%02x",
- __func__, hrsl);
+ dev_dbg(&spi->dev, "unexpected error HRSL=0x%02x",
+ hrsl);
break;
case MAX3421_HRSL_TOGERR:
@@ -940,14 +940,12 @@ max3421_handle_error(struct usb_hcd *hcd, u8 hrsl)
else {
/* Based on ohci.h cc_to_err[]: */
max3421_hcd->urb_done = hrsl_to_error[result_code];
- dev_dbg(&spi->dev, "%s: unexpected error HRSL=0x%02x",
- __func__, hrsl);
+ dev_dbg(&spi->dev, "unexpected error HRSL=0x%02x", hrsl);
}
break;
case MAX3421_HRSL_STALL:
- dev_dbg(&spi->dev, "%s: unexpected error HRSL=0x%02x",
- __func__, hrsl);
+ dev_dbg(&spi->dev, "unexpected error HRSL=0x%02x", hrsl);
max3421_hcd->urb_done = hrsl_to_error[result_code];
break;
--
2.7.4
[-- Attachment #2: 0003-usb-max3421-hcd-Remove-function-name-from-dev_dbg-ca.patch --]
[-- Type: text/x-diff, Size: 2614 bytes --]
From b9a7559d24c0b2cb6e69124d861a943f79272681 Mon Sep 17 00:00:00 2001
From: Alexander Amelkin <amelkin@fastwel.ru>
Date: Fri, 14 Apr 2017 18:01:58 +0300
Subject: [PATCH 3/3] usb: max3421-hcd: Remove function name from dev_dbg calls
The kernel dynamic debugging facility already has a
means for displaying the function name if the developer
wants to (the 'f' flag).
There is no need to hard-code output of the function
name into dev_dbg calls.
Signed-off-by: Alexander Amelkin <alexander@amelkin.msk.ru>
---
drivers/usb/host/max3421-hcd.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index bd59c16..cfca8a2 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -706,8 +706,8 @@ max3421_select_and_start_urb(struct usb_hcd *hcd)
urb = list_first_entry(&ep->urb_list, struct urb,
urb_list);
if (urb->unlinked) {
- dev_dbg(&spi->dev, "%s: URB %p unlinked=%d",
- __func__, urb, urb->unlinked);
+ dev_dbg(&spi->dev, "URB %p unlinked=%d",
+ urb, urb->unlinked);
max3421_hcd->curr_urb = urb;
max3421_hcd->urb_done = 1;
spin_unlock_irqrestore(&max3421_hcd->lock,
@@ -815,8 +815,8 @@ max3421_check_unlink(struct usb_hcd *hcd)
list_for_each_entry_safe(urb, next, &ep->urb_list, urb_list) {
if (urb->unlinked) {
retval = 1;
- dev_dbg(&spi->dev, "%s: URB %p unlinked=%d",
- __func__, urb, urb->unlinked);
+ dev_dbg(&spi->dev, "URB %p unlinked=%d",
+ urb, urb->unlinked);
usb_hcd_unlink_urb_from_ep(hcd, urb);
spin_unlock_irqrestore(&max3421_hcd->lock,
flags);
@@ -912,8 +912,8 @@ max3421_handle_error(struct usb_hcd *hcd, u8 hrsl)
* from; report error
*/
max3421_hcd->urb_done = hrsl_to_error[result_code];
- dev_dbg(&spi->dev, "%s: unexpected error HRSL=0x%02x",
- __func__, hrsl);
+ dev_dbg(&spi->dev, "unexpected error HRSL=0x%02x",
+ hrsl);
break;
case MAX3421_HRSL_TOGERR:
@@ -940,14 +940,12 @@ max3421_handle_error(struct usb_hcd *hcd, u8 hrsl)
else {
/* Based on ohci.h cc_to_err[]: */
max3421_hcd->urb_done = hrsl_to_error[result_code];
- dev_dbg(&spi->dev, "%s: unexpected error HRSL=0x%02x",
- __func__, hrsl);
+ dev_dbg(&spi->dev, "unexpected error HRSL=0x%02x", hrsl);
}
break;
case MAX3421_HRSL_STALL:
- dev_dbg(&spi->dev, "%s: unexpected error HRSL=0x%02x",
- __func__, hrsl);
+ dev_dbg(&spi->dev, "unexpected error HRSL=0x%02x", hrsl);
max3421_hcd->urb_done = hrsl_to_error[result_code];
break;
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/3] usb: max3421-hcd: Remove function name from dev_dbg calls
@ 2017-05-26 18:41 ` Greg Kroah-Hartman
0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2017-05-26 18:41 UTC (permalink / raw)
To: Alexander Amelkin
Cc: linux-usb, devicetree, linux-kernel, Rob Herring, Mark Rutland
On Fri, May 26, 2017 at 02:32:42PM +0300, Alexander Amelkin wrote:
> NOTE:
> Please don't use the plain text here as a patch because it most probably is
> corrupted by my webmail client.
> Attached is a copy of the following text guaranteed to have correct
> tabs/spaces.
Why is this here????
What did you do to add this odd stuff to the patches? Please just use
git send-email like everyone else does...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] usb: max3421-hcd: Remove function name from dev_dbg calls
@ 2017-05-26 18:41 ` Greg Kroah-Hartman
0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2017-05-26 18:41 UTC (permalink / raw)
To: Alexander Amelkin
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
On Fri, May 26, 2017 at 02:32:42PM +0300, Alexander Amelkin wrote:
> NOTE:
> Please don't use the plain text here as a patch because it most probably is
> corrupted by my webmail client.
> Attached is a copy of the following text guaranteed to have correct
> tabs/spaces.
Why is this here????
What did you do to add this odd stuff to the patches? Please just use
git send-email like everyone else does...
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [GIT][PULL] Improvements to max3421-hcd driver
2017-05-26 11:22 ` Alexander Amelkin
` (3 preceding siblings ...)
(?)
@ 2017-05-26 18:40 ` Greg Kroah-Hartman
-1 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2017-05-26 18:40 UTC (permalink / raw)
To: Alexander Amelkin
Cc: linux-usb, devicetree, linux-kernel, Rob Herring, Mark Rutland
On Fri, May 26, 2017 at 02:22:47PM +0300, Alexander Amelkin wrote:
> The following changes since commit 08332893e37af6ae779367e78e444f8f9571511d:
>
> Linux 4.12-rc2 (2017-05-21 19:30:23 -0700)
>
> are available in the git repository at:
>
> https://github.com/AlexanderAmelkin/linux-wandboard.git
> tags/max3421-improvements-1
I don't take random USB pull requests, and never from github, sorry.
Just send patches through email, like all other developers do.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread