[RFC PATCH] net: dm9000: add support for device tree probing
Grant Likely
grant.likely at secretlab.ca
Wed Aug 10 06:51:55 EST 2011
On Tue, Aug 09, 2011 at 09:57:29PM +0200, Daniel Morsing wrote:
> This patch adds support for probing the dm9000 driver through device
> trees.
>
> The patch works by supplying its own platform_data struct when probed
> via device tree. This allows us to rely on the existing options parsing
> in the driver and avoid ifdeffery in the probe function.
>
> Signed-off-by: Daniel Morsing <daniel.morsing at gmail.com>
> ---
> Documentation/devicetree/bindings/net/dm9000.txt | 46 ++++++++++++
> drivers/net/dm9000.c | 85 ++++++++++++++++++++++
> 2 files changed, 131 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/dm9000.txt
>
> diff --git a/Documentation/devicetree/bindings/net/dm9000.txt b/Documentation/devicetree/bindings/net/dm9000.txt
> new file mode 100644
> index 0000000..f13ce20
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dm9000.txt
> @@ -0,0 +1,46 @@
> +Davicom DM9000 ethernet controller
> +
> +Required properties:
> +
> + - compatible : Should be "davicom,dm9000"
> +
> + - interrupts : Interrupt controller specific encoding, which specifies 1
> + or 2 interrupts. The first interrupt is for rx/tx received and is required
> + by the driver to function. The second interrupt is for wake on lan support
> + and is optional.
> +
> + - interrupt-flags : A one cell value which specifies the flag values for
> + the first interrupt, corresponding to the IORESOURCE_IRQ_* flags. An
> + example is using the value <8> for specifying the IORESOURCE_IRQ_LOWLEVEL
> + flag
Don't do this. First of all, the interrupt specifier should be
sufficient for specifying properties of the irq. Second, this is
encoding Linux internal implementation details into the device tree.
It is fine for a property to correspond with the numbers used by
Linux, but they must be explicitly specified. Don't simply say "the
same numbers as used by Linux".
> +
> + - reg : 2 Physical address specifiers, where the first specifies the address
> + register of device, and the second specifies the data register of the device
> +
> +Optional properties:
> +
> + - local-mac-address : Binary data specifying a mac address override
> +
> + - The following properties are all empty properties and specify one of the
> + flags that can be passed with the platform data to the driver. For specifics
> + about what the flags mean, see Documentation/networking/dm9000.txt
Ditto here on Linux values.
> +
> + 8bitonly: DM9000_PLATF_8BITONLY
> + 16bitonly: DM9000_PLATF_16BITONLY
> + 32bitonly: DM9000_PLATF_32BITONLY
You should probably use the reg-io-width property similar to the ns16550
driver.
> + ext-phy: DM9000_PLATF_EXT_PHY
> + no-eeprom: DM9000_PLATF_NO_EEPROM
> + simple-phy: DM9000_PLATF_SIMPLE_PHY
Can you use the phy-device binding used by other ethernet controllers
to specify an external phy?
> +
> +Example:
> +
> +ethernet0 at 2c000000 {
ethernet at 2c000000 (drop the '0'). If you want to enumerate it, use a
property in the /aliases node.
> + compatible = "davicom,dm9000";
> + reg = <0x2c000000 0x04>,
> + <0x2c000400 0x04>;
> + interrupts = <185>;
> + interrupt-flags = <8>; /* IORESOURCE_IRQ_LOWLEVEL */
> + local-mac-address = [02 65 16 01 c0 09];
> + 16bitonly;
> + no-eeprom;
> +};
> diff --git a/drivers/net/dm9000.c b/drivers/net/dm9000.c
> index 8ef31dc..4ad5043 100644
> --- a/drivers/net/dm9000.c
> +++ b/drivers/net/dm9000.c
> @@ -35,6 +35,7 @@
> #include <linux/platform_device.h>
> #include <linux/irq.h>
> #include <linux/slab.h>
> +#include <linux/of.h>
>
> #include <asm/delay.h>
> #include <asm/irq.h>
> @@ -1350,6 +1351,77 @@ static const struct net_device_ops dm9000_netdev_ops = {
> #endif
> };
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id dm9000_of_match[] = {
> + { .compatible = "davicom,dm9000" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, dm9000_of_match);
> +
> +static const struct {
> + const char *property;
> + unsigned int flag;
> +} property_lookup[] __devinitconst = {
> + { "8bitonly", DM9000_PLATF_8BITONLY},
> + { "16bitonly", DM9000_PLATF_16BITONLY},
> + { "32bitonly", DM9000_PLATF_32BITONLY},
> + { "ext-phy", DM9000_PLATF_EXT_PHY},
> + { "no-eeprom", DM9000_PLATF_NO_EEPROM},
> + { "simple-phy", DM9000_PLATF_SIMPLE_PHY},
> + {},
> +};
> +
> +static int __devinit
> +dm9000_dt_fill_platdata(struct platform_device *pdev,
> + struct dm9000_plat_data *pdata)
> +{
> + struct resource *irq_res;
> + struct device_node *of_node = pdev->dev.of_node;
> + const u8 *mac_addr;
> +
> + u32 intflags;
> + int i;
> +
> + if (of_property_read_u32(of_node, "interrupt-flags", &intflags)) {
> + dev_err(&pdev->dev,
> + "interrupt-flags property not found in device tree\n");
> + return -EINVAL;
> + }
Is this really a fatal error?
> +
> + irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (!irq_res) {
> + dev_err(&pdev->dev, "No irq found in device tree\n");
> + return -EINVAL;
> + }
> + irq_res->flags |= intflags;
> +
> + for (i = 0; property_lookup[i].property; i++) {
> + struct property *prop;
> + prop = of_find_property(of_node, property_lookup[i].property,
> + NULL);
> + if (prop) {
> + pdata->flags |= property_lookup[i].flag;
> + dev_dbg(&pdev->dev, "%s property found in device tree\n",
> + property_lookup[i].property);
> + }
> + }
> +
> + mac_addr = of_get_property(of_node, "local-mac-address", NULL);
> + if (mac_addr)
> + memcpy(pdata->dev_addr, mac_addr, 6);
> +
> + return 0;
> +}
> +
> +#else
> +#define dm9000_of_match NULL
> +static inline int dm9000_dt_fill_platdata(struct platform_device *pdev,
> + struct dm9000_plat_data *pdata)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_OF */
> +
> /*
> * Search DM9000 board, allocate space and register it
> */
> @@ -1359,12 +1431,24 @@ dm9000_probe(struct platform_device *pdev)
> struct dm9000_plat_data *pdata = pdev->dev.platform_data;
> struct board_info *db; /* Point a board information structure */
> struct net_device *ndev;
> + struct dm9000_plat_data pdata_of;
> const unsigned char *mac_src;
> int ret = 0;
> int iosize;
> int i;
> u32 id_val;
>
> + if (pdev->dev.of_node) {
> + if (!pdata) {
> + memset(&pdata_of, 0, sizeof(pdata_of));
> + pdata = &pdata_of;
> + }
> +
> + ret = dm9000_dt_fill_platdata(pdev, pdata);
> + if (ret < 0)
> + return ret;
> + }
> +
> /* Init network device */
> ndev = alloc_etherdev(sizeof(struct board_info));
> if (!ndev) {
> @@ -1677,6 +1761,7 @@ static struct platform_driver dm9000_driver = {
> .name = "dm9000",
> .owner = THIS_MODULE,
> .pm = &dm9000_drv_pm_ops,
> + .of_match_table = dm9000_of_match,
> },
> .probe = dm9000_probe,
> .remove = __devexit_p(dm9000_drv_remove),
Otherwise, looks pretty good.
g.
More information about the devicetree-discuss
mailing list