LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] net: dsa: set parent of hwmon device
@ 2015-01-21  0:13 Vivien Didelot
  2015-01-21  0:13 ` [PATCH] net: dsa: set slave MII bus PHY mask Vivien Didelot
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Vivien Didelot @ 2015-01-21  0:13 UTC (permalink / raw)
  To: netdev
  Cc: Vivien Didelot, David S . Miller, Guenter Roeck, linux-kernel, kernel

Set the dsa device as the parent of the hwmon device, in order to link
the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/
sysfs directory.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 3731714..363102a 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
 		hname[j] = '\0';
 		scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), "%s_dsa%d",
 			  hname, index);
-		ds->hwmon_dev = hwmon_device_register_with_groups(NULL,
+		ds->hwmon_dev = hwmon_device_register_with_groups(parent,
 					ds->hwmon_name, ds, dsa_hwmon_groups);
 		if (IS_ERR(ds->hwmon_dev))
 			ds->hwmon_dev = NULL;
-- 
2.2.2


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

* [PATCH] net: dsa: set slave MII bus PHY mask
  2015-01-21  0:13 [PATCH] net: dsa: set parent of hwmon device Vivien Didelot
@ 2015-01-21  0:13 ` Vivien Didelot
  2015-01-21  0:37   ` Florian Fainelli
  2015-01-26  0:01   ` David Miller
  2015-01-21  2:48 ` [PATCH] net: dsa: set parent of hwmon device Guenter Roeck
  2015-01-21  3:50 ` Guenter Roeck
  2 siblings, 2 replies; 8+ messages in thread
From: Vivien Didelot @ 2015-01-21  0:13 UTC (permalink / raw)
  To: netdev
  Cc: Vivien Didelot, David S . Miller, Florian Fainelli, linux-kernel, kernel

When registering a mdio bus, Linux assumes than every port has a PHY and tries
to scan it. If a switch port has no PHY registered, DSA will fail to register
the slave MII bus. To fix this, set the slave MII bus PHY mask to the switch
PHYs mask.

As an example, if we use a Marvell MV88E6352 (which is a 7-port switch with no
registered PHYs for port 5 and port 6), with the following declared names:

	static struct dsa_chip_data switch_cdata = {
		[...]
		.port_names[0] = "sw0",
		.port_names[1] = "sw1",
		.port_names[2] = "sw2",
		.port_names[3] = "sw3",
		.port_names[4] = "sw4",
		.port_names[5] = "cpu",
	};

DSA will fail to create the switch instance. With the PHY mask set for the
slave MII bus, only the PHY for ports 0-4 will be scanned and the instance will
be successfully created.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/slave.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 515569f..589aafd 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -46,6 +46,7 @@ void dsa_slave_mii_bus_init(struct dsa_switch *ds)
 	snprintf(ds->slave_mii_bus->id, MII_BUS_ID_SIZE, "dsa-%d:%.2x",
 			ds->index, ds->pd->sw_addr);
 	ds->slave_mii_bus->parent = ds->master_dev;
+	ds->slave_mii_bus->phy_mask = ~ds->phys_mii_mask;
 }
 
 
-- 
2.2.2


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

* Re: [PATCH] net: dsa: set slave MII bus PHY mask
  2015-01-21  0:13 ` [PATCH] net: dsa: set slave MII bus PHY mask Vivien Didelot
@ 2015-01-21  0:37   ` Florian Fainelli
  2015-01-26  0:01   ` David Miller
  1 sibling, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2015-01-21  0:37 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: David S . Miller, linux-kernel, kernel

On 20/01/15 16:13, Vivien Didelot wrote:
> When registering a mdio bus, Linux assumes than every port has a PHY and tries
> to scan it. If a switch port has no PHY registered, DSA will fail to register
> the slave MII bus. To fix this, set the slave MII bus PHY mask to the switch
> PHYs mask.
> 
> As an example, if we use a Marvell MV88E6352 (which is a 7-port switch with no
> registered PHYs for port 5 and port 6), with the following declared names:
> 
> 	static struct dsa_chip_data switch_cdata = {
> 		[...]
> 		.port_names[0] = "sw0",
> 		.port_names[1] = "sw1",
> 		.port_names[2] = "sw2",
> 		.port_names[3] = "sw3",
> 		.port_names[4] = "sw4",
> 		.port_names[5] = "cpu",
> 	};
> 
> DSA will fail to create the switch instance. With the PHY mask set for the
> slave MII bus, only the PHY for ports 0-4 will be scanned and the instance will
> be successfully created.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Tested-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>

> ---
>  net/dsa/slave.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 515569f..589aafd 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -46,6 +46,7 @@ void dsa_slave_mii_bus_init(struct dsa_switch *ds)
>  	snprintf(ds->slave_mii_bus->id, MII_BUS_ID_SIZE, "dsa-%d:%.2x",
>  			ds->index, ds->pd->sw_addr);
>  	ds->slave_mii_bus->parent = ds->master_dev;
> +	ds->slave_mii_bus->phy_mask = ~ds->phys_mii_mask;
>  }
>  
>  
> 


-- 
Florian

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

* Re: [PATCH] net: dsa: set parent of hwmon device
  2015-01-21  0:13 [PATCH] net: dsa: set parent of hwmon device Vivien Didelot
  2015-01-21  0:13 ` [PATCH] net: dsa: set slave MII bus PHY mask Vivien Didelot
