[PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

The IOT2000 is industrial controller platform, derived from the Intel
Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
IOT2040 has two of them. They can be told apart based on the board asset
tag in the DMI table.

Based on patch by Sascha Weisenberger.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Sascha Weisenberger <sascha.weisenberger@siemens.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 5c9e462276b9..de87c329fb5c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -32,6 +32,7 @@
  */
 struct stmmac_pci_dmi_data {
 	const char *name;
+	const char *asset_tag;
 	unsigned int func;
 	int phy_addr;
 };
@@ -46,6 +47,7 @@ struct stmmac_pci_info {
 static int stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
 {
 	const char *name = dmi_get_system_info(DMI_BOARD_NAME);
+	const char *asset_tag = dmi_get_system_info(DMI_BOARD_ASSET_TAG);
 	unsigned int func = PCI_FUNC(info->pdev->devfn);
 	struct stmmac_pci_dmi_data *dmi;
 
@@ -57,7 +59,8 @@ static int stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
 		return 1;
 
 	for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) {
-		if (!strcmp(dmi->name, name) && dmi->func == func)
+		if (dmi->func == func && !strcmp(dmi->name, name) &&
+		    (!dmi->asset_tag || !strcmp(dmi->asset_tag, asset_tag)))
 			return dmi->phy_addr;
 	}
 
@@ -142,6 +145,24 @@ static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
 		.func = 6,
 		.phy_addr = 1,
 	},
+	{
+		.name = "SIMATIC IOT2000",
+		.asset_tag = "6ES7647-0AA00-0YA2",
+		.func = 6,
+		.phy_addr = 1,
+	},
+	{
+		.name = "SIMATIC IOT2000",
+		.asset_tag = "6ES7647-0AA00-1YA2",
+		.func = 6,
+		.phy_addr = 1,
+	},
+	{
+		.name = "SIMATIC IOT2000",
+		.asset_tag = "6ES7647-0AA00-1YA2",
+		.func = 7,
+		.phy_addr = 1,
+	},
 	{}
 };
 

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Andy Shevchenko]

On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> The IOT2000 is industrial controller platform, derived from the Intel
> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
> IOT2040 has two of them. They can be told apart based on the board asset
> tag in the DMI table.
>
> Based on patch by Sascha Weisenberger.
>

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Signed-off-by: Sascha Weisenberger <sascha.weisenberger@siemens.com>

Shoudn't be ordered other way around?

> +       const char *asset_tag;

I guess this is redundant. See below.

> +       {
> +               .name = "SIMATIC IOT2000",
> +               .asset_tag = "6ES7647-0AA00-0YA2",
> +               .func = 6,
> +               .phy_addr = 1,
> +       },

The below has same definition disregard on asset_tag.

> +       {
> +               .name = "SIMATIC IOT2000",
> +               .asset_tag = "6ES7647-0AA00-1YA2",
> +               .func = 6,
> +               .phy_addr = 1,
> +       },
> +       {
> +               .name = "SIMATIC IOT2000",
> +               .asset_tag = "6ES7647-0AA00-1YA2",
> +               .func = 7,
> +               .phy_addr = 1,
> +       },

How this supposed to work if phy_addr is the same?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

On 2017-04-24 23:27, Andy Shevchenko wrote:
> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> The IOT2000 is industrial controller platform, derived from the Intel
>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>> IOT2040 has two of them. They can be told apart based on the board asset
>> tag in the DMI table.
>>
>> Based on patch by Sascha Weisenberger.
>>
> 
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> Signed-off-by: Sascha Weisenberger <sascha.weisenberger@siemens.com>
> 
> Shoudn't be ordered other way around?

Nope. My changes invalidated Sascha's signed-off on the original patch,
but he signed off again on the final version.

> 
>> +       const char *asset_tag;
> 
> I guess this is redundant. See below.
> 
>> +       {
>> +               .name = "SIMATIC IOT2000",
>> +               .asset_tag = "6ES7647-0AA00-0YA2",
>> +               .func = 6,
>> +               .phy_addr = 1,
>> +       },
> 
> The below has same definition disregard on asset_tag.
> 

