On 18-10-30 12:56, Guenter Roeck wrote:
> On Tue, Oct 30, 2018 at 06:00:26PM +0100, Marco Felsch wrote:
> > On 18-10-30 06:13, Guenter Roeck wrote:
> > > On 10/30/18 3:47 AM, Marco Felsch wrote:
> > > > On 18-10-29 18:12, Guenter Roeck wrote:
> > > > > On 10/29/18 2:16 PM, Trent Piepho wrote:
> >
> > [Snip]
> >
> > > > > >
> > > > > > Consider, what if we had a "classic" hwmon chip, but on this board the
> > > > > > I2C/LPC/SPI interface was not connected as an appropriate master was
> > > > > > not available, and the default configuration of the chip was
> > > > > > acceptable. The chip's alarm outputs are connected to GPIOs, as it
> > > > > > normal for a hwmon chip with alarm outputs.
> > > > > >
> > > > > "If we had" is theory. Do we ? We don't usually add code to the kernel
> > > > > just in case the hardware it supports might be out there.
> > > >
> > > > Yes, there are "good old" hwmon chips without any management interface,
> > > > e.g. LTC694-3.3, LTC7862, ... also there might be a discrete hw circuit.
> > > > I think it's better to handle those "simple" chips by a generic hwmon
> > > > driver. By simple I mean chips which informs the host only by toggling a
> > > > gpio to report a state. For this purpose my driver (including name
> > > > convention) isn't generic enough, I think about a hwmon-simple device.
> > > > Such a device can support reporting states, no voltage/power/temp values.
> > > >
> > >
> > > Yes, I agree. It doesn't really make sense to keep adding single-attribute drivers.
> > > hwmon-gpio-simple, maybe, to indicate that it gets its data from gpio ?
> >
> > hwmon-gpio-simple sounds ok for me.
> >
> > > The most difficult part of such a driver would probably be to define acceptable
> > > devicetree properties.
> >
> > That's true! One possible solution could be:
> >
> > hwmon_dev {
> > compatible = "hwmon-gpio-simple";
>
> This is unlikely to be acceptable for dt maintainers since "hwmon" is a Linuxism.
> Not that I have a better idea for an acceptable "compatible" name.
>
> > name = "gpio-generic-hwmon";
> > update-interval-ms = 100;
>
> We would not want to implement any polling in the kernel.
>
> >
> > hwmon-gpio-simple,dev@0 {
> > reg = <0>;
> > gpio = <gpio3 15 GPIO_ACTIVE_LOW>;
> > hwmon-gpio-simple,type = "in";
> > hwmon-gpio-simple,report = "crit_alarm";
> > };
> >
> > hwmon-gpio-simple,dev@1 {
> > reg = <1>;
> > gpio = <gpio3 19 GPIO_ACTIVE_LOW>;
> > hwmon-gpio-simple,type = "temp";
> > hwmon-gpio-simple,report = "alarm";
> > };
> > };
> >
> > [SNIP]
> >
> > > > > > The ability of the hwmon driver to cause certain other devices to be
> > > > > > removed is a different question.
> > > > > >
> > > > > I disagree. That functionality is essential to the driver as submitted.
> > > >
> > > > You're right thats important for my use-case, but I got you, thats not a
> > > > hwmon related problem. If I understood the device-model correctly there
> > > > are absolut no problems to hot-unplug devices, this is always the case
> > > > if we unbind a driver from user-space. So why we can't handle it
> > > > directly in the kernel. Are there any concerns? If not can you give me
> > > > some hints to get the logic at thr right place.
> > > >
> > > Well, the easiest solution would have been to do nothing code-wise and
> > > register a gpio-keys instance on the gpio pin to create a KEY_POWER event.
> > > Of course that might be considered an abuse of the input subsystem,
> > > as Dmitry has pointed out, and is probably not acceptable. Now, if you
> > > want that functionality, you could probably write some udev rules and
> > > accomplish the same by listening for a change event from a sysfs attribute
> > > supporting it (I think KEY_POWER is not really handled in the kernel,
> > > but I am not entirely sure).
> >
> > I came from the input framework, as you pointed out.
> >
> > > Handling the event in the kernel is more tricky. First, I don't think the
> > > selective device removal as you have suggested would be acceptable anywhere;
> > > it may cause all kinds of trouble. The thermal subsystem supports the
> > > functionality to shut down the kernel in emergencies, but is limited
> > > in its scope (or at least I think so) to thermal events. Anything else
> > > would have to be discussed. I for my part prefer handling it from userspace.
> >
> > Imagine that use-case: There is a custom board design which power off
> > all external devices and leave the host on for a certain time (a few
> > seconds) upon a low-voltage detection. Now the pins from the external
> > devices are floating around and producing a huge amount of interrupts,
> > so the kernel is really busy handling those interrupts and can't handle
> > user-space processes anymore. The "host keep-on time" was intended to be
> > used to save all user data currently processed, but this never happen.
> > Furthermore there can be a high-priority userspace task which can't be
> > preempted and the exec_time for this task is greater than the "host
> > keep-on time". So the task which will unbind the devices gets never
> > scheduled. Hopefully you get me and understand my outlines and why we
> > need to do the unbinding within the kernel-space.
> >
> This would be better handled with a DT overlay and removal of the same
> if power goes bad. The power good signal would then be tied to DT overlay
> insertion and removal, ie it would be be handled like an "insert/remove"
> pin.
>
> > One solution could be to split the drivers into to: hwmon for measuring
> > and notifying, of-unbind to unbind registered devices. For this we need
> > to get a kernel_notification from the hwmon-framework, so the generic
> > unbinding driver gets informed. Are you agree with me?
> >
> At a previous employer we had a kernel module which would trigger DT overlay
> insertion and removal for OIR capable cards with a number of devices on them.
> That worked pretty well. A similar approach might work here. Maybe it is
> even possible to integrate it into extcon-gpio.
Thanks for your hints.
Marco
