All of lore.kernel.org
 help / color / mirror / Atom feed
From: ezequiel.garcia@free-electrons.com (Ezequiel Garcia)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 1/2] ARM: kirkwood: retain MAC address for DT ethernet
Date: Thu, 3 Oct 2013 16:02:02 -0300	[thread overview]
Message-ID: <20131003190201.GA2504@localhost> (raw)
In-Reply-To: <1380826568-18587-1-git-send-email-sebastian.hesselbarth@gmail.com>

On Thu, Oct 03, 2013 at 08:56:08PM +0200, Sebastian Hesselbarth wrote:
> Ethernet IP on Kirkwood SoCs loose their MAC address register content
> if clock gated. To allow modular ethernet driver setups and gated clocks
> also on non-DT capable bootloaders, we fixup port device nodes with no
> valid MAC address property. This patch copies MAC address register
> contents set up by bootloaders early, notably before ethernet clocks
> are gated. While at it, also reorder call sequence in _dt_init.
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Changelog:
> v1->v2:
> - check for gated clock before accessing eth registers
>   (Suggested by Andrew Lunn)
> 
> TBH, as there is no "official" clk_is_enabled but just the __clk_is_enabled
> one, this is close to abuse of CCF API. Actually, there should either be an
> clk_is_enabled or we enable the clock, copy a possible invalid MAC address
> and disable the clock again. (Added Mike, who might have an opinion about
> clk_is_enabled)
> 
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> Cc: Mike Turquette <mturquette@linaro.org>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org
> ---
>  arch/arm/mach-kirkwood/board-dt.c |   77 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 74 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
> index 82d3ad8..f5f35bb 100644
> --- a/arch/arm/mach-kirkwood/board-dt.c
> +++ b/arch/arm/mach-kirkwood/board-dt.c
> @@ -13,6 +13,8 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_net.h>
>  #include <linux/of_platform.h>
>  #include <linux/clk-provider.h>
>  #include <linux/clocksource.h>
> @@ -66,6 +68,76 @@ static void __init kirkwood_legacy_clk_init(void)
>  	clk_prepare_enable(clk);
>  }
>  
> +#define MV643XX_ETH_MAC_ADDR_LOW	0x0414
> +#define MV643XX_ETH_MAC_ADDR_HIGH	0x0418
> +
> +static void __init kirkwood_dt_eth_fixup(void)
> +{
> +	struct device_node *np;
> +
> +	/*
> +	 * The ethernet interfaces forget the MAC address assigned by u-boot
> +	 * if the clocks are turned off. Usually, u-boot on kirkwood boards
> +	 * has no DT support to properly set local-mac-address property.
> +	 * As a workaround, we get the MAC address from mv643xx_eth registers
> +	 * and update the port device node if no valid MAC address is set.
> +	 */
> +	for_each_compatible_node(np, NULL, "marvell,kirkwood-eth-port") {
> +		struct device_node *pnp = of_get_parent(np);
> +		struct clk *clk;
> +		struct property *pmac;
> +		void __iomem *io;
> +		u8 *macaddr;
> +		u32 reg;
> +
> +		if (!pnp || !of_device_is_available(pnp))
> +			continue;
> +
> +		clk = of_clk_get(pnp, 0);
> +		if (!__clk_is_enabled(clk) || of_get_mac_address(np)) {

Mm.. just as you say this usage of __clk_is_enabled is awkard to say the
least.

I'm wondering: is the clock really disabled if the device is not
available (i.e. status == 'ok')? In other words: isn't the
!of_device_is_available() test enough?

Or maybe I'm missing something?
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Jason Cooper <jason@lakedaemon.net>, Andrew Lunn <andrew@lunn.ch>,
	Russell King <linux@arm.linux.org.uk>,
	Grant Likely <grant.likely@linaro.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Jason Gunthorpe <jgunthorpe@obsidianresearch.com>,
	Mike Turquette <mturquette@linaro.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] ARM: kirkwood: retain MAC address for DT ethernet
Date: Thu, 3 Oct 2013 16:02:02 -0300	[thread overview]
Message-ID: <20131003190201.GA2504@localhost> (raw)
In-Reply-To: <1380826568-18587-1-git-send-email-sebastian.hesselbarth@gmail.com>

On Thu, Oct 03, 2013 at 08:56:08PM +0200, Sebastian Hesselbarth wrote:
> Ethernet IP on Kirkwood SoCs loose their MAC address register content
> if clock gated. To allow modular ethernet driver setups and gated clocks
> also on non-DT capable bootloaders, we fixup port device nodes with no
> valid MAC address property. This patch copies MAC address register
> contents set up by bootloaders early, notably before ethernet clocks
> are gated. While at it, also reorder call sequence in _dt_init.
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Changelog:
> v1->v2:
> - check for gated clock before accessing eth registers
>   (Suggested by Andrew Lunn)
> 
> TBH, as there is no "official" clk_is_enabled but just the __clk_is_enabled
> one, this is close to abuse of CCF API. Actually, there should either be an
> clk_is_enabled or we enable the clock, copy a possible invalid MAC address
> and disable the clock again. (Added Mike, who might have an opinion about
> clk_is_enabled)
> 
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> Cc: Mike Turquette <mturquette@linaro.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  arch/arm/mach-kirkwood/board-dt.c |   77 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 74 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
> index 82d3ad8..f5f35bb 100644
> --- a/arch/arm/mach-kirkwood/board-dt.c
> +++ b/arch/arm/mach-kirkwood/board-dt.c
> @@ -13,6 +13,8 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_net.h>
>  #include <linux/of_platform.h>
>  #include <linux/clk-provider.h>
>  #include <linux/clocksource.h>
> @@ -66,6 +68,76 @@ static void __init kirkwood_legacy_clk_init(void)
>  	clk_prepare_enable(clk);
>  }
>  
> +#define MV643XX_ETH_MAC_ADDR_LOW	0x0414
> +#define MV643XX_ETH_MAC_ADDR_HIGH	0x0418
> +
> +static void __init kirkwood_dt_eth_fixup(void)
> +{
> +	struct device_node *np;
> +
> +	/*
> +	 * The ethernet interfaces forget the MAC address assigned by u-boot
> +	 * if the clocks are turned off. Usually, u-boot on kirkwood boards
> +	 * has no DT support to properly set local-mac-address property.
> +	 * As a workaround, we get the MAC address from mv643xx_eth registers
> +	 * and update the port device node if no valid MAC address is set.
> +	 */
> +	for_each_compatible_node(np, NULL, "marvell,kirkwood-eth-port") {
> +		struct device_node *pnp = of_get_parent(np);
> +		struct clk *clk;
> +		struct property *pmac;
> +		void __iomem *io;
> +		u8 *macaddr;
> +		u32 reg;
> +
> +		if (!pnp || !of_device_is_available(pnp))
> +			continue;
> +
> +		clk = of_clk_get(pnp, 0);
> +		if (!__clk_is_enabled(clk) || of_get_mac_address(np)) {

Mm.. just as you say this usage of __clk_is_enabled is awkard to say the
least.

I'm wondering: is the clock really disabled if the device is not
available (i.e. status == 'ok')? In other words: isn't the
!of_device_is_available() test enough?

Or maybe I'm missing something?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

  reply	other threads:[~2013-10-03 19:02 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-03 12:44 [PATCH 1/2] ARM: kirkwood: retain MAC address for DT ethernet Sebastian Hesselbarth
2013-10-03 12:44 ` Sebastian Hesselbarth
2013-10-03 12:44 ` [PATCH 2/2] ARM: kirkwood: remove ethernet clock gate workaround Sebastian Hesselbarth
2013-10-03 12:44   ` Sebastian Hesselbarth
2013-10-03 13:50   ` Ezequiel Garcia
2013-10-03 13:50     ` Ezequiel Garcia
2013-10-03 13:47 ` [PATCH 1/2] ARM: kirkwood: retain MAC address for DT ethernet Andrew Lunn
2013-10-03 13:47   ` Andrew Lunn
2013-10-03 15:15   ` Sebastian Hesselbarth
2013-10-03 15:15     ` Sebastian Hesselbarth
2013-10-03 18:56 ` [PATCH v2 " Sebastian Hesselbarth
2013-10-03 18:56   ` Sebastian Hesselbarth
2013-10-03 19:02   ` Ezequiel Garcia [this message]
2013-10-03 19:02     ` Ezequiel Garcia
2013-10-03 19:04     ` Jason Cooper
2013-10-03 19:04       ` Jason Cooper
2013-10-03 19:44       ` Jason Gunthorpe
2013-10-03 19:44         ` Jason Gunthorpe
2013-10-03 20:14         ` Sebastian Hesselbarth
2013-10-03 20:14           ` Sebastian Hesselbarth
2013-10-03 20:24         ` Andrew Lunn
2013-10-03 20:24           ` Andrew Lunn
2013-10-04 10:13   ` [PATCH v3] ARM: kirkwood: remove ethernet clock gate workaround Sebastian Hesselbarth
2013-10-04 10:13     ` Sebastian Hesselbarth
2013-10-04 10:14     ` Sebastian Hesselbarth
2013-10-04 10:14       ` Sebastian Hesselbarth
2013-10-04 10:17   ` [PATCH RESEND v3] ARM: kirkwood: retain MAC address for DT ethernet Sebastian Hesselbarth
2013-10-04 10:17     ` Sebastian Hesselbarth
2013-10-04 13:28     ` Andrew Lunn
2013-10-04 13:28       ` Andrew Lunn
2013-10-07 20:25     ` [PATCH v4] " Sebastian Hesselbarth
2013-10-07 20:25       ` Sebastian Hesselbarth
2013-10-07 23:30       ` Mike Turquette
2013-10-07 23:30         ` Mike Turquette
2013-10-08 17:03       ` Jason Cooper
2013-10-08 17:03         ` Jason Cooper

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=20131003190201.GA2504@localhost \
    --to=ezequiel.garcia@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.