From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft•com>
To: Jamie Iles <jamie@jamieiles•com>
Cc: devicetree-discuss@ozlabs•org, netdev@vger•kernel.org,
Nicolas Ferre <nicolas.ferre@atmel•com>
Subject: Re: [PATCH 1/1] net/macb: add DT support
Date: Sun, 20 Nov 2011 17:47:40 +0100 [thread overview]
Message-ID: <20111120164740.GC21480@game.jcrosoft.org> (raw)
In-Reply-To: <20111118155824.GA9981@totoro>
On 15:58 Fri 18 Nov , Jamie Iles wrote:
> Hi Jean-Christophe,
>
> On Fri, Nov 18, 2011 at 03:29:25PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > allow the DT to pass the mac address and the phy mode
> >
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft•com>
> > Cc: Jamie Iles <jamie@jamieiles•com>
> > Cc: Nicolas Ferre <nicolas.ferre@atmel•com>
>
> This looks OK to me in principle. I can't easily test this at the
> moment, but as I don't have a DT platform that has the clk framework up
> and running. A couple of nits/questions inline, but thanks for doing
> this!
>
> Jamie
>
> > ---
> > Documentation/devicetree/bindings/net/macb.txt | 22 ++++++++
> > drivers/net/ethernet/cadence/macb.c | 65 +++++++++++++++++++++---
> > drivers/net/ethernet/cadence/macb.h | 2 +
> > 3 files changed, 81 insertions(+), 8 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/net/macb.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
> > new file mode 100644
> > index 0000000..2b727ec
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/macb.txt
> > @@ -0,0 +1,22 @@
> > +* Cadence EMACB
> > +
> > +Implemeted on Atmel AT91 & AVR32 SoC
>
> I think something along the lines of "Binding for the Cadence MACB
> Ethernet controller" rather than listing specific parts might be
> clearer.
I prefer as we will have implementation detail in the binding
>
> > +
> > +Required properties:
> > +- compatible : Should be "atmel,macb" for Atmel
> > +- reg : Address and length of the register set for the device
> > +- interrupts : Should contain macb interrupt
> > +- phy-mode : String, operation mode of the PHY interface.
> > + Supported values are: "mii", "rmii",
> > +
> > +Optional properties:
> > +- local-mac-address : 6 bytes, mac address
> > +
> > +Examples:
> > +
> > + macb0: macb@fffc4000 {
>
> Rob pointed out to me a little while ago that the preferred naming from
> the ePAPR document would be:
>
> macb0: ethernet@fffc4000
>
> so it might be worth being consistent here.
ok
>
> > + compatible = "atmel,macb";
>
> This should be "cdns,macb" as it isn't Atmel specific. I believe cdns
> is the correct stock ticker symbol for Cadence.
here I put "atmel,macb" on purpose to specify the difference of the IP between
the soc, in fact it should have been atmel-at91,macb
>
> > + reg = <oxfffc4000 0x4000>;
> > + interrupts = <21>;
> > + phy-mode = "mii";
> > + };
> > diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> > index a437b46..2c345bc 100644
> > --- a/drivers/net/ethernet/cadence/macb.c
> > +++ b/drivers/net/ethernet/cadence/macb.c
> > @@ -20,6 +20,9 @@
> > #include <linux/etherdevice.h>
> > #include <linux/dma-mapping.h>
> > #include <linux/platform_device.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_net.h>
> > #include <linux/phy.h>
> >
> > #include <mach/board.h>
> > @@ -81,6 +84,20 @@ static void __init macb_get_hwaddr(struct macb *bp)
> > addr[4] = top & 0xff;
> > addr[5] = (top >> 8) & 0xff;
> >
> > +#ifdef CONFIG_OF
> > + /*
> > + * 2) from device tree data
> > + */
> > + if (!is_valid_ether_addr(addr)) {
> > + struct device_node *np = bp->pdev->dev.of_node;
> > + if (np) {
> > + const char *mac = of_get_mac_address(np);
> > + if (mac)
> > + memcpy(addr, mac, sizeof(addr));
> > + }
> > + }
> > +#endif
>
> I'm a bit conflicted here. I think we should always use the MAC address
> from the device tree if it is present even if the current MAC address is
> valid.
if the mac is already programmed in the register we just keep it
I prefer this way if the bootloader set it we keep it
Best Regards,
J.
next prev parent reply other threads:[~2011-11-20 16:50 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-18 14:29 [PATCH 1/1] net/macb: add DT support Jean-Christophe PLAGNIOL-VILLARD
2011-11-18 15:58 ` Jamie Iles
2011-11-20 16:47 ` Jean-Christophe PLAGNIOL-VILLARD [this message]
2011-11-20 17:11 ` Jamie Iles
2011-11-21 10:08 ` Nicolas Ferre
2011-12-02 15:30 ` Nicolas Ferre
2011-12-02 15:38 ` Jamie Iles
2011-12-02 17:14 ` [PATCH] " Nicolas Ferre
[not found] ` <1322846050-4543-1-git-send-email-nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
2011-12-02 17:28 ` Jamie Iles
2011-12-02 17:53 ` Nicolas Ferre
2011-12-05 11:48 ` Jamie Iles
2011-12-05 11:51 ` Nicolas Ferre
2011-12-02 17:43 ` [PATCH v2] " Nicolas Ferre
[not found] ` <1322847782-22650-1-git-send-email-nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
2011-12-02 17:50 ` [PATCH] ARM: at91/net: add macb ethernet controller in 9g45 DT Nicolas Ferre
2011-12-03 5:56 ` Jean-Christophe PLAGNIOL-VILLARD
[not found] ` <20111203055659.GL18533-RQcB7r2h9QmfDR2tN2SG5Ni2O/JbrIOy@public.gmane.org>
2011-12-05 11:39 ` Nicolas Ferre
2011-12-05 11:59 ` [PATCH v3 1/2] net/macb: add DT support for Cadence macb/gem driver Nicolas Ferre
2011-12-05 11:59 ` [PATCH v3 2/2] ARM: at91/net: add macb ethernet controller in 9g45 DT Nicolas Ferre
2011-12-05 15:25 ` Jean-Christophe PLAGNIOL-VILLARD
2011-12-07 13:49 ` [PATCH v3 1/2] net/macb: add DT support for Cadence macb/gem driver Nicolas Ferre
2011-12-07 18:27 ` David Miller
2011-12-02 17:58 ` [PATCH v2] net/macb: add DT support David Miller
[not found] ` <20111202.125832.1208514279272697863.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2011-12-05 11:36 ` Nicolas Ferre
2011-11-21 11:08 ` [PATCH 1/1] " Nicolas Ferre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111120164740.GC21480@game.jcrosoft.org \
--to=plagnioj@jcrosoft$(echo .)com \
--cc=devicetree-discuss@ozlabs$(echo .)org \
--cc=jamie@jamieiles$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=nicolas.ferre@atmel$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox