Re: [PATCH 06/14] net: dsa: Add support for hardware monitoring

[Guenter Roeck <linux@roeck-us.net>]

On Thu, Oct 23, 2014 at 09:55:26PM +0200, Andrew Lunn wrote:
> On Thu, Oct 23, 2014 at 11:43:22AM -0700, Guenter Roeck wrote:
> > On Thu, Oct 23, 2014 at 08:03:57PM +0200, Andrew Lunn wrote:
> > > > No, I am not saying that. The hwmon device's parent device will tell,
> > > > which is how it works for all other hwmon devices.
> > > 
> > > O.K, so parent is important.
> > > 
> > > > Not really. Again, the parent device provides that information. libsensors,
> > > > which is the preferred way of accessing sensors information from user space,
> > > > provides the parent device instance as part of the logical sensor device
> > > > name. In this case, the names will end up being dsa-isa-0000, dsa-isa-0001,
> > > > and so on. With your added tags it would be dsa.0.0-isa-0000, dsa.0.1-isa-0001,
> > > > and so on. I don't see how this would add any value.
> > > 
> > > isa is the name of the ethernet device? Why is it not eth0? Most
> > 
> > isa is just an internal name made up by libsensors, and pretty much just
> > indicates something like "neither i2c nor spi". It is mostly historic,
> > but nowadays almost part of the ABI. It is made up by user space,
> > based on the parent device type, not by the kernel.
> 
> So for DSA, since it is not i2c or spi, the parent is actually
> irrelevant, because libsensor ignores it and says "IsSomethingAlien".
> So the name alone needs to identify it.
> 
>  > You have convinced me that 'dsa' as hwmon attribute name is insufficient,
> > so let's see what we have.
> > 
> > - the parent network device in dst->master_netdev
> > - the dsa device in 'parent'
> > - the host device in host_dev
> > - the switch index in 'index'
> > 
> > First question is what should be the parent device.
> 
> Does it even matter, given the observation above?  I would probably go
> for dsa, since as you said, the Ethernet device may have a temperature
> sensor of its own. DSA is a virtual device...
> 
> > Second is what to choose for the hwmon device 'name' attribute.
> > 'dsa' is not sufficient, but what to choose instead ? dsa.X or dsa_X,
> > where X is the switch index ? At this point I am open to suggestions.
> > Note that we can not use the name returned from the switch probe
> > functions as it may contain spaces and other invalid characters.
> > Including the ethernet device name (eg as in eth0_dsa_0) may also be
> > problematic if it can contain '-', which is illegal for hwmon devices.
> 
> Does hwmon offer a function to sanitise a string?
> 
> The switch index definitely should be used and i would probably
> combine that with a sanitised version of the ethernet device name and
> "dsa".
> 
Here is another naming option:

em1dsa0-virtual-0
Adapter: Virtual device
temp1:        +52.0°C  (high = +100.0°C)

I think I'll go with that one. I get it by not specifying
a parent device when registering with the hwmon subsystem.
It is kind of similar to the thermal sensor data reported
through acpi.

acpitz-virtual-0
Adapter: Virtual device
temp1:        +52.0°C  (crit = +111.0°C)
temp2:        +52.0°C  (crit = +111.0°C)

Guenter