From: Conor Dooley <conor@kernel.org>
To: Colin Huang <u8813345@gmail.com>
Cc: Colin Huang <colin.huang2@amd.com>,
Guenter Roeck <linux@roeck-us.net>, Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Magnus Damm <magnus.damm@gmail.com>,
Grant Peltier <grant.peltier.jg@renesas.com>,
linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: isl68137: Add Renesas RAA229639 and RAA229640
Date: Tue, 15 Sep 2026 18:04:31 +0100 [thread overview]
Message-ID: <20260915-recognize-activist-733d145c0ca7@spud> (raw)
In-Reply-To: <20260915-showcase-reorder-e3c4d45eff60@spud>
[-- Attachment #1: Type: text/plain, Size: 8007 bytes --]
On Tue, Sep 15, 2026 at 06:00:50PM +0100, Conor Dooley wrote:
> On Mon, Sep 14, 2026 at 08:06:25PM +0800, Colin Huang wrote:
> > Conor Dooley <conor@kernel.org> 於 2026年9月9日週三 上午12:57寫道：
> > >
> > > On Tue, Sep 08, 2026 at 02:31:40PM +0800, Colin Huang wrote:
> > > > Conor Dooley <conor@kernel.org> 於 2026年9月8日週二 上午1:01寫道：
> > > > >
> > > > > On Mon, Sep 07, 2026 at 03:03:29PM +0800, Colin Huang wrote:
> > > > > > From: Colin Huang <u8813345@gmail.com>
> > > > > >
> > > > > > Add Device Tree compatible strings for Renesas RAA229639 and
> > > > > > RAA229640 PMBus devices.
> > > > >
> > > > > Driver change suggests fallback compatibles could be used.
> > > > > Why aren't they? If they can be, add them. Otherwise, explain why not in
> > > > > your commit message.
> > > > >
> > > > > pw-bot: changes-requested
> > > > >
> > > > > Thanks,
> > > > > Conor.
> > > > >
> > > > Hi Conor
> > > > Thanks for the review.
> > > >
> > > > I didn't add a fallback compatible because I only have document for
> > > > RAA229639 and RAA229640
> > > > and could not verify full DT level compatibility with any existing
> > > > supported devices. While both devices
> > > > are handled by the existing raa_dmpvr2_2rail driver variant, I don't
> > > > have sufficient information to establish
> > > > a compatible fallback relationship.
> > >
> > > Given that the match data table looks like this:
> > > static const struct of_device_id isl68137_of_match[] = {
> > > { .compatible = "isil,isl68137", .data = (void *)raa_dmpvr1_2rail },
> > > { .compatible = "renesas,isl68220", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl68221", .data = (void *)raa_dmpvr2_3rail },
> > > { .compatible = "renesas,isl68222", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl68223", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl68224", .data = (void *)raa_dmpvr2_3rail },
> > > { .compatible = "renesas,isl68225", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl68226", .data = (void *)raa_dmpvr2_3rail },
> > > { .compatible = "renesas,isl68227", .data = (void *)raa_dmpvr2_1rail },
> > > { .compatible = "renesas,isl68229", .data = (void *)raa_dmpvr2_3rail },
> > > { .compatible = "renesas,isl68233", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl68239", .data = (void *)raa_dmpvr2_3rail },
> > >
> > > { .compatible = "renesas,isl69222", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl69223", .data = (void *)raa_dmpvr2_3rail },
> > > { .compatible = "renesas,isl69224", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl69225", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl69227", .data = (void *)raa_dmpvr2_3rail },
> > > { .compatible = "renesas,isl69228", .data = (void *)raa_dmpvr2_3rail },
> > > { .compatible = "renesas,isl69234", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl69236", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl69239", .data = (void *)raa_dmpvr2_3rail },
> > > { .compatible = "renesas,isl69242", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl69243", .data = (void *)raa_dmpvr2_1rail },
> > > { .compatible = "renesas,isl69247", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl69248", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl69254", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl69255", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl69256", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl69259", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "isil,isl69260", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,isl69268", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "isil,isl69269", .data = (void *)raa_dmpvr2_3rail },
> > > { .compatible = "renesas,isl69298", .data = (void *)raa_dmpvr2_2rail },
> > >
> > > { .compatible = "renesas,raa228000", .data = (void *)raa_dmpvr2_hv },
> > > { .compatible = "renesas,raa228004", .data = (void *)raa_dmpvr2_hv },
> > > { .compatible = "renesas,raa228006", .data = (void *)raa_dmpvr2_hv },
> > > { .compatible = "renesas,raa228228", .data = (void *)raa_dmpvr2_2rail_nontc },
> > > { .compatible = "renesas,raa228244", .data = (void *)raa_dmpvr2_2rail_nontc },
> > > { .compatible = "renesas,raa228246", .data = (void *)raa_dmpvr2_2rail_nontc },
> > > { .compatible = "renesas,raa229001", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,raa229004", .data = (void *)raa_dmpvr2_2rail },
> > > { .compatible = "renesas,raa229621", .data = (void *)raa_dmpvr2_2rail },
> > > { },
> > > };
> > >
> > > It's probably pretty safe to assume that a fallback would work here,
> > > given how many devices are served by the same data structures but
> > > maybe one of the Renesas folks on CC can confirm that for us.
> > > At the very least, you have documents for two devices and should be able
> > > to confirm if they're compatible with one another.
> > >
> > > Thanks,
> > > Conor.
> > >
> > Hi Conor,
> > Thanks for the review.
> > I found a very similar precedent here:
> > Link: https://lore.kernel.org/r/20260325090208.857-2-dawei.liu.jy@renesas.com
> > RAA228942 and RAA228943 use renesas,raa228244 as fallback compatible
> >
> > Therefore, I plan to followthe same approach:
> > RAA229639 and RAA229640 also use renesas,raa228244 as fallback compatible.
> >
> > ```
> > @@ -60,13 +60,13 @@ properties:
> > - renesas,raa229001
> > - renesas,raa229004
> > - renesas,raa229621
> >
> > - items:
> > - enum:
> > - renesas,raa228942
> > - renesas,raa228943
> > + - renesas,raa229639
> > + - renesas,raa229640
> > - const: renesas,raa228244
> >
> > reg:
> > ```
> > Does this look reasonable?
>
> It does, thanks for the update.
Actually no. The idea is right, but the specific fallback is not?
You added to the driver
diff --git a/drivers/hwmon/pmbus/isl68137.c b/drivers/hwmon/pmbus/isl68137.c
index 2f7f825bfb69..53b44775ba1e 100644
--- a/drivers/hwmon/pmbus/isl68137.c
+++ b/drivers/hwmon/pmbus/isl68137.c
@@ -456,6 +456,8 @@ static const struct i2c_device_id raa_dmpvr_id[] = {
{ .name = "raa229004", .driver_data = raa_dmpvr2_2rail },
{ .name = "raa229141", .driver_data = raa_dmpvr2_2rail_pmbus },
{ .name = "raa229621", .driver_data = raa_dmpvr2_2rail },
+ { .name = "raa229639", .driver_data = raa_dmpvr2_2rail },
+ { .name = "raa229640", .driver_data = raa_dmpvr2_2rail },
{ }
};
@@ -506,6 +508,8 @@ static const struct of_device_id isl68137_of_match[] = {
{ .compatible = "renesas,raa229001", .data = (void *)raa_dmpvr2_2rail },
{ .compatible = "renesas,raa229004", .data = (void *)raa_dmpvr2_2rail },
{ .compatible = "renesas,raa229621", .data = (void *)raa_dmpvr2_2rail },
+ { .compatible = "renesas,raa229639", .data = (void *)raa_dmpvr2_2rail },
+ { .compatible = "renesas,raa229640", .data = (void *)raa_dmpvr2_2rail },
but the raa228244 uses a different bit of match data:
> > > { .compatible = "renesas,raa228244", .data = (void *)raa_dmpvr2_2rail_nontc },
So you need something like
- items:
- enum:
- renesas,raa229639
- renesas,raa229640
- const: renesas,raa229001
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2026-09-15 17:04 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-07 7:03 [PATCH 0/2] Support " Colin Huang
2026-09-07 7:03 ` [PATCH 1/2] dt-bindings: hwmon: pmbus: isl68137: Add Renesas " Colin Huang
2026-09-07 17:00 ` Conor Dooley
2026-09-08 6:31 ` Colin Huang
2026-09-08 16:57 ` Conor Dooley
2026-09-14 12:06 ` Colin Huang
2026-09-15 17:00 ` Conor Dooley
2026-09-15 17:04 ` Conor Dooley [this message]
2026-09-17 3:57 ` Colin Huang
2026-09-07 7:03 ` [PATCH 2/2] hwmon: (pmbus/isl68137): " Colin Huang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260915-recognize-activist-733d145c0ca7@spud \
--to=conor@kernel.org \
--cc=colin.huang2@amd.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=geert+renesas@glider.be \
--cc=grant.peltier.jg@renesas.com \
--cc=krzk+dt@kernel.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=magnus.damm@gmail.com \
--cc=robh@kernel.org \
--cc=u8813345@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®