Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757856Ab3CHNgo (ORCPT ); Fri, 8 Mar 2013 08:36:44 -0500 Received: from mail-wg0-f53.google.com ([74.125.82.53]:43788 "EHLO mail-wg0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757659Ab3CHNgm (ORCPT ); Fri, 8 Mar 2013 08:36:42 -0500 MIME-Version: 1.0 In-Reply-To: References: <1359380879-26306-1-git-send-email-prabhakar.lad@ti.com> <1359380879-26306-7-git-send-email-prabhakar.lad@ti.com> <510E5BCF.9080006@ti.com> From: Prabhakar Lad Date: Fri, 8 Mar 2013 18:58:33 +0530 Message-ID: Subject: Re: [PATCH v2 6/6] ARM: davinci: da850: configure system configuration chip(CFGCHIP3) for emac To: Sekhar Nori Cc: linux-arm-kernel@lists.infradead.org, davinci-linux-open-source@linux.davincidsp.com, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org, Heiko Schocher Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4164 Lines: 122 Sekhar, On Mon, Feb 4, 2013 at 10:33 AM, Prabhakar Lad wrote: > Sekhar , > > On Sun, Feb 3, 2013 at 6:15 PM, Sekhar Nori wrote: >> On 1/28/2013 7:17 PM, Prabhakar Lad wrote: >>> From: Lad, Prabhakar >>> >>> The system configuration chip CFGCHIP3, controls the emac module. >>> This patch appropriately configures this register for emac and >>> sets DA850_MII_MDIO_CLKEN_PIN GPIO pin appropriately. >>> >>> Signed-off-by: Lad, Prabhakar >>> Cc: linux-arm-kernel@lists.infradead.org >>> Cc: linux-kernel@vger.kernel.org >>> Cc: davinci-linux-open-source@linux.davincidsp.com >>> Cc: netdev@vger.kernel.org >>> Cc: devicetree-discuss@lists.ozlabs.org >>> Cc: Sekhar Nori >>> Cc: Heiko Schocher >>> --- >>> arch/arm/mach-davinci/da8xx-dt.c | 28 ++++++++++++++++++++++++++++ >>> 1 files changed, 28 insertions(+), 0 deletions(-) >>> >>> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c >>> index e533a0a..4a096e3 100644 >>> --- a/arch/arm/mach-davinci/da8xx-dt.c >>> +++ b/arch/arm/mach-davinci/da8xx-dt.c >>> @@ -8,6 +8,7 @@ >>> * published by the Free Software Foundation. >>> */ >>> #include >>> +#include >>> #include >>> #include >>> #include >>> @@ -39,6 +40,32 @@ static void __init da8xx_init_irq(void) >>> >>> #ifdef CONFIG_ARCH_DAVINCI_DA850 >>> >>> +static void __init da8xx_config_emac(void) >>> +{ >>> +#define DA850_MII_MDIO_CLKEN_PIN GPIO_TO_PIN(2, 6) >>> +#define DA850_EMAC_MODE_SELECT BIT(8) >>> + void __iomem *cfg_chip3_base; >>> + int ret; >>> + u32 val; >>> + >>> + cfg_chip3_base = DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG); >>> + >>> + val = __raw_readl(cfg_chip3_base); >>> + val &= ~DA850_EMAC_MODE_SELECT; >>> + /* configure the CFGCHIP3 register for MII */ >>> + __raw_writel(val, cfg_chip3_base); >> >> Use readl/writel instead. >> > Ok. > >>> + pr_info("EMAC: MII PHY configured\n"); >>> + >>> + ret = gpio_request(DA850_MII_MDIO_CLKEN_PIN, "mdio_clk_en"); >>> + if (ret) { >>> + pr_warn("Cannot open GPIO %d\n", >>> + DA850_MII_MDIO_CLKEN_PIN); >>> + return; >>> + } >>> + /* Enable/Disable MII MDIO clock */ >>> + gpio_direction_output(DA850_MII_MDIO_CLKEN_PIN, 0); >>> +} >>> + >>> struct of_dev_auxdata da8xx_auxdata[] __initdata = { >>> OF_DEV_AUXDATA("ti,davinci_mdio", 0x01e24000, "davinci_mdio.0", NULL), >>> OF_DEV_AUXDATA("ti,davinci-dm6467-emac", 0x01e20000, "davinci_emac.1", >>> @@ -52,6 +79,7 @@ static void __init da850_init_machine(void) >>> da8xx_auxdata, NULL); >>> >>> da8xx_uart_clk_enable(); >>> + da8xx_config_emac(); >> >> There are couple of issues with this implementation. >> >> 1) da8xx_config_emac() is specific to DA850 EVM, but masquerades as >> generic for da8xx. Looks like you need two functions, one for soc >> specific configuration and one board specific. > Ok. > >> 2) da8xx_config_emac() goes through all the time, whether the >> particular board has emac module or not. Shouldn't >> da8xx_config_emac() check if emac is actually enabled in the passed >> dtb and only the do the configuration? > > Ok. Can you give some pointers how we can access the dtb in board files. > Any pointers on this could be helpful. Regards, --Prabhakar >> 3) The function assumes mii is used always, you can use the rmii_en dt >> property to check if rmii/mii is enabled and configure the soc/board >> accordingly. > > Ok. > >> 4) If the same function can work both for da850 and da830, then it can >> be implemented outside of CONFIG_ARCH_DAVINCI_DA850. >> > Ok. > > Regards, > --Prabhakar > >> Thanks, >> Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/