@ 2015-01-21  2:48 ` Guenter Roeck
  2015-01-21  3:50 ` Guenter Roeck
  2 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2015-01-21  2:48 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: David S . Miller, linux-kernel, kernel

On 01/20/2015 04:13 PM, Vivien Didelot wrote:
> Set the dsa device as the parent of the hwmon device, in order to link
> the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/
> sysfs directory.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>   net/dsa/dsa.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 3731714..363102a 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
>   		hname[j] = '\0';
>   		scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), "%s_dsa%d",
>   			  hname, index);
> -		ds->hwmon_dev = hwmon_device_register_with_groups(NULL,
> +		ds->hwmon_dev = hwmon_device_register_with_groups(parent,
>   					ds->hwmon_name, ds, dsa_hwmon_groups);
>   		if (IS_ERR(ds->hwmon_dev))
>   			ds->hwmon_dev = NULL;
>

I'll have to look into this again; not sure if this works because there is
no 1:1 relationship between dsa device and hwmon device (or switch chip).

Guenter


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

* Re: [PATCH] net: dsa: set parent of hwmon device
  2015-01-21  0:13 [PATCH] net: dsa: set parent of hwmon device Vivien Didelot
  2015-01-21  0:13 ` [PATCH] net: dsa: set slave MII bus PHY mask Vivien Didelot
  2015-01-21  2:48 ` [PATCH] net: dsa: set parent of hwmon device Guenter Roeck
@ 2015-01-21  3:50 ` Guenter Roeck
  2015-01-21  5:58   ` Florian Fainelli
  2 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2015-01-21  3:50 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: David S . Miller, linux-kernel, kernel

On 01/20/2015 04:13 PM, Vivien Didelot wrote:
> Set the dsa device as the parent of the hwmon device, in order to link
> the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/
> sysfs directory.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>   net/dsa/dsa.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 3731714..363102a 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
>   		hname[j] = '\0';
>   		scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), "%s_dsa%d",
>   			  hname, index);
> -		ds->hwmon_dev = hwmon_device_register_with_groups(NULL,
> +		ds->hwmon_dev = hwmon_device_register_with_groups(parent,
>   					ds->hwmon_name, ds, dsa_hwmon_groups);
>   		if (IS_ERR(ds->hwmon_dev))
>   			ds->hwmon_dev = NULL;
>

Looking into my old e-mail, turns out we did not add the parent device because
it affected the output of the "sensors" command, and we wanted the device
to be handled as 'virtual device' (which implies no parent). That was an explicit
part of the patch set (v2 of 'net: dsa: Add support for reporting switch chip
temperatures'), compared to v1, which did set the parent device.

I would suggest to keep the code as is.

Thanks,
Guenter


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

* Re: [PATCH] net: dsa: set parent of hwmon device
  2015-01-21  3:50 ` Guenter Roeck
@ 2015-01-21  5:58   ` Florian Fainelli
  2015-01-21 16:26     ` Vivien Didelot
  0 siblings, 1 reply; 8+ messages in thread
From: Florian Fainelli @ 2015-01-21  5:58 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Vivien Didelot, netdev, David S . Miller, linux-kernel, kernel

2015-01-20 19:50 GMT-08:00 Guenter Roeck <linux@roeck-us.net>:
> On 01/20/2015 04:13 PM, Vivien Didelot wrote:
>>
>> Set the dsa device as the parent of the hwmon device, in order to link
>> the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/
>> sysfs directory.
>>
>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> ---
>>   net/dsa/dsa.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>> index 3731714..363102a 100644
>> --- a/net/dsa/dsa.c
>> +++ b/net/dsa/dsa.c
>> @@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int
>> index,
>>                 hname[j] = '\0';
>>                 scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name),
>> "%s_dsa%d",
>>                           hname, index);
>> -               ds->hwmon_dev = hwmon_device_register_with_groups(NULL,
>> +               ds->hwmon_dev = hwmon_device_register_with_groups(parent,
>>                                         ds->hwmon_name, ds,
>> dsa_hwmon_groups);
>>                 if (IS_ERR(ds->hwmon_dev))
>>                         ds->hwmon_dev = NULL;
>>
>
> Looking into my old e-mail, turns out we did not add the parent device
> because
> it affected the output of the "sensors" command, and we wanted the device
> to be handled as 'virtual device' (which implies no parent). That was an
> explicit
> part of the patch set (v2 of 'net: dsa: Add support for reporting switch
> chip
> temperatures'), compared to v1, which did set the parent device.
>
> I would suggest to keep the code as is.

Maybe follow-up with a comment adding that above the call to
hwmon_device_register_with_groups()? I suspect the intent is clear if
you are deep into hwmon devices, but not necessarily for the reader ;)

>
> Thanks,
> Guenter
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Florian

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

* Re: [PATCH] net: dsa: set parent of hwmon device
  2015-01-21  5:58   ` Florian Fainelli
@ 2015-01-21 16:26     ` Vivien Didelot
  0 siblings, 0 replies; 8+ messages in thread
From: Vivien Didelot @ 2015-01-21 16:26 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Florian Fainelli, netdev, David S . Miller, linux-kernel, kernel

Hi Guenter, Florian,

>>> -               ds->hwmon_dev = hwmon_device_register_with_groups(NULL,
>>> +               ds->hwmon_dev = hwmon_device_register_with_groups(parent,
>>>                                         ds->hwmon_name, ds, dsa_hwmon_groups);
>>>                 if (IS_ERR(ds->hwmon_dev))
>>>                         ds->hwmon_dev = NULL;
>>
>> Looking into my old e-mail, turns out we did not add the parent
>> device because it affected the output of the "sensors" command, and
>> we wanted the device to be handled as 'virtual device' (which implies
>> no parent). That was an explicit part of the patch set (v2 of 'net:
>> dsa: Add support for reporting switch chip temperatures'), compared
>> to v1, which did set the parent device.

I don't know about the "sensors" output, but I found that convenient
from the sysfs side to have a hierarchy of sub-devices logically exposed
at the same place, i.e.:

	# ls /sys/devices/platform/dsa.0
	driver    hwmon    net    ...

>> I would suggest to keep the code as is.

> Maybe follow-up with a comment adding that above the call to
> hwmon_device_register_with_groups()? I suspect the intent is clear if
> you are deep into hwmon devices, but not necessarily for the reader ;)

Thanks,
-v

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

* Re: [PATCH] net: dsa: set slave MII bus PHY mask
  2015-01-21  0:13 ` [PATCH] net: dsa: set slave MII bus PHY mask Vivien Didelot
  2015-01-21  0:37   ` Florian Fainelli
@ 2015-01-26  0:01   ` David Miller
  1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2015-01-26  0:01 UTC (permalink / raw)
  To: vivien.didelot; +Cc: netdev, f.fainelli, linux-kernel, kernel

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Tue, 20 Jan 2015 19:13:32 -0500

> When registering a mdio bus, Linux assumes than every port has a PHY and tries
> to scan it. If a switch port has no PHY registered, DSA will fail to register
> the slave MII bus. To fix this, set the slave MII bus PHY mask to the switch
> PHYs mask.
> 
> As an example, if we use a Marvell MV88E6352 (which is a 7-port switch with no
> registered PHYs for port 5 and port 6), with the following declared names:
> 
> 	static struct dsa_chip_data switch_cdata = {
> 		[...]
> 		.port_names[0] = "sw0",
> 		.port_names[1] = "sw1",
> 		.port_names[2] = "sw2",
> 		.port_names[3] = "sw3",
> 		.port_names[4] = "sw4",
> 		.port_names[5] = "cpu",
> 	};
> 
> DSA will fail to create the switch instance. With the PHY mask set for the
> slave MII bus, only the PHY for ports 0-4 will be scanned and the instance will
> be successfully created.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Applied.

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

end of thread, other threads:[~2015-01-26  0:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-21  0:13 [PATCH] net: dsa: set parent of hwmon device Vivien Didelot
2015-01-21  0:13 ` [PATCH] net: dsa: set slave MII bus PHY mask Vivien Didelot
2015-01-21  0:37   ` Florian Fainelli
2015-01-26  0:01   ` David Miller
2015-01-21  2:48 ` [PATCH] net: dsa: set parent of hwmon device Guenter Roeck
2015-01-21  3:50 ` Guenter Roeck
2015-01-21  5:58   ` Florian Fainelli
2015-01-21 16:26     ` Vivien Didelot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).