There is a small difference in the asset tag, just not at the last digit
where one may expect it, look:

...-0YA2 -> IOT2020
...-1YA2 -> IOT2040

>> +       {
>> +               .name = "SIMATIC IOT2000",
>> +               .asset_tag = "6ES7647-0AA00-1YA2",
>> +               .func = 6,
>> +               .phy_addr = 1,
>> +       },
>> +       {
>> +               .name = "SIMATIC IOT2000",
>> +               .asset_tag = "6ES7647-0AA00-1YA2",
>> +               .func = 7,
>> +               .phy_addr = 1,
>> +       },
> 
> How this supposed to work if phy_addr is the same?
> 

That address space is MAC-local, and we have two different MACs here.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-04-24 23:27, Andy Shevchenko wrote:
>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> The IOT2000 is industrial controller platform, derived from the Intel
>>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>>> IOT2040 has two of them. They can be told apart based on the board asset
>>> tag in the DMI table.

>>> +       const char *asset_tag;
>>
>> I guess this is redundant. See below.
>>
>>> +       {
>>> +               .name = "SIMATIC IOT2000",
>>> +               .asset_tag = "6ES7647-0AA00-0YA2",
>>> +               .func = 6,
>>> +               .phy_addr = 1,
>>> +       },
>>
>> The below has same definition disregard on asset_tag.
>>
>
> There is a small difference in the asset tag, just not at the last digit
> where one may expect it, look:
>
> ...-0YA2 -> IOT2020
> ...-1YA2 -> IOT2040

Yes. And how does it change my statement? You may use one record here
instead of two.

>
>>> +       {
>>> +               .name = "SIMATIC IOT2000",
>>> +               .asset_tag = "6ES7647-0AA00-1YA2",
>>> +               .func = 6,
>>> +               .phy_addr = 1,
>>> +       },

>>> +       {
>>> +               .name = "SIMATIC IOT2000",
>>> +               .asset_tag = "6ES7647-0AA00-1YA2",
>>> +               .func = 7,
>>> +               .phy_addr = 1,
>>> +       },
>>
>> How this supposed to work if phy_addr is the same?
> That address space is MAC-local, and we have two different MACs here.

Got it, though asset_tag here is redundant as well.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

On 2017-04-25 09:30, Andy Shevchenko wrote:
> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2017-04-24 23:27, Andy Shevchenko wrote:
>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> The IOT2000 is industrial controller platform, derived from the Intel
>>>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>>>> IOT2040 has two of them. They can be told apart based on the board asset
>>>> tag in the DMI table.
> 
>>>> +       const char *asset_tag;
>>>
>>> I guess this is redundant. See below.
>>>
>>>> +       {
>>>> +               .name = "SIMATIC IOT2000",
>>>> +               .asset_tag = "6ES7647-0AA00-0YA2",
>>>> +               .func = 6,
>>>> +               .phy_addr = 1,
>>>> +       },
>>>
>>> The below has same definition disregard on asset_tag.
>>>
>>
>> There is a small difference in the asset tag, just not at the last digit
>> where one may expect it, look:
>>
>> ...-0YA2 -> IOT2020
>> ...-1YA2 -> IOT2040
> 
> Yes. And how does it change my statement? You may use one record here
> instead of two.

How? Please be more verbose in your comments.

> 
>>
>>>> +       {
>>>> +               .name = "SIMATIC IOT2000",
>>>> +               .asset_tag = "6ES7647-0AA00-1YA2",
>>>> +               .func = 6,
>>>> +               .phy_addr = 1,
>>>> +       },
> 
>>>> +       {
>>>> +               .name = "SIMATIC IOT2000",
>>>> +               .asset_tag = "6ES7647-0AA00-1YA2",
>>>> +               .func = 7,
>>>> +               .phy_addr = 1,
>>>> +       },
>>>
>>> How this supposed to work if phy_addr is the same?
>> That address space is MAC-local, and we have two different MACs here.
> 
> Got it, though asset_tag here is redundant as well.
> 

