| Message ID | 1368962191-32594-1-git-send-email-tomasz.figa@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 05/19/2013 01:16 PM, Tomasz Figa wrote: > +++ b/Documentation/devicetree/bindings/net/davicom-dm9000.txt > @@ -0,0 +1,26 @@ > +Davicom DM9000 Fast Ethernet controller > + > +Required properties: > +- compatible = "davicom,dm9000"; > +- reg : physical addresses and sizes of registers, must contain 2 entries: > + first entry : address register, > + second entry : address register. Two address registers ? Shouldn't one of these be "data register" ? > +Example: > + > + ethernet@18000000 { > + compatible = "davicom,dm9000"; > + reg =<0x18000000 0x2 0x18000004 0x2>; > + interrupt-parent =<&gpn>; > + interrupts =<7 4>; > + local-mac-address = [00 00 de ad be ef]; > + davicom,no-eeprom; > + }; > +static struct dm9000_plat_data *dm9000_parse_dt(struct device *dev) > +{ > + struct dm9000_plat_data *pdata; > + struct device_node *np = dev->of_node; > + const void *mac_addr; > + > + if (!IS_ENABLED(CONFIG_OF) || !np) > + return NULL; Shouldn't ERR_PTR() value be returned here ? > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) { > + dev_err(dev, "failed to allocate platform data struct\n"); There is no need for this error log, k*alloc already logs any failures. > @@ -1373,6 +1402,12 @@ dm9000_probe(struct platform_device *pdev) > int i; > u32 id_val; > > + if (!pdata) { > + pdata = dm9000_parse_dt(&pdev->dev); > + if (IS_ERR(pdata)) > + return PTR_ERR(pdata); > + } Thanks, Sylwester
Hi Sylwester, On Sunday 19 of May 2013 15:27:53 Sylwester Nawrocki wrote: > Hi, > > On 05/19/2013 01:16 PM, Tomasz Figa wrote: > > +++ b/Documentation/devicetree/bindings/net/davicom-dm9000.txt > > @@ -0,0 +1,26 @@ > > +Davicom DM9000 Fast Ethernet controller > > + > > +Required properties: > > +- compatible = "davicom,dm9000"; > > +- reg : physical addresses and sizes of registers, must contain 2 > > entries: + first entry : address register, > > + second entry : address register. > > Two address registers ? Shouldn't one of these be "data register" ? Oops. I thought I already corrected this typo. Thanks. > > +Example: > > + > > + ethernet@18000000 { > > + compatible = "davicom,dm9000"; > > + reg =<0x18000000 0x2 0x18000004 0x2>; > > + interrupt-parent =<&gpn>; > > + interrupts =<7 4>; > > + local-mac-address = [00 00 de ad be ef]; > > + davicom,no-eeprom; > > + }; > > > > +static struct dm9000_plat_data *dm9000_parse_dt(struct device *dev) > > +{ > > + struct dm9000_plat_data *pdata; > > + struct device_node *np = dev->of_node; > > + const void *mac_addr; > > + > > + if (!IS_ENABLED(CONFIG_OF) || !np) > > + return NULL; > > Shouldn't ERR_PTR() value be returned here ? Nope. No platform data is a valid case, so no error here. > > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > > + if (!pdata) { > > + dev_err(dev, "failed to allocate platform data struct\n"); > > There is no need for this error log, k*alloc already logs any failures. Hmm. Does it print what allocation exactly failed? (e.g. a backtrace) Not that it would give anything that could help you in an out of memory condition like this, but in general it's good to know in what point the failure happened. Best regards, Tomasz > > @@ -1373,6 +1402,12 @@ dm9000_probe(struct platform_device *pdev) > > > > int i; > > u32 id_val; > > > > + if (!pdata) { > > + pdata = dm9000_parse_dt(&pdev->dev); > > + if (IS_ERR(pdata)) > > + return PTR_ERR(pdata); > > + } > > Thanks, > > Sylwester
On 05/19/2013 03:41 PM, Tomasz Figa wrote: >>> +static struct dm9000_plat_data *dm9000_parse_dt(struct device *dev) >>> > > +{ >>> > > + struct dm9000_plat_data *pdata; >>> > > + struct device_node *np = dev->of_node; >>> > > + const void *mac_addr; >>> > > + >>> > > + if (!IS_ENABLED(CONFIG_OF) || !np) >>> > > + return NULL; >> > >> > Shouldn't ERR_PTR() value be returned here ? > > Nope. No platform data is a valid case, so no error here. OK, sorry, I should have checked that. >>> > > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >>> > > + if (!pdata) { >>> > > + dev_err(dev, "failed to allocate platform data struct\n"); >> > >> > There is no need for this error log, k*alloc already logs any failures. > > Hmm. Does it print what allocation exactly failed? (e.g. a backtrace) It seems the caller tracking is supported in subset of the kernel configurations. > Not that it would give anything that could help you in an out of memory > condition like this, but in general it's good to know in what point the > failure happened. As long as it is ENOMEM and the driver core reports failed probe I wouldn't really care. Nevertheless I saw patch series removing such already existing "Not enough memory" kind of error logs from the kernel, pointing out that mm already provides relevant error logging. Thanks, Sylwester
diff --git a/Documentation/devicetree/bindings/net/davicom-dm9000.txt b/Documentation/devicetree/bindings/net/davicom-dm9000.txt new file mode 100644 index 0000000..653df17 --- /dev/null +++ b/Documentation/devicetree/bindings/net/davicom-dm9000.txt @@ -0,0 +1,26 @@ +Davicom DM9000 Fast Ethernet controller + +Required properties: +- compatible = "davicom,dm9000"; +- reg : physical addresses and sizes of registers, must contain 2 entries: + first entry : address register, + second entry : address register. +- interrupt-parent : interrupt controller to which the device is connected +- interrupts : interrupt specifier specific to interrupt controller + +Optional properties: +- local-mac-address : A bytestring of 6 bytes specifying Ethernet MAC address + to use (from firmware or bootloader) +- davicom,no-eeprom : Configuration EEPROM is not available +- davicom,ext-phy : Use external PHY + +Example: + + ethernet@18000000 { + compatible = "davicom,dm9000"; + reg = <0x18000000 0x2 0x18000004 0x2>; + interrupt-parent = <&gpn>; + interrupts = <7 4>; + local-mac-address = [00 00 de ad be ef]; + davicom,no-eeprom; + }; diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 6931c43..2fe74e6 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -18,6 +18,7 @@ chrp Common Hardware Reference Platform cirrus Cirrus Logic, Inc. cortina Cortina Systems, Inc. dallas Maxim Integrated Products (formerly Dallas Semiconductor) +davicom DAVICOM Semiconductor, Inc. denx Denx Software Engineering emmicro EM Microelectronic epson Seiko Epson Corp. diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c index 9105465..8b69586 100644 --- a/drivers/net/ethernet/davicom/dm9000.c +++ b/drivers/net/ethernet/davicom/dm9000.c @@ -29,6 +29,8 @@ #include <linux/spinlock.h> #include <linux/crc32.h> #include <linux/mii.h> +#include <linux/of.h> +#include <linux/of_net.h> #include <linux/ethtool.h> #include <linux/dm9000.h> #include <linux/delay.h> @@ -1358,6 +1360,33 @@ static const struct net_device_ops dm9000_netdev_ops = { #endif }; +static struct dm9000_plat_data *dm9000_parse_dt(struct device *dev) +{ + struct dm9000_plat_data *pdata; + struct device_node *np = dev->of_node; + const void *mac_addr; + + if (!IS_ENABLED(CONFIG_OF) || !np) + return NULL; + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) { + dev_err(dev, "failed to allocate platform data struct\n"); + return ERR_PTR(-ENOMEM); + } + + if (of_find_property(np, "davicom,ext-phy", NULL)) + pdata->flags |= DM9000_PLATF_EXT_PHY; + if (of_find_property(np, "davicom,no-eeprom", NULL)) + pdata->flags |= DM9000_PLATF_NO_EEPROM; + + mac_addr = of_get_mac_address(np); + if (mac_addr) + memcpy(pdata->dev_addr, mac_addr, sizeof(pdata->dev_addr)); + + return pdata; +} + /* * Search DM9000 board, allocate space and register it */ @@ -1373,6 +1402,12 @@ dm9000_probe(struct platform_device *pdev) int i; u32 id_val; + if (!pdata) { + pdata = dm9000_parse_dt(&pdev->dev); + if (IS_ERR(pdata)) + return PTR_ERR(pdata); + } + /* Init network device */ ndev = alloc_etherdev(sizeof(struct board_info)); if (!ndev) @@ -1683,11 +1718,20 @@ dm9000_drv_remove(struct platform_device *pdev) return 0; } +#ifdef CONFIG_OF +static const struct of_device_id dm9000_of_matches[] = { + { .compatible = "davicom,dm9000", }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, dm9000_of_matches); +#endif + static struct platform_driver dm9000_driver = { .driver = { .name = "dm9000", .owner = THIS_MODULE, .pm = &dm9000_drv_pm_ops, + .of_match_table = of_match_ptr(dm9000_of_matches), }, .probe = dm9000_probe, .remove = dm9000_drv_remove,
This patch adds Device Tree support to dm9000 driver. Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com> --- Changes since v1: - dropped davicom,simple-phy property as it seems to be of no use - used of_get_mac_address() to get MAC address from device tree - replaced #ifdef CONFIG_OF around dm9000_parse_dt() with IS_ENABLED .../devicetree/bindings/net/davicom-dm9000.txt | 26 +++++++++++++ .../devicetree/bindings/vendor-prefixes.txt | 1 + drivers/net/ethernet/davicom/dm9000.c | 44 ++++++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/davicom-dm9000.txt