All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
       [not found] <1477668756-2651-1-git-send-email-jhofstee@victronenergy.com>
@ 2016-10-28 15:52   ` Tony Lindgren
  0 siblings, 0 replies; 17+ messages in thread
From: Tony Lindgren @ 2016-10-28 15:52 UTC (permalink / raw)
  To: Jeroen Hofstee
  Cc: netdev, Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

* Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
> Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
> id to common file") did not only move the code for an am3517, it also
> added the slave parameter, resulting in an invalid (all zero) mac address
> being returned for an am3517, since it only has a single emac and the slave
> parameter is pointing to the second. So simply always read the first and
> valid mac-address for a ti,am3517-emac.

And others davinci_emac.c users can have more than one. So is the
reason the slave parameter points to the second instance because
of the location in the hardware?

Regards,

Tony

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
@ 2016-10-28 15:52   ` Tony Lindgren
  0 siblings, 0 replies; 17+ messages in thread
From: Tony Lindgren @ 2016-10-28 15:52 UTC (permalink / raw)
  To: Jeroen Hofstee
  Cc: netdev, Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

* Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
> Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
> id to common file") did not only move the code for an am3517, it also
> added the slave parameter, resulting in an invalid (all zero) mac address
> being returned for an am3517, since it only has a single emac and the slave
> parameter is pointing to the second. So simply always read the first and
> valid mac-address for a ti,am3517-emac.

And others davinci_emac.c users can have more than one. So is the
reason the slave parameter points to the second instance because
of the location in the hardware?

Regards,

Tony

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
  2016-10-28 15:52   ` Tony Lindgren
  (?)
@ 2016-10-28 18:17     ` Jeroen Hofstee
  -1 siblings, 0 replies; 17+ messages in thread
From: Jeroen Hofstee @ 2016-10-28 18:17 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: netdev, Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

Hello Tony,

On 28-10-16 17:52, Tony Lindgren wrote:
> * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
>> Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
>> id to common file") did not only move the code for an am3517, it also
>> added the slave parameter, resulting in an invalid (all zero) mac address
>> being returned for an am3517, since it only has a single emac and the slave
>> parameter is pointing to the second. So simply always read the first and
>> valid mac-address for a ti,am3517-emac.
> And others davinci_emac.c users can have more than one. So is the
> reason the slave parameter points to the second instance because
> of the location in the hardware?

Sort of, the slave parameter gets determined by the fact if there is one
or two register range(s) associated with the davinci_emac. In davinci_emac.c

     res_ctrl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
     ...
     rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1,
                           priv->mac_addr);

So it there are two ranges, the slave param becomes 0. It there is only 
one, it
will be 1. Since the am3517 only has a single regs entry it ends up with 
slave 1,
while there is only a single davinci_emac.

Regards,
Jeroen

arch/arm/boot/dts/dm816x.dtsi
-----------------------------
eth0: ethernet@4a100000 {
     compatible = "ti,dm816-emac";
     ti,hwmods = "emac0";
     reg = <0x4a100000 0x800
            0x4a100900 0x3700>;
     clocks = <&sysclk24_ck>;
     syscon = <&scm_conf>;
     ti,davinci-ctrl-reg-offset = <0>;
     ti,davinci-ctrl-mod-reg-offset = <0x900>;
     ti,davinci-ctrl-ram-offset = <0x2000>;
     ti,davinci-ctrl-ram-size = <0x2000>;
     interrupts = <40 41 42 43>;
     phy-handle = <&phy0>;
};

eth1: ethernet@4a120000 {
     compatible = "ti,dm816-emac";
     ti,hwmods = "emac1";
     reg = <0x4a120000 0x4000>;
     clocks = <&sysclk24_ck>;
     syscon = <&scm_conf>;
     ti,davinci-ctrl-reg-offset = <0>;
     ti,davinci-ctrl-mod-reg-offset = <0x900>;
     ti,davinci-ctrl-ram-offset = <0x2000>;
     ti,davinci-ctrl-ram-size = <0x2000>;
     interrupts = <44 45 46 47>;
     phy-handle = <&phy1>;
};

