[PATCH v3 4/7] ARM: davinci: net: davinci_emac: add OF support
Heiko Schocher
hs at denx.de
Thu May 17 16:32:27 EST 2012
Hello Nori,
thanks for the review!
Nori, Sekhar wrote:
> Hi Heiko,
>
> On Mon, Mar 05, 2012 at 16:40:01, Heiko Schocher wrote:
>> add of support for the davinci_emac driver.
>>
>> Signed-off-by: Heiko Schocher <hs at denx.de>
>> Cc: davinci-linux-open-source at linux.davincidsp.com
>> Cc: linux-arm-kernel at lists.infradead.org
>> Cc: devicetree-discuss at lists.ozlabs.org
>> Cc: netdev at vger.kernel.org
>> Cc: Grant Likely <grant.likely at secretlab.ca>
>> Cc: Sekhar Nori <nsekhar at ti.com>
>> Cc: Wolfgang Denk <wd at denx.de>
>> Cc: Anatoly Sivov <mm05 at mail.ru>
>>
>> ---
>> - changes for v2:
>> - add comment from Anatoly Sivov
>> - fix typo in davinci_emac.txt
>> - add comment from Grant Likely:
>> - add prefix "ti,davinci-" to davinci specific property names
>> - remove version property
>> - use compatible name "ti,davinci-dm6460-emac"
>> - use devm_kzalloc()
>> - use of_match_ptr()
>> - document all new properties
>> - remove of_address_to_resource() and do not overwrite
>> resource table
>> - whitespace fixes
>> - remove hw_ram_addr as it is not used in current
>> board code
>> - no changes for v3
>>
>> .../bindings/arm/davinci/davinci_emac.txt | 43 +++++++++
>> drivers/net/ethernet/ti/davinci_emac.c | 94 +++++++++++++++++++-
>> 2 files changed, 136 insertions(+), 1 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/arm/davinci/davinci_emac.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/davinci/davinci_emac.txt b/Documentation/devicetree/bindings/arm/davinci/davinci_emac.txt
>> new file mode 100644
>> index 0000000..a7b0911
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/davinci/davinci_emac.txt
>
> Since DaVinci EMAC driver may be used on platforms other than DaVinci (c6x,
> OMAP), can we place the bindings documentation in bindings/net/?
Done.
>> @@ -0,0 +1,43 @@
>> +* Texas Instruments Davinci EMAC
>> +
>> +This file provides information, what the device node
>> +for the davinci_emac interface contain.
>
> s/contain/contains
fixed.
>> +
>> +Required properties:
>> +- compatible: "ti,davinci-dm6460-emac";
>
> There is no device called dm6460. If you intend to only support
> "version 2" of the EMAC IP at this time, you can use dm6467
> (http://www.ti.com/product/tms320dm6467)
renamed to "ti,davinci-dm6467-emac"
>> +- reg: Offset and length of the register set for the device
>> +- ti,davinci-ctrl-reg-offset: offset to control register
>> +- ti,davinci-ctrl-mod-reg-offset: offset to control module register
>> +- ti,davinci-ctrl-ram-offset: offset to control module ram
>> +- ti,davinci-ctrl-ram-size: size of control module ram
>> +- ti,davinci-rmii-en: use RMII
>> +- ti,davinci-no-bd-ram: has the emac controller BD RAM
>> +- phy-handle: Contains a phandle to an Ethernet PHY.
>> + if not, davinci_emac driver defaults to 100/FULL
>> +- interrupts: interrupt mapping for the davinci emac interrupts sources:
>> + 4 sources: <Receive Threshold Interrupt
>> + Receive Interrupt
>> + Transmit Interrupt
>> + Miscellaneous Interrupt>
>> +- pinmux-handle: Contains a handle to configure the pinmux settings.
removed.
>> +
>> +Optional properties:
>> +- local-mac-address : 6 bytes, mac address
>> +
>> +Example (enbw_cmc board):
>> + eth0: emac at 1e20000 {
>> + compatible = "ti,davinci-dm6460-emac";
>> + reg = <0x220000 0x4000>;
>> + ti,davinci-ctrl-reg-offset = <0x3000>;
>> + ti,davinci-ctrl-mod-reg-offset = <0x2000>;
>> + ti,davinci-ctrl-ram-offset = <0>;
>> + ti,davinci-ctrl-ram-size = <0x2000>;
>> + local-mac-address = [ 00 00 00 00 00 00 ];
>> + interrupts = <33
>> + 34
>> + 35
>> + 36
>> + >;
>> + interrupt-parent = <&intc>;
>> + pinmux-handle = <&emac_pins>;
removed.
>> + };
>> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
>> index 4fa0bcb..56e1c35 100644
>> --- a/drivers/net/ethernet/ti/davinci_emac.c
>> +++ b/drivers/net/ethernet/ti/davinci_emac.c
>> @@ -58,6 +58,12 @@
>> #include <linux/io.h>
>> #include <linux/uaccess.h>
>> #include <linux/davinci_emac.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_net.h>
>> +
>> +#include <mach/mux.h>
>>
>> #include <asm/irq.h>
>> #include <asm/page.h>
>> @@ -339,6 +345,9 @@ struct emac_priv {
>> u32 rx_addr_type;
>> atomic_t cur_tx;
>> const char *phy_id;
>> +#ifdef CONFIG_OF
>> + struct device_node *phy_node;
>> +#endif
>> struct phy_device *phydev;
>> spinlock_t lock;
>> /*platform specific members*/
>> @@ -1760,6 +1769,82 @@ static const struct net_device_ops emac_netdev_ops = {
>> #endif
>> };
>>
>> +#ifdef CONFIG_OF
>> +static struct emac_platform_data
>> + *davinci_emac_of_get_pdata(struct platform_device *pdev,
>> + struct emac_priv *priv)
>> +{
>> + struct device_node *np;
>> + struct device_node *pinmux_np;
>> + struct emac_platform_data *pdata = NULL;
>> + const u8 *mac_addr;
>> + u32 data;
>> + int ret;
>> + int version;
>> +
>> + np = pdev->dev.of_node;
>> + if (!np)
>> + goto nodata;
>> + else
>> + version = EMAC_VERSION_2;
>
> You could set pdata->version directly here.
done.
>> +
>> + pdata = pdev->dev.platform_data;
>> + if (!pdata) {
>> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>> + if (!pdata)
>> + goto nodata;
>> + }
>> + pdata->version = version;
>> +
>> + mac_addr = of_get_mac_address(np);
>> + if (mac_addr)
>> + memcpy(pdata->mac_addr, mac_addr, ETH_ALEN);
>> +
>> + ret = of_property_read_u32(np, "ti,davinci-ctrl-reg-offset", &data);
>> + if (!ret)
>> + pdata->ctrl_reg_offset = data;
>> +
>> + ret = of_property_read_u32(np, "ti,davinci-ctrl-mod-reg-offset",
>> + &data);
>> + if (!ret)
>> + pdata->ctrl_mod_reg_offset = data;
>> +
>> + ret = of_property_read_u32(np, "ti,davinci-ctrl-ram-offset", &data);
>> + if (!ret)
>> + pdata->ctrl_ram_offset = data;
>> +
>> + ret = of_property_read_u32(np, "ti,davinci-ctrl-ram-size", &data);
>> + if (!ret)
>> + pdata->ctrl_ram_size = data;
>> +
>> + ret = of_property_read_u32(np, "ti,davinci-rmii-en", &data);
>> + if (!ret)
>> + pdata->rmii_en = data;
>> +
>> + ret = of_property_read_u32(np, "ti,davinci-no-bd-ram", &data);
>> + if (!ret)
>> + pdata->no_bd_ram = data;
>> +
>> + priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
>> + if (!priv->phy_node)
>> + pdata->phy_id = "";
>> +
>> + pinmux_np = of_parse_phandle(np, "pinmux-handle", 0);
>> + if (pinmux_np)
>> + davinci_cfg_reg_of(pinmux_np);
>
> This is a DaVinci specific pinmux function and this
> driver can be used in non-DaVinci platforms like C6x
> and OMAP. So, it will not be correct to call a DaVinci
> specific function here.
Ah, right!
> Can you drop the pinmux from this patch for now? On DaVinci,
Done ... Hmm.. so I think, I should drop this for all patches
from my patchset, right?
> for pinmux, we need to migrate to drivers/pinctrl/ as well.
Ah, I see ... take a look at it, maybe I find time to do here
something ... or do you know about work in progress here?
> Doing this will also make this patch independent of the rest
> of this series can even be merged separately. Can you please
> make these changes and resend just this patch?
Yep, I do some test with the changes you requested and resend
this patch ... do you prefer some tree, which I should use as
base?
>> +
>> + pdev->dev.platform_data = pdata;
>> +nodata:
>> + return pdata;
>> +}
>> +#else
>> +static struct emac_platform_data
>> + *davinci_emac_of_get_pdata(struct platform_device *pdev,
>> + struct emac_priv *priv)
>> +{
>> + return pdev->dev.platform_data;
>> +}
>> +#endif
>> /**
>> * davinci_emac_probe: EMAC device probe
>> * @pdev: The DaVinci EMAC device that we are removing
>> @@ -1803,7 +1888,7 @@ static int __devinit davinci_emac_probe(struct platform_device *pdev)
>>
>> spin_lock_init(&priv->lock);
>>
>> - pdata = pdev->dev.platform_data;
>> + pdata = davinci_emac_of_get_pdata(pdev, priv);
>> if (!pdata) {
>> dev_err(&pdev->dev, "no platform data\n");
>> rc = -ENODEV;
>> @@ -2013,6 +2098,12 @@ static const struct dev_pm_ops davinci_emac_pm_ops = {
>> .resume = davinci_emac_resume,
>> };
>>
>> +static const struct of_device_id davinci_emac_of_match[] = {
>> + {.compatible = "ti,davinci-dm6460-emac", },
>
> This needs to be ti,davinci-dm6467-emac as well.
Yep, fixed.
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
More information about the devicetree-discuss
mailing list