It's not as it is the only differentiating criteria to tell the
two-ports variant apart from the one-port (and to avoid confusing it
with any potential future variant). We could leave out the name, but I
kept it for documentation purposes.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 12:00 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-04-25 09:30, Andy Shevchenko wrote:
>> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2017-04-24 23:27, Andy Shevchenko wrote:
>>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> The IOT2000 is industrial controller platform, derived from the Intel
>>>>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>>>>> IOT2040 has two of them. They can be told apart based on the board asset
>>>>> tag in the DMI table.
>>
>>>>> +       const char *asset_tag;
>>>>
>>>> I guess this is redundant. See below.
>>>>
>>>>> +       {
>>>>> +               .name = "SIMATIC IOT2000",
>>>>> +               .asset_tag = "6ES7647-0AA00-0YA2",
>>>>> +               .func = 6,
>>>>> +               .phy_addr = 1,
>>>>> +       },
>>>>
>>>> The below has same definition disregard on asset_tag.
>>>>
>>>
>>> There is a small difference in the asset tag, just not at the last digit
>>> where one may expect it, look:
>>>
>>> ...-0YA2 -> IOT2020
>>> ...-1YA2 -> IOT2040
>>
>> Yes. And how does it change my statement? You may use one record here
>> instead of two.
>
> How? Please be more verbose in your comments.

       {
               .name = "SIMATIC IOT2000",
               .func = 6,
               .phy_addr = 1,
       },
       {
               .name = "SIMATIC IOT2000",
               .func = 7,
               .phy_addr = 1,
       },

That's all what you need.

>> Got it, though asset_tag here is redundant as well.

> It's not as it is the only differentiating criteria to tell the
> two-ports variant apart from the one-port (and to avoid confusing it
> with any potential future variant).

And why exactly is it needed?
Sorry, but I don't see any reason to blow the code for no benefit.

> We could leave out the name, but I
> kept it for documentation purposes.

Nobody prevents you to add a comment.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

On 2017-04-25 11:46, Andy Shevchenko wrote:
> On Tue, Apr 25, 2017 at 12:00 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2017-04-25 09:30, Andy Shevchenko wrote:
>>> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2017-04-24 23:27, Andy Shevchenko wrote:
>>>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> The IOT2000 is industrial controller platform, derived from the Intel
>>>>>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>>>>>> IOT2040 has two of them. They can be told apart based on the board asset
>>>>>> tag in the DMI table.
>>>
>>>>>> +       const char *asset_tag;
>>>>>
>>>>> I guess this is redundant. See below.
>>>>>
>>>>>> +       {
>>>>>> +               .name = "SIMATIC IOT2000",
>>>>>> +               .asset_tag = "6ES7647-0AA00-0YA2",
>>>>>> +               .func = 6,
>>>>>> +               .phy_addr = 1,
>>>>>> +       },
>>>>>
>>>>> The below has same definition disregard on asset_tag.
>>>>>
>>>>
>>>> There is a small difference in the asset tag, just not at the last digit
>>>> where one may expect it, look:
>>>>
>>>> ...-0YA2 -> IOT2020
>>>> ...-1YA2 -> IOT2040
>>>
>>> Yes. And how does it change my statement? You may use one record here
>>> instead of two.
>>
>> How? Please be more verbose in your comments.
> 
>        {
>                .name = "SIMATIC IOT2000",
>                .func = 6,
>                .phy_addr = 1,
>        },
>        {
>                .name = "SIMATIC IOT2000",
>                .func = 7,
>                .phy_addr = 1,
>        },
> 
> That's all what you need.

Nope. Again: the asset tag is the way to tell both apart AND to ensure
that we do not match on future devices.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

