Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: isl68137: Add Renesas RAA229639 and RAA229640

[Colin Huang]

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?

Best regards,
Colin Huang

> >
> >   I will clarify this in commit message in v2.
> >
> > Thanks.
> > Colin
> > > >
> > > > Signed-off-by: Colin Huang <u8813345@xxxxxxxxx>
> > > > ---
> > > >  Documentation/devicetree/bindings/hwmon/pmbus/isil,isl68137.yaml | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/isil,isl68137.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/isil,isl68137.yaml
> > > > index 8216cdf758d8..3537c7ddfb09 100644
> > > > --- a/Documentation/devicetree/bindings/hwmon/pmbus/isil,isl68137.yaml
> > > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/isil,isl68137.yaml
> > > > @@ -60,6 +60,8 @@ properties:
> > > >            - renesas,raa229001
> > > >            - renesas,raa229004
> > > >            - renesas,raa229621
> > > > +          - renesas,raa229639
> > > > +          - renesas,raa229640
> > > >
> > > >        - items:
> > > >            - enum:
> > > >
> > > > --
> > > > 2.34.1
> > > >