Thread (51 messages) 51 messages, 7 authors, 2014-10-26

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

From: Guenter Roeck <linux@roeck-us.net>
Date: 2014-10-23 18:43:38
Also in: lkml

On Thu, Oct 23, 2014 at 08:03:57PM +0200, Andrew Lunn wrote:
quoted
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.
quoted
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.
Marvell SoCs used in WiFi Access Points have multiple ethernet
interfaces, so i would hope the parent actually identifies which
ethernet interface it is hanging off.

Now consider the example in

http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/net/dsa/dsa.txt

We have two switches hanging off one ethernet interface. What will the > naming look like in this case?
Ah, that is a good question. You are right, the parent will be the same
for both of those switches but should be unique. Hmm ...
The Marvell DSA tagging scheme allows you to have 16 switches hanging
off one ethernet interface. How is the naming going to work then,
especially if there is a mixture of switch chips, some with
temperature sensors, and some without?
The ones without sensor would not create a hwmon device, so that should
not be an issue. You are right, the other use cases are more tricky.
What would really help is if each switch has a device in the linux
device model. The hwmon parent would then be the switch device. The
EEPROM would then hang off the switch device, not an interface on the
switch device, etc.
Good point. Unfortunately that is not the case.

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. We have three to
choose from. Should it be the parent network device or the dsa device ?
The dsa device seems like a better choice to me, but I am open to
suggestions. A problem with choosing the network device as parent
may be that this device could by itself have a temperature sensor
(some Intel and broadcom Ethernet chips have that), which could
cause confusion.

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.

Thanks,
Guenter
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help