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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <8656b67c-e093-0a18-9c5f-02f5b4264bb2@roeck-us.net>
Date:   Fri, 25 Feb 2022 08:39:40 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     sylv <sylv@...v.io>, 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 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
>