arch/arm/boot/dts/am3517.dtsi
-------------------------------
davinci_emac: ethernet@0x5c000000 {
     compatible = "ti,am3517-emac";
     ti,hwmods = "davinci_emac";
     status = "disabled";
     reg = <0x5c000000 0x30000>;
     interrupts = <67 68 69 70>;
     syscon = <&scm_conf>;
     ti,davinci-ctrl-reg-offset = <0x10000>;
     ti,davinci-ctrl-mod-reg-offset = <0>;
     ti,davinci-ctrl-ram-offset = <0x20000>;
     ti,davinci-ctrl-ram-size = <0x2000>;
     ti,davinci-rmii-en = /bits/ 8 <1>;
     local-mac-address = [ 00 00 00 00 00 00 ];
};

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
@ 2016-10-28 18:17     ` Jeroen Hofstee
  0 siblings, 0 replies; 17+ messages in thread
From: Jeroen Hofstee @ 2016-10-28 18:17 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: netdev, Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

Hello Tony,

On 28-10-16 17:52, Tony Lindgren wrote:
> * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
>> Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
>> id to common file") did not only move the code for an am3517, it also
>> added the slave parameter, resulting in an invalid (all zero) mac address
>> being returned for an am3517, since it only has a single emac and the slave
>> parameter is pointing to the second. So simply always read the first and
>> valid mac-address for a ti,am3517-emac.
> And others davinci_emac.c users can have more than one. So is the
> reason the slave parameter points to the second instance because
> of the location in the hardware?

Sort of, the slave parameter gets determined by the fact if there is one
or two register range(s) associated with the davinci_emac. In davinci_emac.c

     res_ctrl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
     ...
     rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1,
                           priv->mac_addr);

So it there are two ranges, the slave param becomes 0. It there is only 
one, it
will be 1. Since the am3517 only has a single regs entry it ends up with 
slave 1,
while there is only a single davinci_emac.

Regards,
Jeroen

arch/arm/boot/dts/dm816x.dtsi
-----------------------------
eth0: ethernet@4a100000 {
     compatible = "ti,dm816-emac";
     ti,hwmods = "emac0";
     reg = <0x4a100000 0x800
            0x4a100900 0x3700>;
     clocks = <&sysclk24_ck>;
     syscon = <&scm_conf>;
     ti,davinci-ctrl-reg-offset = <0>;
     ti,davinci-ctrl-mod-reg-offset = <0x900>;
     ti,davinci-ctrl-ram-offset = <0x2000>;
     ti,davinci-ctrl-ram-size = <0x2000>;
     interrupts = <40 41 42 43>;
     phy-handle = <&phy0>;
};

eth1: ethernet@4a120000 {
     compatible = "ti,dm816-emac";
     ti,hwmods = "emac1";
     reg = <0x4a120000 0x4000>;
     clocks = <&sysclk24_ck>;
     syscon = <&scm_conf>;
     ti,davinci-ctrl-reg-offset = <0>;
     ti,davinci-ctrl-mod-reg-offset = <0x900>;
     ti,davinci-ctrl-ram-offset = <0x2000>;
     ti,davinci-ctrl-ram-size = <0x2000>;
     interrupts = <44 45 46 47>;
     phy-handle = <&phy1>;
};

arch/arm/boot/dts/am3517.dtsi
-------------------------------
davinci_emac: ethernet@0x5c000000 {
     compatible = "ti,am3517-emac";
     ti,hwmods = "davinci_emac";
     status = "disabled";
     reg = <0x5c000000 0x30000>;
     interrupts = <67 68 69 70>;
     syscon = <&scm_conf>;
     ti,davinci-ctrl-reg-offset = <0x10000>;
     ti,davinci-ctrl-mod-reg-offset = <0>;
     ti,davinci-ctrl-ram-offset = <0x20000>;
     ti,davinci-ctrl-ram-size = <0x2000>;
     ti,davinci-rmii-en = /bits/ 8 <1>;
     local-mac-address = [ 00 00 00 00 00 00 ];
};

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
@ 2016-10-28 18:17     ` Jeroen Hofstee
  0 siblings, 0 replies; 17+ messages in thread
