diff mbox

[v4,08/18] net: davinci_emac: potentially get the MAC address from MTD

Message ID 20180629094039.7543-9-brgl@bgdev.pl (mailing list archive)
State New, archived
Headers show

Commit Message

Bartosz Golaszewski June 29, 2018, 9:40 a.m. UTC
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

On da850-evm board we can read the MAC address from MTD. It's currently
done in the relevant board file, but we want to get rid of all the MAC
reading callbacks from the board file (SPI and NAND). Move the reading
of the MAC address from SPI to the emac driver's probe function.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/net/ethernet/ti/davinci_emac.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Comments

David Lechner June 29, 2018, 8:09 p.m. UTC | #1
On 06/29/2018 04:40 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> On da850-evm board we can read the MAC address from MTD. It's currently
> done in the relevant board file, but we want to get rid of all the MAC
> reading callbacks from the board file (SPI and NAND). Move the reading
> of the MAC address from SPI to the emac driver's probe function.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>   drivers/net/ethernet/ti/davinci_emac.c | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> index a1a6445b5a7e..48e6a7755811 100644
> --- a/drivers/net/ethernet/ti/davinci_emac.c
> +++ b/drivers/net/ethernet/ti/davinci_emac.c
> @@ -67,7 +67,7 @@
>   #include <linux/of_irq.h>
>   #include <linux/of_net.h>
>   #include <linux/mfd/syscon.h>
> -
> +#include <linux/mtd/mtd.h>
>   #include <asm/irq.h>
>   #include <asm/page.h>
>   
> @@ -1783,7 +1783,10 @@ static int davinci_emac_probe(struct platform_device *pdev)
>   	struct cpdma_params dma_params;
>   	struct clk *emac_clk;
>   	unsigned long emac_bus_frequency;
> -
> +#ifdef CONFIG_MTD
> +	size_t mac_addr_len;
> +	struct mtd_info *mtd;
> +#endif /* CONFIG_MTD */
>   
>   	/* obtain emac clock from kernel */
>   	emac_clk = devm_clk_get(&pdev->dev, NULL);
> @@ -1815,6 +1818,19 @@ static int davinci_emac_probe(struct platform_device *pdev)
>   		goto err_free_netdev;
>   	}
>   
> +#ifdef CONFIG_MTD

What about the case when MTD is compiled as a module?

> +	mtd = get_mtd_device_nm("MAC-Address");

What about the case when PTR_ERR(mtd) == -EPROBE_DEFER?

> +	if (!IS_ERR(mtd)) {
> +		rc = mtd_read(mtd, 0, ETH_ALEN,
> +			      &mac_addr_len, priv->mac_addr);
> +		if (rc == 0)
> +			dev_info(&pdev->dev,
> +				 "Read MAC addr from SPI Flash: %pM\n",
> +				 priv->mac_addr);
> +		put_mtd_device(mtd);
> +	}
> +#endif /* CONFIG_MTD */
> +
>   	/* MAC addr and PHY mask , RMII enable info from platform_data */
>   	memcpy(priv->mac_addr, pdata->mac_addr, ETH_ALEN);
>   	priv->phy_id = pdata->phy_id;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Lechner June 29, 2018, 8:35 p.m. UTC | #2
On 06/29/2018 03:09 PM, David Lechner wrote:
> On 06/29/2018 04:40 AM, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> On da850-evm board we can read the MAC address from MTD. It's currently
>> done in the relevant board file, but we want to get rid of all the MAC
>> reading callbacks from the board file (SPI and NAND). Move the reading
>> of the MAC address from SPI to the emac driver's probe function.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>   drivers/net/ethernet/ti/davinci_emac.c | 20 ++++++++++++++++++--
>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
>> index a1a6445b5a7e..48e6a7755811 100644
>> --- a/drivers/net/ethernet/ti/davinci_emac.c
>> +++ b/drivers/net/ethernet/ti/davinci_emac.c
>> @@ -67,7 +67,7 @@
>>   #include <linux/of_irq.h>
>>   #include <linux/of_net.h>
>>   #include <linux/mfd/syscon.h>
>> -
>> +#include <linux/mtd/mtd.h>
>>   #include <asm/irq.h>
>>   #include <asm/page.h>
>> @@ -1783,7 +1783,10 @@ static int davinci_emac_probe(struct platform_device *pdev)
>>       struct cpdma_params dma_params;
>>       struct clk *emac_clk;
>>       unsigned long emac_bus_frequency;
>> -
>> +#ifdef CONFIG_MTD
>> +    size_t mac_addr_len;
>> +    struct mtd_info *mtd;
>> +#endif /* CONFIG_MTD */
>>       /* obtain emac clock from kernel */
>>       emac_clk = devm_clk_get(&pdev->dev, NULL);
>> @@ -1815,6 +1818,19 @@ static int davinci_emac_probe(struct platform_device *pdev)
>>           goto err_free_netdev;
>>       }
>> +#ifdef CONFIG_MTD
> 
> What about the case when MTD is compiled as a module?
> 
>> +    mtd = get_mtd_device_nm("MAC-Address");
> 
> What about the case when PTR_ERR(mtd) == -EPROBE_DEFER?

