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

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 6277 bytes --]

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.

Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]