From: Jeroen Hofstee @ 2016-10-28 18:17 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: netdev, Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

Hello Tony,

On 28-10-16 17:52, Tony Lindgren wrote:
> * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
>> Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
>> id to common file") did not only move the code for an am3517, it also
>> added the slave parameter, resulting in an invalid (all zero) mac address
>> being returned for an am3517, since it only has a single emac and the slave
>> parameter is pointing to the second. So simply always read the first and
>> valid mac-address for a ti,am3517-emac.
> And others davinci_emac.c users can have more than one. So is the
> reason the slave parameter points to the second instance because
> of the location in the hardware?

Sort of, the slave parameter gets determined by the fact if there is one
or two register range(s) associated with the davinci_emac. In davinci_emac.c

     res_ctrl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
     ...
     rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1,
                           priv->mac_addr);

So it there are two ranges, the slave param becomes 0. It there is only 
one, it
will be 1. Since the am3517 only has a single regs entry it ends up with 
slave 1,
while there is only a single davinci_emac.

Regards,
Jeroen

arch/arm/boot/dts/dm816x.dtsi
-----------------------------
eth0: ethernet@4a100000 {
     compatible = "ti,dm816-emac";
     ti,hwmods = "emac0";
     reg = <0x4a100000 0x800
            0x4a100900 0x3700>;
     clocks = <&sysclk24_ck>;
     syscon = <&scm_conf>;
     ti,davinci-ctrl-reg-offset = <0>;
     ti,davinci-ctrl-mod-reg-offset = <0x900>;
     ti,davinci-ctrl-ram-offset = <0x2000>;
     ti,davinci-ctrl-ram-size = <0x2000>;
     interrupts = <40 41 42 43>;
     phy-handle = <&phy0>;
};

eth1: ethernet@4a120000 {
     compatible = "ti,dm816-emac";
     ti,hwmods = "emac1";
     reg = <0x4a120000 0x4000>;
     clocks = <&sysclk24_ck>;
     syscon = <&scm_conf>;
     ti,davinci-ctrl-reg-offset = <0>;
     ti,davinci-ctrl-mod-reg-offset = <0x900>;
     ti,davinci-ctrl-ram-offset = <0x2000>;
     ti,davinci-ctrl-ram-size = <0x2000>;
     interrupts = <44 45 46 47>;
     phy-handle = <&phy1>;
};

arch/arm/boot/dts/am3517.dtsi
-------------------------------
davinci_emac: ethernet@0x5c000000 {
     compatible = "ti,am3517-emac";
     ti,hwmods = "davinci_emac";
     status = "disabled";
     reg = <0x5c000000 0x30000>;
     interrupts = <67 68 69 70>;
     syscon = <&scm_conf>;
     ti,davinci-ctrl-reg-offset = <0x10000>;
     ti,davinci-ctrl-mod-reg-offset = <0>;
     ti,davinci-ctrl-ram-offset = <0x20000>;
     ti,davinci-ctrl-ram-size = <0x2000>;
     ti,davinci-rmii-en = /bits/ 8 <1>;
     local-mac-address = [ 00 00 00 00 00 00 ];
};

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
  2016-10-28 18:17     ` Jeroen Hofstee
@ 2016-10-28 18:19       ` Tony Lindgren
  -1 siblings, 0 replies; 17+ messages in thread
From: Tony Lindgren @ 2016-10-28 18:19 UTC (permalink / raw)
  To: Jeroen Hofstee
  Cc: netdev, Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

