Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: isl68137: Add Renesas RAA229639 and RAA229640
From: Colin Huang
Date: Thu Sep 17 2026 - 00:00:04 EST
Conor Dooley <conor@xxxxxxxxxx> 於 2026年9月16日週三 上午1:04寫道：
>
> 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@xxxxxxxxxx> 於 2026年9月9日週三 上午12:57寫道：
> > > >
> > > > On Tue, Sep 08, 2026 at 02:31:40PM +0800, Colin Huang wrote:
> > > > > Conor Dooley <conor@xxxxxxxxxx> 於 2026年9月8日週二 上午1:01寫道：
> > > > > >
> > > > > > On Mon, Sep 07, 2026 at 03:03:29PM +0800, Colin Huang wrote:
> > > > > > > From: Colin Huang <u8813345@xxxxxxxxx>
> > > > > > >
> > > > > > > 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@xxxxxxxxxxx
> > > 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
Thanks Conor,
I miss some information to you.
I will modify the compatible to following, because they don't
support temperature 3.
+ { .compatible = "renesas,raa229639", .data = (void
*)raa_dmpvr2_2rail_nontc },
+ { .compatible = "renesas,raa229640", .data = (void
*)raa_dmpvr2_2rail_nontc },
so.. fallback will be like following.
- items:
- enum:
- renesas,raa228942
- renesas,raa228943
- renesas,raa229639
- renesas,raa229640
- const: renesas,raa228244
Best regards,
Colin Huang