On 18-11-01 17:41, Trent Piepho wrote: > On Thu, 2018-11-01 at 11:40 +0100, Marco Felsch wrote: > > On 18-10-30 13:11, Guenter Roeck wrote: > > > On Tue, Oct 30, 2018 at 07:34:11PM +0000, Trent Piepho wrote: > > > > > > > voltage@0 { > > > > reg = <0>; > > > > I remember that we have to add a reg property if we want to use xyz@0. > > Kernel disables that dtc warning, but still seems good to follow that > rule. Yes, but the maintainers won't confirm it without the reg property. > > > with some better (acceptable) values for "alarm-type" and the actual fields. > > > > Should we use the @<reg> suffix to map it to in<reg>_*_alarm or should > > we do something like that: > > Usually the node name, e.g. "node@0", isn't used at all by the driver. > It's arbitrary, so the dt author can name it what they want. Thats true, but we have to use that convention to conform the DT if we want to use the reg property. > I've used to identify a device in a udev rule. So I can have a rule > that acts on a device based on what I want the device to do, rather > than what bus and address the device happens to be on. The latter are > subject to change based on other devices, SoC pinmuxing, board routing, > etc. > > Using the "reg" property to identify the input/temp/fan number seems > totally appropriate to me. Okay. > > > > hwmon_dev { > > compatible = "gpio-alarm"; > > > > voltage { > > I think you'd need something like type = "voltage" here instead of > using the node name. > > > bat@0 { > > reg = <0>; > > label = "Battery Pack1 Voltage"; > > }; > > > > bat@1 { > > reg = <1>; > > label = "Battery Pack2 Voltage"; > > }; > > }; > > Seems better to me to have the flatter tree with each alarm having a > "type" attribute rather than grouping them by type. I'm open minded. Guenter what's your prefered solution? > Usually the tree structure of a DT is meant to show a "contains" > relationship, one present in the actual hardware. A bus with devices > behind it. Lines attached to controller. If I've got three op-amps on > GPIOs, two measuring voltage rails and the third on a thermistor, then > IMHO all three are peers. The hardware design doesn't group the two > voltage rail op-amps in some way that excludes the thermistor op-amp. > > > > Now the subnodes imply the type. Since the hwmon-gpio-simple should > > work interrupt driven all the time we should replace the alarm-gpios by > > Not all GPIOs can generate interrupts. It would be an unfortunate > hardware design to use such a GPIO for this. Seems ok to me to defer > that problem to the poor sod who needs to support such hardware if it > ever exists. Please look at the other mails. It's important for Guenter to support the non-interrupt case too. It shouldn't be a big deal to implement both cases. Marco