* Jeroen Hofstee <jhofstee@victronenergy.com> [161028 11:19]:
> Hello Tony,
> 
> On 28-10-16 17:52, Tony Lindgren wrote:
> > * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
> > > Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
> > > id to common file") did not only move the code for an am3517, it also
> > > added the slave parameter, resulting in an invalid (all zero) mac address
> > > being returned for an am3517, since it only has a single emac and the slave
> > > parameter is pointing to the second. So simply always read the first and
> > > valid mac-address for a ti,am3517-emac.
> > And others davinci_emac.c users can have more than one. So is the
> > reason the slave parameter points to the second instance because
> > of the location in the hardware?
> 
> Sort of, the slave parameter gets determined by the fact if there is one
> or two register range(s) associated with the davinci_emac. In davinci_emac.c
> 
>     res_ctrl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>     ...
>     rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1,
>                           priv->mac_addr);
> 
> So it there are two ranges, the slave param becomes 0. It there is only one,
> it
> will be 1. Since the am3517 only has a single regs entry it ends up with
> slave 1,
> while there is only a single davinci_emac.

OK thanks for clarifying it:

Acked-by: Tony Lindgren <tony@atomide.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
@ 2016-10-28 18:19       ` Tony Lindgren
  0 siblings, 0 replies; 17+ messages in thread
From: Tony Lindgren @ 2016-10-28 18:19 UTC (permalink / raw)
  To: Jeroen Hofstee
  Cc: netdev, Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

* Jeroen Hofstee <jhofstee@victronenergy.com> [161028 11:19]:
> Hello Tony,
> 
> On 28-10-16 17:52, Tony Lindgren wrote:
> > * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
> > > Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
> > > id to common file") did not only move the code for an am3517, it also
> > > added the slave parameter, resulting in an invalid (all zero) mac address
> > > being returned for an am3517, since it only has a single emac and the slave
> > > parameter is pointing to the second. So simply always read the first and
> > > valid mac-address for a ti,am3517-emac.
> > And others davinci_emac.c users can have more than one. So is the
> > reason the slave parameter points to the second instance because
> > of the location in the hardware?
> 
> Sort of, the slave parameter gets determined by the fact if there is one
> or two register range(s) associated with the davinci_emac. In davinci_emac.c
> 
>     res_ctrl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>     ...
>     rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1,
>                           priv->mac_addr);
> 
> So it there are two ranges, the slave param becomes 0. It there is only one,
> it
> will be 1. Since the am3517 only has a single regs entry it ends up with
> slave 1,
> while there is only a single davinci_emac.

OK thanks for clarifying it:

Acked-by: Tony Lindgren <tony@atomide.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
  2016-10-28 18:19       ` Tony Lindgren
@ 2019-03-01 14:52         ` Måns Rullgård
  -1 siblings, 0 replies; 17+ messages in thread
From: Måns Rullgård @ 2019-03-01 14:52 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Jeroen Hofstee, netdev, Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

Tony Lindgren <tony@atomide.com> writes:

> * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 11:19]:
>> Hello Tony,
>> 
>> On 28-10-16 17:52, Tony Lindgren wrote:
>> > * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
>> > > Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
>> > > id to common file") did not only move the code for an am3517, it also
>> > > added the slave parameter, resulting in an invalid (all zero) mac address
>> > > being returned for an am3517, since it only has a single emac and the slave
>> > > parameter is pointing to the second. So simply always read the first and
>> > > valid mac-address for a ti,am3517-emac.
>> > And others davinci_emac.c users can have more than one. So is the
>> > reason the slave parameter points to the second instance because
>> > of the location in the hardware?
>> 
>> Sort of, the slave parameter gets determined by the fact if there is one
>> or two register range(s) associated with the davinci_emac. In davinci_emac.c
>> 
>>     res_ctrl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>     ...
>>     rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1,
>>                           priv->mac_addr);
>> 
>> So it there are two ranges, the slave param becomes 0. It there is only one,
>> it
>> will be 1. Since the am3517 only has a single regs entry it ends up with
>> slave 1,
>> while there is only a single davinci_emac.
>
> OK thanks for clarifying it:
>
> Acked-by: Tony Lindgren <tony@atomide.com>

What happened to this patch?

-- 
Måns Rullgård

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
@ 2019-03-01 14:52         ` Måns Rullgård
  0 siblings, 0 replies; 17+ messages in thread
From: Måns Rullgård @ 2019-03-01 14:52 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Jeroen Hofstee, netdev, Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

Tony Lindgren <tony@atomide.com> writes:

> * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 11:19]:
>> Hello Tony,
>> 
>> On 28-10-16 17:52, Tony Lindgren wrote:
>> > * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
>> > > Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
>> > > id to common file") did not only move the code for an am3517, it also
>> > > added the slave parameter, resulting in an invalid (all zero) mac address
>> > > being returned for an am3517, since it only has a single emac and the slave
>> > > parameter is pointing to the second. So simply always read the first and
>> > > valid mac-address for a ti,am3517-emac.
>> > And others davinci_emac.c users can have more than one. So is the
>> > reason the slave parameter points to the second instance because
>> > of the location in the hardware?
>> 
>> Sort of, the slave parameter gets determined by the fact if there is one
>> or two register range(s) associated with the davinci_emac. In davinci_emac.c
>> 
>>     res_ctrl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>     ...
>>     rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1,
>>                           priv->mac_addr);
>> 
>> So it there are two ranges, the slave param becomes 0. It there is only one,
>> it
>> will be 1. Since the am3517 only has a single regs entry it ends up with
>> slave 1,
>> while there is only a single davinci_emac.
>
> OK thanks for clarifying it:
>
> Acked-by: Tony Lindgren <tony@atomide.com>

What happened to this patch?

-- 
Måns Rullgård

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
  2016-10-28 18:19       ` Tony Lindgren
  (?)
  (?)
@ 2023-06-23 14:58       ` Måns Rullgård
  2023-06-23 20:14         ` Simon Horman
  -1 siblings, 1 reply; 17+ messages in thread
From: Måns Rullgård @ 2023-06-23 14:58 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Jeroen Hofstee, netdev, Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

Tony Lindgren <tony@atomide.com> writes:

> * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 11:19]:
>> Hello Tony,
>> 
>> On 28-10-16 17:52, Tony Lindgren wrote:
>> > * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
>> > > Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
>> > > id to common file") did not only move the code for an am3517, it also
>> > > added the slave parameter, resulting in an invalid (all zero) mac address
>> > > being returned for an am3517, since it only has a single emac and the slave
>> > > parameter is pointing to the second. So simply always read the first and
>> > > valid mac-address for a ti,am3517-emac.
>> > And others davinci_emac.c users can have more than one. So is the
>> > reason the slave parameter points to the second instance because
>> > of the location in the hardware?
>> 
>> Sort of, the slave parameter gets determined by the fact if there is one
>> or two register range(s) associated with the davinci_emac. In davinci_emac.c
>> 
>>     res_ctrl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>     ...
>>     rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1,
>>                           priv->mac_addr);
>> 
>> So it there are two ranges, the slave param becomes 0. It there is only one,
>> it
>> will be 1. Since the am3517 only has a single regs entry it ends up with
>> slave 1,
>> while there is only a single davinci_emac.
>
> OK thanks for clarifying it:
>
> Acked-by: Tony Lindgren <tony@atomide.com>

Is there some reason this patch was never picked up, or was it simply
forgotten?

-- 
Måns Rullgård

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
  2023-06-23 14:58       ` Måns Rullgård
@ 2023-06-23 20:14         ` Simon Horman
  2023-06-23 21:13           ` Jeroen Hofstee
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Horman @ 2023-06-23 20:14 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Tony Lindgren, Jeroen Hofstee, netdev, Mugunthan V N,
	Grygorii Strashko, open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

On Fri, Jun 23, 2023 at 03:58:03PM +0100, Måns Rullgård wrote:
> Tony Lindgren <tony@atomide.com> writes:
> 
> > * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 11:19]:
> >> Hello Tony,
> >> 
> >> On 28-10-16 17:52, Tony Lindgren wrote:
> >> > * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
> >> > > Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
> >> > > id to common file") did not only move the code for an am3517, it also
> >> > > added the slave parameter, resulting in an invalid (all zero) mac address
> >> > > being returned for an am3517, since it only has a single emac and the slave
> >> > > parameter is pointing to the second. So simply always read the first and
> >> > > valid mac-address for a ti,am3517-emac.
> >> > And others davinci_emac.c users can have more than one. So is the
> >> > reason the slave parameter points to the second instance because
> >> > of the location in the hardware?
> >> 
> >> Sort of, the slave parameter gets determined by the fact if there is one
> >> or two register range(s) associated with the davinci_emac. In davinci_emac.c
> >> 
> >>     res_ctrl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> >>     ...
> >>     rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1,
> >>                           priv->mac_addr);
> >> 
> >> So it there are two ranges, the slave param becomes 0. It there is only one,
> >> it
> >> will be 1. Since the am3517 only has a single regs entry it ends up with
> >> slave 1,
> >> while there is only a single davinci_emac.
> >
> > OK thanks for clarifying it:
> >
> > Acked-by: Tony Lindgren <tony@atomide.com>
> 
> Is there some reason this patch was never picked up, or was it simply
> forgotten?

