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 於 2026年9月9日週三 上午12:57寫道： > > > > > > On Tue, Sep 08, 2026 at 02:31:40PM +0800, Colin Huang wrote: > > > > Conor Dooley 於 2026年9月8日週二 上午1:01寫道： > > > > > > > > > > On Mon, Sep 07, 2026 at 03:03:29PM +0800, Colin Huang wrote: > > > > > > From: Colin Huang > > > > > > > > > > > > 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