Re: [EXT] Re: [PATCH v13 3/7] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator

[Lee Jones <lee@kernel.org>]

On Thu, 09 Jul 2026, Lakshay Piplani wrote:

> > > --- /dev/null
> > > +++ b/drivers/mfd/p3h2840.c
> > > @@ -0,0 +1,126 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright 2025-2026 NXP
> > 
> > Nit: '\n' here.
> > 
> 
> Sure, will remove the extra blank line.

In future, you only need to reply to review comments that you don't
agree with or require more information.  Snip the rest please.

> > > +static const struct regmap_config p3h2x4x_regmap_config = {
> > > +     .reg_bits = P3H2X4X_REG_BITS,
> > > +     .val_bits = P3H2X4X_VAL_BITS,
> > > +     .max_register = 0xFF,
> > > +};
> > > +
> > > +static int p3h2x4x_device_probe_i3c(struct i3c_device *i3cdev)
> > 
> > How many of these are comming down the pipe?
> > 
> > Might be worth expanding drivers/mfd/simple-mfd-i2c.c instead?
> 
> The P3H2x4x MFD supports both I2C and I3C transports. While the I2C path is conceptually similar to simple-mfd-i2c.c,
> the I3C path additionally requires device matching/probing, devm_regmap_init_i3c(), PID/manufacturer validation, and 
> retaining the parent struct i3c_device * for use by the hub child driver. As these requirements are specific to I3C 
> and not handled by the existing simple-MFD helpers, I believe a dedicated MFD driver is the more suitable approach.

Sure.  What I'm saying is; taking into consideration that this device
doesn't do anything useful (beyond generic device registration that
'simple' provides), if there are lots of I3C based devices coming, it
may be more prudent to create an I3C version.

However, it's probably too early to make that call.

> > >  enum i3c_dcr {
> > >       I3C_DCR_GENERIC_DEVICE = 0,
> > > +     I3C_DCR_HUB = 194,
> > 
> > What is this value?
> 
> 194 is the Device Characteristics Register (DCR) value assigned to I3C Hub devices by the MIPI I3C specification.

Is it described in base-10?  Registers are usually described in hex.

-- 
Lee Jones