I feel like I am missing something here.

The patch possibly dates back to 2016 - but I can't tell because
lore.kernel.org doesn't know either.

https://lore.kernel.org/netdev/20161028155213.2t3nwwe3lqaynaer@atomide.com/

And I see you asked almost the same question in 2019.

If it is still relevant, perhaps it would be good to repost it
for review.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
  2023-06-23 20:14         ` Simon Horman
@ 2023-06-23 21:13           ` Jeroen Hofstee
  2023-06-23 21:41             ` Andrew Lunn
  0 siblings, 1 reply; 17+ messages in thread
From: Jeroen Hofstee @ 2023-06-23 21:13 UTC (permalink / raw)
  To: Simon Horman, Måns Rullgård
  Cc: Tony Lindgren, netdev, Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list


On 23-06-2023 22:14, Simon Horman wrote:
> On Fri, Jun 23, 2023 at 03:58:03PM +0100, Måns Rullgård wrote:
>> Tony Lindgren <tony@atomide.com> writes:
>>
>>> * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 11:19]:
>>>> Hello Tony,
>>>>
>>>> On 28-10-16 17:52, Tony Lindgren wrote:
>>>>> * Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
>>>>>> Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
>>>>>> id to common file") did not only move the code for an am3517, it also
>>>>>> added the slave parameter, resulting in an invalid (all zero) mac address
>>>>>> being returned for an am3517, since it only has a single emac and the slave
>>>>>> parameter is pointing to the second. So simply always read the first and
>>>>>> valid mac-address for a ti,am3517-emac.
>>>>> And others davinci_emac.c users can have more than one. So is the
>>>>> reason the slave parameter points to the second instance because
>>>>> of the location in the hardware?
>>>> Sort of, the slave parameter gets determined by the fact if there is one
>>>> or two register range(s) associated with the davinci_emac. In davinci_emac.c
>>>>
>>>>      res_ctrl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>>>      ...
>>>>      rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1,
>>>>                            priv->mac_addr);
>>>>
>>>> So it there are two ranges, the slave param becomes 0. It there is only one,
>>>> it
>>>> will be 1. Since the am3517 only has a single regs entry it ends up with
>>>> slave 1,
>>>> while there is only a single davinci_emac.
>>> OK thanks for clarifying it:
>>>
>>> Acked-by: Tony Lindgren <tony@atomide.com>
>> Is there some reason this patch was never picked up, or was it simply
>> forgotten?
> I feel like I am missing something here.

That is a weird response, you feel like something is missing, while
it is a recognized bug, acknowledged by the maintainer at that time
and apparently still not fixed and Måns simply asked it there is a
reason for that.

This is not a really helpful response,

Jeroen


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
  2023-06-23 21:13           ` Jeroen Hofstee
@ 2023-06-23 21:41             ` Andrew Lunn
  2023-06-24 14:55               ` Simon Horman
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Lunn @ 2023-06-23 21:41 UTC (permalink / raw)
  To: Jeroen Hofstee
  Cc: Simon Horman, Måns Rullgård, Tony Lindgren, netdev,
	Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

> > I feel like I am missing something here.
> 
> That is a weird response, you feel like something is missing

There is. The patch.

Maintainers have a slightly better memory than a goldfish, but given
the high volume of patches, we don't remember threads from 2016. Also,
all our infrastructure has limited memory, this patch is not in lore,
and it is not in patchworks. So in terms of getting merged, it does
not exist.