To answer my own question: because get_mtd_device_nm() doesn't
ever return -EPROBE_DEFER.

I'm trying to make this work on LCDK, but the emac driver probes
before any mtd device is registered, so, I just get -ENODEV even
though I've added a partition to the device tree labeled
"MAC-Address". You can see in the kernel messages that MTD is
not probed until later.

> 
>> +    if (!IS_ERR(mtd)) {
>> +        rc = mtd_read(mtd, 0, ETH_ALEN,
>> +                  &mac_addr_len, priv->mac_addr);
>> +        if (rc == 0)
>> +            dev_info(&pdev->dev,
>> +                 "Read MAC addr from SPI Flash: %pM\n",
>> +                 priv->mac_addr);
>> +        put_mtd_device(mtd);
>> +    }
>> +#endif /* CONFIG_MTD */
>> +
>>       /* MAC addr and PHY mask , RMII enable info from platform_data */
>>       memcpy(priv->mac_addr, pdata->mac_addr, ETH_ALEN);
>>       priv->phy_id = pdata->phy_id;
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartosz Golaszewski July 2, 2018, 7:41 a.m. UTC | #3
2018-06-29 22:35 GMT+02:00 David Lechner <david@lechnology.com>:
> On 06/29/2018 03:09 PM, David Lechner wrote:
>>
>> On 06/29/2018 04:40 AM, Bartosz Golaszewski wrote:
>>>
>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>
>>> On da850-evm board we can read the MAC address from MTD. It's currently
>>> done in the relevant board file, but we want to get rid of all the MAC
>>> reading callbacks from the board file (SPI and NAND). Move the reading
>>> of the MAC address from SPI to the emac driver's probe function.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> ---
>>>   drivers/net/ethernet/ti/davinci_emac.c | 20 ++++++++++++++++++--
>>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/ti/davinci_emac.c
>>> b/drivers/net/ethernet/ti/davinci_emac.c
>>> index a1a6445b5a7e..48e6a7755811 100644
>>> --- a/drivers/net/ethernet/ti/davinci_emac.c
>>> +++ b/drivers/net/ethernet/ti/davinci_emac.c
>>> @@ -67,7 +67,7 @@
>>>   #include <linux/of_irq.h>
>>>   #include <linux/of_net.h>
>>>   #include <linux/mfd/syscon.h>
>>> -
>>> +#include <linux/mtd/mtd.h>
>>>   #include <asm/irq.h>
>>>   #include <asm/page.h>
>>> @@ -1783,7 +1783,10 @@ static int davinci_emac_probe(struct
>>> platform_device *pdev)
>>>       struct cpdma_params dma_params;
>>>       struct clk *emac_clk;
>>>       unsigned long emac_bus_frequency;
>>> -
>>> +#ifdef CONFIG_MTD
>>> +    size_t mac_addr_len;
>>> +    struct mtd_info *mtd;
>>> +#endif /* CONFIG_MTD */
>>>       /* obtain emac clock from kernel */
>>>       emac_clk = devm_clk_get(&pdev->dev, NULL);
>>> @@ -1815,6 +1818,19 @@ static int davinci_emac_probe(struct
>>> platform_device *pdev)
>>>           goto err_free_netdev;
>>>       }
>>> +#ifdef CONFIG_MTD
>>
>>
>> What about the case when MTD is compiled as a module?
>>
>>> +    mtd = get_mtd_device_nm("MAC-Address");
>>
>>
>> What about the case when PTR_ERR(mtd) == -EPROBE_DEFER?
>
>
> To answer my own question: because get_mtd_device_nm() doesn't
> ever return -EPROBE_DEFER.
>
> I'm trying to make this work on LCDK, but the emac driver probes
> before any mtd device is registered, so, I just get -ENODEV even
> though I've added a partition to the device tree labeled
> "MAC-Address". You can see in the kernel messages that MTD is
> not probed until later.
>

I tested it on da850-evm - all MTD & SPI related modules need to be
built-in for it to work, so:

CONFIG_MTD=y
CONFIG_MTD_M25P80=y
CONFIG_SPI_DAVINCI=y

Best regards,
Bartosz Golaszewski

>
>>
>>> +    if (!IS_ERR(mtd)) {
>>> +        rc = mtd_read(mtd, 0, ETH_ALEN,
>>> +                  &mac_addr_len, priv->mac_addr);
>>> +        if (rc == 0)
>>> +            dev_info(&pdev->dev,
>>> +                 "Read MAC addr from SPI Flash: %pM\n",
>>> +                 priv->mac_addr);
>>> +        put_mtd_device(mtd);
>>> +    }
>>> +#endif /* CONFIG_MTD */
>>> +
>>>       /* MAC addr and PHY mask , RMII enable info from platform_data */
>>>       memcpy(priv->mac_addr, pdata->mac_addr, ETH_ALEN);
>>>       priv->phy_id = pdata->phy_id;
>>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Fainelli July 3, 2018, 4:39 p.m. UTC | #4
On 06/29/2018 02:40 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> On da850-evm board we can read the MAC address from MTD. It's currently
> done in the relevant board file, but we want to get rid of all the MAC
> reading callbacks from the board file (SPI and NAND). Move the reading
> of the MAC address from SPI to the emac driver's probe function.

This should be made something generic to all drivers, not just something
the davinci_emac driver does, something like this actually:

https://lkml.org/lkml/2018/3/24/312

> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/net/ethernet/ti/davinci_emac.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> index a1a6445b5a7e..48e6a7755811 100644
> --- a/drivers/net/ethernet/ti/davinci_emac.c
> +++ b/drivers/net/ethernet/ti/davinci_emac.c
> @@ -67,7 +67,7 @@
>  #include <linux/of_irq.h>
>  #include <linux/of_net.h>
>  #include <linux/mfd/syscon.h>
> -
> +#include <linux/mtd/mtd.h>
>  #include <asm/irq.h>
>  #include <asm/page.h>
>  
> @@ -1783,7 +1783,10 @@ static int davinci_emac_probe(struct platform_device *pdev)
>  	struct cpdma_params dma_params;
>  	struct clk *emac_clk;
>  	unsigned long emac_bus_frequency;
> -
> +#ifdef CONFIG_MTD
> +	size_t mac_addr_len;
> +	struct mtd_info *mtd;
> +#endif /* CONFIG_MTD */
>  
>  	/* obtain emac clock from kernel */
>  	emac_clk = devm_clk_get(&pdev->dev, NULL);
> @@ -1815,6 +1818,19 @@ static int davinci_emac_probe(struct platform_device *pdev)
>  		goto err_free_netdev;
>  	}
>  
> +#ifdef CONFIG_MTD
> +	mtd = get_mtd_device_nm("MAC-Address");
> +	if (!IS_ERR(mtd)) {
> +		rc = mtd_read(mtd, 0, ETH_ALEN,
> +			      &mac_addr_len, priv->mac_addr);
> +		if (rc == 0)
> +			dev_info(&pdev->dev,
> +				 "Read MAC addr from SPI Flash: %pM\n",
> +				 priv->mac_addr);
> +		put_mtd_device(mtd);
> +	}
> +#endif /* CONFIG_MTD */
> +
>  	/* MAC addr and PHY mask , RMII enable info from platform_data */
>  	memcpy(priv->mac_addr, pdata->mac_addr, ETH_ALEN);
>  	priv->phy_id = pdata->phy_id;
>
David Lechner July 3, 2018, 4:47 p.m. UTC | #5
On 07/03/2018 11:39 AM, Florian Fainelli wrote:
> 
> 
> On 06/29/2018 02:40 AM, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> On da850-evm board we can read the MAC address from MTD. It's currently
>> done in the relevant board file, but we want to get rid of all the MAC
>> reading callbacks from the board file (SPI and NAND). Move the reading
>> of the MAC address from SPI to the emac driver's probe function.
> 
> This should be made something generic to all drivers, not just something
> the davinci_emac driver does, something like this actually:
> 
> https://lkml.org/lkml/2018/3/24/312
> 

I was thinking about suggesting adding a nvmem provider for MTD devices
as well. It would fix the kernel config dependency problems I was running
into since nvmem lookups can do -EPROBE_DEFER.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ladislav Michl July 4, 2018, 7:09 a.m. UTC | #6
On Tue, Jul 03, 2018 at 09:39:51AM -0700, Florian Fainelli wrote:
> 
> 
> On 06/29/2018 02:40 AM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > 
> > On da850-evm board we can read the MAC address from MTD. It's currently
> > done in the relevant board file, but we want to get rid of all the MAC
> > reading callbacks from the board file (SPI and NAND). Move the reading
> > of the MAC address from SPI to the emac driver's probe function.
> 
> This should be made something generic to all drivers, not just something
> the davinci_emac driver does, something like this actually:
> 
> https://lkml.org/lkml/2018/3/24/312

...and that's would also make it work when MAC address is stored
in 24c08 EEPROM, which is quite common.

> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > ---
> >  drivers/net/ethernet/ti/davinci_emac.c | 20 ++++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> > index a1a6445b5a7e..48e6a7755811 100644
> > --- a/drivers/net/ethernet/ti/davinci_emac.c
> > +++ b/drivers/net/ethernet/ti/davinci_emac.c
> > @@ -67,7 +67,7 @@
> >  #include <linux/of_irq.h>
> >  #include <linux/of_net.h>
> >  #include <linux/mfd/syscon.h>
> > -
> > +#include <linux/mtd/mtd.h>
> >  #include <asm/irq.h>
> >  #include <asm/page.h>
> >  
> > @@ -1783,7 +1783,10 @@ static int davinci_emac_probe(struct platform_device *pdev)
> >  	struct cpdma_params dma_params;
> >  	struct clk *emac_clk;
> >  	unsigned long emac_bus_frequency;
> > -
> > +#ifdef CONFIG_MTD
> > +	size_t mac_addr_len;
> > +	struct mtd_info *mtd;
> > +#endif /* CONFIG_MTD */
> >  
> >  	/* obtain emac clock from kernel */
> >  	emac_clk = devm_clk_get(&pdev->dev, NULL);
> > @@ -1815,6 +1818,19 @@ static int davinci_emac_probe(struct platform_device *pdev)
> >  		goto err_free_netdev;
> >  	}
> >  
> > +#ifdef CONFIG_MTD
> > +	mtd = get_mtd_device_nm("MAC-Address");
> > +	if (!IS_ERR(mtd)) {
> > +		rc = mtd_read(mtd, 0, ETH_ALEN,
> > +			      &mac_addr_len, priv->mac_addr);
> > +		if (rc == 0)
> > +			dev_info(&pdev->dev,
> > +				 "Read MAC addr from SPI Flash: %pM\n",
> > +				 priv->mac_addr);
> > +		put_mtd_device(mtd);
> > +	}
> > +#endif /* CONFIG_MTD */
> > +
> >  	/* MAC addr and PHY mask , RMII enable info from platform_data */
> >  	memcpy(priv->mac_addr, pdata->mac_addr, ETH_ALEN);
> >  	priv->phy_id = pdata->phy_id;
> > 
> 
> -- 
> Florian
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartosz Golaszewski July 4, 2018, 8:29 a.m. UTC | #7
2018-07-04 9:09 GMT+02:00 Ladislav Michl <ladis@linux-mips.org>:
> On Tue, Jul 03, 2018 at 09:39:51AM -0700, Florian Fainelli wrote:
>>
>>
>> On 06/29/2018 02:40 AM, Bartosz Golaszewski wrote:
>> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> >
>> > On da850-evm board we can read the MAC address from MTD. It's currently
>> > done in the relevant board file, but we want to get rid of all the MAC
>> > reading callbacks from the board file (SPI and NAND). Move the reading
>> > of the MAC address from SPI to the emac driver's probe function.
>>
>> This should be made something generic to all drivers, not just something
>> the davinci_emac driver does, something like this actually:
>>
>> https://lkml.org/lkml/2018/3/24/312
>
> ...and that's would also make it work when MAC address is stored
> in 24c08 EEPROM, which is quite common.
>

This is what the second patch for davinci_emac in this series does. I
agree that this should become more generic at some point - we should
probably have a routine somewhere in net that would try to get the MAC
address from all possible sources (nvmem, of etc.). This is somewhat
related to the work I want to do on nvmem to make the at24 setup()
callback more generic.

Unfortunately we don't have it yet and I will not have time to work on
it before v4.20 so if there are no serious objections, I'd like to get
this series merged for v4.19 and then we can refactor the MAC reading
later.

How does it sound?

Best regards,
Bartosz Golaszewski

>> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> > ---
>> >  drivers/net/ethernet/ti/davinci_emac.c | 20 ++++++++++++++++++--
>> >  1 file changed, 18 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
>> > index a1a6445b5a7e..48e6a7755811 100644
>> > --- a/drivers/net/ethernet/ti/davinci_emac.c
>> > +++ b/drivers/net/ethernet/ti/davinci_emac.c
>> > @@ -67,7 +67,7 @@
>> >  #include <linux/of_irq.h>
>> >  #include <linux/of_net.h>
>> >  #include <linux/mfd/syscon.h>
>> > -
>> > +#include <linux/mtd/mtd.h>
>> >  #include <asm/irq.h>
>> >  #include <asm/page.h>
>> >
>> > @@ -1783,7 +1783,10 @@ static int davinci_emac_probe(struct platform_device *pdev)
>> >     struct cpdma_params dma_params;
>> >     struct clk *emac_clk;
>> >     unsigned long emac_bus_frequency;
>> > -
>> > +#ifdef CONFIG_MTD
>> > +   size_t mac_addr_len;
>> > +   struct mtd_info *mtd;
>> > +#endif /* CONFIG_MTD */
>> >
>> >     /* obtain emac clock from kernel */
>> >     emac_clk = devm_clk_get(&pdev->dev, NULL);
>> > @@ -1815,6 +1818,19 @@ static int davinci_emac_probe(struct platform_device *pdev)
>> >             goto err_free_netdev;
>> >     }
>> >
>> > +#ifdef CONFIG_MTD
>> > +   mtd = get_mtd_device_nm("MAC-Address");
>> > +   if (!IS_ERR(mtd)) {
>> > +           rc = mtd_read(mtd, 0, ETH_ALEN,
>> > +                         &mac_addr_len, priv->mac_addr);
>> > +           if (rc == 0)
>> > +                   dev_info(&pdev->dev,
>> > +                            "Read MAC addr from SPI Flash: %pM\n",
>> > +                            priv->mac_addr);
>> > +           put_mtd_device(mtd);
>> > +   }
>> > +#endif /* CONFIG_MTD */
>> > +
>> >     /* MAC addr and PHY mask , RMII enable info from platform_data */
>> >     memcpy(priv->mac_addr, pdata->mac_addr, ETH_ALEN);
>> >     priv->phy_id = pdata->phy_id;
>> >
>>
>> --
>> Florian
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori July 4, 2018, 9:04 a.m. UTC | #8
On Wednesday 04 July 2018 01:59 PM, Bartosz Golaszewski wrote:
> 2018-07-04 9:09 GMT+02:00 Ladislav Michl <ladis@linux-mips.org>:
>> On Tue, Jul 03, 2018 at 09:39:51AM -0700, Florian Fainelli wrote:
>>>
>>>
>>> On 06/29/2018 02:40 AM, Bartosz Golaszewski wrote:
>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>
>>>> On da850-evm board we can read the MAC address from MTD. It's currently
>>>> done in the relevant board file, but we want to get rid of all the MAC
>>>> reading callbacks from the board file (SPI and NAND). Move the reading
>>>> of the MAC address from SPI to the emac driver's probe function.
>>>
>>> This should be made something generic to all drivers, not just something
>>> the davinci_emac driver does, something like this actually:
>>>
>>> https://lkml.org/lkml/2018/3/24/312
>>
>> ...and that's would also make it work when MAC address is stored
>> in 24c08 EEPROM, which is quite common.
>>
> 
> This is what the second patch for davinci_emac in this series does. I
> agree that this should become more generic at some point - we should
> probably have a routine somewhere in net that would try to get the MAC
> address from all possible sources (nvmem, of etc.). This is somewhat
> related to the work I want to do on nvmem to make the at24 setup()
> callback more generic.
> 
> Unfortunately we don't have it yet and I will not have time to work on
> it before v4.20 so if there are no serious objections, I'd like to get
> this series merged for v4.19 and then we can refactor the MAC reading
> later.
> 
> How does it sound?

I don't think the series introduces any regressions. We need to have MTD
and SPI flash built into the kernel even today to get mac address on
DA850 EVM. So from that perspective, I don't have objections (I need to
actually test still).

OTOH, it will be nice to do the conversion once and not piecemeal. That
way there is less churn and scope for regressions.

So from a mach-davinci perspective, I don't have a very strong position
either way.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartosz Golaszewski July 13, 2018, 6 p.m. UTC | #9
2018-07-04 11:04 GMT+02:00 Sekhar Nori <nsekhar@ti.com>:
> On Wednesday 04 July 2018 01:59 PM, Bartosz Golaszewski wrote:
>> 2018-07-04 9:09 GMT+02:00 Ladislav Michl <ladis@linux-mips.org>:
>>> On Tue, Jul 03, 2018 at 09:39:51AM -0700, Florian Fainelli wrote:
>>>>
>>>>
>>>> On 06/29/2018 02:40 AM, Bartosz Golaszewski wrote:
>>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>
>>>>> On da850-evm board we can read the MAC address from MTD. It's currently
>>>>> done in the relevant board file, but we want to get rid of all the MAC
>>>>> reading callbacks from the board file (SPI and NAND). Move the reading
>>>>> of the MAC address from SPI to the emac driver's probe function.
>>>>
>>>> This should be made something generic to all drivers, not just something
>>>> the davinci_emac driver does, something like this actually:
>>>>
>>>> https://lkml.org/lkml/2018/3/24/312
>>>
>>> ...and that's would also make it work when MAC address is stored
>>> in 24c08 EEPROM, which is quite common.
>>>
>>
>> This is what the second patch for davinci_emac in this series does. I
>> agree that this should become more generic at some point - we should
>> probably have a routine somewhere in net that would try to get the MAC
>> address from all possible sources (nvmem, of etc.). This is somewhat
>> related to the work I want to do on nvmem to make the at24 setup()
>> callback more generic.
>>
>> Unfortunately we don't have it yet and I will not have time to work on
>> it before v4.20 so if there are no serious objections, I'd like to get
>> this series merged for v4.19 and then we can refactor the MAC reading
>> later.
>>
>> How does it sound?
>
> I don't think the series introduces any regressions. We need to have MTD
> and SPI flash built into the kernel even today to get mac address on
> DA850 EVM. So from that perspective, I don't have objections (I need to
> actually test still).
>
> OTOH, it will be nice to do the conversion once and not piecemeal. That
> way there is less churn and scope for regressions.
>
> So from a mach-davinci perspective, I don't have a very strong position
> either way.
>
> Thanks,
> Sekhar

