Sekhar ,

On Sun, Feb 3, 2013 at 6:15 PM, Sekhar Nori [off-list ref] wrote:

On 1/28/2013 7:17 PM, Prabhakar Lad wrote:

quoted

From: Lad, Prabhakar <redacted>

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 <redacted>
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 <redacted>
Cc: Heiko Schocher <redacted>
---
 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 <linux/io.h>
+#include <linux/gpio.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/irqdomain.h>

@@ -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.

quoted

+     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.

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