Re: [PATCH v2 2/2] hwmon: add generic GPIO brownout support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux