* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-25 9:44 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-25 9:44 UTC (permalink / raw)
To: netdev
Cc: devicetree-discuss, linux-arm-kernel, patches, Shawn Guo,
Grant Likely, Steve Glendinning, David S. Miller
It adds device tree probe support for smsc911x driver.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Steve Glendinning <steve.glendinning@smsc.com>
Cc: David S. Miller <davem@davemloft.net>
---
Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
2 files changed, 132 insertions(+), 25 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
new file mode 100644
index 0000000..1920695
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc.txt
@@ -0,0 +1,34 @@
+* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
+
+Required properties:
+- compatible : Should be "smsc,lan<model>""smsc,lan"
+- reg : Address and length of the io space for SMSC LAN
+- smsc-int-gpios : Should specify the GPIO for SMSC LAN interrupt line
+- phy-mode : String, operation mode of the PHY interface.
+ Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
+ "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
+
+Optional properties:
+- smsc,irq-active-high : Indicates the IRQ polarity is active-low
+- smsc,irq-push-pull : Indicates the IRQ type is push-pull
+- smsc,register-needs-shift : Indicates the register access needs shift
+- smsc,access-in-32bit : Indicates the access to controller is in 32-bit
+ mode
+- smsc,force-internal-phy : Forces SMSC LAN controller to use
+ internal PHY
+- smsc,force-external-phy : Forces SMSC LAN controller to use
+ external PHY
+- smsc,save-mac-address : Indicates that mac address needs to be saved
+ before resetting the controller
+- local-mac-address : 6 bytes, mac address
+
+Examples:
+
+lan9220@f4000000 {
+ compatible = "smsc,lan9220", "smsc,lan";
+ reg = <0xf4000000 0x2000000>;
+ phy-mode = "mii";
+ smsc-int-gpios = <&gpio1 31 0>; /* GPIO2_31 */
+ smsc,irq-push-pull;
+ smsc,access-in-32bit;
+};
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index b9016a3..0097048 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -53,6 +53,10 @@
#include <linux/phy.h>
#include <linux/smsc911x.h>
#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/of_net.h>
#include "smsc911x.h"
#define SMSC_CHIPNAME "smsc911x"
@@ -2095,25 +2099,67 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
.tx_writefifo = smsc911x_tx_writefifo_shift,
};
+#ifdef CONFIG_OF
+static int __devinit smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ const char *mac;
+
+ if (!np)
+ return -ENODEV;
+
+ config->phy_interface = of_get_phy_mode(np);
+
+ mac = of_get_mac_address(np);
+ if (mac)
+ memcpy(config->mac, mac, ETH_ALEN);
+
+ if (of_get_property(np, "smsc,irq-active-high", NULL))
+ config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
+
+ if (of_get_property(np, "smsc,irq-push-pull", NULL))
+ config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
+
+ if (of_get_property(np, "smsc,register-needs-shift", NULL))
+ config->shift = 1;
+
+ if (of_get_property(np, "smsc,access-in-32bit", NULL))
+ config->flags |= SMSC911X_USE_32BIT;
+
+ if (of_get_property(np, "smsc,force-internal-phy", NULL))
+ config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,force-external-phy", NULL))
+ config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,save-mac-address", NULL))
+ config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
+
+ return 0;
+}
+#else
+static inline int smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ return -ENODEV;
+}
+#endif /* CONFIG_OF */
+
static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
{
+ struct device_node *np = pdev->dev.of_node;
struct net_device *dev;
struct smsc911x_data *pdata;
struct smsc911x_platform_config *config = pdev->dev.platform_data;
struct resource *res, *irq_res;
unsigned int intcfg = 0;
- int res_size, irq_flags;
- int retval;
+ int irq_gpio, res_size, irq_flags = 0;
+ int retval = 0;
pr_info("Driver version %s\n", SMSC_DRV_VERSION);
- /* platform data specifies irq & dynamic bus configuration */
- if (!pdev->dev.platform_data) {
- pr_warn("platform_data not provided\n");
- retval = -ENODEV;
- goto out_0;
- }
-
res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"smsc911x-memory");
if (!res)
@@ -2125,13 +2171,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
}
res_size = resource_size(res);
- irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!irq_res) {
- pr_warn("Could not allocate irq resource\n");
- retval = -ENODEV;
- goto out_0;
- }
-
if (!request_mem_region(res->start, res_size, SMSC_CHIPNAME)) {
retval = -EBUSY;
goto out_0;
@@ -2148,26 +2187,53 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
pdata = netdev_priv(dev);
- dev->irq = irq_res->start;
- irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
- pdata->ioaddr = ioremap_nocache(res->start, res_size);
-
- /* copy config parameters across to pdata */
- memcpy(&pdata->config, config, sizeof(pdata->config));
+ if (np) {
+ irq_gpio = of_get_named_gpio(np, "smsc-int-gpios", 0);
+ retval = gpio_request_one(irq_gpio, GPIOF_IN, "smsc-int-gpio");
+ if (!retval)
+ dev->irq = gpio_to_irq(irq_gpio);
+ } else {
+ irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (irq_res) {
+ dev->irq = irq_res->start;
+ irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
+ } else {
+ retval = -ENODEV;
+ }
+ }
- pdata->dev = dev;
- pdata->msg_enable = ((1 << debug) - 1);
+ if (retval) {
+ SMSC_WARN(pdata, probe, "Error smsc911x irq not found");
+ retval = -EINVAL;
+ goto out_free_netdev_2;
+ }
+ pdata->ioaddr = ioremap_nocache(res->start, res_size);
if (pdata->ioaddr == NULL) {
SMSC_WARN(pdata, probe, "Error smsc911x base address invalid");
retval = -ENOMEM;
goto out_free_netdev_2;
}
+ pdata->dev = dev;
+ pdata->msg_enable = ((1 << debug) - 1);
+
+ retval = smsc911x_probe_config_dt(&pdata->config, np);
+ if (retval && config) {
+ /* copy config parameters across to pdata */
+ memcpy(&pdata->config, config, sizeof(pdata->config));
+ retval = 0;
+ }
+
+ if (retval) {
+ SMSC_WARN(pdata, probe, "Error smsc911x config not found");
+ goto out_unmap_io_3;
+ }
+
/* assume standard, non-shifted, access to HW registers */
pdata->ops = &standard_smsc911x_ops;
/* apply the right access if shifting is needed */
- if (config->shift)
+ if (pdata->config.shift)
pdata->ops = &shifted_smsc911x_ops;
retval = smsc911x_init(dev);
@@ -2314,6 +2380,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
#define SMSC911X_PM_OPS NULL
#endif
+static const struct of_device_id smsc_dt_ids[] = {
+ { .compatible = "smsc,lan", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, smsc_dt_ids);
+
static struct platform_driver smsc911x_driver = {
.probe = smsc911x_drv_probe,
.remove = __devexit_p(smsc911x_drv_remove),
@@ -2321,6 +2393,7 @@ static struct platform_driver smsc911x_driver = {
.name = SMSC_CHIPNAME,
.owner = THIS_MODULE,
.pm = SMSC911X_PM_OPS,
+ .of_match_table = smsc_dt_ids,
},
};
--
1.7.4.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-25 9:44 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-25 9:44 UTC (permalink / raw)
To: netdev
Cc: devicetree-discuss, linux-arm-kernel, patches, Shawn Guo,
Grant Likely, Steve Glendinning, David S. Miller
It adds device tree probe support for smsc911x driver.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Steve Glendinning <steve.glendinning@smsc.com>
Cc: David S. Miller <davem@davemloft.net>
---
Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
2 files changed, 132 insertions(+), 25 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
new file mode 100644
index 0000000..1920695
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc.txt
@@ -0,0 +1,34 @@
+* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
+
+Required properties:
+- compatible : Should be "smsc,lan<model>""smsc,lan"
+- reg : Address and length of the io space for SMSC LAN
+- smsc-int-gpios : Should specify the GPIO for SMSC LAN interrupt line
+- phy-mode : String, operation mode of the PHY interface.
+ Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
+ "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
+
+Optional properties:
+- smsc,irq-active-high : Indicates the IRQ polarity is active-low
+- smsc,irq-push-pull : Indicates the IRQ type is push-pull
+- smsc,register-needs-shift : Indicates the register access needs shift
+- smsc,access-in-32bit : Indicates the access to controller is in 32-bit
+ mode
+- smsc,force-internal-phy : Forces SMSC LAN controller to use
+ internal PHY
+- smsc,force-external-phy : Forces SMSC LAN controller to use
+ external PHY
+- smsc,save-mac-address : Indicates that mac address needs to be saved
+ before resetting the controller
+- local-mac-address : 6 bytes, mac address
+
+Examples:
+
+lan9220@f4000000 {
+ compatible = "smsc,lan9220", "smsc,lan";
+ reg = <0xf4000000 0x2000000>;
+ phy-mode = "mii";
+ smsc-int-gpios = <&gpio1 31 0>; /* GPIO2_31 */
+ smsc,irq-push-pull;
+ smsc,access-in-32bit;
+};
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index b9016a3..0097048 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -53,6 +53,10 @@
#include <linux/phy.h>
#include <linux/smsc911x.h>
#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/of_net.h>
#include "smsc911x.h"
#define SMSC_CHIPNAME "smsc911x"
@@ -2095,25 +2099,67 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
.tx_writefifo = smsc911x_tx_writefifo_shift,
};
+#ifdef CONFIG_OF
+static int __devinit smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ const char *mac;
+
+ if (!np)
+ return -ENODEV;
+
+ config->phy_interface = of_get_phy_mode(np);
+
+ mac = of_get_mac_address(np);
+ if (mac)
+ memcpy(config->mac, mac, ETH_ALEN);
+
+ if (of_get_property(np, "smsc,irq-active-high", NULL))
+ config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
+
+ if (of_get_property(np, "smsc,irq-push-pull", NULL))
+ config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
+
+ if (of_get_property(np, "smsc,register-needs-shift", NULL))
+ config->shift = 1;
+
+ if (of_get_property(np, "smsc,access-in-32bit", NULL))
+ config->flags |= SMSC911X_USE_32BIT;
+
+ if (of_get_property(np, "smsc,force-internal-phy", NULL))
+ config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,force-external-phy", NULL))
+ config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,save-mac-address", NULL))
+ config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
+
+ return 0;
+}
+#else
+static inline int smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ return -ENODEV;
+}
+#endif /* CONFIG_OF */
+
static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
{
+ struct device_node *np = pdev->dev.of_node;
struct net_device *dev;
struct smsc911x_data *pdata;
struct smsc911x_platform_config *config = pdev->dev.platform_data;
struct resource *res, *irq_res;
unsigned int intcfg = 0;
- int res_size, irq_flags;
- int retval;
+ int irq_gpio, res_size, irq_flags = 0;
+ int retval = 0;
pr_info("Driver version %s\n", SMSC_DRV_VERSION);
- /* platform data specifies irq & dynamic bus configuration */
- if (!pdev->dev.platform_data) {
- pr_warn("platform_data not provided\n");
- retval = -ENODEV;
- goto out_0;
- }
-
res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"smsc911x-memory");
if (!res)
@@ -2125,13 +2171,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
}
res_size = resource_size(res);
- irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!irq_res) {
- pr_warn("Could not allocate irq resource\n");
- retval = -ENODEV;
- goto out_0;
- }
-
if (!request_mem_region(res->start, res_size, SMSC_CHIPNAME)) {
retval = -EBUSY;
goto out_0;
@@ -2148,26 +2187,53 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
pdata = netdev_priv(dev);
- dev->irq = irq_res->start;
- irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
- pdata->ioaddr = ioremap_nocache(res->start, res_size);
-
- /* copy config parameters across to pdata */
- memcpy(&pdata->config, config, sizeof(pdata->config));
+ if (np) {
+ irq_gpio = of_get_named_gpio(np, "smsc-int-gpios", 0);
+ retval = gpio_request_one(irq_gpio, GPIOF_IN, "smsc-int-gpio");
+ if (!retval)
+ dev->irq = gpio_to_irq(irq_gpio);
+ } else {
+ irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (irq_res) {
+ dev->irq = irq_res->start;
+ irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
+ } else {
+ retval = -ENODEV;
+ }
+ }
- pdata->dev = dev;
- pdata->msg_enable = ((1 << debug) - 1);
+ if (retval) {
+ SMSC_WARN(pdata, probe, "Error smsc911x irq not found");
+ retval = -EINVAL;
+ goto out_free_netdev_2;
+ }
+ pdata->ioaddr = ioremap_nocache(res->start, res_size);
if (pdata->ioaddr == NULL) {
SMSC_WARN(pdata, probe, "Error smsc911x base address invalid");
retval = -ENOMEM;
goto out_free_netdev_2;
}
+ pdata->dev = dev;
+ pdata->msg_enable = ((1 << debug) - 1);
+
+ retval = smsc911x_probe_config_dt(&pdata->config, np);
+ if (retval && config) {
+ /* copy config parameters across to pdata */
+ memcpy(&pdata->config, config, sizeof(pdata->config));
+ retval = 0;
+ }
+
+ if (retval) {
+ SMSC_WARN(pdata, probe, "Error smsc911x config not found");
+ goto out_unmap_io_3;
+ }
+
/* assume standard, non-shifted, access to HW registers */
pdata->ops = &standard_smsc911x_ops;
/* apply the right access if shifting is needed */
- if (config->shift)
+ if (pdata->config.shift)
pdata->ops = &shifted_smsc911x_ops;
retval = smsc911x_init(dev);
@@ -2314,6 +2380,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
#define SMSC911X_PM_OPS NULL
#endif
+static const struct of_device_id smsc_dt_ids[] = {
+ { .compatible = "smsc,lan", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, smsc_dt_ids);
+
static struct platform_driver smsc911x_driver = {
.probe = smsc911x_drv_probe,
.remove = __devexit_p(smsc911x_drv_remove),
@@ -2321,6 +2393,7 @@ static struct platform_driver smsc911x_driver = {
.name = SMSC_CHIPNAME,
.owner = THIS_MODULE,
.pm = SMSC911X_PM_OPS,
+ .of_match_table = smsc_dt_ids,
},
};
--
1.7.4.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-25 9:44 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-25 9:44 UTC (permalink / raw)
To: linux-arm-kernel
It adds device tree probe support for smsc911x driver.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Steve Glendinning <steve.glendinning@smsc.com>
Cc: David S. Miller <davem@davemloft.net>
---
Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
2 files changed, 132 insertions(+), 25 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
new file mode 100644
index 0000000..1920695
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc.txt
@@ -0,0 +1,34 @@
+* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
+
+Required properties:
+- compatible : Should be "smsc,lan<model>""smsc,lan"
+- reg : Address and length of the io space for SMSC LAN
+- smsc-int-gpios : Should specify the GPIO for SMSC LAN interrupt line
+- phy-mode : String, operation mode of the PHY interface.
+ Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
+ "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
+
+Optional properties:
+- smsc,irq-active-high : Indicates the IRQ polarity is active-low
+- smsc,irq-push-pull : Indicates the IRQ type is push-pull
+- smsc,register-needs-shift : Indicates the register access needs shift
+- smsc,access-in-32bit : Indicates the access to controller is in 32-bit
+ mode
+- smsc,force-internal-phy : Forces SMSC LAN controller to use
+ internal PHY
+- smsc,force-external-phy : Forces SMSC LAN controller to use
+ external PHY
+- smsc,save-mac-address : Indicates that mac address needs to be saved
+ before resetting the controller
+- local-mac-address : 6 bytes, mac address
+
+Examples:
+
+lan9220 at f4000000 {
+ compatible = "smsc,lan9220", "smsc,lan";
+ reg = <0xf4000000 0x2000000>;
+ phy-mode = "mii";
+ smsc-int-gpios = <&gpio1 31 0>; /* GPIO2_31 */
+ smsc,irq-push-pull;
+ smsc,access-in-32bit;
+};
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index b9016a3..0097048 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -53,6 +53,10 @@
#include <linux/phy.h>
#include <linux/smsc911x.h>
#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/of_net.h>
#include "smsc911x.h"
#define SMSC_CHIPNAME "smsc911x"
@@ -2095,25 +2099,67 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
.tx_writefifo = smsc911x_tx_writefifo_shift,
};
+#ifdef CONFIG_OF
+static int __devinit smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ const char *mac;
+
+ if (!np)
+ return -ENODEV;
+
+ config->phy_interface = of_get_phy_mode(np);
+
+ mac = of_get_mac_address(np);
+ if (mac)
+ memcpy(config->mac, mac, ETH_ALEN);
+
+ if (of_get_property(np, "smsc,irq-active-high", NULL))
+ config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
+
+ if (of_get_property(np, "smsc,irq-push-pull", NULL))
+ config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
+
+ if (of_get_property(np, "smsc,register-needs-shift", NULL))
+ config->shift = 1;
+
+ if (of_get_property(np, "smsc,access-in-32bit", NULL))
+ config->flags |= SMSC911X_USE_32BIT;
+
+ if (of_get_property(np, "smsc,force-internal-phy", NULL))
+ config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,force-external-phy", NULL))
+ config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,save-mac-address", NULL))
+ config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
+
+ return 0;
+}
+#else
+static inline int smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ return -ENODEV;
+}
+#endif /* CONFIG_OF */
+
static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
{
+ struct device_node *np = pdev->dev.of_node;
struct net_device *dev;
struct smsc911x_data *pdata;
struct smsc911x_platform_config *config = pdev->dev.platform_data;
struct resource *res, *irq_res;
unsigned int intcfg = 0;
- int res_size, irq_flags;
- int retval;
+ int irq_gpio, res_size, irq_flags = 0;
+ int retval = 0;
pr_info("Driver version %s\n", SMSC_DRV_VERSION);
- /* platform data specifies irq & dynamic bus configuration */
- if (!pdev->dev.platform_data) {
- pr_warn("platform_data not provided\n");
- retval = -ENODEV;
- goto out_0;
- }
-
res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"smsc911x-memory");
if (!res)
@@ -2125,13 +2171,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
}
res_size = resource_size(res);
- irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!irq_res) {
- pr_warn("Could not allocate irq resource\n");
- retval = -ENODEV;
- goto out_0;
- }
-
if (!request_mem_region(res->start, res_size, SMSC_CHIPNAME)) {
retval = -EBUSY;
goto out_0;
@@ -2148,26 +2187,53 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
pdata = netdev_priv(dev);
- dev->irq = irq_res->start;
- irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
- pdata->ioaddr = ioremap_nocache(res->start, res_size);
-
- /* copy config parameters across to pdata */
- memcpy(&pdata->config, config, sizeof(pdata->config));
+ if (np) {
+ irq_gpio = of_get_named_gpio(np, "smsc-int-gpios", 0);
+ retval = gpio_request_one(irq_gpio, GPIOF_IN, "smsc-int-gpio");
+ if (!retval)
+ dev->irq = gpio_to_irq(irq_gpio);
+ } else {
+ irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (irq_res) {
+ dev->irq = irq_res->start;
+ irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
+ } else {
+ retval = -ENODEV;
+ }
+ }
- pdata->dev = dev;
- pdata->msg_enable = ((1 << debug) - 1);
+ if (retval) {
+ SMSC_WARN(pdata, probe, "Error smsc911x irq not found");
+ retval = -EINVAL;
+ goto out_free_netdev_2;
+ }
+ pdata->ioaddr = ioremap_nocache(res->start, res_size);
if (pdata->ioaddr == NULL) {
SMSC_WARN(pdata, probe, "Error smsc911x base address invalid");
retval = -ENOMEM;
goto out_free_netdev_2;
}
+ pdata->dev = dev;
+ pdata->msg_enable = ((1 << debug) - 1);
+
+ retval = smsc911x_probe_config_dt(&pdata->config, np);
+ if (retval && config) {
+ /* copy config parameters across to pdata */
+ memcpy(&pdata->config, config, sizeof(pdata->config));
+ retval = 0;
+ }
+
+ if (retval) {
+ SMSC_WARN(pdata, probe, "Error smsc911x config not found");
+ goto out_unmap_io_3;
+ }
+
/* assume standard, non-shifted, access to HW registers */
pdata->ops = &standard_smsc911x_ops;
/* apply the right access if shifting is needed */
- if (config->shift)
+ if (pdata->config.shift)
pdata->ops = &shifted_smsc911x_ops;
retval = smsc911x_init(dev);
@@ -2314,6 +2380,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
#define SMSC911X_PM_OPS NULL
#endif
+static const struct of_device_id smsc_dt_ids[] = {
+ { .compatible = "smsc,lan", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, smsc_dt_ids);
+
static struct platform_driver smsc911x_driver = {
.probe = smsc911x_drv_probe,
.remove = __devexit_p(smsc911x_drv_remove),
@@ -2321,6 +2393,7 @@ static struct platform_driver smsc911x_driver = {
.name = SMSC_CHIPNAME,
.owner = THIS_MODULE,
.pm = SMSC911X_PM_OPS,
+ .of_match_table = smsc_dt_ids,
},
};
--
1.7.4.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
2011-07-25 9:44 ` Shawn Guo
@ 2011-07-25 21:37 ` Grant Likely
-1 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-07-25 21:37 UTC (permalink / raw)
To: Shawn Guo
Cc: netdev, devicetree-discuss, linux-arm-kernel, patches,
Steve Glendinning, David S. Miller
On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> It adds device tree probe support for smsc911x driver.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Steve Glendinning <steve.glendinning@smsc.com>
> Cc: David S. Miller <davem@davemloft.net>
> ---
> Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> 2 files changed, 132 insertions(+), 25 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
>
> diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> new file mode 100644
> index 0000000..1920695
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/smsc.txt
> @@ -0,0 +1,34 @@
> +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> +
> +Required properties:
> +- compatible : Should be "smsc,lan<model>""smsc,lan"
Drop "smsc,lan". That's far too generic.
> +- reg : Address and length of the io space for SMSC LAN
> +- smsc-int-gpios : Should specify the GPIO for SMSC LAN interrupt line
This looks broken. Shouldn't this be specified as a normal
"interrupts" property?
> +- phy-mode : String, operation mode of the PHY interface.
> + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
> +
> +Optional properties:
> +- smsc,irq-active-high : Indicates the IRQ polarity is active-low
> +- smsc,irq-push-pull : Indicates the IRQ type is push-pull
> +- smsc,register-needs-shift : Indicates the register access needs shift
> +- smsc,access-in-32bit : Indicates the access to controller is in 32-bit
> + mode
Currently, reg-io-width and reg-shift are being used to manipulate
register access on ns16550 serial ports. The same thing can be used
here. See bindings/tty/serial/of-serial.txt
> +- smsc,force-internal-phy : Forces SMSC LAN controller to use
> + internal PHY
> +- smsc,force-external-phy : Forces SMSC LAN controller to use
> + external PHY
I would expect using an external phy would also expect a phy-device
property to connect to the phy node.
> +- smsc,save-mac-address : Indicates that mac address needs to be saved
> + before resetting the controller
> +- local-mac-address : 6 bytes, mac address
> +
> +Examples:
> +
> +lan9220@f4000000 {
> + compatible = "smsc,lan9220", "smsc,lan";
> + reg = <0xf4000000 0x2000000>;
> + phy-mode = "mii";
> + smsc-int-gpios = <&gpio1 31 0>; /* GPIO2_31 */
> + smsc,irq-push-pull;
> + smsc,access-in-32bit;
> +};
> diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> index b9016a3..0097048 100644
> --- a/drivers/net/smsc911x.c
> +++ b/drivers/net/smsc911x.c
> @@ -53,6 +53,10 @@
> #include <linux/phy.h>
> #include <linux/smsc911x.h>
> #include <linux/device.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_net.h>
> #include "smsc911x.h"
>
> #define SMSC_CHIPNAME "smsc911x"
> @@ -2095,25 +2099,67 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
> .tx_writefifo = smsc911x_tx_writefifo_shift,
> };
>
> +#ifdef CONFIG_OF
> +static int __devinit smsc911x_probe_config_dt(
> + struct smsc911x_platform_config *config,
> + struct device_node *np)
> +{
> + const char *mac;
> +
> + if (!np)
> + return -ENODEV;
> +
> + config->phy_interface = of_get_phy_mode(np);
> +
> + mac = of_get_mac_address(np);
> + if (mac)
> + memcpy(config->mac, mac, ETH_ALEN);
> +
> + if (of_get_property(np, "smsc,irq-active-high", NULL))
> + config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
> +
> + if (of_get_property(np, "smsc,irq-push-pull", NULL))
> + config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
> +
> + if (of_get_property(np, "smsc,register-needs-shift", NULL))
> + config->shift = 1;
> +
> + if (of_get_property(np, "smsc,access-in-32bit", NULL))
> + config->flags |= SMSC911X_USE_32BIT;
> +
> + if (of_get_property(np, "smsc,force-internal-phy", NULL))
> + config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
> +
> + if (of_get_property(np, "smsc,force-external-phy", NULL))
> + config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
> +
> + if (of_get_property(np, "smsc,save-mac-address", NULL))
> + config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
> +
> + return 0;
> +}
> +#else
> +static inline int smsc911x_probe_config_dt(
> + struct smsc911x_platform_config *config,
> + struct device_node *np)
> +{
> + return -ENODEV;
> +}
> +#endif /* CONFIG_OF */
> +
> static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> {
> + struct device_node *np = pdev->dev.of_node;
> struct net_device *dev;
> struct smsc911x_data *pdata;
> struct smsc911x_platform_config *config = pdev->dev.platform_data;
> struct resource *res, *irq_res;
> unsigned int intcfg = 0;
> - int res_size, irq_flags;
> - int retval;
> + int irq_gpio, res_size, irq_flags = 0;
> + int retval = 0;
>
> pr_info("Driver version %s\n", SMSC_DRV_VERSION);
>
> - /* platform data specifies irq & dynamic bus configuration */
> - if (!pdev->dev.platform_data) {
> - pr_warn("platform_data not provided\n");
> - retval = -ENODEV;
> - goto out_0;
> - }
> -
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "smsc911x-memory");
> if (!res)
> @@ -2125,13 +2171,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> }
> res_size = resource_size(res);
>
> - irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> - if (!irq_res) {
> - pr_warn("Could not allocate irq resource\n");
> - retval = -ENODEV;
> - goto out_0;
> - }
> -
This should still work for the device-tree situation. Why remove it?
> if (!request_mem_region(res->start, res_size, SMSC_CHIPNAME)) {
> retval = -EBUSY;
> goto out_0;
> @@ -2148,26 +2187,53 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
>
> pdata = netdev_priv(dev);
>
> - dev->irq = irq_res->start;
> - irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> - pdata->ioaddr = ioremap_nocache(res->start, res_size);
> -
> - /* copy config parameters across to pdata */
> - memcpy(&pdata->config, config, sizeof(pdata->config));
> + if (np) {
> + irq_gpio = of_get_named_gpio(np, "smsc-int-gpios", 0);
> + retval = gpio_request_one(irq_gpio, GPIOF_IN, "smsc-int-gpio");
> + if (!retval)
> + dev->irq = gpio_to_irq(irq_gpio);
Yeah, that's definitely the wrong way to handle this. If the
device it wired to a gpio controller, then the gpio controller also
need to be an interrupt controller to ensure that it can map interrupt
numbers.
> + } else {
> + irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (irq_res) {
> + dev->irq = irq_res->start;
> + irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> + } else {
> + retval = -ENODEV;
> + }
> + }
>
> - pdata->dev = dev;
> - pdata->msg_enable = ((1 << debug) - 1);
> + if (retval) {
> + SMSC_WARN(pdata, probe, "Error smsc911x irq not found");
> + retval = -EINVAL;
> + goto out_free_netdev_2;
> + }
>
> + pdata->ioaddr = ioremap_nocache(res->start, res_size);
> if (pdata->ioaddr == NULL) {
> SMSC_WARN(pdata, probe, "Error smsc911x base address invalid");
> retval = -ENOMEM;
> goto out_free_netdev_2;
> }
>
> + pdata->dev = dev;
> + pdata->msg_enable = ((1 << debug) - 1);
> +
> + retval = smsc911x_probe_config_dt(&pdata->config, np);
> + if (retval && config) {
> + /* copy config parameters across to pdata */
> + memcpy(&pdata->config, config, sizeof(pdata->config));
> + retval = 0;
> + }
> +
> + if (retval) {
> + SMSC_WARN(pdata, probe, "Error smsc911x config not found");
> + goto out_unmap_io_3;
> + }
> +
> /* assume standard, non-shifted, access to HW registers */
> pdata->ops = &standard_smsc911x_ops;
> /* apply the right access if shifting is needed */
> - if (config->shift)
> + if (pdata->config.shift)
> pdata->ops = &shifted_smsc911x_ops;
>
> retval = smsc911x_init(dev);
> @@ -2314,6 +2380,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
> #define SMSC911X_PM_OPS NULL
> #endif
>
> +static const struct of_device_id smsc_dt_ids[] = {
> + { .compatible = "smsc,lan", },
As mentioned above, "smsc,lan" is far too generic.
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, smsc_dt_ids);
> +
> static struct platform_driver smsc911x_driver = {
> .probe = smsc911x_drv_probe,
> .remove = __devexit_p(smsc911x_drv_remove),
> @@ -2321,6 +2393,7 @@ static struct platform_driver smsc911x_driver = {
> .name = SMSC_CHIPNAME,
> .owner = THIS_MODULE,
> .pm = SMSC911X_PM_OPS,
> + .of_match_table = smsc_dt_ids,
> },
> };
>
> --
> 1.7.4.1
>
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-25 21:37 ` Grant Likely
0 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-07-25 21:37 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> It adds device tree probe support for smsc911x driver.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Steve Glendinning <steve.glendinning@smsc.com>
> Cc: David S. Miller <davem@davemloft.net>
> ---
> Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> 2 files changed, 132 insertions(+), 25 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
>
> diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> new file mode 100644
> index 0000000..1920695
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/smsc.txt
> @@ -0,0 +1,34 @@
> +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> +
> +Required properties:
> +- compatible : Should be "smsc,lan<model>""smsc,lan"
Drop "smsc,lan". That's far too generic.
> +- reg : Address and length of the io space for SMSC LAN
> +- smsc-int-gpios : Should specify the GPIO for SMSC LAN interrupt line
This looks broken. Shouldn't this be specified as a normal
"interrupts" property?
> +- phy-mode : String, operation mode of the PHY interface.
> + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
> +
> +Optional properties:
> +- smsc,irq-active-high : Indicates the IRQ polarity is active-low
> +- smsc,irq-push-pull : Indicates the IRQ type is push-pull
> +- smsc,register-needs-shift : Indicates the register access needs shift
> +- smsc,access-in-32bit : Indicates the access to controller is in 32-bit
> + mode
Currently, reg-io-width and reg-shift are being used to manipulate
register access on ns16550 serial ports. The same thing can be used
here. See bindings/tty/serial/of-serial.txt
> +- smsc,force-internal-phy : Forces SMSC LAN controller to use
> + internal PHY
> +- smsc,force-external-phy : Forces SMSC LAN controller to use
> + external PHY
I would expect using an external phy would also expect a phy-device
property to connect to the phy node.
> +- smsc,save-mac-address : Indicates that mac address needs to be saved
> + before resetting the controller
> +- local-mac-address : 6 bytes, mac address
> +
> +Examples:
> +
> +lan9220 at f4000000 {
> + compatible = "smsc,lan9220", "smsc,lan";
> + reg = <0xf4000000 0x2000000>;
> + phy-mode = "mii";
> + smsc-int-gpios = <&gpio1 31 0>; /* GPIO2_31 */
> + smsc,irq-push-pull;
> + smsc,access-in-32bit;
> +};
> diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> index b9016a3..0097048 100644
> --- a/drivers/net/smsc911x.c
> +++ b/drivers/net/smsc911x.c
> @@ -53,6 +53,10 @@
> #include <linux/phy.h>
> #include <linux/smsc911x.h>
> #include <linux/device.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_net.h>
> #include "smsc911x.h"
>
> #define SMSC_CHIPNAME "smsc911x"
> @@ -2095,25 +2099,67 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
> .tx_writefifo = smsc911x_tx_writefifo_shift,
> };
>
> +#ifdef CONFIG_OF
> +static int __devinit smsc911x_probe_config_dt(
> + struct smsc911x_platform_config *config,
> + struct device_node *np)
> +{
> + const char *mac;
> +
> + if (!np)
> + return -ENODEV;
> +
> + config->phy_interface = of_get_phy_mode(np);
> +
> + mac = of_get_mac_address(np);
> + if (mac)
> + memcpy(config->mac, mac, ETH_ALEN);
> +
> + if (of_get_property(np, "smsc,irq-active-high", NULL))
> + config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
> +
> + if (of_get_property(np, "smsc,irq-push-pull", NULL))
> + config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
> +
> + if (of_get_property(np, "smsc,register-needs-shift", NULL))
> + config->shift = 1;
> +
> + if (of_get_property(np, "smsc,access-in-32bit", NULL))
> + config->flags |= SMSC911X_USE_32BIT;
> +
> + if (of_get_property(np, "smsc,force-internal-phy", NULL))
> + config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
> +
> + if (of_get_property(np, "smsc,force-external-phy", NULL))
> + config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
> +
> + if (of_get_property(np, "smsc,save-mac-address", NULL))
> + config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
> +
> + return 0;
> +}
> +#else
> +static inline int smsc911x_probe_config_dt(
> + struct smsc911x_platform_config *config,
> + struct device_node *np)
> +{
> + return -ENODEV;
> +}
> +#endif /* CONFIG_OF */
> +
> static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> {
> + struct device_node *np = pdev->dev.of_node;
> struct net_device *dev;
> struct smsc911x_data *pdata;
> struct smsc911x_platform_config *config = pdev->dev.platform_data;
> struct resource *res, *irq_res;
> unsigned int intcfg = 0;
> - int res_size, irq_flags;
> - int retval;
> + int irq_gpio, res_size, irq_flags = 0;
> + int retval = 0;
>
> pr_info("Driver version %s\n", SMSC_DRV_VERSION);
>
> - /* platform data specifies irq & dynamic bus configuration */
> - if (!pdev->dev.platform_data) {
> - pr_warn("platform_data not provided\n");
> - retval = -ENODEV;
> - goto out_0;
> - }
> -
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "smsc911x-memory");
> if (!res)
> @@ -2125,13 +2171,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> }
> res_size = resource_size(res);
>
> - irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> - if (!irq_res) {
> - pr_warn("Could not allocate irq resource\n");
> - retval = -ENODEV;
> - goto out_0;
> - }
> -
This should still work for the device-tree situation. Why remove it?
> if (!request_mem_region(res->start, res_size, SMSC_CHIPNAME)) {
> retval = -EBUSY;
> goto out_0;
> @@ -2148,26 +2187,53 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
>
> pdata = netdev_priv(dev);
>
> - dev->irq = irq_res->start;
> - irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> - pdata->ioaddr = ioremap_nocache(res->start, res_size);
> -
> - /* copy config parameters across to pdata */
> - memcpy(&pdata->config, config, sizeof(pdata->config));
> + if (np) {
> + irq_gpio = of_get_named_gpio(np, "smsc-int-gpios", 0);
> + retval = gpio_request_one(irq_gpio, GPIOF_IN, "smsc-int-gpio");
> + if (!retval)
> + dev->irq = gpio_to_irq(irq_gpio);
Yeah, that's definitely the wrong way to handle this. If the
device it wired to a gpio controller, then the gpio controller also
need to be an interrupt controller to ensure that it can map interrupt
numbers.
> + } else {
> + irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (irq_res) {
> + dev->irq = irq_res->start;
> + irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> + } else {
> + retval = -ENODEV;
> + }
> + }
>
> - pdata->dev = dev;
> - pdata->msg_enable = ((1 << debug) - 1);
> + if (retval) {
> + SMSC_WARN(pdata, probe, "Error smsc911x irq not found");
> + retval = -EINVAL;
> + goto out_free_netdev_2;
> + }
>
> + pdata->ioaddr = ioremap_nocache(res->start, res_size);
> if (pdata->ioaddr == NULL) {
> SMSC_WARN(pdata, probe, "Error smsc911x base address invalid");
> retval = -ENOMEM;
> goto out_free_netdev_2;
> }
>
> + pdata->dev = dev;
> + pdata->msg_enable = ((1 << debug) - 1);
> +
> + retval = smsc911x_probe_config_dt(&pdata->config, np);
> + if (retval && config) {
> + /* copy config parameters across to pdata */
> + memcpy(&pdata->config, config, sizeof(pdata->config));
> + retval = 0;
> + }
> +
> + if (retval) {
> + SMSC_WARN(pdata, probe, "Error smsc911x config not found");
> + goto out_unmap_io_3;
> + }
> +
> /* assume standard, non-shifted, access to HW registers */
> pdata->ops = &standard_smsc911x_ops;
> /* apply the right access if shifting is needed */
> - if (config->shift)
> + if (pdata->config.shift)
> pdata->ops = &shifted_smsc911x_ops;
>
> retval = smsc911x_init(dev);
> @@ -2314,6 +2380,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
> #define SMSC911X_PM_OPS NULL
> #endif
>
> +static const struct of_device_id smsc_dt_ids[] = {
> + { .compatible = "smsc,lan", },
As mentioned above, "smsc,lan" is far too generic.
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, smsc_dt_ids);
> +
> static struct platform_driver smsc911x_driver = {
> .probe = smsc911x_drv_probe,
> .remove = __devexit_p(smsc911x_drv_remove),
> @@ -2321,6 +2393,7 @@ static struct platform_driver smsc911x_driver = {
> .name = SMSC_CHIPNAME,
> .owner = THIS_MODULE,
> .pm = SMSC911X_PM_OPS,
> + .of_match_table = smsc_dt_ids,
> },
> };
>
> --
> 1.7.4.1
>
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
2011-07-25 21:37 ` Grant Likely
@ 2011-07-26 1:01 ` Shawn Guo
-1 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-26 1:01 UTC (permalink / raw)
To: Grant Likely
Cc: patches, netdev, devicetree-discuss, Steve Glendinning,
Shawn Guo, David S. Miller, linux-arm-kernel
On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > It adds device tree probe support for smsc911x driver.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > ---
> > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > 2 files changed, 132 insertions(+), 25 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > new file mode 100644
> > index 0000000..1920695
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > @@ -0,0 +1,34 @@
> > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > +
> > +Required properties:
> > +- compatible : Should be "smsc,lan<model>""smsc,lan"
>
> Drop "smsc,lan". That's far too generic.
>
The following devices are supported by the driver.
LAN9115, LAN9116, LAN9117, LAN9118
LAN9215, LAN9216, LAN9217, LAN9218
LAN9210, LAN9211
LAN9220, LAN9221
If we only keep specific <model> as the compatible, we will have a
long match table which is actually used nowhere to distinguish the
device.
So we need some level generic compatible to save the meaningless
long match table. What about:
static const struct of_device_id smsc_dt_ids[] = {
{ .compatible = "smsc,lan9", },
{ /* sentinel */ }
};
Or:
static const struct of_device_id smsc_dt_ids[] = {
{ .compatible = "smsc,lan91", },
{ .compatible = "smsc,lan92", },
{ /* sentinel */ }
};
> > +- reg : Address and length of the io space for SMSC LAN
> > +- smsc-int-gpios : Should specify the GPIO for SMSC LAN interrupt line
>
> This looks broken. Shouldn't this be specified as a normal
> "interrupts" property?
>
> > +- phy-mode : String, operation mode of the PHY interface.
> > + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> > + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
> > +
> > +Optional properties:
> > +- smsc,irq-active-high : Indicates the IRQ polarity is active-low
> > +- smsc,irq-push-pull : Indicates the IRQ type is push-pull
> > +- smsc,register-needs-shift : Indicates the register access needs shift
> > +- smsc,access-in-32bit : Indicates the access to controller is in 32-bit
> > + mode
>
> Currently, reg-io-width and reg-shift are being used to manipulate
> register access on ns16550 serial ports. The same thing can be used
> here. See bindings/tty/serial/of-serial.txt
>
They are not exactly same. reg-io-width and reg-shift in of-serial.txt
are giving a number, while register-needs-shift and access-in-32bit
here just tells a flag. But if you have a strong position to make
them consistent, I can change them to reg-io-width and reg-shift and
get smsc911x parse numbers instead of flags.
>
> > +- smsc,force-internal-phy : Forces SMSC LAN controller to use
> > + internal PHY
> > +- smsc,force-external-phy : Forces SMSC LAN controller to use
> > + external PHY
>
> I would expect using an external phy would also expect a phy-device
> property to connect to the phy node.
>
I do not understand the details. But from the comment below in the
code, I guess the "external phy" is not external?
/* Autodetects and enables external phy if present on supported chips.
* autodetection can be overridden by specifying SMSC911X_FORCE_INTERNAL_PHY
* or SMSC911X_FORCE_EXTERNAL_PHY in the platform_data flags. */
> > +- smsc,save-mac-address : Indicates that mac address needs to be saved
> > + before resetting the controller
> > +- local-mac-address : 6 bytes, mac address
> > +
> > +Examples:
> > +
> > +lan9220@f4000000 {
> > + compatible = "smsc,lan9220", "smsc,lan";
> > + reg = <0xf4000000 0x2000000>;
> > + phy-mode = "mii";
> > + smsc-int-gpios = <&gpio1 31 0>; /* GPIO2_31 */
> > + smsc,irq-push-pull;
> > + smsc,access-in-32bit;
> > +};
> > diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> > index b9016a3..0097048 100644
> > --- a/drivers/net/smsc911x.c
> > +++ b/drivers/net/smsc911x.c
> > @@ -53,6 +53,10 @@
> > #include <linux/phy.h>
> > #include <linux/smsc911x.h>
> > #include <linux/device.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/of_net.h>
> > #include "smsc911x.h"
> >
> > #define SMSC_CHIPNAME "smsc911x"
> > @@ -2095,25 +2099,67 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
> > .tx_writefifo = smsc911x_tx_writefifo_shift,
> > };
> >
> > +#ifdef CONFIG_OF
> > +static int __devinit smsc911x_probe_config_dt(
> > + struct smsc911x_platform_config *config,
> > + struct device_node *np)
> > +{
> > + const char *mac;
> > +
> > + if (!np)
> > + return -ENODEV;
> > +
> > + config->phy_interface = of_get_phy_mode(np);
> > +
> > + mac = of_get_mac_address(np);
> > + if (mac)
> > + memcpy(config->mac, mac, ETH_ALEN);
> > +
> > + if (of_get_property(np, "smsc,irq-active-high", NULL))
> > + config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
> > +
> > + if (of_get_property(np, "smsc,irq-push-pull", NULL))
> > + config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
> > +
> > + if (of_get_property(np, "smsc,register-needs-shift", NULL))
> > + config->shift = 1;
> > +
> > + if (of_get_property(np, "smsc,access-in-32bit", NULL))
> > + config->flags |= SMSC911X_USE_32BIT;
> > +
> > + if (of_get_property(np, "smsc,force-internal-phy", NULL))
> > + config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
> > +
> > + if (of_get_property(np, "smsc,force-external-phy", NULL))
> > + config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
> > +
> > + if (of_get_property(np, "smsc,save-mac-address", NULL))
> > + config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
> > +
> > + return 0;
> > +}
> > +#else
> > +static inline int smsc911x_probe_config_dt(
> > + struct smsc911x_platform_config *config,
> > + struct device_node *np)
> > +{
> > + return -ENODEV;
> > +}
> > +#endif /* CONFIG_OF */
> > +
> > static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > {
> > + struct device_node *np = pdev->dev.of_node;
> > struct net_device *dev;
> > struct smsc911x_data *pdata;
> > struct smsc911x_platform_config *config = pdev->dev.platform_data;
> > struct resource *res, *irq_res;
> > unsigned int intcfg = 0;
> > - int res_size, irq_flags;
> > - int retval;
> > + int irq_gpio, res_size, irq_flags = 0;
> > + int retval = 0;
> >
> > pr_info("Driver version %s\n", SMSC_DRV_VERSION);
> >
> > - /* platform data specifies irq & dynamic bus configuration */
> > - if (!pdev->dev.platform_data) {
> > - pr_warn("platform_data not provided\n");
> > - retval = -ENODEV;
> > - goto out_0;
> > - }
> > -
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > "smsc911x-memory");
> > if (!res)
> > @@ -2125,13 +2171,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > }
> > res_size = resource_size(res);
> >
> > - irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > - if (!irq_res) {
> > - pr_warn("Could not allocate irq resource\n");
> > - retval = -ENODEV;
> > - goto out_0;
> > - }
> > -
>
> This should still work for the device-tree situation. Why remove it?
>
> > if (!request_mem_region(res->start, res_size, SMSC_CHIPNAME)) {
> > retval = -EBUSY;
> > goto out_0;
> > @@ -2148,26 +2187,53 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> >
> > pdata = netdev_priv(dev);
> >
> > - dev->irq = irq_res->start;
> > - irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> > - pdata->ioaddr = ioremap_nocache(res->start, res_size);
> > -
> > - /* copy config parameters across to pdata */
> > - memcpy(&pdata->config, config, sizeof(pdata->config));
> > + if (np) {
> > + irq_gpio = of_get_named_gpio(np, "smsc-int-gpios", 0);
> > + retval = gpio_request_one(irq_gpio, GPIOF_IN, "smsc-int-gpio");
> > + if (!retval)
> > + dev->irq = gpio_to_irq(irq_gpio);
>
> Yeah, that's definitely the wrong way to handle this. If the
> device it wired to a gpio controller, then the gpio controller also
> need to be an interrupt controller to ensure that it can map interrupt
> numbers.
>
Here it is. Please let me know if it is what you mean.
diff --git a/arch/arm/boot/dts/imx53-ard.dts b/arch/arm/boot/dts/imx53-ard.dts
index 6a007f1..0b17af8 100644
--- a/arch/arm/boot/dts/imx53-ard.dts
+++ b/arch/arm/boot/dts/imx53-ard.dts
@@ -320,7 +320,8 @@
compatible = "smsc,lan9220", "smsc,lan";
reg = <0xf4000000 0x2000000>;
phy-mode = "mii";
- smsc-int-gpios = <&gpio1 31 0>; /* GPIO2_31 */
+ interrupt-parent = <&gpio1>;
+ interrupts = <31>;
smsc,irq-push-pull;
smsc,access-in-32bit;
};
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index 746221c..224f67f 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -89,6 +89,8 @@
interrupts = <50 51>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
gpio1: gpio@53f88000 { /* GPIO2 */
@@ -97,6 +99,8 @@
interrupts = <52 53>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
gpio2: gpio@53f8c000 { /* GPIO3 */
@@ -105,6 +109,8 @@
interrupts = <54 55>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
gpio3: gpio@53f90000 { /* GPIO4 */
@@ -113,6 +119,8 @@
interrupts = <56 57>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
wdt@53f98000 { /* WDOG1 */
@@ -950,6 +958,8 @@
interrupts = <103 104>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
gpio5: gpio@53fe0000 { /* GPIO6 */
@@ -958,6 +968,8 @@
interrupts = <105 106>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
gpio6: gpio@53fe4000 { /* GPIO7 */
@@ -966,6 +978,8 @@
interrupts = <107 108>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
i2c@53fec000 { /* I2C3 */
diff --git a/arch/arm/mach-mx5/imx53-dt.c b/arch/arm/mach-mx5/imx53-dt.c
index ac06f04..04dc1ab 100644
--- a/arch/arm/mach-mx5/imx53-dt.c
+++ b/arch/arm/mach-mx5/imx53-dt.c
@@ -51,6 +51,11 @@ static const struct of_device_id imx53_tzic_of_match[] __initconst = {
{ /* sentinel */ }
};
+static const struct of_device_id imx53_gpio_of_match[] __initconst = {
+ { .compatible = "fsl,imx53-gpio", },
+ { /* sentinel */ }
+};
+
static const struct of_device_id imx53_iomuxc_of_match[] __initconst = {
{ .compatible = "fsl,imx53-iomuxc", },
{ /* sentinel */ }
@@ -90,12 +95,28 @@ static void __init imx53_ard_eim_config(void)
static void __init imx53_dt_init(void)
{
+ int gpio_irq = MXC_INTERNAL_IRQS + ARCH_NR_GPIOS;
+
if (of_machine_is_compatible("fsl,imx53-ard"))
imx53_ard_eim_config();
mxc_iomuxc_dt_init(imx53_iomuxc_of_match);
irq_domain_generate_simple(imx53_tzic_of_match, MX53_TZIC_BASE_ADDR, 0);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO1_BASE_ADDR, gpio_irq);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO2_BASE_ADDR, gpio_irq);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO3_BASE_ADDR, gpio_irq);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO4_BASE_ADDR, gpio_irq);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO5_BASE_ADDR, gpio_irq);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO6_BASE_ADDR, gpio_irq);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO7_BASE_ADDR, gpio_irq);
of_platform_populate(NULL, of_default_bus_match_table,
imx53_auxdata_lookup, NULL);
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index 0097048..6dd025e 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -2187,25 +2187,10 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
pdata = netdev_priv(dev);
- if (np) {
- irq_gpio = of_get_named_gpio(np, "smsc-int-gpios", 0);
- retval = gpio_request_one(irq_gpio, GPIOF_IN, "smsc-int-gpio");
- if (!retval)
- dev->irq = gpio_to_irq(irq_gpio);
- } else {
- irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (irq_res) {
- dev->irq = irq_res->start;
- irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
- } else {
- retval = -ENODEV;
- }
- }
-
- if (retval) {
- SMSC_WARN(pdata, probe, "Error smsc911x irq not found");
- retval = -EINVAL;
- goto out_free_netdev_2;
+ irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (irq_res) {
+ dev->irq = irq_res->start;
+ irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
}
pdata->ioaddr = ioremap_nocache(res->start, res_size);
> > + } else {
> > + irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > + if (irq_res) {
> > + dev->irq = irq_res->start;
> > + irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> > + } else {
> > + retval = -ENODEV;
> > + }
> > + }
> >
> > - pdata->dev = dev;
> > - pdata->msg_enable = ((1 << debug) - 1);
> > + if (retval) {
> > + SMSC_WARN(pdata, probe, "Error smsc911x irq not found");
> > + retval = -EINVAL;
> > + goto out_free_netdev_2;
> > + }
> >
> > + pdata->ioaddr = ioremap_nocache(res->start, res_size);
> > if (pdata->ioaddr == NULL) {
> > SMSC_WARN(pdata, probe, "Error smsc911x base address invalid");
> > retval = -ENOMEM;
> > goto out_free_netdev_2;
> > }
> >
> > + pdata->dev = dev;
> > + pdata->msg_enable = ((1 << debug) - 1);
> > +
> > + retval = smsc911x_probe_config_dt(&pdata->config, np);
> > + if (retval && config) {
> > + /* copy config parameters across to pdata */
> > + memcpy(&pdata->config, config, sizeof(pdata->config));
> > + retval = 0;
> > + }
> > +
> > + if (retval) {
> > + SMSC_WARN(pdata, probe, "Error smsc911x config not found");
> > + goto out_unmap_io_3;
> > + }
> > +
> > /* assume standard, non-shifted, access to HW registers */
> > pdata->ops = &standard_smsc911x_ops;
> > /* apply the right access if shifting is needed */
> > - if (config->shift)
> > + if (pdata->config.shift)
> > pdata->ops = &shifted_smsc911x_ops;
> >
> > retval = smsc911x_init(dev);
> > @@ -2314,6 +2380,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
> > #define SMSC911X_PM_OPS NULL
> > #endif
> >
> > +static const struct of_device_id smsc_dt_ids[] = {
> > + { .compatible = "smsc,lan", },
>
> As mentioned above, "smsc,lan" is far too generic.
>
Again
static const struct of_device_id smsc_dt_ids[] = {
{ .compatible = "smsc,lan9", },
{ /* sentinel */ }
};
Or:
static const struct of_device_id smsc_dt_ids[] = {
{ .compatible = "smsc,lan91", },
{ .compatible = "smsc,lan92", },
{ /* sentinel */ }
};
You pick :)
--
Regards,
Shawn
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-26 1:01 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-26 1:01 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > It adds device tree probe support for smsc911x driver.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > ---
> > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > 2 files changed, 132 insertions(+), 25 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > new file mode 100644
> > index 0000000..1920695
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > @@ -0,0 +1,34 @@
> > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > +
> > +Required properties:
> > +- compatible : Should be "smsc,lan<model>""smsc,lan"
>
> Drop "smsc,lan". That's far too generic.
>
The following devices are supported by the driver.
LAN9115, LAN9116, LAN9117, LAN9118
LAN9215, LAN9216, LAN9217, LAN9218
LAN9210, LAN9211
LAN9220, LAN9221
If we only keep specific <model> as the compatible, we will have a
long match table which is actually used nowhere to distinguish the
device.
So we need some level generic compatible to save the meaningless
long match table. What about:
static const struct of_device_id smsc_dt_ids[] = {
{ .compatible = "smsc,lan9", },
{ /* sentinel */ }
};
Or:
static const struct of_device_id smsc_dt_ids[] = {
{ .compatible = "smsc,lan91", },
{ .compatible = "smsc,lan92", },
{ /* sentinel */ }
};
> > +- reg : Address and length of the io space for SMSC LAN
> > +- smsc-int-gpios : Should specify the GPIO for SMSC LAN interrupt line
>
> This looks broken. Shouldn't this be specified as a normal
> "interrupts" property?
>
> > +- phy-mode : String, operation mode of the PHY interface.
> > + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> > + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
> > +
> > +Optional properties:
> > +- smsc,irq-active-high : Indicates the IRQ polarity is active-low
> > +- smsc,irq-push-pull : Indicates the IRQ type is push-pull
> > +- smsc,register-needs-shift : Indicates the register access needs shift
> > +- smsc,access-in-32bit : Indicates the access to controller is in 32-bit
> > + mode
>
> Currently, reg-io-width and reg-shift are being used to manipulate
> register access on ns16550 serial ports. The same thing can be used
> here. See bindings/tty/serial/of-serial.txt
>
They are not exactly same. reg-io-width and reg-shift in of-serial.txt
are giving a number, while register-needs-shift and access-in-32bit
here just tells a flag. But if you have a strong position to make
them consistent, I can change them to reg-io-width and reg-shift and
get smsc911x parse numbers instead of flags.
>
> > +- smsc,force-internal-phy : Forces SMSC LAN controller to use
> > + internal PHY
> > +- smsc,force-external-phy : Forces SMSC LAN controller to use
> > + external PHY
>
> I would expect using an external phy would also expect a phy-device
> property to connect to the phy node.
>
I do not understand the details. But from the comment below in the
code, I guess the "external phy" is not external?
/* Autodetects and enables external phy if present on supported chips.
* autodetection can be overridden by specifying SMSC911X_FORCE_INTERNAL_PHY
* or SMSC911X_FORCE_EXTERNAL_PHY in the platform_data flags. */
> > +- smsc,save-mac-address : Indicates that mac address needs to be saved
> > + before resetting the controller
> > +- local-mac-address : 6 bytes, mac address
> > +
> > +Examples:
> > +
> > +lan9220 at f4000000 {
> > + compatible = "smsc,lan9220", "smsc,lan";
> > + reg = <0xf4000000 0x2000000>;
> > + phy-mode = "mii";
> > + smsc-int-gpios = <&gpio1 31 0>; /* GPIO2_31 */
> > + smsc,irq-push-pull;
> > + smsc,access-in-32bit;
> > +};
> > diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> > index b9016a3..0097048 100644
> > --- a/drivers/net/smsc911x.c
> > +++ b/drivers/net/smsc911x.c
> > @@ -53,6 +53,10 @@
> > #include <linux/phy.h>
> > #include <linux/smsc911x.h>
> > #include <linux/device.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/of_net.h>
> > #include "smsc911x.h"
> >
> > #define SMSC_CHIPNAME "smsc911x"
> > @@ -2095,25 +2099,67 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
> > .tx_writefifo = smsc911x_tx_writefifo_shift,
> > };
> >
> > +#ifdef CONFIG_OF
> > +static int __devinit smsc911x_probe_config_dt(
> > + struct smsc911x_platform_config *config,
> > + struct device_node *np)
> > +{
> > + const char *mac;
> > +
> > + if (!np)
> > + return -ENODEV;
> > +
> > + config->phy_interface = of_get_phy_mode(np);
> > +
> > + mac = of_get_mac_address(np);
> > + if (mac)
> > + memcpy(config->mac, mac, ETH_ALEN);
> > +
> > + if (of_get_property(np, "smsc,irq-active-high", NULL))
> > + config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
> > +
> > + if (of_get_property(np, "smsc,irq-push-pull", NULL))
> > + config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
> > +
> > + if (of_get_property(np, "smsc,register-needs-shift", NULL))
> > + config->shift = 1;
> > +
> > + if (of_get_property(np, "smsc,access-in-32bit", NULL))
> > + config->flags |= SMSC911X_USE_32BIT;
> > +
> > + if (of_get_property(np, "smsc,force-internal-phy", NULL))
> > + config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
> > +
> > + if (of_get_property(np, "smsc,force-external-phy", NULL))
> > + config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
> > +
> > + if (of_get_property(np, "smsc,save-mac-address", NULL))
> > + config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
> > +
> > + return 0;
> > +}
> > +#else
> > +static inline int smsc911x_probe_config_dt(
> > + struct smsc911x_platform_config *config,
> > + struct device_node *np)
> > +{
> > + return -ENODEV;
> > +}
> > +#endif /* CONFIG_OF */
> > +
> > static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > {
> > + struct device_node *np = pdev->dev.of_node;
> > struct net_device *dev;
> > struct smsc911x_data *pdata;
> > struct smsc911x_platform_config *config = pdev->dev.platform_data;
> > struct resource *res, *irq_res;
> > unsigned int intcfg = 0;
> > - int res_size, irq_flags;
> > - int retval;
> > + int irq_gpio, res_size, irq_flags = 0;
> > + int retval = 0;
> >
> > pr_info("Driver version %s\n", SMSC_DRV_VERSION);
> >
> > - /* platform data specifies irq & dynamic bus configuration */
> > - if (!pdev->dev.platform_data) {
> > - pr_warn("platform_data not provided\n");
> > - retval = -ENODEV;
> > - goto out_0;
> > - }
> > -
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > "smsc911x-memory");
> > if (!res)
> > @@ -2125,13 +2171,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > }
> > res_size = resource_size(res);
> >
> > - irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > - if (!irq_res) {
> > - pr_warn("Could not allocate irq resource\n");
> > - retval = -ENODEV;
> > - goto out_0;
> > - }
> > -
>
> This should still work for the device-tree situation. Why remove it?
>
> > if (!request_mem_region(res->start, res_size, SMSC_CHIPNAME)) {
> > retval = -EBUSY;
> > goto out_0;
> > @@ -2148,26 +2187,53 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> >
> > pdata = netdev_priv(dev);
> >
> > - dev->irq = irq_res->start;
> > - irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> > - pdata->ioaddr = ioremap_nocache(res->start, res_size);
> > -
> > - /* copy config parameters across to pdata */
> > - memcpy(&pdata->config, config, sizeof(pdata->config));
> > + if (np) {
> > + irq_gpio = of_get_named_gpio(np, "smsc-int-gpios", 0);
> > + retval = gpio_request_one(irq_gpio, GPIOF_IN, "smsc-int-gpio");
> > + if (!retval)
> > + dev->irq = gpio_to_irq(irq_gpio);
>
> Yeah, that's definitely the wrong way to handle this. If the
> device it wired to a gpio controller, then the gpio controller also
> need to be an interrupt controller to ensure that it can map interrupt
> numbers.
>
Here it is. Please let me know if it is what you mean.
diff --git a/arch/arm/boot/dts/imx53-ard.dts b/arch/arm/boot/dts/imx53-ard.dts
index 6a007f1..0b17af8 100644
--- a/arch/arm/boot/dts/imx53-ard.dts
+++ b/arch/arm/boot/dts/imx53-ard.dts
@@ -320,7 +320,8 @@
compatible = "smsc,lan9220", "smsc,lan";
reg = <0xf4000000 0x2000000>;
phy-mode = "mii";
- smsc-int-gpios = <&gpio1 31 0>; /* GPIO2_31 */
+ interrupt-parent = <&gpio1>;
+ interrupts = <31>;
smsc,irq-push-pull;
smsc,access-in-32bit;
};
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index 746221c..224f67f 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -89,6 +89,8 @@
interrupts = <50 51>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
gpio1: gpio at 53f88000 { /* GPIO2 */
@@ -97,6 +99,8 @@
interrupts = <52 53>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
gpio2: gpio at 53f8c000 { /* GPIO3 */
@@ -105,6 +109,8 @@
interrupts = <54 55>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
gpio3: gpio at 53f90000 { /* GPIO4 */
@@ -113,6 +119,8 @@
interrupts = <56 57>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
wdt at 53f98000 { /* WDOG1 */
@@ -950,6 +958,8 @@
interrupts = <103 104>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
gpio5: gpio at 53fe0000 { /* GPIO6 */
@@ -958,6 +968,8 @@
interrupts = <105 106>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
gpio6: gpio at 53fe4000 { /* GPIO7 */
@@ -966,6 +978,8 @@
interrupts = <107 108>;
gpio-controller;
#gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
};
i2c at 53fec000 { /* I2C3 */
diff --git a/arch/arm/mach-mx5/imx53-dt.c b/arch/arm/mach-mx5/imx53-dt.c
index ac06f04..04dc1ab 100644
--- a/arch/arm/mach-mx5/imx53-dt.c
+++ b/arch/arm/mach-mx5/imx53-dt.c
@@ -51,6 +51,11 @@ static const struct of_device_id imx53_tzic_of_match[] __initconst = {
{ /* sentinel */ }
};
+static const struct of_device_id imx53_gpio_of_match[] __initconst = {
+ { .compatible = "fsl,imx53-gpio", },
+ { /* sentinel */ }
+};
+
static const struct of_device_id imx53_iomuxc_of_match[] __initconst = {
{ .compatible = "fsl,imx53-iomuxc", },
{ /* sentinel */ }
@@ -90,12 +95,28 @@ static void __init imx53_ard_eim_config(void)
static void __init imx53_dt_init(void)
{
+ int gpio_irq = MXC_INTERNAL_IRQS + ARCH_NR_GPIOS;
+
if (of_machine_is_compatible("fsl,imx53-ard"))
imx53_ard_eim_config();
mxc_iomuxc_dt_init(imx53_iomuxc_of_match);
irq_domain_generate_simple(imx53_tzic_of_match, MX53_TZIC_BASE_ADDR, 0);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO1_BASE_ADDR, gpio_irq);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO2_BASE_ADDR, gpio_irq);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO3_BASE_ADDR, gpio_irq);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO4_BASE_ADDR, gpio_irq);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO5_BASE_ADDR, gpio_irq);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO6_BASE_ADDR, gpio_irq);
+ gpio_irq -= 32;
+ irq_domain_generate_simple(imx53_gpio_of_match, MX53_GPIO7_BASE_ADDR, gpio_irq);
of_platform_populate(NULL, of_default_bus_match_table,
imx53_auxdata_lookup, NULL);
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index 0097048..6dd025e 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -2187,25 +2187,10 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
pdata = netdev_priv(dev);
- if (np) {
- irq_gpio = of_get_named_gpio(np, "smsc-int-gpios", 0);
- retval = gpio_request_one(irq_gpio, GPIOF_IN, "smsc-int-gpio");
- if (!retval)
- dev->irq = gpio_to_irq(irq_gpio);
- } else {
- irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (irq_res) {
- dev->irq = irq_res->start;
- irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
- } else {
- retval = -ENODEV;
- }
- }
-
- if (retval) {
- SMSC_WARN(pdata, probe, "Error smsc911x irq not found");
- retval = -EINVAL;
- goto out_free_netdev_2;
+ irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (irq_res) {
+ dev->irq = irq_res->start;
+ irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
}
pdata->ioaddr = ioremap_nocache(res->start, res_size);
> > + } else {
> > + irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > + if (irq_res) {
> > + dev->irq = irq_res->start;
> > + irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> > + } else {
> > + retval = -ENODEV;
> > + }
> > + }
> >
> > - pdata->dev = dev;
> > - pdata->msg_enable = ((1 << debug) - 1);
> > + if (retval) {
> > + SMSC_WARN(pdata, probe, "Error smsc911x irq not found");
> > + retval = -EINVAL;
> > + goto out_free_netdev_2;
> > + }
> >
> > + pdata->ioaddr = ioremap_nocache(res->start, res_size);
> > if (pdata->ioaddr == NULL) {
> > SMSC_WARN(pdata, probe, "Error smsc911x base address invalid");
> > retval = -ENOMEM;
> > goto out_free_netdev_2;
> > }
> >
> > + pdata->dev = dev;
> > + pdata->msg_enable = ((1 << debug) - 1);
> > +
> > + retval = smsc911x_probe_config_dt(&pdata->config, np);
> > + if (retval && config) {
> > + /* copy config parameters across to pdata */
> > + memcpy(&pdata->config, config, sizeof(pdata->config));
> > + retval = 0;
> > + }
> > +
> > + if (retval) {
> > + SMSC_WARN(pdata, probe, "Error smsc911x config not found");
> > + goto out_unmap_io_3;
> > + }
> > +
> > /* assume standard, non-shifted, access to HW registers */
> > pdata->ops = &standard_smsc911x_ops;
> > /* apply the right access if shifting is needed */
> > - if (config->shift)
> > + if (pdata->config.shift)
> > pdata->ops = &shifted_smsc911x_ops;
> >
> > retval = smsc911x_init(dev);
> > @@ -2314,6 +2380,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
> > #define SMSC911X_PM_OPS NULL
> > #endif
> >
> > +static const struct of_device_id smsc_dt_ids[] = {
> > + { .compatible = "smsc,lan", },
>
> As mentioned above, "smsc,lan" is far too generic.
>
Again
static const struct of_device_id smsc_dt_ids[] = {
{ .compatible = "smsc,lan9", },
{ /* sentinel */ }
};
Or:
static const struct of_device_id smsc_dt_ids[] = {
{ .compatible = "smsc,lan91", },
{ .compatible = "smsc,lan92", },
{ /* sentinel */ }
};
You pick :)
--
Regards,
Shawn
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
2011-07-26 1:01 ` Shawn Guo
@ 2011-07-26 1:16 ` Nicolas Pitre
-1 siblings, 0 replies; 53+ messages in thread
From: Nicolas Pitre @ 2011-07-26 1:16 UTC (permalink / raw)
To: Shawn Guo
Cc: Grant Likely, patches, netdev, devicetree-discuss,
Steve Glendinning, Shawn Guo, David S. Miller, linux-arm-kernel
On Tue, 26 Jul 2011, Shawn Guo wrote:
> On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > It adds device tree probe support for smsc911x driver.
> > >
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > Cc: David S. Miller <davem@davemloft.net>
> > > ---
> > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > new file mode 100644
> > > index 0000000..1920695
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > @@ -0,0 +1,34 @@
> > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > +
> > > +Required properties:
> > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> >
> > Drop "smsc,lan". That's far too generic.
> >
> The following devices are supported by the driver.
>
> LAN9115, LAN9116, LAN9117, LAN9118
> LAN9215, LAN9216, LAN9217, LAN9218
> LAN9210, LAN9211
> LAN9220, LAN9221
>
> If we only keep specific <model> as the compatible, we will have a
> long match table which is actually used nowhere to distinguish the
> device.
>
> So we need some level generic compatible to save the meaningless
> long match table. What about:
>
> static const struct of_device_id smsc_dt_ids[] = {
> { .compatible = "smsc,lan9", },
> { /* sentinel */ }
> };
>
> Or:
>
> static const struct of_device_id smsc_dt_ids[] = {
> { .compatible = "smsc,lan91", },
> { .compatible = "smsc,lan92", },
> { /* sentinel */ }
> };
None of this unambiguously distinguish the devices supported by this
driver and the smc91x driver which supports LAN91C92, LAN91C94,
LAN91C95, LAN91C96, LAN91C100, LAN91C110.
Nicolas
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-26 1:16 ` Nicolas Pitre
0 siblings, 0 replies; 53+ messages in thread
From: Nicolas Pitre @ 2011-07-26 1:16 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 26 Jul 2011, Shawn Guo wrote:
> On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > It adds device tree probe support for smsc911x driver.
> > >
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > Cc: David S. Miller <davem@davemloft.net>
> > > ---
> > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > new file mode 100644
> > > index 0000000..1920695
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > @@ -0,0 +1,34 @@
> > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > +
> > > +Required properties:
> > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> >
> > Drop "smsc,lan". That's far too generic.
> >
> The following devices are supported by the driver.
>
> LAN9115, LAN9116, LAN9117, LAN9118
> LAN9215, LAN9216, LAN9217, LAN9218
> LAN9210, LAN9211
> LAN9220, LAN9221
>
> If we only keep specific <model> as the compatible, we will have a
> long match table which is actually used nowhere to distinguish the
> device.
>
> So we need some level generic compatible to save the meaningless
> long match table. What about:
>
> static const struct of_device_id smsc_dt_ids[] = {
> { .compatible = "smsc,lan9", },
> { /* sentinel */ }
> };
>
> Or:
>
> static const struct of_device_id smsc_dt_ids[] = {
> { .compatible = "smsc,lan91", },
> { .compatible = "smsc,lan92", },
> { /* sentinel */ }
> };
None of this unambiguously distinguish the devices supported by this
driver and the smc91x driver which supports LAN91C92, LAN91C94,
LAN91C95, LAN91C96, LAN91C100, LAN91C110.
Nicolas
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
2011-07-26 1:16 ` Nicolas Pitre
@ 2011-07-26 1:30 ` Shawn Guo
-1 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-26 1:30 UTC (permalink / raw)
To: Nicolas Pitre
Cc: patches-QSEj5FYQhm4dnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Steve Glendinning,
David S. Miller,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> On Tue, 26 Jul 2011, Shawn Guo wrote:
>
> > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > It adds device tree probe support for smsc911x driver.
> > > >
> > > > Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > > Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> > > > Cc: Steve Glendinning <steve.glendinning-sdUf+H5yV5I@public.gmane.org>
> > > > Cc: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> > > > ---
> > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > new file mode 100644
> > > > index 0000000..1920695
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > @@ -0,0 +1,34 @@
> > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > +
> > > > +Required properties:
> > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > >
> > > Drop "smsc,lan". That's far too generic.
> > >
> > The following devices are supported by the driver.
> >
> > LAN9115, LAN9116, LAN9117, LAN9118
> > LAN9215, LAN9216, LAN9217, LAN9218
> > LAN9210, LAN9211
> > LAN9220, LAN9221
> >
> > If we only keep specific <model> as the compatible, we will have a
> > long match table which is actually used nowhere to distinguish the
> > device.
> >
> > So we need some level generic compatible to save the meaningless
> > long match table. What about:
> >
> > static const struct of_device_id smsc_dt_ids[] = {
> > { .compatible = "smsc,lan9", },
> > { /* sentinel */ }
> > };
> >
> > Or:
> >
> > static const struct of_device_id smsc_dt_ids[] = {
> > { .compatible = "smsc,lan91", },
> > { .compatible = "smsc,lan92", },
> > { /* sentinel */ }
> > };
>
> None of this unambiguously distinguish the devices supported by this
> driver and the smc91x driver which supports LAN91C92, LAN91C94,
> LAN91C95, LAN91C96, LAN91C100, LAN91C110.
>
So you suggest to make a long list to explicitly tell the device type
that the driver supports?
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-26 1:30 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-26 1:30 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> On Tue, 26 Jul 2011, Shawn Guo wrote:
>
> > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > It adds device tree probe support for smsc911x driver.
> > > >
> > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > Cc: David S. Miller <davem@davemloft.net>
> > > > ---
> > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > new file mode 100644
> > > > index 0000000..1920695
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > @@ -0,0 +1,34 @@
> > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > +
> > > > +Required properties:
> > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > >
> > > Drop "smsc,lan". That's far too generic.
> > >
> > The following devices are supported by the driver.
> >
> > LAN9115, LAN9116, LAN9117, LAN9118
> > LAN9215, LAN9216, LAN9217, LAN9218
> > LAN9210, LAN9211
> > LAN9220, LAN9221
> >
> > If we only keep specific <model> as the compatible, we will have a
> > long match table which is actually used nowhere to distinguish the
> > device.
> >
> > So we need some level generic compatible to save the meaningless
> > long match table. What about:
> >
> > static const struct of_device_id smsc_dt_ids[] = {
> > { .compatible = "smsc,lan9", },
> > { /* sentinel */ }
> > };
> >
> > Or:
> >
> > static const struct of_device_id smsc_dt_ids[] = {
> > { .compatible = "smsc,lan91", },
> > { .compatible = "smsc,lan92", },
> > { /* sentinel */ }
> > };
>
> None of this unambiguously distinguish the devices supported by this
> driver and the smc91x driver which supports LAN91C92, LAN91C94,
> LAN91C95, LAN91C96, LAN91C100, LAN91C110.
>
So you suggest to make a long list to explicitly tell the device type
that the driver supports?
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
2011-07-26 1:30 ` Shawn Guo
@ 2011-07-26 2:28 ` Nicolas Pitre
-1 siblings, 0 replies; 53+ messages in thread
From: Nicolas Pitre @ 2011-07-26 2:28 UTC (permalink / raw)
To: Shawn Guo
Cc: Grant Likely, patches, netdev, devicetree-discuss,
Steve Glendinning, Shawn Guo, David S. Miller, linux-arm-kernel
On Tue, 26 Jul 2011, Shawn Guo wrote:
> On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> > On Tue, 26 Jul 2011, Shawn Guo wrote:
> >
> > > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > > It adds device tree probe support for smsc911x driver.
> > > > >
> > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > ---
> > > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > new file mode 100644
> > > > > index 0000000..1920695
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > @@ -0,0 +1,34 @@
> > > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > > +
> > > > > +Required properties:
> > > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > > >
> > > > Drop "smsc,lan". That's far too generic.
> > > >
> > > The following devices are supported by the driver.
> > >
> > > LAN9115, LAN9116, LAN9117, LAN9118
> > > LAN9215, LAN9216, LAN9217, LAN9218
> > > LAN9210, LAN9211
> > > LAN9220, LAN9221
> > >
> > > If we only keep specific <model> as the compatible, we will have a
> > > long match table which is actually used nowhere to distinguish the
> > > device.
> > >
> > > So we need some level generic compatible to save the meaningless
> > > long match table. What about:
> > >
> > > static const struct of_device_id smsc_dt_ids[] = {
> > > { .compatible = "smsc,lan9", },
> > > { /* sentinel */ }
> > > };
> > >
> > > Or:
> > >
> > > static const struct of_device_id smsc_dt_ids[] = {
> > > { .compatible = "smsc,lan91", },
> > > { .compatible = "smsc,lan92", },
> > > { /* sentinel */ }
> > > };
> >
> > None of this unambiguously distinguish the devices supported by this
> > driver and the smc91x driver which supports LAN91C92, LAN91C94,
> > LAN91C95, LAN91C96, LAN91C100, LAN91C110.
> >
> So you suggest to make a long list to explicitly tell the device type
> that the driver supports?
I'm not suggesting anything. :-) I'm merely pointing out that the
above .compatible = "smsc,lan9" or .compatible = "smsc,lan91" are too
generic given that there is another driver with different devices to
which they could also apply.
Nicolas
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-26 2:28 ` Nicolas Pitre
0 siblings, 0 replies; 53+ messages in thread
From: Nicolas Pitre @ 2011-07-26 2:28 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 26 Jul 2011, Shawn Guo wrote:
> On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> > On Tue, 26 Jul 2011, Shawn Guo wrote:
> >
> > > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > > It adds device tree probe support for smsc911x driver.
> > > > >
> > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > ---
> > > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > new file mode 100644
> > > > > index 0000000..1920695
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > @@ -0,0 +1,34 @@
> > > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > > +
> > > > > +Required properties:
> > > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > > >
> > > > Drop "smsc,lan". That's far too generic.
> > > >
> > > The following devices are supported by the driver.
> > >
> > > LAN9115, LAN9116, LAN9117, LAN9118
> > > LAN9215, LAN9216, LAN9217, LAN9218
> > > LAN9210, LAN9211
> > > LAN9220, LAN9221
> > >
> > > If we only keep specific <model> as the compatible, we will have a
> > > long match table which is actually used nowhere to distinguish the
> > > device.
> > >
> > > So we need some level generic compatible to save the meaningless
> > > long match table. What about:
> > >
> > > static const struct of_device_id smsc_dt_ids[] = {
> > > { .compatible = "smsc,lan9", },
> > > { /* sentinel */ }
> > > };
> > >
> > > Or:
> > >
> > > static const struct of_device_id smsc_dt_ids[] = {
> > > { .compatible = "smsc,lan91", },
> > > { .compatible = "smsc,lan92", },
> > > { /* sentinel */ }
> > > };
> >
> > None of this unambiguously distinguish the devices supported by this
> > driver and the smc91x driver which supports LAN91C92, LAN91C94,
> > LAN91C95, LAN91C96, LAN91C100, LAN91C110.
> >
> So you suggest to make a long list to explicitly tell the device type
> that the driver supports?
I'm not suggesting anything. :-) I'm merely pointing out that the
above .compatible = "smsc,lan9" or .compatible = "smsc,lan91" are too
generic given that there is another driver with different devices to
which they could also apply.
Nicolas
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
2011-07-26 2:28 ` Nicolas Pitre
(?)
@ 2011-07-29 2:36 ` Shawn Guo
-1 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 2:36 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Grant Likely, patches, netdev, devicetree-discuss,
Steve Glendinning, Shawn Guo, David S. Miller, linux-arm-kernel
On Mon, Jul 25, 2011 at 10:28:05PM -0400, Nicolas Pitre wrote:
> On Tue, 26 Jul 2011, Shawn Guo wrote:
>
> > On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> > > On Tue, 26 Jul 2011, Shawn Guo wrote:
> > >
> > > > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > > > It adds device tree probe support for smsc911x driver.
> > > > > >
> > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > > ---
> > > > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > new file mode 100644
> > > > > > index 0000000..1920695
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > @@ -0,0 +1,34 @@
> > > > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > > > +
> > > > > > +Required properties:
> > > > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > > > >
> > > > > Drop "smsc,lan". That's far too generic.
> > > > >
> > > > The following devices are supported by the driver.
> > > >
> > > > LAN9115, LAN9116, LAN9117, LAN9118
> > > > LAN9215, LAN9216, LAN9217, LAN9218
> > > > LAN9210, LAN9211
> > > > LAN9220, LAN9221
> > > >
> > > > If we only keep specific <model> as the compatible, we will have a
> > > > long match table which is actually used nowhere to distinguish the
> > > > device.
> > > >
> > > > So we need some level generic compatible to save the meaningless
> > > > long match table. What about:
> > > >
> > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > { .compatible = "smsc,lan9", },
> > > > { /* sentinel */ }
> > > > };
> > > >
> > > > Or:
> > > >
> > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > { .compatible = "smsc,lan91", },
> > > > { .compatible = "smsc,lan92", },
> > > > { /* sentinel */ }
> > > > };
> > >
> > > None of this unambiguously distinguish the devices supported by this
> > > driver and the smc91x driver which supports LAN91C92, LAN91C94,
> > > LAN91C95, LAN91C96, LAN91C100, LAN91C110.
> > >
> > So you suggest to make a long list to explicitly tell the device type
> > that the driver supports?
>
> I'm not suggesting anything. :-) I'm merely pointing out that the
> above .compatible = "smsc,lan9" or .compatible = "smsc,lan91" are too
> generic given that there is another driver with different devices to
> which they could also apply.
>
Since I do not get any suggestion here, I'm going to follow the driver
name to use '911' as the model number. Please tell me if there is any
better one.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-29 2:36 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 2:36 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Grant Likely, patches, netdev, devicetree-discuss,
Steve Glendinning, Shawn Guo, David S. Miller, linux-arm-kernel
On Mon, Jul 25, 2011 at 10:28:05PM -0400, Nicolas Pitre wrote:
> On Tue, 26 Jul 2011, Shawn Guo wrote:
>
> > On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> > > On Tue, 26 Jul 2011, Shawn Guo wrote:
> > >
> > > > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > > > It adds device tree probe support for smsc911x driver.
> > > > > >
> > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > > ---
> > > > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > new file mode 100644
> > > > > > index 0000000..1920695
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > @@ -0,0 +1,34 @@
> > > > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > > > +
> > > > > > +Required properties:
> > > > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > > > >
> > > > > Drop "smsc,lan". That's far too generic.
> > > > >
> > > > The following devices are supported by the driver.
> > > >
> > > > LAN9115, LAN9116, LAN9117, LAN9118
> > > > LAN9215, LAN9216, LAN9217, LAN9218
> > > > LAN9210, LAN9211
> > > > LAN9220, LAN9221
> > > >
> > > > If we only keep specific <model> as the compatible, we will have a
> > > > long match table which is actually used nowhere to distinguish the
> > > > device.
> > > >
> > > > So we need some level generic compatible to save the meaningless
> > > > long match table. What about:
> > > >
> > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > { .compatible = "smsc,lan9", },
> > > > { /* sentinel */ }
> > > > };
> > > >
> > > > Or:
> > > >
> > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > { .compatible = "smsc,lan91", },
> > > > { .compatible = "smsc,lan92", },
> > > > { /* sentinel */ }
> > > > };
> > >
> > > None of this unambiguously distinguish the devices supported by this
> > > driver and the smc91x driver which supports LAN91C92, LAN91C94,
> > > LAN91C95, LAN91C96, LAN91C100, LAN91C110.
> > >
> > So you suggest to make a long list to explicitly tell the device type
> > that the driver supports?
>
> I'm not suggesting anything. :-) I'm merely pointing out that the
> above .compatible = "smsc,lan9" or .compatible = "smsc,lan91" are too
> generic given that there is another driver with different devices to
> which they could also apply.
>
Since I do not get any suggestion here, I'm going to follow the driver
name to use '911' as the model number. Please tell me if there is any
better one.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-29 2:36 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 2:36 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 25, 2011 at 10:28:05PM -0400, Nicolas Pitre wrote:
> On Tue, 26 Jul 2011, Shawn Guo wrote:
>
> > On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> > > On Tue, 26 Jul 2011, Shawn Guo wrote:
> > >
> > > > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > > > It adds device tree probe support for smsc911x driver.
> > > > > >
> > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > > ---
> > > > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > new file mode 100644
> > > > > > index 0000000..1920695
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > @@ -0,0 +1,34 @@
> > > > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > > > +
> > > > > > +Required properties:
> > > > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > > > >
> > > > > Drop "smsc,lan". That's far too generic.
> > > > >
> > > > The following devices are supported by the driver.
> > > >
> > > > LAN9115, LAN9116, LAN9117, LAN9118
> > > > LAN9215, LAN9216, LAN9217, LAN9218
> > > > LAN9210, LAN9211
> > > > LAN9220, LAN9221
> > > >
> > > > If we only keep specific <model> as the compatible, we will have a
> > > > long match table which is actually used nowhere to distinguish the
> > > > device.
> > > >
> > > > So we need some level generic compatible to save the meaningless
> > > > long match table. What about:
> > > >
> > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > { .compatible = "smsc,lan9", },
> > > > { /* sentinel */ }
> > > > };
> > > >
> > > > Or:
> > > >
> > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > { .compatible = "smsc,lan91", },
> > > > { .compatible = "smsc,lan92", },
> > > > { /* sentinel */ }
> > > > };
> > >
> > > None of this unambiguously distinguish the devices supported by this
> > > driver and the smc91x driver which supports LAN91C92, LAN91C94,
> > > LAN91C95, LAN91C96, LAN91C100, LAN91C110.
> > >
> > So you suggest to make a long list to explicitly tell the device type
> > that the driver supports?
>
> I'm not suggesting anything. :-) I'm merely pointing out that the
> above .compatible = "smsc,lan9" or .compatible = "smsc,lan91" are too
> generic given that there is another driver with different devices to
> which they could also apply.
>
Since I do not get any suggestion here, I'm going to follow the driver
name to use '911' as the model number. Please tell me if there is any
better one.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v2] net/smsc911x: add device tree probe support
2011-07-25 9:44 ` Shawn Guo
@ 2011-07-29 8:43 ` Shawn Guo
-1 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 8:43 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: patches-QSEj5FYQhm4dnm+yROfE0A,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Steve Glendinning,
David S. Miller,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
It adds device tree probe support for smsc911x driver.
Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: Steve Glendinning <steve.glendinning-sdUf+H5yV5I@public.gmane.org>
Cc: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
---
Changes since v1:
* Instead of getting irq line from gpio number, it use irq domain
to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
* Use 'lan9115' the first model that smsc911x supports in the match
table
* Use reg-shift and reg-io-width which already used in of_serial for
shift and access size binding
Documentation/devicetree/bindings/net/smsc911x.txt | 38 +++++++++
drivers/net/smsc911x.c | 85 +++++++++++++++++---
2 files changed, 112 insertions(+), 11 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/smsc911x.txt
diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt
new file mode 100644
index 0000000..271c454
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc911x.txt
@@ -0,0 +1,38 @@
+* Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
+
+Required properties:
+- compatible : Should be "smsc,lan<model>", "smsc,lan9115"
+- reg : Address and length of the io space for SMSC LAN
+- interrupts : Should contain SMSC LAN interrupt line
+- interrupt-parent : Should be the phandle for the interrupt controller
+ that services interrupts for this device
+- phy-mode : String, operation mode of the PHY interface.
+ Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
+ "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
+
+Optional properties:
+- reg-shift : Specify the quantity to shift the register offsets by
+- reg-io-width : Specify the size (in bytes) of the IO accesses that
+ should be performed on the device. Valid value for SMSC LAN is
+ 2 or 4. If it's omitted or invalid, the size would be 2.
+- smsc,irq-active-high : Indicates the IRQ polarity is active-low
+- smsc,irq-push-pull : Indicates the IRQ type is push-pull
+- smsc,force-internal-phy : Forces SMSC LAN controller to use
+ internal PHY
+- smsc,force-external-phy : Forces SMSC LAN controller to use
+ external PHY
+- smsc,save-mac-address : Indicates that mac address needs to be saved
+ before resetting the controller
+- local-mac-address : 6 bytes, mac address
+
+Examples:
+
+lan9220@f4000000 {
+ compatible = "smsc,lan9220", "smsc,lan9115";
+ reg = <0xf4000000 0x2000000>;
+ phy-mode = "mii";
+ interrupt-parent = <&gpio1>;
+ interrupts = <31>;
+ reg-io-width = <4>;
+ smsc,irq-push-pull;
+};
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index b9016a3..75c08a5 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -53,6 +53,10 @@
#include <linux/phy.h>
#include <linux/smsc911x.h>
#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/of_net.h>
#include "smsc911x.h"
#define SMSC_CHIPNAME "smsc911x"
@@ -2095,8 +2099,58 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
.tx_writefifo = smsc911x_tx_writefifo_shift,
};
+#ifdef CONFIG_OF
+static int __devinit smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ const char *mac;
+ u32 width = 0;
+
+ if (!np)
+ return -ENODEV;
+
+ config->phy_interface = of_get_phy_mode(np);
+
+ mac = of_get_mac_address(np);
+ if (mac)
+ memcpy(config->mac, mac, ETH_ALEN);
+
+ of_property_read_u32(np, "reg-shift", &config->shift);
+
+ of_property_read_u32(np, "reg-io-width", &width);
+ if (width == 4)
+ config->flags |= SMSC911X_USE_32BIT;
+
+ if (of_get_property(np, "smsc,irq-active-high", NULL))
+ config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
+
+ if (of_get_property(np, "smsc,irq-push-pull", NULL))
+ config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
+
+ if (of_get_property(np, "smsc,force-internal-phy", NULL))
+ config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,force-external-phy", NULL))
+ config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,save-mac-address", NULL))
+ config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
+
+ return 0;
+}
+#else
+static inline int smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ return -ENODEV;
+}
+#endif /* CONFIG_OF */
+
static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
{
+ struct device_node *np = pdev->dev.of_node;
struct net_device *dev;
struct smsc911x_data *pdata;
struct smsc911x_platform_config *config = pdev->dev.platform_data;
@@ -2107,13 +2161,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
pr_info("Driver version %s\n", SMSC_DRV_VERSION);
- /* platform data specifies irq & dynamic bus configuration */
- if (!pdev->dev.platform_data) {
- pr_warn("platform_data not provided\n");
- retval = -ENODEV;
- goto out_0;
- }
-
res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"smsc911x-memory");
if (!res)
@@ -2152,9 +2199,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
pdata->ioaddr = ioremap_nocache(res->start, res_size);
- /* copy config parameters across to pdata */
- memcpy(&pdata->config, config, sizeof(pdata->config));
-
pdata->dev = dev;
pdata->msg_enable = ((1 << debug) - 1);
@@ -2164,10 +2208,22 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
goto out_free_netdev_2;
}
+ retval = smsc911x_probe_config_dt(&pdata->config, np);
+ if (retval && config) {
+ /* copy config parameters across to pdata */
+ memcpy(&pdata->config, config, sizeof(pdata->config));
+ retval = 0;
+ }
+
+ if (retval) {
+ SMSC_WARN(pdata, probe, "Error smsc911x config not found");
+ goto out_unmap_io_3;
+ }
+
/* assume standard, non-shifted, access to HW registers */
pdata->ops = &standard_smsc911x_ops;
/* apply the right access if shifting is needed */
- if (config->shift)
+ if (pdata->config.shift)
pdata->ops = &shifted_smsc911x_ops;
retval = smsc911x_init(dev);
@@ -2314,6 +2370,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
#define SMSC911X_PM_OPS NULL
#endif
+static const struct of_device_id smsc911x_dt_ids[] = {
+ { .compatible = "smsc,lan9115", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, smsc911x_dt_ids);
+
static struct platform_driver smsc911x_driver = {
.probe = smsc911x_drv_probe,
.remove = __devexit_p(smsc911x_drv_remove),
@@ -2321,6 +2383,7 @@ static struct platform_driver smsc911x_driver = {
.name = SMSC_CHIPNAME,
.owner = THIS_MODULE,
.pm = SMSC911X_PM_OPS,
+ .of_match_table = smsc911x_dt_ids,
},
};
--
1.7.4.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v2] net/smsc911x: add device tree probe support
@ 2011-07-29 8:43 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 8:43 UTC (permalink / raw)
To: linux-arm-kernel
It adds device tree probe support for smsc911x driver.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Steve Glendinning <steve.glendinning@smsc.com>
Cc: David S. Miller <davem@davemloft.net>
---
Changes since v1:
* Instead of getting irq line from gpio number, it use irq domain
to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
* Use 'lan9115' the first model that smsc911x supports in the match
table
* Use reg-shift and reg-io-width which already used in of_serial for
shift and access size binding
Documentation/devicetree/bindings/net/smsc911x.txt | 38 +++++++++
drivers/net/smsc911x.c | 85 +++++++++++++++++---
2 files changed, 112 insertions(+), 11 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/smsc911x.txt
diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt
new file mode 100644
index 0000000..271c454
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc911x.txt
@@ -0,0 +1,38 @@
+* Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
+
+Required properties:
+- compatible : Should be "smsc,lan<model>", "smsc,lan9115"
+- reg : Address and length of the io space for SMSC LAN
+- interrupts : Should contain SMSC LAN interrupt line
+- interrupt-parent : Should be the phandle for the interrupt controller
+ that services interrupts for this device
+- phy-mode : String, operation mode of the PHY interface.
+ Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
+ "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
+
+Optional properties:
+- reg-shift : Specify the quantity to shift the register offsets by
+- reg-io-width : Specify the size (in bytes) of the IO accesses that
+ should be performed on the device. Valid value for SMSC LAN is
+ 2 or 4. If it's omitted or invalid, the size would be 2.
+- smsc,irq-active-high : Indicates the IRQ polarity is active-low
+- smsc,irq-push-pull : Indicates the IRQ type is push-pull
+- smsc,force-internal-phy : Forces SMSC LAN controller to use
+ internal PHY
+- smsc,force-external-phy : Forces SMSC LAN controller to use
+ external PHY
+- smsc,save-mac-address : Indicates that mac address needs to be saved
+ before resetting the controller
+- local-mac-address : 6 bytes, mac address
+
+Examples:
+
+lan9220 at f4000000 {
+ compatible = "smsc,lan9220", "smsc,lan9115";
+ reg = <0xf4000000 0x2000000>;
+ phy-mode = "mii";
+ interrupt-parent = <&gpio1>;
+ interrupts = <31>;
+ reg-io-width = <4>;
+ smsc,irq-push-pull;
+};
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index b9016a3..75c08a5 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -53,6 +53,10 @@
#include <linux/phy.h>
#include <linux/smsc911x.h>
#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/of_net.h>
#include "smsc911x.h"
#define SMSC_CHIPNAME "smsc911x"
@@ -2095,8 +2099,58 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
.tx_writefifo = smsc911x_tx_writefifo_shift,
};
+#ifdef CONFIG_OF
+static int __devinit smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ const char *mac;
+ u32 width = 0;
+
+ if (!np)
+ return -ENODEV;
+
+ config->phy_interface = of_get_phy_mode(np);
+
+ mac = of_get_mac_address(np);
+ if (mac)
+ memcpy(config->mac, mac, ETH_ALEN);
+
+ of_property_read_u32(np, "reg-shift", &config->shift);
+
+ of_property_read_u32(np, "reg-io-width", &width);
+ if (width == 4)
+ config->flags |= SMSC911X_USE_32BIT;
+
+ if (of_get_property(np, "smsc,irq-active-high", NULL))
+ config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
+
+ if (of_get_property(np, "smsc,irq-push-pull", NULL))
+ config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
+
+ if (of_get_property(np, "smsc,force-internal-phy", NULL))
+ config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,force-external-phy", NULL))
+ config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,save-mac-address", NULL))
+ config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
+
+ return 0;
+}
+#else
+static inline int smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ return -ENODEV;
+}
+#endif /* CONFIG_OF */
+
static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
{
+ struct device_node *np = pdev->dev.of_node;
struct net_device *dev;
struct smsc911x_data *pdata;
struct smsc911x_platform_config *config = pdev->dev.platform_data;
@@ -2107,13 +2161,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
pr_info("Driver version %s\n", SMSC_DRV_VERSION);
- /* platform data specifies irq & dynamic bus configuration */
- if (!pdev->dev.platform_data) {
- pr_warn("platform_data not provided\n");
- retval = -ENODEV;
- goto out_0;
- }
-
res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"smsc911x-memory");
if (!res)
@@ -2152,9 +2199,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
pdata->ioaddr = ioremap_nocache(res->start, res_size);
- /* copy config parameters across to pdata */
- memcpy(&pdata->config, config, sizeof(pdata->config));
-
pdata->dev = dev;
pdata->msg_enable = ((1 << debug) - 1);
@@ -2164,10 +2208,22 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
goto out_free_netdev_2;
}
+ retval = smsc911x_probe_config_dt(&pdata->config, np);
+ if (retval && config) {
+ /* copy config parameters across to pdata */
+ memcpy(&pdata->config, config, sizeof(pdata->config));
+ retval = 0;
+ }
+
+ if (retval) {
+ SMSC_WARN(pdata, probe, "Error smsc911x config not found");
+ goto out_unmap_io_3;
+ }
+
/* assume standard, non-shifted, access to HW registers */
pdata->ops = &standard_smsc911x_ops;
/* apply the right access if shifting is needed */
- if (config->shift)
+ if (pdata->config.shift)
pdata->ops = &shifted_smsc911x_ops;
retval = smsc911x_init(dev);
@@ -2314,6 +2370,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
#define SMSC911X_PM_OPS NULL
#endif
+static const struct of_device_id smsc911x_dt_ids[] = {
+ { .compatible = "smsc,lan9115", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, smsc911x_dt_ids);
+
static struct platform_driver smsc911x_driver = {
.probe = smsc911x_drv_probe,
.remove = __devexit_p(smsc911x_drv_remove),
@@ -2321,6 +2383,7 @@ static struct platform_driver smsc911x_driver = {
.name = SMSC_CHIPNAME,
.owner = THIS_MODULE,
.pm = SMSC911X_PM_OPS,
+ .of_match_table = smsc911x_dt_ids,
},
};
--
1.7.4.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
2011-07-26 1:30 ` Shawn Guo
@ 2011-07-29 15:45 ` Grant Likely
-1 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-07-29 15:45 UTC (permalink / raw)
To: Shawn Guo
Cc: Nicolas Pitre, patches, netdev, devicetree-discuss,
Steve Glendinning, Shawn Guo, David S. Miller, linux-arm-kernel
On Tue, Jul 26, 2011 at 09:30:26AM +0800, Shawn Guo wrote:
> On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> > On Tue, 26 Jul 2011, Shawn Guo wrote:
> >
> > > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > > It adds device tree probe support for smsc911x driver.
> > > > >
> > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > ---
> > > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > new file mode 100644
> > > > > index 0000000..1920695
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > @@ -0,0 +1,34 @@
> > > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > > +
> > > > > +Required properties:
> > > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > > >
> > > > Drop "smsc,lan". That's far too generic.
> > > >
> > > The following devices are supported by the driver.
> > >
> > > LAN9115, LAN9116, LAN9117, LAN9118
> > > LAN9215, LAN9216, LAN9217, LAN9218
> > > LAN9210, LAN9211
> > > LAN9220, LAN9221
> > >
> > > If we only keep specific <model> as the compatible, we will have a
> > > long match table which is actually used nowhere to distinguish the
> > > device.
> > >
> > > So we need some level generic compatible to save the meaningless
> > > long match table. What about:
> > >
> > > static const struct of_device_id smsc_dt_ids[] = {
> > > { .compatible = "smsc,lan9", },
> > > { /* sentinel */ }
> > > };
> > >
> > > Or:
> > >
> > > static const struct of_device_id smsc_dt_ids[] = {
> > > { .compatible = "smsc,lan91", },
> > > { .compatible = "smsc,lan92", },
> > > { /* sentinel */ }
> > > };
> >
> > None of this unambiguously distinguish the devices supported by this
> > driver and the smc91x driver which supports LAN91C92, LAN91C94,
> > LAN91C95, LAN91C96, LAN91C100, LAN91C110.
> >
> So you suggest to make a long list to explicitly tell the device type
> that the driver supports?
If newer devices are 100% backwards compatible with an older device,
then the newer device doesn't need to appear in this list because the
device node can claim compatibility.
If it isn't backwards compatible, then you do need an entry in this
list.
g.
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-29 15:45 ` Grant Likely
0 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-07-29 15:45 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 26, 2011 at 09:30:26AM +0800, Shawn Guo wrote:
> On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> > On Tue, 26 Jul 2011, Shawn Guo wrote:
> >
> > > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > > It adds device tree probe support for smsc911x driver.
> > > > >
> > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > ---
> > > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > new file mode 100644
> > > > > index 0000000..1920695
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > @@ -0,0 +1,34 @@
> > > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > > +
> > > > > +Required properties:
> > > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > > >
> > > > Drop "smsc,lan". That's far too generic.
> > > >
> > > The following devices are supported by the driver.
> > >
> > > LAN9115, LAN9116, LAN9117, LAN9118
> > > LAN9215, LAN9216, LAN9217, LAN9218
> > > LAN9210, LAN9211
> > > LAN9220, LAN9221
> > >
> > > If we only keep specific <model> as the compatible, we will have a
> > > long match table which is actually used nowhere to distinguish the
> > > device.
> > >
> > > So we need some level generic compatible to save the meaningless
> > > long match table. What about:
> > >
> > > static const struct of_device_id smsc_dt_ids[] = {
> > > { .compatible = "smsc,lan9", },
> > > { /* sentinel */ }
> > > };
> > >
> > > Or:
> > >
> > > static const struct of_device_id smsc_dt_ids[] = {
> > > { .compatible = "smsc,lan91", },
> > > { .compatible = "smsc,lan92", },
> > > { /* sentinel */ }
> > > };
> >
> > None of this unambiguously distinguish the devices supported by this
> > driver and the smc91x driver which supports LAN91C92, LAN91C94,
> > LAN91C95, LAN91C96, LAN91C100, LAN91C110.
> >
> So you suggest to make a long list to explicitly tell the device type
> that the driver supports?
If newer devices are 100% backwards compatible with an older device,
then the newer device doesn't need to appear in this list because the
device node can claim compatibility.
If it isn't backwards compatible, then you do need an entry in this
list.
g.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
2011-07-29 2:36 ` Shawn Guo
@ 2011-07-29 15:47 ` Grant Likely
-1 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-07-29 15:47 UTC (permalink / raw)
To: Shawn Guo
Cc: Nicolas Pitre, patches, netdev, devicetree-discuss,
Steve Glendinning, Shawn Guo, David S. Miller, linux-arm-kernel
On Fri, Jul 29, 2011 at 10:36:26AM +0800, Shawn Guo wrote:
> On Mon, Jul 25, 2011 at 10:28:05PM -0400, Nicolas Pitre wrote:
> > On Tue, 26 Jul 2011, Shawn Guo wrote:
> >
> > > On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> > > > On Tue, 26 Jul 2011, Shawn Guo wrote:
> > > >
> > > > > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > > > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > > > > It adds device tree probe support for smsc911x driver.
> > > > > > >
> > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > > > ---
> > > > > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > > > > >
> > > > > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > new file mode 100644
> > > > > > > index 0000000..1920695
> > > > > > > --- /dev/null
> > > > > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > @@ -0,0 +1,34 @@
> > > > > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > > > > +
> > > > > > > +Required properties:
> > > > > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > > > > >
> > > > > > Drop "smsc,lan". That's far too generic.
> > > > > >
> > > > > The following devices are supported by the driver.
> > > > >
> > > > > LAN9115, LAN9116, LAN9117, LAN9118
> > > > > LAN9215, LAN9216, LAN9217, LAN9218
> > > > > LAN9210, LAN9211
> > > > > LAN9220, LAN9221
> > > > >
> > > > > If we only keep specific <model> as the compatible, we will have a
> > > > > long match table which is actually used nowhere to distinguish the
> > > > > device.
> > > > >
> > > > > So we need some level generic compatible to save the meaningless
> > > > > long match table. What about:
> > > > >
> > > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > > { .compatible = "smsc,lan9", },
> > > > > { /* sentinel */ }
> > > > > };
> > > > >
> > > > > Or:
> > > > >
> > > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > > { .compatible = "smsc,lan91", },
> > > > > { .compatible = "smsc,lan92", },
> > > > > { /* sentinel */ }
> > > > > };
> > > >
> > > > None of this unambiguously distinguish the devices supported by this
> > > > driver and the smc91x driver which supports LAN91C92, LAN91C94,
> > > > LAN91C95, LAN91C96, LAN91C100, LAN91C110.
> > > >
> > > So you suggest to make a long list to explicitly tell the device type
> > > that the driver supports?
> >
> > I'm not suggesting anything. :-) I'm merely pointing out that the
> > above .compatible = "smsc,lan9" or .compatible = "smsc,lan91" are too
> > generic given that there is another driver with different devices to
> > which they could also apply.
> >
> Since I do not get any suggestion here, I'm going to follow the driver
> name to use '911' as the model number. Please tell me if there is any
> better one.
What is the manufacturer part name? lan9111 or lan91c11? The
manufacturer's documented part number is almost always preferred.
g.
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-29 15:47 ` Grant Likely
0 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-07-29 15:47 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jul 29, 2011 at 10:36:26AM +0800, Shawn Guo wrote:
> On Mon, Jul 25, 2011 at 10:28:05PM -0400, Nicolas Pitre wrote:
> > On Tue, 26 Jul 2011, Shawn Guo wrote:
> >
> > > On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> > > > On Tue, 26 Jul 2011, Shawn Guo wrote:
> > > >
> > > > > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > > > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > > > > It adds device tree probe support for smsc911x driver.
> > > > > > >
> > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > > > ---
> > > > > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > > > > >
> > > > > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > new file mode 100644
> > > > > > > index 0000000..1920695
> > > > > > > --- /dev/null
> > > > > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > @@ -0,0 +1,34 @@
> > > > > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > > > > +
> > > > > > > +Required properties:
> > > > > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > > > > >
> > > > > > Drop "smsc,lan". That's far too generic.
> > > > > >
> > > > > The following devices are supported by the driver.
> > > > >
> > > > > LAN9115, LAN9116, LAN9117, LAN9118
> > > > > LAN9215, LAN9216, LAN9217, LAN9218
> > > > > LAN9210, LAN9211
> > > > > LAN9220, LAN9221
> > > > >
> > > > > If we only keep specific <model> as the compatible, we will have a
> > > > > long match table which is actually used nowhere to distinguish the
> > > > > device.
> > > > >
> > > > > So we need some level generic compatible to save the meaningless
> > > > > long match table. What about:
> > > > >
> > > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > > { .compatible = "smsc,lan9", },
> > > > > { /* sentinel */ }
> > > > > };
> > > > >
> > > > > Or:
> > > > >
> > > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > > { .compatible = "smsc,lan91", },
> > > > > { .compatible = "smsc,lan92", },
> > > > > { /* sentinel */ }
> > > > > };
> > > >
> > > > None of this unambiguously distinguish the devices supported by this
> > > > driver and the smc91x driver which supports LAN91C92, LAN91C94,
> > > > LAN91C95, LAN91C96, LAN91C100, LAN91C110.
> > > >
> > > So you suggest to make a long list to explicitly tell the device type
> > > that the driver supports?
> >
> > I'm not suggesting anything. :-) I'm merely pointing out that the
> > above .compatible = "smsc,lan9" or .compatible = "smsc,lan91" are too
> > generic given that there is another driver with different devices to
> > which they could also apply.
> >
> Since I do not get any suggestion here, I'm going to follow the driver
> name to use '911' as the model number. Please tell me if there is any
> better one.
What is the manufacturer part name? lan9111 or lan91c11? The
manufacturer's documented part number is almost always preferred.
g.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2] net/smsc911x: add device tree probe support
2011-07-29 8:43 ` Shawn Guo
@ 2011-07-29 15:53 ` Grant Likely
-1 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-07-29 15:53 UTC (permalink / raw)
To: Shawn Guo
Cc: netdev, devicetree-discuss, linux-arm-kernel, patches,
Steve Glendinning, David S. Miller
On Fri, Jul 29, 2011 at 04:43:16PM +0800, Shawn Guo wrote:
> It adds device tree probe support for smsc911x driver.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Steve Glendinning <steve.glendinning@smsc.com>
> Cc: David S. Miller <davem@davemloft.net>
Some comments below, and I asked a question on the older version about
the actual model name vs. compatible, but otherwise it looks right and
you can add my:
Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> Changes since v1:
> * Instead of getting irq line from gpio number, it use irq domain
> to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> * Use 'lan9115' the first model that smsc911x supports in the match
> table
> * Use reg-shift and reg-io-width which already used in of_serial for
> shift and access size binding
>
> Documentation/devicetree/bindings/net/smsc911x.txt | 38 +++++++++
> drivers/net/smsc911x.c | 85 +++++++++++++++++---
> 2 files changed, 112 insertions(+), 11 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/smsc911x.txt
>
> diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt
> new file mode 100644
> index 0000000..271c454
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/smsc911x.txt
> @@ -0,0 +1,38 @@
> +* Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
> +
> +Required properties:
> +- compatible : Should be "smsc,lan<model>", "smsc,lan9115"
> +- reg : Address and length of the io space for SMSC LAN
> +- interrupts : Should contain SMSC LAN interrupt line
> +- interrupt-parent : Should be the phandle for the interrupt controller
> + that services interrupts for this device
> +- phy-mode : String, operation mode of the PHY interface.
> + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
> +
> +Optional properties:
> +- reg-shift : Specify the quantity to shift the register offsets by
> +- reg-io-width : Specify the size (in bytes) of the IO accesses that
> + should be performed on the device. Valid value for SMSC LAN is
> + 2 or 4. If it's omitted or invalid, the size would be 2.
> +- smsc,irq-active-high : Indicates the IRQ polarity is active-low
Which is it? Active high or active low? The property doesn't match
the description.
> +- smsc,irq-push-pull : Indicates the IRQ type is push-pull
What exactly does "push-pull" mean here?
> +- smsc,force-internal-phy : Forces SMSC LAN controller to use
> + internal PHY
> +- smsc,force-external-phy : Forces SMSC LAN controller to use
> + external PHY
> +- smsc,save-mac-address : Indicates that mac address needs to be saved
> + before resetting the controller
> +- local-mac-address : 6 bytes, mac address
> +
> +Examples:
> +
> +lan9220@f4000000 {
> + compatible = "smsc,lan9220", "smsc,lan9115";
> + reg = <0xf4000000 0x2000000>;
> + phy-mode = "mii";
> + interrupt-parent = <&gpio1>;
> + interrupts = <31>;
> + reg-io-width = <4>;
> + smsc,irq-push-pull;
> +};
> diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> index b9016a3..75c08a5 100644
> --- a/drivers/net/smsc911x.c
> +++ b/drivers/net/smsc911x.c
> @@ -53,6 +53,10 @@
> #include <linux/phy.h>
> #include <linux/smsc911x.h>
> #include <linux/device.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_net.h>
> #include "smsc911x.h"
>
> #define SMSC_CHIPNAME "smsc911x"
> @@ -2095,8 +2099,58 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
> .tx_writefifo = smsc911x_tx_writefifo_shift,
> };
>
> +#ifdef CONFIG_OF
> +static int __devinit smsc911x_probe_config_dt(
> + struct smsc911x_platform_config *config,
> + struct device_node *np)
> +{
> + const char *mac;
> + u32 width = 0;
> +
> + if (!np)
> + return -ENODEV;
> +
> + config->phy_interface = of_get_phy_mode(np);
> +
> + mac = of_get_mac_address(np);
> + if (mac)
> + memcpy(config->mac, mac, ETH_ALEN);
> +
> + of_property_read_u32(np, "reg-shift", &config->shift);
> +
> + of_property_read_u32(np, "reg-io-width", &width);
> + if (width == 4)
> + config->flags |= SMSC911X_USE_32BIT;
> +
> + if (of_get_property(np, "smsc,irq-active-high", NULL))
> + config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
> +
> + if (of_get_property(np, "smsc,irq-push-pull", NULL))
> + config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
> +
> + if (of_get_property(np, "smsc,force-internal-phy", NULL))
> + config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
> +
> + if (of_get_property(np, "smsc,force-external-phy", NULL))
> + config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
> +
> + if (of_get_property(np, "smsc,save-mac-address", NULL))
> + config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
> +
> + return 0;
> +}
> +#else
> +static inline int smsc911x_probe_config_dt(
> + struct smsc911x_platform_config *config,
> + struct device_node *np)
> +{
> + return -ENODEV;
> +}
> +#endif /* CONFIG_OF */
> +
> static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> {
> + struct device_node *np = pdev->dev.of_node;
> struct net_device *dev;
> struct smsc911x_data *pdata;
> struct smsc911x_platform_config *config = pdev->dev.platform_data;
> @@ -2107,13 +2161,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
>
> pr_info("Driver version %s\n", SMSC_DRV_VERSION);
>
> - /* platform data specifies irq & dynamic bus configuration */
> - if (!pdev->dev.platform_data) {
> - pr_warn("platform_data not provided\n");
> - retval = -ENODEV;
> - goto out_0;
> - }
> -
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "smsc911x-memory");
> if (!res)
> @@ -2152,9 +2199,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> pdata->ioaddr = ioremap_nocache(res->start, res_size);
>
> - /* copy config parameters across to pdata */
> - memcpy(&pdata->config, config, sizeof(pdata->config));
> -
> pdata->dev = dev;
> pdata->msg_enable = ((1 << debug) - 1);
>
> @@ -2164,10 +2208,22 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> goto out_free_netdev_2;
> }
>
> + retval = smsc911x_probe_config_dt(&pdata->config, np);
> + if (retval && config) {
> + /* copy config parameters across to pdata */
> + memcpy(&pdata->config, config, sizeof(pdata->config));
The following will do the same but is type-safe:
pdata->config = *config;
> + retval = 0;
> + }
> +
> + if (retval) {
> + SMSC_WARN(pdata, probe, "Error smsc911x config not found");
> + goto out_unmap_io_3;
> + }
> +
> /* assume standard, non-shifted, access to HW registers */
> pdata->ops = &standard_smsc911x_ops;
> /* apply the right access if shifting is needed */
> - if (config->shift)
> + if (pdata->config.shift)
> pdata->ops = &shifted_smsc911x_ops;
>
> retval = smsc911x_init(dev);
> @@ -2314,6 +2370,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
> #define SMSC911X_PM_OPS NULL
> #endif
>
> +static const struct of_device_id smsc911x_dt_ids[] = {
> + { .compatible = "smsc,lan9115", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, smsc911x_dt_ids);
> +
> static struct platform_driver smsc911x_driver = {
> .probe = smsc911x_drv_probe,
> .remove = __devexit_p(smsc911x_drv_remove),
> @@ -2321,6 +2383,7 @@ static struct platform_driver smsc911x_driver = {
> .name = SMSC_CHIPNAME,
> .owner = THIS_MODULE,
> .pm = SMSC911X_PM_OPS,
> + .of_match_table = smsc911x_dt_ids,
> },
> };
>
> --
> 1.7.4.1
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v2] net/smsc911x: add device tree probe support
@ 2011-07-29 15:53 ` Grant Likely
0 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-07-29 15:53 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jul 29, 2011 at 04:43:16PM +0800, Shawn Guo wrote:
> It adds device tree probe support for smsc911x driver.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Steve Glendinning <steve.glendinning@smsc.com>
> Cc: David S. Miller <davem@davemloft.net>
Some comments below, and I asked a question on the older version about
the actual model name vs. compatible, but otherwise it looks right and
you can add my:
Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> Changes since v1:
> * Instead of getting irq line from gpio number, it use irq domain
> to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> * Use 'lan9115' the first model that smsc911x supports in the match
> table
> * Use reg-shift and reg-io-width which already used in of_serial for
> shift and access size binding
>
> Documentation/devicetree/bindings/net/smsc911x.txt | 38 +++++++++
> drivers/net/smsc911x.c | 85 +++++++++++++++++---
> 2 files changed, 112 insertions(+), 11 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/smsc911x.txt
>
> diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt
> new file mode 100644
> index 0000000..271c454
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/smsc911x.txt
> @@ -0,0 +1,38 @@
> +* Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
> +
> +Required properties:
> +- compatible : Should be "smsc,lan<model>", "smsc,lan9115"
> +- reg : Address and length of the io space for SMSC LAN
> +- interrupts : Should contain SMSC LAN interrupt line
> +- interrupt-parent : Should be the phandle for the interrupt controller
> + that services interrupts for this device
> +- phy-mode : String, operation mode of the PHY interface.
> + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
> +
> +Optional properties:
> +- reg-shift : Specify the quantity to shift the register offsets by
> +- reg-io-width : Specify the size (in bytes) of the IO accesses that
> + should be performed on the device. Valid value for SMSC LAN is
> + 2 or 4. If it's omitted or invalid, the size would be 2.
> +- smsc,irq-active-high : Indicates the IRQ polarity is active-low
Which is it? Active high or active low? The property doesn't match
the description.
> +- smsc,irq-push-pull : Indicates the IRQ type is push-pull
What exactly does "push-pull" mean here?
> +- smsc,force-internal-phy : Forces SMSC LAN controller to use
> + internal PHY
> +- smsc,force-external-phy : Forces SMSC LAN controller to use
> + external PHY
> +- smsc,save-mac-address : Indicates that mac address needs to be saved
> + before resetting the controller
> +- local-mac-address : 6 bytes, mac address
> +
> +Examples:
> +
> +lan9220 at f4000000 {
> + compatible = "smsc,lan9220", "smsc,lan9115";
> + reg = <0xf4000000 0x2000000>;
> + phy-mode = "mii";
> + interrupt-parent = <&gpio1>;
> + interrupts = <31>;
> + reg-io-width = <4>;
> + smsc,irq-push-pull;
> +};
> diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> index b9016a3..75c08a5 100644
> --- a/drivers/net/smsc911x.c
> +++ b/drivers/net/smsc911x.c
> @@ -53,6 +53,10 @@
> #include <linux/phy.h>
> #include <linux/smsc911x.h>
> #include <linux/device.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_net.h>
> #include "smsc911x.h"
>
> #define SMSC_CHIPNAME "smsc911x"
> @@ -2095,8 +2099,58 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
> .tx_writefifo = smsc911x_tx_writefifo_shift,
> };
>
> +#ifdef CONFIG_OF
> +static int __devinit smsc911x_probe_config_dt(
> + struct smsc911x_platform_config *config,
> + struct device_node *np)
> +{
> + const char *mac;
> + u32 width = 0;
> +
> + if (!np)
> + return -ENODEV;
> +
> + config->phy_interface = of_get_phy_mode(np);
> +
> + mac = of_get_mac_address(np);
> + if (mac)
> + memcpy(config->mac, mac, ETH_ALEN);
> +
> + of_property_read_u32(np, "reg-shift", &config->shift);
> +
> + of_property_read_u32(np, "reg-io-width", &width);
> + if (width == 4)
> + config->flags |= SMSC911X_USE_32BIT;
> +
> + if (of_get_property(np, "smsc,irq-active-high", NULL))
> + config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
> +
> + if (of_get_property(np, "smsc,irq-push-pull", NULL))
> + config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
> +
> + if (of_get_property(np, "smsc,force-internal-phy", NULL))
> + config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
> +
> + if (of_get_property(np, "smsc,force-external-phy", NULL))
> + config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
> +
> + if (of_get_property(np, "smsc,save-mac-address", NULL))
> + config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
> +
> + return 0;
> +}
> +#else
> +static inline int smsc911x_probe_config_dt(
> + struct smsc911x_platform_config *config,
> + struct device_node *np)
> +{
> + return -ENODEV;
> +}
> +#endif /* CONFIG_OF */
> +
> static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> {
> + struct device_node *np = pdev->dev.of_node;
> struct net_device *dev;
> struct smsc911x_data *pdata;
> struct smsc911x_platform_config *config = pdev->dev.platform_data;
> @@ -2107,13 +2161,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
>
> pr_info("Driver version %s\n", SMSC_DRV_VERSION);
>
> - /* platform data specifies irq & dynamic bus configuration */
> - if (!pdev->dev.platform_data) {
> - pr_warn("platform_data not provided\n");
> - retval = -ENODEV;
> - goto out_0;
> - }
> -
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "smsc911x-memory");
> if (!res)
> @@ -2152,9 +2199,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> pdata->ioaddr = ioremap_nocache(res->start, res_size);
>
> - /* copy config parameters across to pdata */
> - memcpy(&pdata->config, config, sizeof(pdata->config));
> -
> pdata->dev = dev;
> pdata->msg_enable = ((1 << debug) - 1);
>
> @@ -2164,10 +2208,22 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> goto out_free_netdev_2;
> }
>
> + retval = smsc911x_probe_config_dt(&pdata->config, np);
> + if (retval && config) {
> + /* copy config parameters across to pdata */
> + memcpy(&pdata->config, config, sizeof(pdata->config));
The following will do the same but is type-safe:
pdata->config = *config;
> + retval = 0;
> + }
> +
> + if (retval) {
> + SMSC_WARN(pdata, probe, "Error smsc911x config not found");
> + goto out_unmap_io_3;
> + }
> +
> /* assume standard, non-shifted, access to HW registers */
> pdata->ops = &standard_smsc911x_ops;
> /* apply the right access if shifting is needed */
> - if (config->shift)
> + if (pdata->config.shift)
> pdata->ops = &shifted_smsc911x_ops;
>
> retval = smsc911x_init(dev);
> @@ -2314,6 +2370,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
> #define SMSC911X_PM_OPS NULL
> #endif
>
> +static const struct of_device_id smsc911x_dt_ids[] = {
> + { .compatible = "smsc,lan9115", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, smsc911x_dt_ids);
> +
> static struct platform_driver smsc911x_driver = {
> .probe = smsc911x_drv_probe,
> .remove = __devexit_p(smsc911x_drv_remove),
> @@ -2321,6 +2383,7 @@ static struct platform_driver smsc911x_driver = {
> .name = SMSC_CHIPNAME,
> .owner = THIS_MODULE,
> .pm = SMSC911X_PM_OPS,
> + .of_match_table = smsc911x_dt_ids,
> },
> };
>
> --
> 1.7.4.1
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
2011-07-29 15:47 ` Grant Likely
(?)
@ 2011-07-29 16:03 ` Shawn Guo
-1 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 16:03 UTC (permalink / raw)
To: Grant Likely
Cc: Nicolas Pitre, patches, netdev, devicetree-discuss,
Steve Glendinning, Shawn Guo, David S. Miller, linux-arm-kernel
On Fri, Jul 29, 2011 at 09:47:23AM -0600, Grant Likely wrote:
> On Fri, Jul 29, 2011 at 10:36:26AM +0800, Shawn Guo wrote:
> > On Mon, Jul 25, 2011 at 10:28:05PM -0400, Nicolas Pitre wrote:
> > > On Tue, 26 Jul 2011, Shawn Guo wrote:
> > >
> > > > On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> > > > > On Tue, 26 Jul 2011, Shawn Guo wrote:
> > > > >
> > > > > > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > > > > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > > > > > It adds device tree probe support for smsc911x driver.
> > > > > > > >
> > > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > > > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > > > > ---
> > > > > > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > > > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > > > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > > > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > >
> > > > > > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > > new file mode 100644
> > > > > > > > index 0000000..1920695
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > > @@ -0,0 +1,34 @@
> > > > > > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > > > > > +
> > > > > > > > +Required properties:
> > > > > > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > > > > > >
> > > > > > > Drop "smsc,lan". That's far too generic.
> > > > > > >
> > > > > > The following devices are supported by the driver.
> > > > > >
> > > > > > LAN9115, LAN9116, LAN9117, LAN9118
> > > > > > LAN9215, LAN9216, LAN9217, LAN9218
> > > > > > LAN9210, LAN9211
> > > > > > LAN9220, LAN9221
> > > > > >
> > > > > > If we only keep specific <model> as the compatible, we will have a
> > > > > > long match table which is actually used nowhere to distinguish the
> > > > > > device.
> > > > > >
> > > > > > So we need some level generic compatible to save the meaningless
> > > > > > long match table. What about:
> > > > > >
> > > > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > > > { .compatible = "smsc,lan9", },
> > > > > > { /* sentinel */ }
> > > > > > };
> > > > > >
> > > > > > Or:
> > > > > >
> > > > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > > > { .compatible = "smsc,lan91", },
> > > > > > { .compatible = "smsc,lan92", },
> > > > > > { /* sentinel */ }
> > > > > > };
> > > > >
> > > > > None of this unambiguously distinguish the devices supported by this
> > > > > driver and the smc91x driver which supports LAN91C92, LAN91C94,
> > > > > LAN91C95, LAN91C96, LAN91C100, LAN91C110.
> > > > >
> > > > So you suggest to make a long list to explicitly tell the device type
> > > > that the driver supports?
> > >
> > > I'm not suggesting anything. :-) I'm merely pointing out that the
> > > above .compatible = "smsc,lan9" or .compatible = "smsc,lan91" are too
> > > generic given that there is another driver with different devices to
> > > which they could also apply.
> > >
> > Since I do not get any suggestion here, I'm going to follow the driver
> > name to use '911' as the model number. Please tell me if there is any
> > better one.
>
> What is the manufacturer part name? lan9111 or lan91c11? The
> manufacturer's documented part number is almost always preferred.
>
I just posted the v2 of the patch, and chose to use 'smsc,lan9115'
which is the first model supported in the driver. This is the same
approach I used with i.mx device bindings.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-29 16:03 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 16:03 UTC (permalink / raw)
To: Grant Likely
Cc: Nicolas Pitre, patches, netdev, devicetree-discuss,
Steve Glendinning, Shawn Guo, David S. Miller, linux-arm-kernel
On Fri, Jul 29, 2011 at 09:47:23AM -0600, Grant Likely wrote:
> On Fri, Jul 29, 2011 at 10:36:26AM +0800, Shawn Guo wrote:
> > On Mon, Jul 25, 2011 at 10:28:05PM -0400, Nicolas Pitre wrote:
> > > On Tue, 26 Jul 2011, Shawn Guo wrote:
> > >
> > > > On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> > > > > On Tue, 26 Jul 2011, Shawn Guo wrote:
> > > > >
> > > > > > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > > > > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > > > > > It adds device tree probe support for smsc911x driver.
> > > > > > > >
> > > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > > > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > > > > ---
> > > > > > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > > > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > > > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > > > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > >
> > > > > > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > > new file mode 100644
> > > > > > > > index 0000000..1920695
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > > @@ -0,0 +1,34 @@
> > > > > > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > > > > > +
> > > > > > > > +Required properties:
> > > > > > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > > > > > >
> > > > > > > Drop "smsc,lan". That's far too generic.
> > > > > > >
> > > > > > The following devices are supported by the driver.
> > > > > >
> > > > > > LAN9115, LAN9116, LAN9117, LAN9118
> > > > > > LAN9215, LAN9216, LAN9217, LAN9218
> > > > > > LAN9210, LAN9211
> > > > > > LAN9220, LAN9221
> > > > > >
> > > > > > If we only keep specific <model> as the compatible, we will have a
> > > > > > long match table which is actually used nowhere to distinguish the
> > > > > > device.
> > > > > >
> > > > > > So we need some level generic compatible to save the meaningless
> > > > > > long match table. What about:
> > > > > >
> > > > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > > > { .compatible = "smsc,lan9", },
> > > > > > { /* sentinel */ }
> > > > > > };
> > > > > >
> > > > > > Or:
> > > > > >
> > > > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > > > { .compatible = "smsc,lan91", },
> > > > > > { .compatible = "smsc,lan92", },
> > > > > > { /* sentinel */ }
> > > > > > };
> > > > >
> > > > > None of this unambiguously distinguish the devices supported by this
> > > > > driver and the smc91x driver which supports LAN91C92, LAN91C94,
> > > > > LAN91C95, LAN91C96, LAN91C100, LAN91C110.
> > > > >
> > > > So you suggest to make a long list to explicitly tell the device type
> > > > that the driver supports?
> > >
> > > I'm not suggesting anything. :-) I'm merely pointing out that the
> > > above .compatible = "smsc,lan9" or .compatible = "smsc,lan91" are too
> > > generic given that there is another driver with different devices to
> > > which they could also apply.
> > >
> > Since I do not get any suggestion here, I'm going to follow the driver
> > name to use '911' as the model number. Please tell me if there is any
> > better one.
>
> What is the manufacturer part name? lan9111 or lan91c11? The
> manufacturer's documented part number is almost always preferred.
>
I just posted the v2 of the patch, and chose to use 'smsc,lan9115'
which is the first model supported in the driver. This is the same
approach I used with i.mx device bindings.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-29 16:03 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 16:03 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jul 29, 2011 at 09:47:23AM -0600, Grant Likely wrote:
> On Fri, Jul 29, 2011 at 10:36:26AM +0800, Shawn Guo wrote:
> > On Mon, Jul 25, 2011 at 10:28:05PM -0400, Nicolas Pitre wrote:
> > > On Tue, 26 Jul 2011, Shawn Guo wrote:
> > >
> > > > On Mon, Jul 25, 2011 at 09:16:40PM -0400, Nicolas Pitre wrote:
> > > > > On Tue, 26 Jul 2011, Shawn Guo wrote:
> > > > >
> > > > > > On Mon, Jul 25, 2011 at 03:37:23PM -0600, Grant Likely wrote:
> > > > > > > On Mon, Jul 25, 2011 at 05:44:00PM +0800, Shawn Guo wrote:
> > > > > > > > It adds device tree probe support for smsc911x driver.
> > > > > > > >
> > > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > > > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > > > > > Cc: David S. Miller <davem@davemloft.net>
> > > > > > > > ---
> > > > > > > > Documentation/devicetree/bindings/net/smsc.txt | 34 +++++++
> > > > > > > > drivers/net/smsc911x.c | 123 +++++++++++++++++++-----
> > > > > > > > 2 files changed, 132 insertions(+), 25 deletions(-)
> > > > > > > > create mode 100644 Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > >
> > > > > > > > diff --git a/Documentation/devicetree/bindings/net/smsc.txt b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > > new file mode 100644
> > > > > > > > index 0000000..1920695
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/Documentation/devicetree/bindings/net/smsc.txt
> > > > > > > > @@ -0,0 +1,34 @@
> > > > > > > > +* Smart Mixed-Signal Connectivity (SMSC) LAN Controller
> > > > > > > > +
> > > > > > > > +Required properties:
> > > > > > > > +- compatible : Should be "smsc,lan<model>""smsc,lan"
> > > > > > >
> > > > > > > Drop "smsc,lan". That's far too generic.
> > > > > > >
> > > > > > The following devices are supported by the driver.
> > > > > >
> > > > > > LAN9115, LAN9116, LAN9117, LAN9118
> > > > > > LAN9215, LAN9216, LAN9217, LAN9218
> > > > > > LAN9210, LAN9211
> > > > > > LAN9220, LAN9221
> > > > > >
> > > > > > If we only keep specific <model> as the compatible, we will have a
> > > > > > long match table which is actually used nowhere to distinguish the
> > > > > > device.
> > > > > >
> > > > > > So we need some level generic compatible to save the meaningless
> > > > > > long match table. What about:
> > > > > >
> > > > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > > > { .compatible = "smsc,lan9", },
> > > > > > { /* sentinel */ }
> > > > > > };
> > > > > >
> > > > > > Or:
> > > > > >
> > > > > > static const struct of_device_id smsc_dt_ids[] = {
> > > > > > { .compatible = "smsc,lan91", },
> > > > > > { .compatible = "smsc,lan92", },
> > > > > > { /* sentinel */ }
> > > > > > };
> > > > >
> > > > > None of this unambiguously distinguish the devices supported by this
> > > > > driver and the smc91x driver which supports LAN91C92, LAN91C94,
> > > > > LAN91C95, LAN91C96, LAN91C100, LAN91C110.
> > > > >
> > > > So you suggest to make a long list to explicitly tell the device type
> > > > that the driver supports?
> > >
> > > I'm not suggesting anything. :-) I'm merely pointing out that the
> > > above .compatible = "smsc,lan9" or .compatible = "smsc,lan91" are too
> > > generic given that there is another driver with different devices to
> > > which they could also apply.
> > >
> > Since I do not get any suggestion here, I'm going to follow the driver
> > name to use '911' as the model number. Please tell me if there is any
> > better one.
>
> What is the manufacturer part name? lan9111 or lan91c11? The
> manufacturer's documented part number is almost always preferred.
>
I just posted the v2 of the patch, and chose to use 'smsc,lan9115'
which is the first model supported in the driver. This is the same
approach I used with i.mx device bindings.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2] net/smsc911x: add device tree probe support
2011-07-29 15:53 ` Grant Likely
(?)
@ 2011-07-29 16:16 ` Shawn Guo
-1 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 16:16 UTC (permalink / raw)
To: Grant Likely
Cc: Shawn Guo, netdev, devicetree-discuss, linux-arm-kernel, patches,
Steve Glendinning, David S. Miller
On Fri, Jul 29, 2011 at 09:53:54AM -0600, Grant Likely wrote:
> On Fri, Jul 29, 2011 at 04:43:16PM +0800, Shawn Guo wrote:
> > It adds device tree probe support for smsc911x driver.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > Cc: David S. Miller <davem@davemloft.net>
>
> Some comments below, and I asked a question on the older version about
> the actual model name vs. compatible, but otherwise it looks right and
> you can add my:
>
> Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
>
Thanks, Grant.
> > ---
> > Changes since v1:
> > * Instead of getting irq line from gpio number, it use irq domain
> > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > * Use 'lan9115' the first model that smsc911x supports in the match
> > table
> > * Use reg-shift and reg-io-width which already used in of_serial for
> > shift and access size binding
> >
> > Documentation/devicetree/bindings/net/smsc911x.txt | 38 +++++++++
> > drivers/net/smsc911x.c | 85 +++++++++++++++++---
> > 2 files changed, 112 insertions(+), 11 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/net/smsc911x.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt
> > new file mode 100644
> > index 0000000..271c454
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/smsc911x.txt
> > @@ -0,0 +1,38 @@
> > +* Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
> > +
> > +Required properties:
> > +- compatible : Should be "smsc,lan<model>", "smsc,lan9115"
> > +- reg : Address and length of the io space for SMSC LAN
> > +- interrupts : Should contain SMSC LAN interrupt line
> > +- interrupt-parent : Should be the phandle for the interrupt controller
> > + that services interrupts for this device
> > +- phy-mode : String, operation mode of the PHY interface.
> > + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> > + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
> > +
> > +Optional properties:
> > +- reg-shift : Specify the quantity to shift the register offsets by
> > +- reg-io-width : Specify the size (in bytes) of the IO accesses that
> > + should be performed on the device. Valid value for SMSC LAN is
> > + 2 or 4. If it's omitted or invalid, the size would be 2.
> > +- smsc,irq-active-high : Indicates the IRQ polarity is active-low
>
> Which is it? Active high or active low? The property doesn't match
> the description.
>
Sorry. The description is wrong.
> > +- smsc,irq-push-pull : Indicates the IRQ type is push-pull
>
> What exactly does "push-pull" mean here?
>
I do not understand exactly. But I see the term used in driver was
originally from SMSC data sheet.
> > +- smsc,force-internal-phy : Forces SMSC LAN controller to use
> > + internal PHY
> > +- smsc,force-external-phy : Forces SMSC LAN controller to use
> > + external PHY
> > +- smsc,save-mac-address : Indicates that mac address needs to be saved
> > + before resetting the controller
> > +- local-mac-address : 6 bytes, mac address
> > +
> > +Examples:
> > +
> > +lan9220@f4000000 {
> > + compatible = "smsc,lan9220", "smsc,lan9115";
> > + reg = <0xf4000000 0x2000000>;
> > + phy-mode = "mii";
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <31>;
> > + reg-io-width = <4>;
> > + smsc,irq-push-pull;
> > +};
> > diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> > index b9016a3..75c08a5 100644
> > --- a/drivers/net/smsc911x.c
> > +++ b/drivers/net/smsc911x.c
> > @@ -53,6 +53,10 @@
> > #include <linux/phy.h>
> > #include <linux/smsc911x.h>
> > #include <linux/device.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/of_net.h>
> > #include "smsc911x.h"
> >
> > #define SMSC_CHIPNAME "smsc911x"
> > @@ -2095,8 +2099,58 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
> > .tx_writefifo = smsc911x_tx_writefifo_shift,
> > };
> >
> > +#ifdef CONFIG_OF
> > +static int __devinit smsc911x_probe_config_dt(
> > + struct smsc911x_platform_config *config,
> > + struct device_node *np)
> > +{
> > + const char *mac;
> > + u32 width = 0;
> > +
> > + if (!np)
> > + return -ENODEV;
> > +
> > + config->phy_interface = of_get_phy_mode(np);
> > +
> > + mac = of_get_mac_address(np);
> > + if (mac)
> > + memcpy(config->mac, mac, ETH_ALEN);
> > +
> > + of_property_read_u32(np, "reg-shift", &config->shift);
> > +
> > + of_property_read_u32(np, "reg-io-width", &width);
> > + if (width == 4)
> > + config->flags |= SMSC911X_USE_32BIT;
> > +
> > + if (of_get_property(np, "smsc,irq-active-high", NULL))
> > + config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
> > +
> > + if (of_get_property(np, "smsc,irq-push-pull", NULL))
> > + config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
> > +
> > + if (of_get_property(np, "smsc,force-internal-phy", NULL))
> > + config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
> > +
> > + if (of_get_property(np, "smsc,force-external-phy", NULL))
> > + config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
> > +
> > + if (of_get_property(np, "smsc,save-mac-address", NULL))
> > + config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
> > +
> > + return 0;
> > +}
> > +#else
> > +static inline int smsc911x_probe_config_dt(
> > + struct smsc911x_platform_config *config,
> > + struct device_node *np)
> > +{
> > + return -ENODEV;
> > +}
> > +#endif /* CONFIG_OF */
> > +
> > static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > {
> > + struct device_node *np = pdev->dev.of_node;
> > struct net_device *dev;
> > struct smsc911x_data *pdata;
> > struct smsc911x_platform_config *config = pdev->dev.platform_data;
> > @@ -2107,13 +2161,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> >
> > pr_info("Driver version %s\n", SMSC_DRV_VERSION);
> >
> > - /* platform data specifies irq & dynamic bus configuration */
> > - if (!pdev->dev.platform_data) {
> > - pr_warn("platform_data not provided\n");
> > - retval = -ENODEV;
> > - goto out_0;
> > - }
> > -
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > "smsc911x-memory");
> > if (!res)
> > @@ -2152,9 +2199,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> > pdata->ioaddr = ioremap_nocache(res->start, res_size);
> >
> > - /* copy config parameters across to pdata */
> > - memcpy(&pdata->config, config, sizeof(pdata->config));
> > -
> > pdata->dev = dev;
> > pdata->msg_enable = ((1 << debug) - 1);
> >
> > @@ -2164,10 +2208,22 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > goto out_free_netdev_2;
> > }
> >
> > + retval = smsc911x_probe_config_dt(&pdata->config, np);
> > + if (retval && config) {
> > + /* copy config parameters across to pdata */
> > + memcpy(&pdata->config, config, sizeof(pdata->config));
>
> The following will do the same but is type-safe:
> pdata->config = *config;
>
I choose to keep the original taste :)
> > + retval = 0;
> > + }
> > +
> > + if (retval) {
> > + SMSC_WARN(pdata, probe, "Error smsc911x config not found");
> > + goto out_unmap_io_3;
> > + }
> > +
> > /* assume standard, non-shifted, access to HW registers */
> > pdata->ops = &standard_smsc911x_ops;
> > /* apply the right access if shifting is needed */
> > - if (config->shift)
> > + if (pdata->config.shift)
> > pdata->ops = &shifted_smsc911x_ops;
> >
> > retval = smsc911x_init(dev);
> > @@ -2314,6 +2370,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
> > #define SMSC911X_PM_OPS NULL
> > #endif
> >
> > +static const struct of_device_id smsc911x_dt_ids[] = {
> > + { .compatible = "smsc,lan9115", },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, smsc911x_dt_ids);
> > +
> > static struct platform_driver smsc911x_driver = {
> > .probe = smsc911x_drv_probe,
> > .remove = __devexit_p(smsc911x_drv_remove),
> > @@ -2321,6 +2383,7 @@ static struct platform_driver smsc911x_driver = {
> > .name = SMSC_CHIPNAME,
> > .owner = THIS_MODULE,
> > .pm = SMSC911X_PM_OPS,
> > + .of_match_table = smsc911x_dt_ids,
> > },
> > };
> >
> > --
> > 1.7.4.1
> >
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2] net/smsc911x: add device tree probe support
@ 2011-07-29 16:16 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 16:16 UTC (permalink / raw)
To: Grant Likely
Cc: Shawn Guo, netdev, devicetree-discuss, linux-arm-kernel, patches,
Steve Glendinning, David S. Miller
On Fri, Jul 29, 2011 at 09:53:54AM -0600, Grant Likely wrote:
> On Fri, Jul 29, 2011 at 04:43:16PM +0800, Shawn Guo wrote:
> > It adds device tree probe support for smsc911x driver.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > Cc: David S. Miller <davem@davemloft.net>
>
> Some comments below, and I asked a question on the older version about
> the actual model name vs. compatible, but otherwise it looks right and
> you can add my:
>
> Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
>
Thanks, Grant.
> > ---
> > Changes since v1:
> > * Instead of getting irq line from gpio number, it use irq domain
> > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > * Use 'lan9115' the first model that smsc911x supports in the match
> > table
> > * Use reg-shift and reg-io-width which already used in of_serial for
> > shift and access size binding
> >
> > Documentation/devicetree/bindings/net/smsc911x.txt | 38 +++++++++
> > drivers/net/smsc911x.c | 85 +++++++++++++++++---
> > 2 files changed, 112 insertions(+), 11 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/net/smsc911x.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt
> > new file mode 100644
> > index 0000000..271c454
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/smsc911x.txt
> > @@ -0,0 +1,38 @@
> > +* Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
> > +
> > +Required properties:
> > +- compatible : Should be "smsc,lan<model>", "smsc,lan9115"
> > +- reg : Address and length of the io space for SMSC LAN
> > +- interrupts : Should contain SMSC LAN interrupt line
> > +- interrupt-parent : Should be the phandle for the interrupt controller
> > + that services interrupts for this device
> > +- phy-mode : String, operation mode of the PHY interface.
> > + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> > + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
> > +
> > +Optional properties:
> > +- reg-shift : Specify the quantity to shift the register offsets by
> > +- reg-io-width : Specify the size (in bytes) of the IO accesses that
> > + should be performed on the device. Valid value for SMSC LAN is
> > + 2 or 4. If it's omitted or invalid, the size would be 2.
> > +- smsc,irq-active-high : Indicates the IRQ polarity is active-low
>
> Which is it? Active high or active low? The property doesn't match
> the description.
>
Sorry. The description is wrong.
> > +- smsc,irq-push-pull : Indicates the IRQ type is push-pull
>
> What exactly does "push-pull" mean here?
>
I do not understand exactly. But I see the term used in driver was
originally from SMSC data sheet.
> > +- smsc,force-internal-phy : Forces SMSC LAN controller to use
> > + internal PHY
> > +- smsc,force-external-phy : Forces SMSC LAN controller to use
> > + external PHY
> > +- smsc,save-mac-address : Indicates that mac address needs to be saved
> > + before resetting the controller
> > +- local-mac-address : 6 bytes, mac address
> > +
> > +Examples:
> > +
> > +lan9220@f4000000 {
> > + compatible = "smsc,lan9220", "smsc,lan9115";
> > + reg = <0xf4000000 0x2000000>;
> > + phy-mode = "mii";
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <31>;
> > + reg-io-width = <4>;
> > + smsc,irq-push-pull;
> > +};
> > diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> > index b9016a3..75c08a5 100644
> > --- a/drivers/net/smsc911x.c
> > +++ b/drivers/net/smsc911x.c
> > @@ -53,6 +53,10 @@
> > #include <linux/phy.h>
> > #include <linux/smsc911x.h>
> > #include <linux/device.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/of_net.h>
> > #include "smsc911x.h"
> >
> > #define SMSC_CHIPNAME "smsc911x"
> > @@ -2095,8 +2099,58 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
> > .tx_writefifo = smsc911x_tx_writefifo_shift,
> > };
> >
> > +#ifdef CONFIG_OF
> > +static int __devinit smsc911x_probe_config_dt(
> > + struct smsc911x_platform_config *config,
> > + struct device_node *np)
> > +{
> > + const char *mac;
> > + u32 width = 0;
> > +
> > + if (!np)
> > + return -ENODEV;
> > +
> > + config->phy_interface = of_get_phy_mode(np);
> > +
> > + mac = of_get_mac_address(np);
> > + if (mac)
> > + memcpy(config->mac, mac, ETH_ALEN);
> > +
> > + of_property_read_u32(np, "reg-shift", &config->shift);
> > +
> > + of_property_read_u32(np, "reg-io-width", &width);
> > + if (width == 4)
> > + config->flags |= SMSC911X_USE_32BIT;
> > +
> > + if (of_get_property(np, "smsc,irq-active-high", NULL))
> > + config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
> > +
> > + if (of_get_property(np, "smsc,irq-push-pull", NULL))
> > + config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
> > +
> > + if (of_get_property(np, "smsc,force-internal-phy", NULL))
> > + config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
> > +
> > + if (of_get_property(np, "smsc,force-external-phy", NULL))
> > + config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
> > +
> > + if (of_get_property(np, "smsc,save-mac-address", NULL))
> > + config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
> > +
> > + return 0;
> > +}
> > +#else
> > +static inline int smsc911x_probe_config_dt(
> > + struct smsc911x_platform_config *config,
> > + struct device_node *np)
> > +{
> > + return -ENODEV;
> > +}
> > +#endif /* CONFIG_OF */
> > +
> > static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > {
> > + struct device_node *np = pdev->dev.of_node;
> > struct net_device *dev;
> > struct smsc911x_data *pdata;
> > struct smsc911x_platform_config *config = pdev->dev.platform_data;
> > @@ -2107,13 +2161,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> >
> > pr_info("Driver version %s\n", SMSC_DRV_VERSION);
> >
> > - /* platform data specifies irq & dynamic bus configuration */
> > - if (!pdev->dev.platform_data) {
> > - pr_warn("platform_data not provided\n");
> > - retval = -ENODEV;
> > - goto out_0;
> > - }
> > -
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > "smsc911x-memory");
> > if (!res)
> > @@ -2152,9 +2199,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> > pdata->ioaddr = ioremap_nocache(res->start, res_size);
> >
> > - /* copy config parameters across to pdata */
> > - memcpy(&pdata->config, config, sizeof(pdata->config));
> > -
> > pdata->dev = dev;
> > pdata->msg_enable = ((1 << debug) - 1);
> >
> > @@ -2164,10 +2208,22 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > goto out_free_netdev_2;
> > }
> >
> > + retval = smsc911x_probe_config_dt(&pdata->config, np);
> > + if (retval && config) {
> > + /* copy config parameters across to pdata */
> > + memcpy(&pdata->config, config, sizeof(pdata->config));
>
> The following will do the same but is type-safe:
> pdata->config = *config;
>
I choose to keep the original taste :)
> > + retval = 0;
> > + }
> > +
> > + if (retval) {
> > + SMSC_WARN(pdata, probe, "Error smsc911x config not found");
> > + goto out_unmap_io_3;
> > + }
> > +
> > /* assume standard, non-shifted, access to HW registers */
> > pdata->ops = &standard_smsc911x_ops;
> > /* apply the right access if shifting is needed */
> > - if (config->shift)
> > + if (pdata->config.shift)
> > pdata->ops = &shifted_smsc911x_ops;
> >
> > retval = smsc911x_init(dev);
> > @@ -2314,6 +2370,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
> > #define SMSC911X_PM_OPS NULL
> > #endif
> >
> > +static const struct of_device_id smsc911x_dt_ids[] = {
> > + { .compatible = "smsc,lan9115", },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, smsc911x_dt_ids);
> > +
> > static struct platform_driver smsc911x_driver = {
> > .probe = smsc911x_drv_probe,
> > .remove = __devexit_p(smsc911x_drv_remove),
> > @@ -2321,6 +2383,7 @@ static struct platform_driver smsc911x_driver = {
> > .name = SMSC_CHIPNAME,
> > .owner = THIS_MODULE,
> > .pm = SMSC911X_PM_OPS,
> > + .of_match_table = smsc911x_dt_ids,
> > },
> > };
> >
> > --
> > 1.7.4.1
> >
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v2] net/smsc911x: add device tree probe support
@ 2011-07-29 16:16 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 16:16 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jul 29, 2011 at 09:53:54AM -0600, Grant Likely wrote:
> On Fri, Jul 29, 2011 at 04:43:16PM +0800, Shawn Guo wrote:
> > It adds device tree probe support for smsc911x driver.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > Cc: David S. Miller <davem@davemloft.net>
>
> Some comments below, and I asked a question on the older version about
> the actual model name vs. compatible, but otherwise it looks right and
> you can add my:
>
> Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
>
Thanks, Grant.
> > ---
> > Changes since v1:
> > * Instead of getting irq line from gpio number, it use irq domain
> > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > * Use 'lan9115' the first model that smsc911x supports in the match
> > table
> > * Use reg-shift and reg-io-width which already used in of_serial for
> > shift and access size binding
> >
> > Documentation/devicetree/bindings/net/smsc911x.txt | 38 +++++++++
> > drivers/net/smsc911x.c | 85 +++++++++++++++++---
> > 2 files changed, 112 insertions(+), 11 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/net/smsc911x.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt
> > new file mode 100644
> > index 0000000..271c454
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/smsc911x.txt
> > @@ -0,0 +1,38 @@
> > +* Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
> > +
> > +Required properties:
> > +- compatible : Should be "smsc,lan<model>", "smsc,lan9115"
> > +- reg : Address and length of the io space for SMSC LAN
> > +- interrupts : Should contain SMSC LAN interrupt line
> > +- interrupt-parent : Should be the phandle for the interrupt controller
> > + that services interrupts for this device
> > +- phy-mode : String, operation mode of the PHY interface.
> > + Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> > + "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
> > +
> > +Optional properties:
> > +- reg-shift : Specify the quantity to shift the register offsets by
> > +- reg-io-width : Specify the size (in bytes) of the IO accesses that
> > + should be performed on the device. Valid value for SMSC LAN is
> > + 2 or 4. If it's omitted or invalid, the size would be 2.
> > +- smsc,irq-active-high : Indicates the IRQ polarity is active-low
>
> Which is it? Active high or active low? The property doesn't match
> the description.
>
Sorry. The description is wrong.
> > +- smsc,irq-push-pull : Indicates the IRQ type is push-pull
>
> What exactly does "push-pull" mean here?
>
I do not understand exactly. But I see the term used in driver was
originally from SMSC data sheet.
> > +- smsc,force-internal-phy : Forces SMSC LAN controller to use
> > + internal PHY
> > +- smsc,force-external-phy : Forces SMSC LAN controller to use
> > + external PHY
> > +- smsc,save-mac-address : Indicates that mac address needs to be saved
> > + before resetting the controller
> > +- local-mac-address : 6 bytes, mac address
> > +
> > +Examples:
> > +
> > +lan9220 at f4000000 {
> > + compatible = "smsc,lan9220", "smsc,lan9115";
> > + reg = <0xf4000000 0x2000000>;
> > + phy-mode = "mii";
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <31>;
> > + reg-io-width = <4>;
> > + smsc,irq-push-pull;
> > +};
> > diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> > index b9016a3..75c08a5 100644
> > --- a/drivers/net/smsc911x.c
> > +++ b/drivers/net/smsc911x.c
> > @@ -53,6 +53,10 @@
> > #include <linux/phy.h>
> > #include <linux/smsc911x.h>
> > #include <linux/device.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/of_net.h>
> > #include "smsc911x.h"
> >
> > #define SMSC_CHIPNAME "smsc911x"
> > @@ -2095,8 +2099,58 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
> > .tx_writefifo = smsc911x_tx_writefifo_shift,
> > };
> >
> > +#ifdef CONFIG_OF
> > +static int __devinit smsc911x_probe_config_dt(
> > + struct smsc911x_platform_config *config,
> > + struct device_node *np)
> > +{
> > + const char *mac;
> > + u32 width = 0;
> > +
> > + if (!np)
> > + return -ENODEV;
> > +
> > + config->phy_interface = of_get_phy_mode(np);
> > +
> > + mac = of_get_mac_address(np);
> > + if (mac)
> > + memcpy(config->mac, mac, ETH_ALEN);
> > +
> > + of_property_read_u32(np, "reg-shift", &config->shift);
> > +
> > + of_property_read_u32(np, "reg-io-width", &width);
> > + if (width == 4)
> > + config->flags |= SMSC911X_USE_32BIT;
> > +
> > + if (of_get_property(np, "smsc,irq-active-high", NULL))
> > + config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
> > +
> > + if (of_get_property(np, "smsc,irq-push-pull", NULL))
> > + config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
> > +
> > + if (of_get_property(np, "smsc,force-internal-phy", NULL))
> > + config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
> > +
> > + if (of_get_property(np, "smsc,force-external-phy", NULL))
> > + config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
> > +
> > + if (of_get_property(np, "smsc,save-mac-address", NULL))
> > + config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
> > +
> > + return 0;
> > +}
> > +#else
> > +static inline int smsc911x_probe_config_dt(
> > + struct smsc911x_platform_config *config,
> > + struct device_node *np)
> > +{
> > + return -ENODEV;
> > +}
> > +#endif /* CONFIG_OF */
> > +
> > static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > {
> > + struct device_node *np = pdev->dev.of_node;
> > struct net_device *dev;
> > struct smsc911x_data *pdata;
> > struct smsc911x_platform_config *config = pdev->dev.platform_data;
> > @@ -2107,13 +2161,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> >
> > pr_info("Driver version %s\n", SMSC_DRV_VERSION);
> >
> > - /* platform data specifies irq & dynamic bus configuration */
> > - if (!pdev->dev.platform_data) {
> > - pr_warn("platform_data not provided\n");
> > - retval = -ENODEV;
> > - goto out_0;
> > - }
> > -
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > "smsc911x-memory");
> > if (!res)
> > @@ -2152,9 +2199,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> > pdata->ioaddr = ioremap_nocache(res->start, res_size);
> >
> > - /* copy config parameters across to pdata */
> > - memcpy(&pdata->config, config, sizeof(pdata->config));
> > -
> > pdata->dev = dev;
> > pdata->msg_enable = ((1 << debug) - 1);
> >
> > @@ -2164,10 +2208,22 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> > goto out_free_netdev_2;
> > }
> >
> > + retval = smsc911x_probe_config_dt(&pdata->config, np);
> > + if (retval && config) {
> > + /* copy config parameters across to pdata */
> > + memcpy(&pdata->config, config, sizeof(pdata->config));
>
> The following will do the same but is type-safe:
> pdata->config = *config;
>
I choose to keep the original taste :)
> > + retval = 0;
> > + }
> > +
> > + if (retval) {
> > + SMSC_WARN(pdata, probe, "Error smsc911x config not found");
> > + goto out_unmap_io_3;
> > + }
> > +
> > /* assume standard, non-shifted, access to HW registers */
> > pdata->ops = &standard_smsc911x_ops;
> > /* apply the right access if shifting is needed */
> > - if (config->shift)
> > + if (pdata->config.shift)
> > pdata->ops = &shifted_smsc911x_ops;
> >
> > retval = smsc911x_init(dev);
> > @@ -2314,6 +2370,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
> > #define SMSC911X_PM_OPS NULL
> > #endif
> >
> > +static const struct of_device_id smsc911x_dt_ids[] = {
> > + { .compatible = "smsc,lan9115", },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, smsc911x_dt_ids);
> > +
> > static struct platform_driver smsc911x_driver = {
> > .probe = smsc911x_drv_probe,
> > .remove = __devexit_p(smsc911x_drv_remove),
> > @@ -2321,6 +2383,7 @@ static struct platform_driver smsc911x_driver = {
> > .name = SMSC_CHIPNAME,
> > .owner = THIS_MODULE,
> > .pm = SMSC911X_PM_OPS,
> > + .of_match_table = smsc911x_dt_ids,
> > },
> > };
> >
> > --
> > 1.7.4.1
> >
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
2011-07-29 16:03 ` Shawn Guo
@ 2011-07-29 16:26 ` Grant Likely
-1 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-07-29 16:26 UTC (permalink / raw)
To: Shawn Guo
Cc: Nicolas Pitre, patches, netdev, devicetree-discuss,
Steve Glendinning, Shawn Guo, David S. Miller, linux-arm-kernel
On Sat, Jul 30, 2011 at 12:03:44AM +0800, Shawn Guo wrote:
> On Fri, Jul 29, 2011 at 09:47:23AM -0600, Grant Likely wrote:
> > On Fri, Jul 29, 2011 at 10:36:26AM +0800, Shawn Guo wrote:
> > > Since I do not get any suggestion here, I'm going to follow the driver
> > > name to use '911' as the model number. Please tell me if there is any
> > > better one.
> >
> > What is the manufacturer part name? lan9111 or lan91c11? The
> > manufacturer's documented part number is almost always preferred.
> >
> I just posted the v2 of the patch, and chose to use 'smsc,lan9115'
> which is the first model supported in the driver. This is the same
> approach I used with i.mx device bindings.
You haven't answered the question.
g.
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-29 16:26 ` Grant Likely
0 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-07-29 16:26 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Jul 30, 2011 at 12:03:44AM +0800, Shawn Guo wrote:
> On Fri, Jul 29, 2011 at 09:47:23AM -0600, Grant Likely wrote:
> > On Fri, Jul 29, 2011 at 10:36:26AM +0800, Shawn Guo wrote:
> > > Since I do not get any suggestion here, I'm going to follow the driver
> > > name to use '911' as the model number. Please tell me if there is any
> > > better one.
> >
> > What is the manufacturer part name? lan9111 or lan91c11? The
> > manufacturer's documented part number is almost always preferred.
> >
> I just posted the v2 of the patch, and chose to use 'smsc,lan9115'
> which is the first model supported in the driver. This is the same
> approach I used with i.mx device bindings.
You haven't answered the question.
g.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
2011-07-29 16:26 ` Grant Likely
(?)
@ 2011-07-29 17:13 ` Shawn Guo
-1 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 17:13 UTC (permalink / raw)
To: Grant Likely
Cc: Nicolas Pitre, patches, netdev, devicetree-discuss,
Steve Glendinning, Shawn Guo, David S. Miller, linux-arm-kernel
On Fri, Jul 29, 2011 at 10:26:29AM -0600, Grant Likely wrote:
> On Sat, Jul 30, 2011 at 12:03:44AM +0800, Shawn Guo wrote:
> > On Fri, Jul 29, 2011 at 09:47:23AM -0600, Grant Likely wrote:
> > > On Fri, Jul 29, 2011 at 10:36:26AM +0800, Shawn Guo wrote:
> > > > Since I do not get any suggestion here, I'm going to follow the driver
> > > > name to use '911' as the model number. Please tell me if there is any
> > > > better one.
> > >
> > > What is the manufacturer part name? lan9111 or lan91c11? The
> > > manufacturer's documented part number is almost always preferred.
> > >
> > I just posted the v2 of the patch, and chose to use 'smsc,lan9115'
> > which is the first model supported in the driver. This is the same
> > approach I used with i.mx device bindings.
>
> You haven't answered the question.
>
For 9115 example, the part number in data sheet is "LAN9115".
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-29 17:13 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 17:13 UTC (permalink / raw)
To: Grant Likely
Cc: Nicolas Pitre, patches, netdev, devicetree-discuss,
Steve Glendinning, Shawn Guo, David S. Miller, linux-arm-kernel
On Fri, Jul 29, 2011 at 10:26:29AM -0600, Grant Likely wrote:
> On Sat, Jul 30, 2011 at 12:03:44AM +0800, Shawn Guo wrote:
> > On Fri, Jul 29, 2011 at 09:47:23AM -0600, Grant Likely wrote:
> > > On Fri, Jul 29, 2011 at 10:36:26AM +0800, Shawn Guo wrote:
> > > > Since I do not get any suggestion here, I'm going to follow the driver
> > > > name to use '911' as the model number. Please tell me if there is any
> > > > better one.
> > >
> > > What is the manufacturer part name? lan9111 or lan91c11? The
> > > manufacturer's documented part number is almost always preferred.
> > >
> > I just posted the v2 of the patch, and chose to use 'smsc,lan9115'
> > which is the first model supported in the driver. This is the same
> > approach I used with i.mx device bindings.
>
> You haven't answered the question.
>
For 9115 example, the part number in data sheet is "LAN9115".
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-29 17:13 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-29 17:13 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jul 29, 2011 at 10:26:29AM -0600, Grant Likely wrote:
> On Sat, Jul 30, 2011 at 12:03:44AM +0800, Shawn Guo wrote:
> > On Fri, Jul 29, 2011 at 09:47:23AM -0600, Grant Likely wrote:
> > > On Fri, Jul 29, 2011 at 10:36:26AM +0800, Shawn Guo wrote:
> > > > Since I do not get any suggestion here, I'm going to follow the driver
> > > > name to use '911' as the model number. Please tell me if there is any
> > > > better one.
> > >
> > > What is the manufacturer part name? lan9111 or lan91c11? The
> > > manufacturer's documented part number is almost always preferred.
> > >
> > I just posted the v2 of the patch, and chose to use 'smsc,lan9115'
> > which is the first model supported in the driver. This is the same
> > approach I used with i.mx device bindings.
>
> You haven't answered the question.
>
For 9115 example, the part number in data sheet is "LAN9115".
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v3] net/smsc911x: add device tree probe support
2011-07-25 9:44 ` Shawn Guo
@ 2011-07-30 18:26 ` Shawn Guo
-1 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-30 18:26 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: patches-QSEj5FYQhm4dnm+yROfE0A,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Steve Glendinning,
David S. Miller,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
It adds device tree probe support for smsc911x driver.
Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: Steve Glendinning <steve.glendinning-sdUf+H5yV5I@public.gmane.org>
Cc: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Reviewed-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---
Changes since v2:
* Fix a typo in smsc911x.txt
Changes since v1:
* Instead of getting irq line from gpio number, it use irq domain
to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
* Use 'lan9115' the first model that smsc911x supports in the match
table
* Use reg-shift and reg-io-width which already used in of_serial for
shift and access size binding
Documentation/devicetree/bindings/net/smsc911x.txt | 38 +++++++++
drivers/net/smsc911x.c | 85 +++++++++++++++++---
2 files changed, 112 insertions(+), 11 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/smsc911x.txt
diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt
new file mode 100644
index 0000000..adb5b57
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc911x.txt
@@ -0,0 +1,38 @@
+* Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
+
+Required properties:
+- compatible : Should be "smsc,lan<model>", "smsc,lan9115"
+- reg : Address and length of the io space for SMSC LAN
+- interrupts : Should contain SMSC LAN interrupt line
+- interrupt-parent : Should be the phandle for the interrupt controller
+ that services interrupts for this device
+- phy-mode : String, operation mode of the PHY interface.
+ Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
+ "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
+
+Optional properties:
+- reg-shift : Specify the quantity to shift the register offsets by
+- reg-io-width : Specify the size (in bytes) of the IO accesses that
+ should be performed on the device. Valid value for SMSC LAN is
+ 2 or 4. If it's omitted or invalid, the size would be 2.
+- smsc,irq-active-high : Indicates the IRQ polarity is active-high
+- smsc,irq-push-pull : Indicates the IRQ type is push-pull
+- smsc,force-internal-phy : Forces SMSC LAN controller to use
+ internal PHY
+- smsc,force-external-phy : Forces SMSC LAN controller to use
+ external PHY
+- smsc,save-mac-address : Indicates that mac address needs to be saved
+ before resetting the controller
+- local-mac-address : 6 bytes, mac address
+
+Examples:
+
+lan9220@f4000000 {
+ compatible = "smsc,lan9220", "smsc,lan9115";
+ reg = <0xf4000000 0x2000000>;
+ phy-mode = "mii";
+ interrupt-parent = <&gpio1>;
+ interrupts = <31>;
+ reg-io-width = <4>;
+ smsc,irq-push-pull;
+};
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index b9016a3..75c08a5 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -53,6 +53,10 @@
#include <linux/phy.h>
#include <linux/smsc911x.h>
#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/of_net.h>
#include "smsc911x.h"
#define SMSC_CHIPNAME "smsc911x"
@@ -2095,8 +2099,58 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
.tx_writefifo = smsc911x_tx_writefifo_shift,
};
+#ifdef CONFIG_OF
+static int __devinit smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ const char *mac;
+ u32 width = 0;
+
+ if (!np)
+ return -ENODEV;
+
+ config->phy_interface = of_get_phy_mode(np);
+
+ mac = of_get_mac_address(np);
+ if (mac)
+ memcpy(config->mac, mac, ETH_ALEN);
+
+ of_property_read_u32(np, "reg-shift", &config->shift);
+
+ of_property_read_u32(np, "reg-io-width", &width);
+ if (width == 4)
+ config->flags |= SMSC911X_USE_32BIT;
+
+ if (of_get_property(np, "smsc,irq-active-high", NULL))
+ config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
+
+ if (of_get_property(np, "smsc,irq-push-pull", NULL))
+ config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
+
+ if (of_get_property(np, "smsc,force-internal-phy", NULL))
+ config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,force-external-phy", NULL))
+ config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,save-mac-address", NULL))
+ config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
+
+ return 0;
+}
+#else
+static inline int smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ return -ENODEV;
+}
+#endif /* CONFIG_OF */
+
static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
{
+ struct device_node *np = pdev->dev.of_node;
struct net_device *dev;
struct smsc911x_data *pdata;
struct smsc911x_platform_config *config = pdev->dev.platform_data;
@@ -2107,13 +2161,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
pr_info("Driver version %s\n", SMSC_DRV_VERSION);
- /* platform data specifies irq & dynamic bus configuration */
- if (!pdev->dev.platform_data) {
- pr_warn("platform_data not provided\n");
- retval = -ENODEV;
- goto out_0;
- }
-
res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"smsc911x-memory");
if (!res)
@@ -2152,9 +2199,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
pdata->ioaddr = ioremap_nocache(res->start, res_size);
- /* copy config parameters across to pdata */
- memcpy(&pdata->config, config, sizeof(pdata->config));
-
pdata->dev = dev;
pdata->msg_enable = ((1 << debug) - 1);
@@ -2164,10 +2208,22 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
goto out_free_netdev_2;
}
+ retval = smsc911x_probe_config_dt(&pdata->config, np);
+ if (retval && config) {
+ /* copy config parameters across to pdata */
+ memcpy(&pdata->config, config, sizeof(pdata->config));
+ retval = 0;
+ }
+
+ if (retval) {
+ SMSC_WARN(pdata, probe, "Error smsc911x config not found");
+ goto out_unmap_io_3;
+ }
+
/* assume standard, non-shifted, access to HW registers */
pdata->ops = &standard_smsc911x_ops;
/* apply the right access if shifting is needed */
- if (config->shift)
+ if (pdata->config.shift)
pdata->ops = &shifted_smsc911x_ops;
retval = smsc911x_init(dev);
@@ -2314,6 +2370,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
#define SMSC911X_PM_OPS NULL
#endif
+static const struct of_device_id smsc911x_dt_ids[] = {
+ { .compatible = "smsc,lan9115", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, smsc911x_dt_ids);
+
static struct platform_driver smsc911x_driver = {
.probe = smsc911x_drv_probe,
.remove = __devexit_p(smsc911x_drv_remove),
@@ -2321,6 +2383,7 @@ static struct platform_driver smsc911x_driver = {
.name = SMSC_CHIPNAME,
.owner = THIS_MODULE,
.pm = SMSC911X_PM_OPS,
+ .of_match_table = smsc911x_dt_ids,
},
};
--
1.7.4.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v3] net/smsc911x: add device tree probe support
@ 2011-07-30 18:26 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-07-30 18:26 UTC (permalink / raw)
To: linux-arm-kernel
It adds device tree probe support for smsc911x driver.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Steve Glendinning <steve.glendinning@smsc.com>
Cc: David S. Miller <davem@davemloft.net>
Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
---
Changes since v2:
* Fix a typo in smsc911x.txt
Changes since v1:
* Instead of getting irq line from gpio number, it use irq domain
to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
* Use 'lan9115' the first model that smsc911x supports in the match
table
* Use reg-shift and reg-io-width which already used in of_serial for
shift and access size binding
Documentation/devicetree/bindings/net/smsc911x.txt | 38 +++++++++
drivers/net/smsc911x.c | 85 +++++++++++++++++---
2 files changed, 112 insertions(+), 11 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/smsc911x.txt
diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt
new file mode 100644
index 0000000..adb5b57
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc911x.txt
@@ -0,0 +1,38 @@
+* Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
+
+Required properties:
+- compatible : Should be "smsc,lan<model>", "smsc,lan9115"
+- reg : Address and length of the io space for SMSC LAN
+- interrupts : Should contain SMSC LAN interrupt line
+- interrupt-parent : Should be the phandle for the interrupt controller
+ that services interrupts for this device
+- phy-mode : String, operation mode of the PHY interface.
+ Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
+ "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
+
+Optional properties:
+- reg-shift : Specify the quantity to shift the register offsets by
+- reg-io-width : Specify the size (in bytes) of the IO accesses that
+ should be performed on the device. Valid value for SMSC LAN is
+ 2 or 4. If it's omitted or invalid, the size would be 2.
+- smsc,irq-active-high : Indicates the IRQ polarity is active-high
+- smsc,irq-push-pull : Indicates the IRQ type is push-pull
+- smsc,force-internal-phy : Forces SMSC LAN controller to use
+ internal PHY
+- smsc,force-external-phy : Forces SMSC LAN controller to use
+ external PHY
+- smsc,save-mac-address : Indicates that mac address needs to be saved
+ before resetting the controller
+- local-mac-address : 6 bytes, mac address
+
+Examples:
+
+lan9220 at f4000000 {
+ compatible = "smsc,lan9220", "smsc,lan9115";
+ reg = <0xf4000000 0x2000000>;
+ phy-mode = "mii";
+ interrupt-parent = <&gpio1>;
+ interrupts = <31>;
+ reg-io-width = <4>;
+ smsc,irq-push-pull;
+};
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index b9016a3..75c08a5 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -53,6 +53,10 @@
#include <linux/phy.h>
#include <linux/smsc911x.h>
#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/of_net.h>
#include "smsc911x.h"
#define SMSC_CHIPNAME "smsc911x"
@@ -2095,8 +2099,58 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
.tx_writefifo = smsc911x_tx_writefifo_shift,
};
+#ifdef CONFIG_OF
+static int __devinit smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ const char *mac;
+ u32 width = 0;
+
+ if (!np)
+ return -ENODEV;
+
+ config->phy_interface = of_get_phy_mode(np);
+
+ mac = of_get_mac_address(np);
+ if (mac)
+ memcpy(config->mac, mac, ETH_ALEN);
+
+ of_property_read_u32(np, "reg-shift", &config->shift);
+
+ of_property_read_u32(np, "reg-io-width", &width);
+ if (width == 4)
+ config->flags |= SMSC911X_USE_32BIT;
+
+ if (of_get_property(np, "smsc,irq-active-high", NULL))
+ config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
+
+ if (of_get_property(np, "smsc,irq-push-pull", NULL))
+ config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
+
+ if (of_get_property(np, "smsc,force-internal-phy", NULL))
+ config->flags |= SMSC911X_FORCE_INTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,force-external-phy", NULL))
+ config->flags |= SMSC911X_FORCE_EXTERNAL_PHY;
+
+ if (of_get_property(np, "smsc,save-mac-address", NULL))
+ config->flags |= SMSC911X_SAVE_MAC_ADDRESS;
+
+ return 0;
+}
+#else
+static inline int smsc911x_probe_config_dt(
+ struct smsc911x_platform_config *config,
+ struct device_node *np)
+{
+ return -ENODEV;
+}
+#endif /* CONFIG_OF */
+
static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
{
+ struct device_node *np = pdev->dev.of_node;
struct net_device *dev;
struct smsc911x_data *pdata;
struct smsc911x_platform_config *config = pdev->dev.platform_data;
@@ -2107,13 +2161,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
pr_info("Driver version %s\n", SMSC_DRV_VERSION);
- /* platform data specifies irq & dynamic bus configuration */
- if (!pdev->dev.platform_data) {
- pr_warn("platform_data not provided\n");
- retval = -ENODEV;
- goto out_0;
- }
-
res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"smsc911x-memory");
if (!res)
@@ -2152,9 +2199,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
pdata->ioaddr = ioremap_nocache(res->start, res_size);
- /* copy config parameters across to pdata */
- memcpy(&pdata->config, config, sizeof(pdata->config));
-
pdata->dev = dev;
pdata->msg_enable = ((1 << debug) - 1);
@@ -2164,10 +2208,22 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
goto out_free_netdev_2;
}
+ retval = smsc911x_probe_config_dt(&pdata->config, np);
+ if (retval && config) {
+ /* copy config parameters across to pdata */
+ memcpy(&pdata->config, config, sizeof(pdata->config));
+ retval = 0;
+ }
+
+ if (retval) {
+ SMSC_WARN(pdata, probe, "Error smsc911x config not found");
+ goto out_unmap_io_3;
+ }
+
/* assume standard, non-shifted, access to HW registers */
pdata->ops = &standard_smsc911x_ops;
/* apply the right access if shifting is needed */
- if (config->shift)
+ if (pdata->config.shift)
pdata->ops = &shifted_smsc911x_ops;
retval = smsc911x_init(dev);
@@ -2314,6 +2370,12 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
#define SMSC911X_PM_OPS NULL
#endif
+static const struct of_device_id smsc911x_dt_ids[] = {
+ { .compatible = "smsc,lan9115", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, smsc911x_dt_ids);
+
static struct platform_driver smsc911x_driver = {
.probe = smsc911x_drv_probe,
.remove = __devexit_p(smsc911x_drv_remove),
@@ -2321,6 +2383,7 @@ static struct platform_driver smsc911x_driver = {
.name = SMSC_CHIPNAME,
.owner = THIS_MODULE,
.pm = SMSC911X_PM_OPS,
+ .of_match_table = smsc911x_dt_ids,
},
};
--
1.7.4.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH] net/smsc911x: add device tree probe support
2011-07-29 17:13 ` Shawn Guo
@ 2011-07-31 0:42 ` Grant Likely
-1 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-07-31 0:42 UTC (permalink / raw)
To: Shawn Guo
Cc: Nicolas Pitre, patches, netdev, devicetree-discuss,
Steve Glendinning, Shawn Guo, David S. Miller, linux-arm-kernel
On Sat, Jul 30, 2011 at 01:13:15AM +0800, Shawn Guo wrote:
> On Fri, Jul 29, 2011 at 10:26:29AM -0600, Grant Likely wrote:
> > On Sat, Jul 30, 2011 at 12:03:44AM +0800, Shawn Guo wrote:
> > > On Fri, Jul 29, 2011 at 09:47:23AM -0600, Grant Likely wrote:
> > > > On Fri, Jul 29, 2011 at 10:36:26AM +0800, Shawn Guo wrote:
> > > > > Since I do not get any suggestion here, I'm going to follow the driver
> > > > > name to use '911' as the model number. Please tell me if there is any
> > > > > better one.
> > > >
> > > > What is the manufacturer part name? lan9111 or lan91c11? The
> > > > manufacturer's documented part number is almost always preferred.
> > > >
> > > I just posted the v2 of the patch, and chose to use 'smsc,lan9115'
> > > which is the first model supported in the driver. This is the same
> > > approach I used with i.mx device bindings.
> >
> > You haven't answered the question.
> >
> For 9115 example, the part number in data sheet is "LAN9115".
Okay, good. Thanks.
g.
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] net/smsc911x: add device tree probe support
@ 2011-07-31 0:42 ` Grant Likely
0 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-07-31 0:42 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Jul 30, 2011 at 01:13:15AM +0800, Shawn Guo wrote:
> On Fri, Jul 29, 2011 at 10:26:29AM -0600, Grant Likely wrote:
> > On Sat, Jul 30, 2011 at 12:03:44AM +0800, Shawn Guo wrote:
> > > On Fri, Jul 29, 2011 at 09:47:23AM -0600, Grant Likely wrote:
> > > > On Fri, Jul 29, 2011 at 10:36:26AM +0800, Shawn Guo wrote:
> > > > > Since I do not get any suggestion here, I'm going to follow the driver
> > > > > name to use '911' as the model number. Please tell me if there is any
> > > > > better one.
> > > >
> > > > What is the manufacturer part name? lan9111 or lan91c11? The
> > > > manufacturer's documented part number is almost always preferred.
> > > >
> > > I just posted the v2 of the patch, and chose to use 'smsc,lan9115'
> > > which is the first model supported in the driver. This is the same
> > > approach I used with i.mx device bindings.
> >
> > You haven't answered the question.
> >
> For 9115 example, the part number in data sheet is "LAN9115".
Okay, good. Thanks.
g.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v3] net/smsc911x: add device tree probe support
2011-07-30 18:26 ` Shawn Guo
@ 2011-08-02 1:01 ` David Miller
-1 siblings, 0 replies; 53+ messages in thread
From: David Miller @ 2011-08-02 1:01 UTC (permalink / raw)
To: shawn.guo
Cc: patches, netdev, devicetree-discuss, grant.likely,
steve.glendinning, linux-arm-kernel
From: Shawn Guo <shawn.guo@linaro.org>
Date: Sun, 31 Jul 2011 02:26:00 +0800
> It adds device tree probe support for smsc911x driver.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Steve Glendinning <steve.glendinning@smsc.com>
> Cc: David S. Miller <davem@davemloft.net>
> Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
Queued up for net-next, thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v3] net/smsc911x: add device tree probe support
@ 2011-08-02 1:01 ` David Miller
0 siblings, 0 replies; 53+ messages in thread
From: David Miller @ 2011-08-02 1:01 UTC (permalink / raw)
To: linux-arm-kernel
From: Shawn Guo <shawn.guo@linaro.org>
Date: Sun, 31 Jul 2011 02:26:00 +0800
> It adds device tree probe support for smsc911x driver.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Steve Glendinning <steve.glendinning@smsc.com>
> Cc: David S. Miller <davem@davemloft.net>
> Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
Queued up for net-next, thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v3] net/smsc911x: add device tree probe support
2011-07-30 18:26 ` Shawn Guo
@ 2011-09-08 14:59 ` Dave Martin
-1 siblings, 0 replies; 53+ messages in thread
From: Dave Martin @ 2011-09-08 14:59 UTC (permalink / raw)
To: Shawn Guo
Cc: netdev, patches, devicetree-discuss, Grant Likely,
Steve Glendinning, David S. Miller, linux-arm-kernel
Hi Shawn,
On Sun, Jul 31, 2011 at 02:26:00AM +0800, Shawn Guo wrote:
> It adds device tree probe support for smsc911x driver.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Steve Glendinning <steve.glendinning@smsc.com>
> Cc: David S. Miller <davem@davemloft.net>
> Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> Changes since v2:
> * Fix a typo in smsc911x.txt
>
> Changes since v1:
> * Instead of getting irq line from gpio number, it use irq domain
> to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> * Use 'lan9115' the first model that smsc911x supports in the match
> table
> * Use reg-shift and reg-io-width which already used in of_serial for
> shift and access size binding
When using this patch with vexpress, I found that 16-bit register access
mode doesn't seem to be getting set correctly.
Can you take a look at this additional patch and let me know if it looks
correct?
Cheers
---Dave
From: Dave Martin <dave.martin@linaro.org>
Date: Wed, 7 Sep 2011 17:26:31 +0100
Subject: [PATCH] net/smsc911x: Correctly configure 16-bit register access from DT
The SMSC911X_USE_16BIT needs to be set when using 16-bit register
access. However, currently no flag is set if the DT doesn't specify
32-bit access.
This patch should set the SMSC911X_USE_16BIT flag in a manner consistent
with the documented DT bindings.
Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
drivers/net/smsc911x.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index 75c08a5..1a35c25 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -2121,6 +2121,8 @@ static int __devinit smsc911x_probe_config_dt(
of_property_read_u32(np, "reg-io-width", &width);
if (width == 4)
config->flags |= SMSC911X_USE_32BIT;
+ else
+ config->flags |= SMSC911X_USE_16BIT;
if (of_get_property(np, "smsc,irq-active-high", NULL))
config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v3] net/smsc911x: add device tree probe support
@ 2011-09-08 14:59 ` Dave Martin
0 siblings, 0 replies; 53+ messages in thread
From: Dave Martin @ 2011-09-08 14:59 UTC (permalink / raw)
To: linux-arm-kernel
Hi Shawn,
On Sun, Jul 31, 2011 at 02:26:00AM +0800, Shawn Guo wrote:
> It adds device tree probe support for smsc911x driver.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Steve Glendinning <steve.glendinning@smsc.com>
> Cc: David S. Miller <davem@davemloft.net>
> Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> Changes since v2:
> * Fix a typo in smsc911x.txt
>
> Changes since v1:
> * Instead of getting irq line from gpio number, it use irq domain
> to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> * Use 'lan9115' the first model that smsc911x supports in the match
> table
> * Use reg-shift and reg-io-width which already used in of_serial for
> shift and access size binding
When using this patch with vexpress, I found that 16-bit register access
mode doesn't seem to be getting set correctly.
Can you take a look at this additional patch and let me know if it looks
correct?
Cheers
---Dave
From: Dave Martin <dave.martin@linaro.org>
Date: Wed, 7 Sep 2011 17:26:31 +0100
Subject: [PATCH] net/smsc911x: Correctly configure 16-bit register access from DT
The SMSC911X_USE_16BIT needs to be set when using 16-bit register
access. However, currently no flag is set if the DT doesn't specify
32-bit access.
This patch should set the SMSC911X_USE_16BIT flag in a manner consistent
with the documented DT bindings.
Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
drivers/net/smsc911x.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index 75c08a5..1a35c25 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -2121,6 +2121,8 @@ static int __devinit smsc911x_probe_config_dt(
of_property_read_u32(np, "reg-io-width", &width);
if (width == 4)
config->flags |= SMSC911X_USE_32BIT;
+ else
+ config->flags |= SMSC911X_USE_16BIT;
if (of_get_property(np, "smsc,irq-active-high", NULL))
config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v3] net/smsc911x: add device tree probe support
2011-09-08 14:59 ` Dave Martin
@ 2011-09-08 18:29 ` Grant Likely
-1 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-09-08 18:29 UTC (permalink / raw)
To: Dave Martin
Cc: patches-QSEj5FYQhm4dnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Steve Glendinning,
David S. Miller,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Thu, Sep 08, 2011 at 03:59:46PM +0100, Dave Martin wrote:
> Hi Shawn,
>
> On Sun, Jul 31, 2011 at 02:26:00AM +0800, Shawn Guo wrote:
> > It adds device tree probe support for smsc911x driver.
> >
> > Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> > Cc: Steve Glendinning <steve.glendinning-sdUf+H5yV5I@public.gmane.org>
> > Cc: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> > Reviewed-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> > ---
> > Changes since v2:
> > * Fix a typo in smsc911x.txt
> >
> > Changes since v1:
> > * Instead of getting irq line from gpio number, it use irq domain
> > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > * Use 'lan9115' the first model that smsc911x supports in the match
> > table
> > * Use reg-shift and reg-io-width which already used in of_serial for
> > shift and access size binding
>
> When using this patch with vexpress, I found that 16-bit register access
> mode doesn't seem to be getting set correctly.
>
> Can you take a look at this additional patch and let me know if it looks
> correct?
>
> Cheers
> ---Dave
>
> From: Dave Martin <dave.martin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Date: Wed, 7 Sep 2011 17:26:31 +0100
> Subject: [PATCH] net/smsc911x: Correctly configure 16-bit register access from DT
>
> The SMSC911X_USE_16BIT needs to be set when using 16-bit register
> access. However, currently no flag is set if the DT doesn't specify
> 32-bit access.
>
> This patch should set the SMSC911X_USE_16BIT flag in a manner consistent
> with the documented DT bindings.
>
> Signed-off-by: Dave Martin <dave.martin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> drivers/net/smsc911x.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> index 75c08a5..1a35c25 100644
> --- a/drivers/net/smsc911x.c
> +++ b/drivers/net/smsc911x.c
> @@ -2121,6 +2121,8 @@ static int __devinit smsc911x_probe_config_dt(
> of_property_read_u32(np, "reg-io-width", &width);
> if (width == 4)
> config->flags |= SMSC911X_USE_32BIT;
> + else
> + config->flags |= SMSC911X_USE_16BIT;
Would it be better to do "else if (width == 2)"? (completely
uninformed comment. I've not looked at what the non-DT probe path
does on this driver.)
g.
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v3] net/smsc911x: add device tree probe support
@ 2011-09-08 18:29 ` Grant Likely
0 siblings, 0 replies; 53+ messages in thread
From: Grant Likely @ 2011-09-08 18:29 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 08, 2011 at 03:59:46PM +0100, Dave Martin wrote:
> Hi Shawn,
>
> On Sun, Jul 31, 2011 at 02:26:00AM +0800, Shawn Guo wrote:
> > It adds device tree probe support for smsc911x driver.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> > ---
> > Changes since v2:
> > * Fix a typo in smsc911x.txt
> >
> > Changes since v1:
> > * Instead of getting irq line from gpio number, it use irq domain
> > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > * Use 'lan9115' the first model that smsc911x supports in the match
> > table
> > * Use reg-shift and reg-io-width which already used in of_serial for
> > shift and access size binding
>
> When using this patch with vexpress, I found that 16-bit register access
> mode doesn't seem to be getting set correctly.
>
> Can you take a look at this additional patch and let me know if it looks
> correct?
>
> Cheers
> ---Dave
>
> From: Dave Martin <dave.martin@linaro.org>
> Date: Wed, 7 Sep 2011 17:26:31 +0100
> Subject: [PATCH] net/smsc911x: Correctly configure 16-bit register access from DT
>
> The SMSC911X_USE_16BIT needs to be set when using 16-bit register
> access. However, currently no flag is set if the DT doesn't specify
> 32-bit access.
>
> This patch should set the SMSC911X_USE_16BIT flag in a manner consistent
> with the documented DT bindings.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
> drivers/net/smsc911x.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> index 75c08a5..1a35c25 100644
> --- a/drivers/net/smsc911x.c
> +++ b/drivers/net/smsc911x.c
> @@ -2121,6 +2121,8 @@ static int __devinit smsc911x_probe_config_dt(
> of_property_read_u32(np, "reg-io-width", &width);
> if (width == 4)
> config->flags |= SMSC911X_USE_32BIT;
> + else
> + config->flags |= SMSC911X_USE_16BIT;
Would it be better to do "else if (width == 2)"? (completely
uninformed comment. I've not looked at what the non-DT probe path
does on this driver.)
g.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v3] net/smsc911x: add device tree probe support
2011-09-08 18:29 ` Grant Likely
@ 2011-09-09 8:50 ` Dave Martin
-1 siblings, 0 replies; 53+ messages in thread
From: Dave Martin @ 2011-09-09 8:50 UTC (permalink / raw)
To: Grant Likely
Cc: Shawn Guo, netdev, patches, devicetree-discuss,
Steve Glendinning, David S. Miller, linux-arm-kernel
On Thu, Sep 08, 2011 at 11:29:20AM -0700, Grant Likely wrote:
> On Thu, Sep 08, 2011 at 03:59:46PM +0100, Dave Martin wrote:
> > Hi Shawn,
> >
> > On Sun, Jul 31, 2011 at 02:26:00AM +0800, Shawn Guo wrote:
> > > It adds device tree probe support for smsc911x driver.
> > >
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > Cc: David S. Miller <davem@davemloft.net>
> > > Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> > > ---
> > > Changes since v2:
> > > * Fix a typo in smsc911x.txt
> > >
> > > Changes since v1:
> > > * Instead of getting irq line from gpio number, it use irq domain
> > > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > > * Use 'lan9115' the first model that smsc911x supports in the match
> > > table
> > > * Use reg-shift and reg-io-width which already used in of_serial for
> > > shift and access size binding
> >
> > When using this patch with vexpress, I found that 16-bit register access
> > mode doesn't seem to be getting set correctly.
> >
> > Can you take a look at this additional patch and let me know if it looks
> > correct?
> >
> > Cheers
> > ---Dave
> >
> > From: Dave Martin <dave.martin@linaro.org>
> > Date: Wed, 7 Sep 2011 17:26:31 +0100
> > Subject: [PATCH] net/smsc911x: Correctly configure 16-bit register access from DT
> >
> > The SMSC911X_USE_16BIT needs to be set when using 16-bit register
> > access. However, currently no flag is set if the DT doesn't specify
> > 32-bit access.
> >
> > This patch should set the SMSC911X_USE_16BIT flag in a manner consistent
> > with the documented DT bindings.
> >
> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > ---
> > drivers/net/smsc911x.c | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> > index 75c08a5..1a35c25 100644
> > --- a/drivers/net/smsc911x.c
> > +++ b/drivers/net/smsc911x.c
> > @@ -2121,6 +2121,8 @@ static int __devinit smsc911x_probe_config_dt(
> > of_property_read_u32(np, "reg-io-width", &width);
> > if (width == 4)
> > config->flags |= SMSC911X_USE_32BIT;
> > + else
> > + config->flags |= SMSC911X_USE_16BIT;
>
> Would it be better to do "else if (width == 2)"? (completely
> uninformed comment. I've not looked at what the non-DT probe path
> does on this driver.)
I wouldn't have a problem with that. But currently the binding
documentation says that any value other than 4, or a missing property,
implies 16-bit register access.
So the binding documentation would need to change too in that case.
Personally I think this would be better, but it's just an opinion.
Cheers
---Dave
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v3] net/smsc911x: add device tree probe support
@ 2011-09-09 8:50 ` Dave Martin
0 siblings, 0 replies; 53+ messages in thread
From: Dave Martin @ 2011-09-09 8:50 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 08, 2011 at 11:29:20AM -0700, Grant Likely wrote:
> On Thu, Sep 08, 2011 at 03:59:46PM +0100, Dave Martin wrote:
> > Hi Shawn,
> >
> > On Sun, Jul 31, 2011 at 02:26:00AM +0800, Shawn Guo wrote:
> > > It adds device tree probe support for smsc911x driver.
> > >
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > Cc: David S. Miller <davem@davemloft.net>
> > > Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> > > ---
> > > Changes since v2:
> > > * Fix a typo in smsc911x.txt
> > >
> > > Changes since v1:
> > > * Instead of getting irq line from gpio number, it use irq domain
> > > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > > * Use 'lan9115' the first model that smsc911x supports in the match
> > > table
> > > * Use reg-shift and reg-io-width which already used in of_serial for
> > > shift and access size binding
> >
> > When using this patch with vexpress, I found that 16-bit register access
> > mode doesn't seem to be getting set correctly.
> >
> > Can you take a look at this additional patch and let me know if it looks
> > correct?
> >
> > Cheers
> > ---Dave
> >
> > From: Dave Martin <dave.martin@linaro.org>
> > Date: Wed, 7 Sep 2011 17:26:31 +0100
> > Subject: [PATCH] net/smsc911x: Correctly configure 16-bit register access from DT
> >
> > The SMSC911X_USE_16BIT needs to be set when using 16-bit register
> > access. However, currently no flag is set if the DT doesn't specify
> > 32-bit access.
> >
> > This patch should set the SMSC911X_USE_16BIT flag in a manner consistent
> > with the documented DT bindings.
> >
> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > ---
> > drivers/net/smsc911x.c | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> > index 75c08a5..1a35c25 100644
> > --- a/drivers/net/smsc911x.c
> > +++ b/drivers/net/smsc911x.c
> > @@ -2121,6 +2121,8 @@ static int __devinit smsc911x_probe_config_dt(
> > of_property_read_u32(np, "reg-io-width", &width);
> > if (width == 4)
> > config->flags |= SMSC911X_USE_32BIT;
> > + else
> > + config->flags |= SMSC911X_USE_16BIT;
>
> Would it be better to do "else if (width == 2)"? (completely
> uninformed comment. I've not looked at what the non-DT probe path
> does on this driver.)
I wouldn't have a problem with that. But currently the binding
documentation says that any value other than 4, or a missing property,
implies 16-bit register access.
So the binding documentation would need to change too in that case.
Personally I think this would be better, but it's just an opinion.
Cheers
---Dave
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v3] net/smsc911x: add device tree probe support
2011-09-08 14:59 ` Dave Martin
(?)
@ 2011-09-09 11:48 ` Shawn Guo
-1 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-09-09 11:48 UTC (permalink / raw)
To: Dave Martin
Cc: Shawn Guo, netdev, patches, devicetree-discuss, Grant Likely,
Steve Glendinning, David S. Miller, linux-arm-kernel
On Thu, Sep 08, 2011 at 03:59:46PM +0100, Dave Martin wrote:
> Hi Shawn,
>
> On Sun, Jul 31, 2011 at 02:26:00AM +0800, Shawn Guo wrote:
> > It adds device tree probe support for smsc911x driver.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> > ---
> > Changes since v2:
> > * Fix a typo in smsc911x.txt
> >
> > Changes since v1:
> > * Instead of getting irq line from gpio number, it use irq domain
> > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > * Use 'lan9115' the first model that smsc911x supports in the match
> > table
> > * Use reg-shift and reg-io-width which already used in of_serial for
> > shift and access size binding
>
> When using this patch with vexpress, I found that 16-bit register access
> mode doesn't seem to be getting set correctly.
>
> Can you take a look at this additional patch and let me know if it looks
> correct?
>
> Cheers
> ---Dave
>
> From: Dave Martin <dave.martin@linaro.org>
> Date: Wed, 7 Sep 2011 17:26:31 +0100
> Subject: [PATCH] net/smsc911x: Correctly configure 16-bit register access from DT
>
> The SMSC911X_USE_16BIT needs to be set when using 16-bit register
> access. However, currently no flag is set if the DT doesn't specify
> 32-bit access.
>
> This patch should set the SMSC911X_USE_16BIT flag in a manner consistent
> with the documented DT bindings.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
Acked-by: Shawn Guo <shawn.guo@linaro.org>
> drivers/net/smsc911x.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> index 75c08a5..1a35c25 100644
> --- a/drivers/net/smsc911x.c
> +++ b/drivers/net/smsc911x.c
> @@ -2121,6 +2121,8 @@ static int __devinit smsc911x_probe_config_dt(
> of_property_read_u32(np, "reg-io-width", &width);
> if (width == 4)
> config->flags |= SMSC911X_USE_32BIT;
> + else
> + config->flags |= SMSC911X_USE_16BIT;
>
> if (of_get_property(np, "smsc,irq-active-high", NULL))
> config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v3] net/smsc911x: add device tree probe support
@ 2011-09-09 11:48 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-09-09 11:48 UTC (permalink / raw)
To: Dave Martin
Cc: Shawn Guo, netdev, patches, devicetree-discuss, Grant Likely,
Steve Glendinning, David S. Miller, linux-arm-kernel
On Thu, Sep 08, 2011 at 03:59:46PM +0100, Dave Martin wrote:
> Hi Shawn,
>
> On Sun, Jul 31, 2011 at 02:26:00AM +0800, Shawn Guo wrote:
> > It adds device tree probe support for smsc911x driver.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> > ---
> > Changes since v2:
> > * Fix a typo in smsc911x.txt
> >
> > Changes since v1:
> > * Instead of getting irq line from gpio number, it use irq domain
> > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > * Use 'lan9115' the first model that smsc911x supports in the match
> > table
> > * Use reg-shift and reg-io-width which already used in of_serial for
> > shift and access size binding
>
> When using this patch with vexpress, I found that 16-bit register access
> mode doesn't seem to be getting set correctly.
>
> Can you take a look at this additional patch and let me know if it looks
> correct?
>
> Cheers
> ---Dave
>
> From: Dave Martin <dave.martin@linaro.org>
> Date: Wed, 7 Sep 2011 17:26:31 +0100
> Subject: [PATCH] net/smsc911x: Correctly configure 16-bit register access from DT
>
> The SMSC911X_USE_16BIT needs to be set when using 16-bit register
> access. However, currently no flag is set if the DT doesn't specify
> 32-bit access.
>
> This patch should set the SMSC911X_USE_16BIT flag in a manner consistent
> with the documented DT bindings.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
Acked-by: Shawn Guo <shawn.guo@linaro.org>
> drivers/net/smsc911x.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> index 75c08a5..1a35c25 100644
> --- a/drivers/net/smsc911x.c
> +++ b/drivers/net/smsc911x.c
> @@ -2121,6 +2121,8 @@ static int __devinit smsc911x_probe_config_dt(
> of_property_read_u32(np, "reg-io-width", &width);
> if (width == 4)
> config->flags |= SMSC911X_USE_32BIT;
> + else
> + config->flags |= SMSC911X_USE_16BIT;
>
> if (of_get_property(np, "smsc,irq-active-high", NULL))
> config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v3] net/smsc911x: add device tree probe support
@ 2011-09-09 11:48 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-09-09 11:48 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 08, 2011 at 03:59:46PM +0100, Dave Martin wrote:
> Hi Shawn,
>
> On Sun, Jul 31, 2011 at 02:26:00AM +0800, Shawn Guo wrote:
> > It adds device tree probe support for smsc911x driver.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> > ---
> > Changes since v2:
> > * Fix a typo in smsc911x.txt
> >
> > Changes since v1:
> > * Instead of getting irq line from gpio number, it use irq domain
> > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > * Use 'lan9115' the first model that smsc911x supports in the match
> > table
> > * Use reg-shift and reg-io-width which already used in of_serial for
> > shift and access size binding
>
> When using this patch with vexpress, I found that 16-bit register access
> mode doesn't seem to be getting set correctly.
>
> Can you take a look at this additional patch and let me know if it looks
> correct?
>
> Cheers
> ---Dave
>
> From: Dave Martin <dave.martin@linaro.org>
> Date: Wed, 7 Sep 2011 17:26:31 +0100
> Subject: [PATCH] net/smsc911x: Correctly configure 16-bit register access from DT
>
> The SMSC911X_USE_16BIT needs to be set when using 16-bit register
> access. However, currently no flag is set if the DT doesn't specify
> 32-bit access.
>
> This patch should set the SMSC911X_USE_16BIT flag in a manner consistent
> with the documented DT bindings.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
Acked-by: Shawn Guo <shawn.guo@linaro.org>
> drivers/net/smsc911x.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> index 75c08a5..1a35c25 100644
> --- a/drivers/net/smsc911x.c
> +++ b/drivers/net/smsc911x.c
> @@ -2121,6 +2121,8 @@ static int __devinit smsc911x_probe_config_dt(
> of_property_read_u32(np, "reg-io-width", &width);
> if (width == 4)
> config->flags |= SMSC911X_USE_32BIT;
> + else
> + config->flags |= SMSC911X_USE_16BIT;
>
> if (of_get_property(np, "smsc,irq-active-high", NULL))
> config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v3] net/smsc911x: add device tree probe support
2011-09-09 8:50 ` Dave Martin
(?)
@ 2011-09-09 12:59 ` Shawn Guo
-1 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-09-09 12:59 UTC (permalink / raw)
To: Dave Martin
Cc: Grant Likely, Shawn Guo, netdev, patches, devicetree-discuss,
Steve Glendinning, David S. Miller, linux-arm-kernel
On Fri, Sep 09, 2011 at 09:50:30AM +0100, Dave Martin wrote:
> On Thu, Sep 08, 2011 at 11:29:20AM -0700, Grant Likely wrote:
> > On Thu, Sep 08, 2011 at 03:59:46PM +0100, Dave Martin wrote:
> > > Hi Shawn,
> > >
> > > On Sun, Jul 31, 2011 at 02:26:00AM +0800, Shawn Guo wrote:
> > > > It adds device tree probe support for smsc911x driver.
> > > >
> > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > Cc: David S. Miller <davem@davemloft.net>
> > > > Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> > > > ---
> > > > Changes since v2:
> > > > * Fix a typo in smsc911x.txt
> > > >
> > > > Changes since v1:
> > > > * Instead of getting irq line from gpio number, it use irq domain
> > > > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > > > * Use 'lan9115' the first model that smsc911x supports in the match
> > > > table
> > > > * Use reg-shift and reg-io-width which already used in of_serial for
> > > > shift and access size binding
> > >
> > > When using this patch with vexpress, I found that 16-bit register access
> > > mode doesn't seem to be getting set correctly.
> > >
> > > Can you take a look at this additional patch and let me know if it looks
> > > correct?
> > >
> > > Cheers
> > > ---Dave
> > >
> > > From: Dave Martin <dave.martin@linaro.org>
> > > Date: Wed, 7 Sep 2011 17:26:31 +0100
> > > Subject: [PATCH] net/smsc911x: Correctly configure 16-bit register access from DT
> > >
> > > The SMSC911X_USE_16BIT needs to be set when using 16-bit register
> > > access. However, currently no flag is set if the DT doesn't specify
> > > 32-bit access.
> > >
> > > This patch should set the SMSC911X_USE_16BIT flag in a manner consistent
> > > with the documented DT bindings.
> > >
> > > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > > ---
> > > drivers/net/smsc911x.c | 2 ++
> > > 1 files changed, 2 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> > > index 75c08a5..1a35c25 100644
> > > --- a/drivers/net/smsc911x.c
> > > +++ b/drivers/net/smsc911x.c
> > > @@ -2121,6 +2121,8 @@ static int __devinit smsc911x_probe_config_dt(
> > > of_property_read_u32(np, "reg-io-width", &width);
> > > if (width == 4)
> > > config->flags |= SMSC911X_USE_32BIT;
> > > + else
> > > + config->flags |= SMSC911X_USE_16BIT;
> >
> > Would it be better to do "else if (width == 2)"? (completely
> > uninformed comment. I've not looked at what the non-DT probe path
> > does on this driver.)
>
> I wouldn't have a problem with that. But currently the binding
> documentation says that any value other than 4, or a missing property,
> implies 16-bit register access.
>
> So the binding documentation would need to change too in that case.
>
> Personally I think this would be better, but it's just an opinion.
>
Yes. SMSC911X_USE_32BIT and SMSC911X_USE_16BIT are the only two cases
supported by the driver.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v3] net/smsc911x: add device tree probe support
@ 2011-09-09 12:59 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-09-09 12:59 UTC (permalink / raw)
To: Dave Martin
Cc: Grant Likely, Shawn Guo, netdev, patches, devicetree-discuss,
Steve Glendinning, David S. Miller, linux-arm-kernel
On Fri, Sep 09, 2011 at 09:50:30AM +0100, Dave Martin wrote:
> On Thu, Sep 08, 2011 at 11:29:20AM -0700, Grant Likely wrote:
> > On Thu, Sep 08, 2011 at 03:59:46PM +0100, Dave Martin wrote:
> > > Hi Shawn,
> > >
> > > On Sun, Jul 31, 2011 at 02:26:00AM +0800, Shawn Guo wrote:
> > > > It adds device tree probe support for smsc911x driver.
> > > >
> > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > Cc: David S. Miller <davem@davemloft.net>
> > > > Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> > > > ---
> > > > Changes since v2:
> > > > * Fix a typo in smsc911x.txt
> > > >
> > > > Changes since v1:
> > > > * Instead of getting irq line from gpio number, it use irq domain
> > > > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > > > * Use 'lan9115' the first model that smsc911x supports in the match
> > > > table
> > > > * Use reg-shift and reg-io-width which already used in of_serial for
> > > > shift and access size binding
> > >
> > > When using this patch with vexpress, I found that 16-bit register access
> > > mode doesn't seem to be getting set correctly.
> > >
> > > Can you take a look at this additional patch and let me know if it looks
> > > correct?
> > >
> > > Cheers
> > > ---Dave
> > >
> > > From: Dave Martin <dave.martin@linaro.org>
> > > Date: Wed, 7 Sep 2011 17:26:31 +0100
> > > Subject: [PATCH] net/smsc911x: Correctly configure 16-bit register access from DT
> > >
> > > The SMSC911X_USE_16BIT needs to be set when using 16-bit register
> > > access. However, currently no flag is set if the DT doesn't specify
> > > 32-bit access.
> > >
> > > This patch should set the SMSC911X_USE_16BIT flag in a manner consistent
> > > with the documented DT bindings.
> > >
> > > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > > ---
> > > drivers/net/smsc911x.c | 2 ++
> > > 1 files changed, 2 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> > > index 75c08a5..1a35c25 100644
> > > --- a/drivers/net/smsc911x.c
> > > +++ b/drivers/net/smsc911x.c
> > > @@ -2121,6 +2121,8 @@ static int __devinit smsc911x_probe_config_dt(
> > > of_property_read_u32(np, "reg-io-width", &width);
> > > if (width == 4)
> > > config->flags |= SMSC911X_USE_32BIT;
> > > + else
> > > + config->flags |= SMSC911X_USE_16BIT;
> >
> > Would it be better to do "else if (width == 2)"? (completely
> > uninformed comment. I've not looked at what the non-DT probe path
> > does on this driver.)
>
> I wouldn't have a problem with that. But currently the binding
> documentation says that any value other than 4, or a missing property,
> implies 16-bit register access.
>
> So the binding documentation would need to change too in that case.
>
> Personally I think this would be better, but it's just an opinion.
>
Yes. SMSC911X_USE_32BIT and SMSC911X_USE_16BIT are the only two cases
supported by the driver.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v3] net/smsc911x: add device tree probe support
@ 2011-09-09 12:59 ` Shawn Guo
0 siblings, 0 replies; 53+ messages in thread
From: Shawn Guo @ 2011-09-09 12:59 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 09, 2011 at 09:50:30AM +0100, Dave Martin wrote:
> On Thu, Sep 08, 2011 at 11:29:20AM -0700, Grant Likely wrote:
> > On Thu, Sep 08, 2011 at 03:59:46PM +0100, Dave Martin wrote:
> > > Hi Shawn,
> > >
> > > On Sun, Jul 31, 2011 at 02:26:00AM +0800, Shawn Guo wrote:
> > > > It adds device tree probe support for smsc911x driver.
> > > >
> > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > > Cc: Steve Glendinning <steve.glendinning@smsc.com>
> > > > Cc: David S. Miller <davem@davemloft.net>
> > > > Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
> > > > ---
> > > > Changes since v2:
> > > > * Fix a typo in smsc911x.txt
> > > >
> > > > Changes since v1:
> > > > * Instead of getting irq line from gpio number, it use irq domain
> > > > to keep platform_get_resource(IORESOURCE_IRQ) works for dt too.
> > > > * Use 'lan9115' the first model that smsc911x supports in the match
> > > > table
> > > > * Use reg-shift and reg-io-width which already used in of_serial for
> > > > shift and access size binding
> > >
> > > When using this patch with vexpress, I found that 16-bit register access
> > > mode doesn't seem to be getting set correctly.
> > >
> > > Can you take a look at this additional patch and let me know if it looks
> > > correct?
> > >
> > > Cheers
> > > ---Dave
> > >
> > > From: Dave Martin <dave.martin@linaro.org>
> > > Date: Wed, 7 Sep 2011 17:26:31 +0100
> > > Subject: [PATCH] net/smsc911x: Correctly configure 16-bit register access from DT
> > >
> > > The SMSC911X_USE_16BIT needs to be set when using 16-bit register
> > > access. However, currently no flag is set if the DT doesn't specify
> > > 32-bit access.
> > >
> > > This patch should set the SMSC911X_USE_16BIT flag in a manner consistent
> > > with the documented DT bindings.
> > >
> > > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > > ---
> > > drivers/net/smsc911x.c | 2 ++
> > > 1 files changed, 2 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> > > index 75c08a5..1a35c25 100644
> > > --- a/drivers/net/smsc911x.c
> > > +++ b/drivers/net/smsc911x.c
> > > @@ -2121,6 +2121,8 @@ static int __devinit smsc911x_probe_config_dt(
> > > of_property_read_u32(np, "reg-io-width", &width);
> > > if (width == 4)
> > > config->flags |= SMSC911X_USE_32BIT;
> > > + else
> > > + config->flags |= SMSC911X_USE_16BIT;
> >
> > Would it be better to do "else if (width == 2)"? (completely
> > uninformed comment. I've not looked at what the non-DT probe path
> > does on this driver.)
>
> I wouldn't have a problem with that. But currently the binding
> documentation says that any value other than 4, or a missing property,
> implies 16-bit register access.
>
> So the binding documentation would need to change too in that case.
>
> Personally I think this would be better, but it's just an opinion.
>
Yes. SMSC911X_USE_32BIT and SMSC911X_USE_16BIT are the only two cases
supported by the driver.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 53+ messages in thread
end of thread, other threads:[~2011-09-09 12:59 UTC | newest]
Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-25 9:44 [PATCH] net/smsc911x: add device tree probe support Shawn Guo
2011-07-25 9:44 ` Shawn Guo
2011-07-25 9:44 ` Shawn Guo
2011-07-25 21:37 ` Grant Likely
2011-07-25 21:37 ` Grant Likely
2011-07-26 1:01 ` Shawn Guo
2011-07-26 1:01 ` Shawn Guo
2011-07-26 1:16 ` Nicolas Pitre
2011-07-26 1:16 ` Nicolas Pitre
[not found] ` <alpine.LFD.2.00.1107252105561.12766-QuJgVwGFrdf/9pzu0YdTqQ@public.gmane.org>
2011-07-26 1:30 ` Shawn Guo
2011-07-26 1:30 ` Shawn Guo
2011-07-26 2:28 ` Nicolas Pitre
2011-07-26 2:28 ` Nicolas Pitre
2011-07-29 2:36 ` Shawn Guo
2011-07-29 2:36 ` Shawn Guo
2011-07-29 2:36 ` Shawn Guo
2011-07-29 15:47 ` Grant Likely
2011-07-29 15:47 ` Grant Likely
2011-07-29 16:03 ` Shawn Guo
2011-07-29 16:03 ` Shawn Guo
2011-07-29 16:03 ` Shawn Guo
2011-07-29 16:26 ` Grant Likely
2011-07-29 16:26 ` Grant Likely
2011-07-29 17:13 ` Shawn Guo
2011-07-29 17:13 ` Shawn Guo
2011-07-29 17:13 ` Shawn Guo
2011-07-31 0:42 ` Grant Likely
2011-07-31 0:42 ` Grant Likely
2011-07-29 15:45 ` Grant Likely
2011-07-29 15:45 ` Grant Likely
[not found] ` <1311587040-8988-1-git-send-email-shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2011-07-29 8:43 ` [PATCH v2] " Shawn Guo
2011-07-29 8:43 ` Shawn Guo
2011-07-29 15:53 ` Grant Likely
2011-07-29 15:53 ` Grant Likely
2011-07-29 16:16 ` Shawn Guo
2011-07-29 16:16 ` Shawn Guo
2011-07-29 16:16 ` Shawn Guo
2011-07-30 18:26 ` [PATCH v3] " Shawn Guo
2011-07-30 18:26 ` Shawn Guo
2011-08-02 1:01 ` David Miller
2011-08-02 1:01 ` David Miller
2011-09-08 14:59 ` Dave Martin
2011-09-08 14:59 ` Dave Martin
[not found] ` <20110908145946.GE2070-5wv7dgnIgG8@public.gmane.org>
2011-09-08 18:29 ` Grant Likely
2011-09-08 18:29 ` Grant Likely
2011-09-09 8:50 ` Dave Martin
2011-09-09 8:50 ` Dave Martin
2011-09-09 12:59 ` Shawn Guo
2011-09-09 12:59 ` Shawn Guo
2011-09-09 12:59 ` Shawn Guo
2011-09-09 11:48 ` Shawn Guo
2011-09-09 11:48 ` Shawn Guo
2011-09-09 11:48 ` Shawn Guo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.