On 2017-04-25 12:07, Jan Kiszka wrote:
> On 2017-04-25 11:46, Andy Shevchenko wrote:
>> On Tue, Apr 25, 2017 at 12:00 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2017-04-25 09:30, Andy Shevchenko wrote:
>>>> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> On 2017-04-24 23:27, Andy Shevchenko wrote:
>>>>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>> The IOT2000 is industrial controller platform, derived from the Intel
>>>>>>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>>>>>>> IOT2040 has two of them. They can be told apart based on the board asset
>>>>>>> tag in the DMI table.
>>>>
>>>>>>> +       const char *asset_tag;
>>>>>>
>>>>>> I guess this is redundant. See below.
>>>>>>
>>>>>>> +       {
>>>>>>> +               .name = "SIMATIC IOT2000",
>>>>>>> +               .asset_tag = "6ES7647-0AA00-0YA2",
>>>>>>> +               .func = 6,
>>>>>>> +               .phy_addr = 1,
>>>>>>> +       },
>>>>>>
>>>>>> The below has same definition disregard on asset_tag.
>>>>>>
>>>>>
>>>>> There is a small difference in the asset tag, just not at the last digit
>>>>> where one may expect it, look:
>>>>>
>>>>> ...-0YA2 -> IOT2020
>>>>> ...-1YA2 -> IOT2040
>>>>
>>>> Yes. And how does it change my statement? You may use one record here
>>>> instead of two.
>>>
>>> How? Please be more verbose in your comments.
>>
>>        {
>>                .name = "SIMATIC IOT2000",
>>                .func = 6,
>>                .phy_addr = 1,
>>        },
>>        {
>>                .name = "SIMATIC IOT2000",
>>                .func = 7,
>>                .phy_addr = 1,
>>        },
>>
>> That's all what you need.
> 
> Nope. Again: the asset tag is the way to tell both apart AND to ensure
> that we do not match on future devices.

To be more verbose: your version (which is our old one) would even
enable the second, not connected port on the IOT2020. Incorrectly. Plus
the risk to match different future devices.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 1:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-04-25 11:46, Andy Shevchenko wrote:
>> On Tue, Apr 25, 2017 at 12:00 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2017-04-25 09:30, Andy Shevchenko wrote:
>>>> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> On 2017-04-24 23:27, Andy Shevchenko wrote:
>>>>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

>>>>>>> +       {
>>>>>>> +               .name = "SIMATIC IOT2000",
>>>>>>> +               .asset_tag = "6ES7647-0AA00-0YA2",
>>>>>>> +               .func = 6,
>>>>>>> +               .phy_addr = 1,
>>>>>>> +       },
>>>>>>
>>>>>> The below has same definition disregard on asset_tag.
>>>>>>
>>>>>
>>>>> There is a small difference in the asset tag, just not at the last digit
>>>>> where one may expect it, look:
>>>>>
>>>>> ...-0YA2 -> IOT2020
>>>>> ...-1YA2 -> IOT2040
>>>>
>>>> Yes. And how does it change my statement? You may use one record here
>>>> instead of two.
>>>
>>> How? Please be more verbose in your comments.
>>
>>        {
>>                .name = "SIMATIC IOT2000",
>>                .func = 6,
>>                .phy_addr = 1,
>>        },
>>        {
>>                .name = "SIMATIC IOT2000",
>>                .func = 7,
>>                .phy_addr = 1,
>>        },
>>
>> That's all what you need.
>
> Nope. Again: the asset tag is the way to tell both apart AND to ensure
> that we do not match on future devices.

One step at a time. We don't care of future devices. When we will have
an issue we will look at it.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 1:09 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-04-25 12:07, Jan Kiszka wrote:
>> On 2017-04-25 11:46, Andy Shevchenko wrote:
>>> On Tue, Apr 25, 2017 at 12:00 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2017-04-25 09:30, Andy Shevchenko wrote:
>>>>> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> On 2017-04-24 23:27, Andy Shevchenko wrote:
>>>>>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

>>>        {
>>>                .name = "SIMATIC IOT2000",
>>>                .func = 6,
>>>                .phy_addr = 1,
>>>        },
>>>        {
>>>                .name = "SIMATIC IOT2000",
>>>                .func = 7,
>>>                .phy_addr = 1,
>>>        },
>>>
>>> That's all what you need.
>>
>> Nope. Again: the asset tag is the way to tell both apart AND to ensure
>> that we do not match on future devices.

> To be more verbose: your version (which is our old one) would even
> enable the second, not connected port on the IOT2020. Incorrectly.

