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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <337fe0598837b35ee96773339e9cdc8345a7c16e.camel@sylv.io>
Date:   Mon, 21 Feb 2022 13:13:15 +0100
From:   sylv <sylv@...v.io>
To:     Guenter Roeck <linux@...ck-us.net>,
        Jean Delvare <jdelvare@...e.com>,
        Jonathan Corbet <corbet@....net>
Cc:     linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org,
        Patrick Rudolph <patrick.rudolph@...ements.com>,
        linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 2/3] hwmon (xdpe12284): Add support for xdpe11280

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?

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