Re: [PATCH v2 2/3] hwmon (xdpe12284): Add support for xdpe11280

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

 



On 2/21/22 04:13, sylv wrote:
On Thu, 2022-02-17 at 11:25 -0800, Guenter Roeck wrote:
On 2/17/22 10:38, sylv wrote:
[ ... ]


That makes me wonder if the chip needs to be added to this driver
in
the first
place, or if it could be added to pmbus.c instead. Any idea ?

Oh, we did wrote a standalone driver too, and it works fine.
Maybe it's better to upsteam it instead. :)

No, I meant if it would make sense to just add something like

         {"xdpe11280", (kernel_ulong_t)&pmbus_info_one },

to drivers/hwmon/pmbus/pmbus.c.

You only really need a standalone driver if it does something
special, such as a workaround for some register access (like
the xdpe12284 driver), or if support for manufacturer specific
registers is desired or needed. That would, for example, be useful
if the xdpe11280 supports per-phase sensors.

Thanks,
Guenter

Hi,

I tested if the xdpe11280 can use the generic pmbus driver. Everything
works fine except it does only detect READ_TEMPERATURE_1 on page 0.
Looking at the pmbus_find_sensor_groups function it looks like only
some commands are probed on each page (READ_VOUT, READ_IOUT, and
READ_POUT) but not READ_TEMPERATURE_1.
The PMBus spec 1.3.1 tells us: "Each page may offer the full range of
PMBus commands available for each output or non-PMBus device." How
could we adapt the generic driver so that it is possible to probe
commands for each page?


The problem is the "may". Some chips don't implement multi-page
support especially for temperature sensors. So we'll have to go
with adding support to the existing driver.

Guenter

Furthermore, It would be great to add regulator and DT support. I
created a WIP branch on GitHub with a possible way to implement this:
https://github.com/9elements/linux/tree/upstreaming_pmbus_regulator_wip

What do you think?

Thanks,
Marcello





[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