We do however recommend that if a patch has not been merged within 2
weeks, it is rebased, any Acked-by: etc tags are added and the patch
reposted.

	Andrew

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
  2023-06-23 21:41             ` Andrew Lunn
@ 2023-06-24 14:55               ` Simon Horman
  2023-06-24 15:02                 ` Måns Rullgård
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Horman @ 2023-06-24 14:55 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jeroen Hofstee, Måns Rullgård, Tony Lindgren, netdev,
	Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

On Fri, Jun 23, 2023 at 11:41:10PM +0200, Andrew Lunn wrote:
> > > I feel like I am missing something here.
> > 
> > That is a weird response, you feel like something is missing
> 
> There is. The patch.
> 
> Maintainers have a slightly better memory than a goldfish, but given
> the high volume of patches, we don't remember threads from 2016. Also,
> all our infrastructure has limited memory, this patch is not in lore,
> and it is not in patchworks. So in terms of getting merged, it does
> not exist.
> 
> We do however recommend that if a patch has not been merged within 2
> weeks, it is rebased, any Acked-by: etc tags are added and the patch
> reposted.

Thanks Andrew, that is also my position.

A ping for a multi-year old patch is unusual (for me).
I was wondering if there was a back story. I guess not.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
  2023-06-24 14:55               ` Simon Horman
@ 2023-06-24 15:02                 ` Måns Rullgård
  2023-06-24 15:53                   ` Simon Horman
  0 siblings, 1 reply; 17+ messages in thread
From: Måns Rullgård @ 2023-06-24 15:02 UTC (permalink / raw)
  To: Simon Horman
  Cc: Andrew Lunn, Jeroen Hofstee, Tony Lindgren, netdev,
	Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

Simon Horman <simon.horman@corigine.com> writes:

> On Fri, Jun 23, 2023 at 11:41:10PM +0200, Andrew Lunn wrote:
>> > > I feel like I am missing something here.
>> > 
>> > That is a weird response, you feel like something is missing
>> 
>> There is. The patch.
>> 
>> Maintainers have a slightly better memory than a goldfish, but given
>> the high volume of patches, we don't remember threads from 2016. Also,
>> all our infrastructure has limited memory, this patch is not in lore,
>> and it is not in patchworks. So in terms of getting merged, it does
>> not exist.
>> 
>> We do however recommend that if a patch has not been merged within 2
>> weeks, it is rebased, any Acked-by: etc tags are added and the patch
>> reposted.
>
> Thanks Andrew, that is also my position.
>
> A ping for a multi-year old patch is unusual (for me).
> I was wondering if there was a back story. I guess not.

The only story here is that I was reviewing the set of patches we apply
to our kernels, and I noticed that this one, judging by the discussion,
should have been applied to some tree or other ages ago.

Now if it takes 6 years to get a one-line patch (a fix for a regression,
no less) accepted, I have better things to spend my time on.

-- 
Måns Rullgård

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
  2023-06-24 15:02                 ` Måns Rullgård
@ 2023-06-24 15:53                   ` Simon Horman
  2023-06-24 15:59                     ` Måns Rullgård
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Horman @ 2023-06-24 15:53 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Andrew Lunn, Jeroen Hofstee, Tony Lindgren, netdev,
	Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

On Sat, Jun 24, 2023 at 04:02:07PM +0100, Måns Rullgård wrote:
> Simon Horman <simon.horman@corigine.com> writes:
> 
> > On Fri, Jun 23, 2023 at 11:41:10PM +0200, Andrew Lunn wrote:
> >> > > I feel like I am missing something here.
> >> > 
> >> > That is a weird response, you feel like something is missing
> >> 
> >> There is. The patch.
> >> 
> >> Maintainers have a slightly better memory than a goldfish, but given
> >> the high volume of patches, we don't remember threads from 2016. Also,
> >> all our infrastructure has limited memory, this patch is not in lore,
> >> and it is not in patchworks. So in terms of getting merged, it does
> >> not exist.
> >> 
> >> We do however recommend that if a patch has not been merged within 2
> >> weeks, it is rebased, any Acked-by: etc tags are added and the patch
> >> reposted.
> >
> > Thanks Andrew, that is also my position.
> >
> > A ping for a multi-year old patch is unusual (for me).
> > I was wondering if there was a back story. I guess not.
> 
> The only story here is that I was reviewing the set of patches we apply
> to our kernels, and I noticed that this one, judging by the discussion,
> should have been applied to some tree or other ages ago.
> 
> Now if it takes 6 years to get a one-line patch (a fix for a regression,
> no less) accepted, I have better things to spend my time on.

A long time to be sure. As Andrew explained, the patch is now stale.
It will need to be rebased and reposted in ordered to be considered for
upstream.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
  2023-06-24 15:53                   ` Simon Horman