So, name has 2000 for 2020 device? It's clear bug in DMI table you have. :-(

What else do you have in DMI which can be used to distinguish those devices?

> Plus
> the risk to match different future devices.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

On 2017-04-25 13:42, Andy Shevchenko wrote:
> On Tue, Apr 25, 2017 at 1:09 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2017-04-25 12:07, Jan Kiszka wrote:
>>> On 2017-04-25 11:46, Andy Shevchenko wrote:
>>>> On Tue, Apr 25, 2017 at 12:00 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> On 2017-04-25 09:30, Andy Shevchenko wrote:
>>>>>> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>> On 2017-04-24 23:27, Andy Shevchenko wrote:
>>>>>>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> 
>>>>        {
>>>>                .name = "SIMATIC IOT2000",
>>>>                .func = 6,
>>>>                .phy_addr = 1,
>>>>        },
>>>>        {
>>>>                .name = "SIMATIC IOT2000",
>>>>                .func = 7,
>>>>                .phy_addr = 1,
>>>>        },
>>>>
>>>> That's all what you need.
>>>
>>> Nope. Again: the asset tag is the way to tell both apart AND to ensure
>>> that we do not match on future devices.
> 
>> To be more verbose: your version (which is our old one) would even
>> enable the second, not connected port on the IOT2020. Incorrectly.
> 
> So, name has 2000 for 2020 device? It's clear bug in DMI table you have. :-(
> 
> What else do you have in DMI which can be used to distinguish those devices?

Andy, there are devices out there in the field, if we as engineers like
it or not, that are all called "IOT2000" although they are sightly
different inside. This patch accounts for that. And it does that even
without adding "platform_data" hacks like in other patches of mine. ;)

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 3:15 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-04-25 13:42, Andy Shevchenko wrote:
>> On Tue, Apr 25, 2017 at 1:09 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2017-04-25 12:07, Jan Kiszka wrote:
>>>> On 2017-04-25 11:46, Andy Shevchenko wrote:
>>>>> On Tue, Apr 25, 2017 at 12:00 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

>>>>>        {
>>>>>                .name = "SIMATIC IOT2000",
>>>>>                .func = 6,
>>>>>                .phy_addr = 1,
>>>>>        },
>>>>>        {
>>>>>                .name = "SIMATIC IOT2000",
>>>>>                .func = 7,
>>>>>                .phy_addr = 1,
>>>>>        },
>>>>>
>>>>> That's all what you need.
>>>>
>>>> Nope. Again: the asset tag is the way to tell both apart AND to ensure
>>>> that we do not match on future devices.
>>
>>> To be more verbose: your version (which is our old one) would even
>>> enable the second, not connected port on the IOT2020. Incorrectly.
>>
>> So, name has 2000 for 2020 device? It's clear bug in DMI table you have. :-(
>>

>> What else do you have in DMI which can be used to distinguish those devices?

So, except asset tag is there anything else to use?

Whatever you choose would be nice to split this long conditional:

if (!strcmp(dmi->name, name) && dmi->func == func) {
    /* ASSET_TAG is optional */
    if (dmi->asset_tag && strcmp(dmi->asset_tag, asset_tag))
        continue;
    return dmi->phy_addr;
}

> Andy, there are devices out there in the field, if we as engineers like
> it or not, that are all called "IOT2000" although they are sightly
> different inside. This patch accounts for that. And it does that even
> without adding "platform_data" hacks like in other patches of mine. ;)

Yes, which is good.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[David Miller]

From: Jan Kiszka <jan.kiszka@siemens.com>
Date: Mon, 24 Apr 2017 21:27:15 +0200

> The IOT2000 is industrial controller platform, derived from the Intel
> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
> IOT2040 has two of them. They can be told apart based on the board asset
> tag in the DMI table.
> 
> Based on patch by Sascha Weisenberger.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Signed-off-by: Sascha Weisenberger <sascha.weisenberger@siemens.com>

It looks like there is still some discussion going on about precise
detection and disambiguation of these devices.

Once things have settled down please resubmit the final patch.

Thank you.