We're getting close to rc5 so I'd like to make a case for this series again.

I understand that there's more to do than just the changes introduced
here, but we shouldn't try to fix several problems in many different
places at once. There's just too many moving pieces. I'd rather start
merging small improvements right away.

The idea behind this series is to remove (almost) all users of
at24_platform_data. The davinci_emac patches are there only because we
need to remove some MAC adress reading stuff from the board files.
Having this code there and calling it back from EEPROM/MTD drivers is
already wrong and we should work towards using nvmem for that anyway.

Currently for MTD the nvmem support series seems to be dead and it's
going to take some time before anything gets upstream.

So I'd like to again ask you to consider picking up the patches from
this series to your respective trees or at the very least: I'd like to
ask Srinivas to pick up the nvmem patches and Sekhar to take the
first, non-controversial batch of davinci platform changes so that
we'll have less code to carry for the next release.

Best regards,
Bartosz Golaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori July 16, 2018, 8:50 a.m. UTC | #10
On Friday 13 July 2018 11:30 PM, Bartosz Golaszewski wrote:

> We're getting close to rc5 so I'd like to make a case for this series again.
> 
> I understand that there's more to do than just the changes introduced
> here, but we shouldn't try to fix several problems in many different
> places at once. There's just too many moving pieces. I'd rather start
> merging small improvements right away.
> 
> The idea behind this series is to remove (almost) all users of
> at24_platform_data. The davinci_emac patches are there only because we
> need to remove some MAC adress reading stuff from the board files.
> Having this code there and calling it back from EEPROM/MTD drivers is
> already wrong and we should work towards using nvmem for that anyway.
> 
> Currently for MTD the nvmem support series seems to be dead and it's
> going to take some time before anything gets upstream.
> 
> So I'd like to again ask you to consider picking up the patches from
> this series to your respective trees or at the very least: I'd like to
> ask Srinivas to pick up the nvmem patches and Sekhar to take the
> first, non-controversial batch of davinci platform changes so that
> we'll have less code to carry for the next release.

I think those are patches 3-7. I can take those if I get an immutable
commit over v4.18-rc1 from Srinivas with patches 1 & 2 applied.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Srinivas Kandagatla July 16, 2018, 8:56 a.m. UTC | #11
On 16/07/18 09:50, Sekhar Nori wrote:
> On Friday 13 July 2018 11:30 PM, Bartosz Golaszewski wrote:
> 
>> We're getting close to rc5 so I'd like to make a case for this series again.
>>
>> I understand that there's more to do than just the changes introduced
>> here, but we shouldn't try to fix several problems in many different
>> places at once. There's just too many moving pieces. I'd rather start
>> merging small improvements right away.
>>
>> The idea behind this series is to remove (almost) all users of
>> at24_platform_data. The davinci_emac patches are there only because we
>> need to remove some MAC adress reading stuff from the board files.
>> Having this code there and calling it back from EEPROM/MTD drivers is
>> already wrong and we should work towards using nvmem for that anyway.
>>
>> Currently for MTD the nvmem support series seems to be dead and it's
>> going to take some time before anything gets upstream.
>>
>> So I'd like to again ask you to consider picking up the patches from
>> this series to your respective trees or at the very least: I'd like to
>> ask Srinivas to pick up the nvmem patches and Sekhar to take the
>> first, non-controversial batch of davinci platform changes so that
>> we'll have less code to carry for the next release.
> 
> I think those are patches 3-7. I can take those if I get an immutable
> commit over v4.18-rc1 from Srinivas with patches 1 & 2 applied.
nvmem patches go via Greg KH char-misc tree, if it makes things easy I 
can provide Ack on nvmem patches, so that you can take these patches via 
your tree?

Let me know.

--srini
> 
> Thanks,
> Sekhar
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori July 16, 2018, 11:57 a.m. UTC | #12
On Monday 16 July 2018 02:26 PM, Srinivas Kandagatla wrote:
> 
> 
> On 16/07/18 09:50, Sekhar Nori wrote:
>> On Friday 13 July 2018 11:30 PM, Bartosz Golaszewski wrote:
>>
>>> We're getting close to rc5 so I'd like to make a case for this series
>>> again.
>>>
>>> I understand that there's more to do than just the changes introduced
>>> here, but we shouldn't try to fix several problems in many different
>>> places at once. There's just too many moving pieces. I'd rather start
>>> merging small improvements right away.
>>>
>>> The idea behind this series is to remove (almost) all users of
>>> at24_platform_data. The davinci_emac patches are there only because we
>>> need to remove some MAC adress reading stuff from the board files.
>>> Having this code there and calling it back from EEPROM/MTD drivers is
>>> already wrong and we should work towards using nvmem for that anyway.
>>>
>>> Currently for MTD the nvmem support series seems to be dead and it's
>>> going to take some time before anything gets upstream.
>>>
>>> So I'd like to again ask you to consider picking up the patches from
>>> this series to your respective trees or at the very least: I'd like to
>>> ask Srinivas to pick up the nvmem patches and Sekhar to take the
>>> first, non-controversial batch of davinci platform changes so that
>>> we'll have less code to carry for the next release.
>>
>> I think those are patches 3-7. I can take those if I get an immutable
>> commit over v4.18-rc1 from Srinivas with patches 1 & 2 applied.