@ 2023-06-24 15:59                     ` Måns Rullgård
  0 siblings, 0 replies; 17+ messages in thread
From: Måns Rullgård @ 2023-06-24 15:59 UTC (permalink / raw)
  To: Simon Horman
  Cc: Andrew Lunn, Jeroen Hofstee, Tony Lindgren, netdev,
	Mugunthan V N, Grygorii Strashko,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	open list

Simon Horman <simon.horman@corigine.com> writes:

> On Sat, Jun 24, 2023 at 04:02:07PM +0100, Måns Rullgård wrote:
>> Simon Horman <simon.horman@corigine.com> writes:
>> 
>> > On Fri, Jun 23, 2023 at 11:41:10PM +0200, Andrew Lunn wrote:
>> >> > > I feel like I am missing something here.
>> >> > 
>> >> > That is a weird response, you feel like something is missing
>> >> 
>> >> There is. The patch.
>> >> 
>> >> Maintainers have a slightly better memory than a goldfish, but given
>> >> the high volume of patches, we don't remember threads from 2016. Also,
>> >> all our infrastructure has limited memory, this patch is not in lore,
>> >> and it is not in patchworks. So in terms of getting merged, it does
>> >> not exist.
>> >> 
>> >> We do however recommend that if a patch has not been merged within 2
>> >> weeks, it is rebased, any Acked-by: etc tags are added and the patch
>> >> reposted.
>> >
>> > Thanks Andrew, that is also my position.
>> >
>> > A ping for a multi-year old patch is unusual (for me).
>> > I was wondering if there was a back story. I guess not.
>> 
>> The only story here is that I was reviewing the set of patches we apply
>> to our kernels, and I noticed that this one, judging by the discussion,
>> should have been applied to some tree or other ages ago.
>> 
>> Now if it takes 6 years to get a one-line patch (a fix for a regression,
>> no less) accepted, I have better things to spend my time on.
>
> A long time to be sure. As Andrew explained, the patch is now stale.
> It will need to be rebased and reposted in ordered to be considered for
> upstream.

I already did that, only to get more snark in return.

-- 
Måns Rullgård

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2023-06-24 15:59 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1477668756-2651-1-git-send-email-jhofstee@victronenergy.com>
2016-10-28 15:52 ` [PATCH v2] net: cpsw: fix obtaining mac address for am3517 Tony Lindgren
2016-10-28 15:52   ` Tony Lindgren
2016-10-28 18:17   ` Jeroen Hofstee
2016-10-28 18:17     ` Jeroen Hofstee
2016-10-28 18:17     ` Jeroen Hofstee
2016-10-28 18:19     ` Tony Lindgren
2016-10-28 18:19       ` Tony Lindgren
2019-03-01 14:52       ` Måns Rullgård
2019-03-01 14:52         ` Måns Rullgård
2023-06-23 14:58       ` Måns Rullgård
2023-06-23 20:14         ` Simon Horman
2023-06-23 21:13           ` Jeroen Hofstee
2023-06-23 21:41             ` Andrew Lunn
2023-06-24 14:55               ` Simon Horman
2023-06-24 15:02                 ` Måns Rullgård
2023-06-24 15:53                   ` Simon Horman
2023-06-24 15:59                     ` Måns Rullgård

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.