netdev - Re: [PATCH v2 6/6] ARM: davinci: da850: configure system configuration chip(CFGCHIP3) for emac

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CA+V-a8uUFRRKrGRyq3wDs8ks_GzaGZGrTz+SoW9b2VQ8pb+DxA@mail.gmail.com>
Date:	Fri, 8 Mar 2013 18:58:33 +0530
From:	Prabhakar Lad <prabhakar.csengg@...il.com>
To:	Sekhar Nori <nsekhar@...com>
Cc:	linux-arm-kernel@...ts.infradead.org,
	davinci-linux-open-source@...ux.davincidsp.com,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	devicetree-discuss@...ts.ozlabs.org, Heiko Schocher <hs@...x.de>
Subject: Re: [PATCH v2 6/6] ARM: davinci: da850: configure system
 configuration chip(CFGCHIP3) for emac

Sekhar,

On Mon, Feb 4, 2013 at 10:33 AM, Prabhakar Lad
<prabhakar.csengg@...il.com> wrote:
> Sekhar ,
>
> On Sun, Feb 3, 2013 at 6:15 PM, Sekhar Nori <nsekhar@...com> wrote:
>> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>>> From: Lad, Prabhakar <prabhakar.lad@...com>
>>>
>>> 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 <prabhakar.lad@...com>
>>> Cc: linux-arm-kernel@...ts.infradead.org
>>> Cc: linux-kernel@...r.kernel.org
>>> Cc: davinci-linux-open-source@...ux.davincidsp.com
>>> Cc: netdev@...r.kernel.org
>>> Cc: devicetree-discuss@...ts.ozlabs.org
>>> Cc: Sekhar Nori <nsekhar@...com>
>>> Cc: Heiko Schocher <hs@...x.de>
>>> ---
>>>  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.
>
>>> +     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 netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html