> nvmem patches go via Greg KH char-misc tree, if it makes things easy I
> can provide Ack on nvmem patches, so that you can take these patches via
> your tree?
> 
> Let me know.

I can do that.

Greg, are you fine with this? It will be great to have your ack for
patches 1/8 and 2/18.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg Kroah-Hartman July 16, 2018, 12:10 p.m. UTC | #13
On Mon, Jul 16, 2018 at 05:27:00PM +0530, Sekhar Nori wrote:
> On Monday 16 July 2018 02:26 PM, Srinivas Kandagatla wrote:
> > 
> > 
> > On 16/07/18 09:50, Sekhar Nori wrote:
> >> On Friday 13 July 2018 11:30 PM, Bartosz Golaszewski wrote:
> >>
> >>> We're getting close to rc5 so I'd like to make a case for this series
> >>> again.
> >>>
> >>> I understand that there's more to do than just the changes introduced
> >>> here, but we shouldn't try to fix several problems in many different
> >>> places at once. There's just too many moving pieces. I'd rather start
> >>> merging small improvements right away.
> >>>
> >>> The idea behind this series is to remove (almost) all users of
> >>> at24_platform_data. The davinci_emac patches are there only because we
> >>> need to remove some MAC adress reading stuff from the board files.
> >>> Having this code there and calling it back from EEPROM/MTD drivers is
> >>> already wrong and we should work towards using nvmem for that anyway.
> >>>
> >>> Currently for MTD the nvmem support series seems to be dead and it's
> >>> going to take some time before anything gets upstream.
> >>>
> >>> So I'd like to again ask you to consider picking up the patches from
> >>> this series to your respective trees or at the very least: I'd like to
> >>> ask Srinivas to pick up the nvmem patches and Sekhar to take the
> >>> first, non-controversial batch of davinci platform changes so that
> >>> we'll have less code to carry for the next release.
> >>
> >> I think those are patches 3-7. I can take those if I get an immutable
> >> commit over v4.18-rc1 from Srinivas with patches 1 & 2 applied.
> 
> > nvmem patches go via Greg KH char-misc tree, if it makes things easy I
> > can provide Ack on nvmem patches, so that you can take these patches via
> > your tree?
> > 
> > Let me know.
> 
> I can do that.
> 
> Greg, are you fine with this? It will be great to have your ack for
> patches 1/8 and 2/18.

I'm not the nvmem maintainer :)

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori July 16, 2018, 12:13 p.m. UTC | #14
On Monday 16 July 2018 05:40 PM, Greg Kroah-Hartman wrote:
> On Mon, Jul 16, 2018 at 05:27:00PM +0530, Sekhar Nori wrote:
>> On Monday 16 July 2018 02:26 PM, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 16/07/18 09:50, Sekhar Nori wrote:
>>>> On Friday 13 July 2018 11:30 PM, Bartosz Golaszewski wrote:
>>>>
>>>>> We're getting close to rc5 so I'd like to make a case for this series
>>>>> again.
>>>>>
>>>>> I understand that there's more to do than just the changes introduced
>>>>> here, but we shouldn't try to fix several problems in many different
>>>>> places at once. There's just too many moving pieces. I'd rather start
>>>>> merging small improvements right away.
>>>>>
>>>>> The idea behind this series is to remove (almost) all users of
>>>>> at24_platform_data. The davinci_emac patches are there only because we
>>>>> need to remove some MAC adress reading stuff from the board files.
>>>>> Having this code there and calling it back from EEPROM/MTD drivers is
>>>>> already wrong and we should work towards using nvmem for that anyway.
>>>>>
>>>>> Currently for MTD the nvmem support series seems to be dead and it's
>>>>> going to take some time before anything gets upstream.
>>>>>
>>>>> So I'd like to again ask you to consider picking up the patches from
>>>>> this series to your respective trees or at the very least: I'd like to
>>>>> ask Srinivas to pick up the nvmem patches and Sekhar to take the
>>>>> first, non-controversial batch of davinci platform changes so that
>>>>> we'll have less code to carry for the next release.
>>>>
>>>> I think those are patches 3-7. I can take those if I get an immutable
>>>> commit over v4.18-rc1 from Srinivas with patches 1 & 2 applied.
>>
>>> nvmem patches go via Greg KH char-misc tree, if it makes things easy I
>>> can provide Ack on nvmem patches, so that you can take these patches via
>>> your tree?
>>>
>>> Let me know.
>>
>> I can do that.
>>
>> Greg, are you fine with this? It will be great to have your ack for
>> patches 1/8 and 2/18.
> 
> I'm not the nvmem maintainer :)

Got it. Will apply with Srinivas's ack.

Regards,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori July 20, 2018, 5:04 a.m. UTC | #15
On Monday 16 July 2018 02:26 PM, Srinivas Kandagatla wrote:
> 
> 
> On 16/07/18 09:50, Sekhar Nori wrote:
>> On Friday 13 July 2018 11:30 PM, Bartosz Golaszewski wrote:
>>
>>> We're getting close to rc5 so I'd like to make a case for this series
>>> again.
>>>
>>> I understand that there's more to do than just the changes introduced
>>> here, but we shouldn't try to fix several problems in many different
>>> places at once. There's just too many moving pieces. I'd rather start
>>> merging small improvements right away.
>>>
>>> The idea behind this series is to remove (almost) all users of
>>> at24_platform_data. The davinci_emac patches are there only because we
>>> need to remove some MAC adress reading stuff from the board files.
>>> Having this code there and calling it back from EEPROM/MTD drivers is
>>> already wrong and we should work towards using nvmem for that anyway.
>>>
>>> Currently for MTD the nvmem support series seems to be dead and it's
>>> going to take some time before anything gets upstream.
>>>
>>> So I'd like to again ask you to consider picking up the patches from
>>> this series to your respective trees or at the very least: I'd like to
>>> ask Srinivas to pick up the nvmem patches and Sekhar to take the
>>> first, non-controversial batch of davinci platform changes so that
>>> we'll have less code to carry for the next release.
>>
>> I think those are patches 3-7. I can take those if I get an immutable
>> commit over v4.18-rc1 from Srinivas with patches 1 & 2 applied.
> nvmem patches go via Greg KH char-misc tree, if it makes things easy I
> can provide Ack on nvmem patches, so that you can take these patches via
> your tree?

There is a lot of follow-up traffic on how exactly to develop the needed
interfaces for reading mac address in mtd and/or network subsystem.

But, I don't think any of that negates the need for nvmem lookups that
work for non-device-tree and populating the lookups in mach-davinci
board code.

I am going to send patches 1-7 to ARM-SoC soon, so please do say if
there is any disagreement on this.

Thanks,
Sekhar

> 
> Let me know.
> 
> --srini
>>
>> Thanks,
>> Sekhar
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori July 27, 2018, 9:52 a.m. UTC | #16
On Friday 20 July 2018 10:34 AM, Sekhar Nori wrote:

> There is a lot of follow-up traffic on how exactly to develop the needed
> interfaces for reading mac address in mtd and/or network subsystem.
> 
> But, I don't think any of that negates the need for nvmem lookups that
> work for non-device-tree and populating the lookups in mach-davinci
> board code.
> 
> I am going to send patches 1-7 to ARM-SoC soon, so please do say if
> there is any disagreement on this.

I owed a response on this. After thinking over some and discussing with
Bartosz as well, I decided not to merge any patches from  this series.
Its better to merge anything after the actual path we are talking is
fully clear.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index a1a6445b5a7e..48e6a7755811 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -67,7 +67,7 @@ 
 #include <linux/of_irq.h>
 #include <linux/of_net.h>
 #include <linux/mfd/syscon.h>
-
+#include <linux/mtd/mtd.h>
 #include <asm/irq.h>
 #include <asm/page.h>
 
@@ -1783,7 +1783,10 @@  static int davinci_emac_probe(struct platform_device *pdev)
 	struct cpdma_params dma_params;
 	struct clk *emac_clk;
 	unsigned long emac_bus_frequency;
-
+#ifdef CONFIG_MTD
+	size_t mac_addr_len;
+	struct mtd_info *mtd;
+#endif /* CONFIG_MTD */
 
 	/* obtain emac clock from kernel */
 	emac_clk = devm_clk_get(&pdev->dev, NULL);
@@ -1815,6 +1818,19 @@  static int davinci_emac_probe(struct platform_device *pdev)
 		goto err_free_netdev;
 	}
 
+#ifdef CONFIG_MTD
+	mtd = get_mtd_device_nm("MAC-Address");
+	if (!IS_ERR(mtd)) {
+		rc = mtd_read(mtd, 0, ETH_ALEN,
+			      &mac_addr_len, priv->mac_addr);
+		if (rc == 0)
+			dev_info(&pdev->dev,
+				 "Read MAC addr from SPI Flash: %pM\n",
+				 priv->mac_addr);
+		put_mtd_device(mtd);
+	}
+#endif /* CONFIG_MTD */
+
 	/* MAC addr and PHY mask , RMII enable info from platform_data */
 	memcpy(priv->mac_addr, pdata->mac_addr, ETH_ALEN);
 	priv->phy_id = pdata->phy_id;