Re: [PATCH v2 2/2] hwmon: ltc4282: add support for the LTC4282 chip

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

 



On Thu, 2023-11-30 at 21:15 +0100, Linus Walleij wrote:
> On Thu, Nov 30, 2023 at 4:20 PM Nuno Sá <noname.nuno@xxxxxxxxx> wrote:
> 
> > Well, I did spent some time on the gpio thing so I would like to have it in but
> > yeah,
> > no hard feelings if it does not go in.
> 
> I think it's a good idea to include it, especially if you can, in the
> commit message,
> illustrate how you test it with the libgpiod toolset. If you can test it all the
> way such that you have real hardware connected to real electronics where
> you can observe the values on these pins or read them: even better.
> 
> GPIOs tend to get used, and then we are prepared.
> 

Yeah, I would also like to have this in but since I'm adding the driver to hwmon,
I'll leave the final word to Guenter.

I did not used libgpiod but I did tested it with gpio-sysfs. Well, I could
effectively see the pull down behaviour but since my eval board has no pull-ups I
could not drive the line high.

> > So, I actually talk with some hw guys and the pull_low is not really like a
> > pull_low
> > resistor.
> 
> We usually call it pull down, so the PIN_CONFIG_BIAS_PULL_DOWN
> config property.
> 
> This is vital to e.g. create a bit-banged I2C link, which is something I
> suspect could be useful on this hardware.
> 
> > These pins are effectively an open drain. Which means, setting them as
> > input means setting them in high-z (turning off the mosffet) - and I do have a
> > bug in
> > my code regarding this -
> 
> The gpiolib framework assumes we can do open drain emulation by
> setting lines as input. It is used as fallback unless the hardware has
> an explicit open drain setting.

Yeah, I did look at that after you pointed that out. There's just something I'm still
not getting. This HW has no explicit open drain setting because open drain is all
that it is. So, I guess we could just specify the flag in devicetree so gpiolib could
use the emulation but I wonder how would we have things in case we have the HW setup
to drive the pin high (so having this as GPOs)?
> 
> > Or if you want them as outputs you can set the level low
> > (and it will always be low - just turn on the mosffet) or you can also set high-z
> > which means it will be either low or high depending on your external circuitry.
> > The
> > point is, you can still have your pin acting as a normal gpo if you accommodate
> > your
> > circuitry for it (can also use these pins for things like buses).
> 
> Yeah that is just standard open drain behaviour, by the book.
> Also documented in
> https://docs.kernel.org/driver-api/gpio/driver.html
> under the heading "GPIO lines with open drain/source support".
> 
> > Also got me thinking if a gpi vs gpo devicetree property would make sense. But I
> > would likely leave it more generic/relaxed for now (even though I think you would
> > need to be creative and actually use more HW to have the possibility of using
> > these
> > pins as GPIs and GPOs at the same time).
> 
> We don't define that in the device tree currently, we just make the driver
> not support output on input-only pins and vice versa, by returning error
> codes on the .set_direction() callbacks.
> 

I see, but in this case, the pins could be outputs depending on the HW setup but
there's no way for us to know that in the driver. And given the fact that (I think)
it's highly unlikely for pins like this to ever be GPIs and GPOs at the same time, I
brought the devicetree property to define input and output only. So, roughly, what I
have in mind now for the chip is;

.set_config() -> with PULL_DOWN and HIGH_IMPEDANCE support
.direction_input() -> This is important for gpio1 where we do have an hw setting to
set the direction. On the other pins I was thinking in just forcing high-z. Or maybe
can I just rely on gpio_set_bias()?
.direction_ouput() -> Would only matter for gpio1
.get/set_value() -> And in this case we just assume that high value might or might
not be possible (depending on the hw setup). Note that reading the pin state is
always possible.

This means that I assume we can have both directions but that is not really case and
one needs to know what it is doing :). Or in cases like this, we just ignore the
possibility of having GPO's and we let gpiolib do the emulation?

Sounds reasonable or not really how I should handle this open-drain only pins?

Thanks for the help!
- Nuno Sá






[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