Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <337fe0598837b35ee96773339e9cdc8345a7c16e.camel@sylv.io>
Subject: Re: [PATCH v2 2/3] hwmon (xdpe12284): Add support for xdpe11280
From:   sylv <sylv@sylv.io>
To:     Guenter Roeck <linux@roeck-us.net>,
        Jean Delvare <jdelvare@suse.com>,
        Jonathan Corbet <corbet@lwn.net>
Cc:     linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org,
        Patrick Rudolph <patrick.rudolph@9elements.com>,
        linux-doc@vger.kernel.org
Date:   Mon, 21 Feb 2022 13:13:15 +0100
In-Reply-To: <aa82f347-1b1a-8c40-b5cb-9efe292ae0ec@roeck-us.net>
References: <cover.1645108566.git.sylv@sylv.io>
         <6103836f1116a045a25919ae3e5c18dfdc485902.1645108566.git.sylv@sylv.io>
         <f1e13a4e-2e86-1d53-f34d-a04838e8e8f9@roeck-us.net>
         <246de83631dd5e165f95904d7f1628502f030e02.camel@sylv.io>
         <66d4ea0b-0377-c9ae-68eb-7303c32cf87e@roeck-us.net>
         <7a567823d62b6ed00cbdc2ce239dc4a69e48b411.camel@sylv.io>
         <aa82f347-1b1a-8c40-b5cb-9efe292ae0ec@roeck-us.net>
Content-Type: text/plain; charset="UTF-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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