Messages in this thread | | | Date | Fri, 11 Feb 2022 13:01:35 +0200 | From | Matti Vaittinen <> | Subject | Re: [PATCH V5 4/6] regulator: Add a regulator driver for the PM8008 PMIC |
| |
Hi Satya,
It's always nice to see new PMIC drivers :) I just one question after reading your patch - please ignore it if it has already been discussed before - for some reason this version was caught by my filters where the previous versions didn't. It means I do not know the full history.
On 2/8/22 16:52, Satya Priya wrote: > Qualcomm Technologies, Inc. PM8008 is an I2C controlled PMIC > containing 7 LDO regulators. Add a PM8008 regulator driver to > support PMIC regulator management via the regulator framework. > > Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> > ---
snip
> + > +static int pm8008_regulator_of_parse(struct device_node *node, > + const struct regulator_desc *desc, > + struct regulator_config *config) > +{ > + struct pm8008_regulator *pm8008_reg = config->driver_data; > + int rc; > + unsigned int reg; > + > + /* get slew rate */ > + rc = regmap_bulk_read(pm8008_reg->regmap, > + LDO_STEPPER_CTL_REG(pm8008_reg->base), ®, 1); > + if (rc < 0) { > + dev_err(pm8008_reg->dev, > + "%s: failed to read step rate configuration rc=%d\n", > + pm8008_reg->rdesc.name, rc); > + return rc; > + } > + reg &= STEP_RATE_MASK; > + pm8008_reg->step_rate = DEFAULT_VOLTAGE_STEPPER_RATE >> reg; > + > + return 0; > +}
I wonder why this is done in the of_parse_cb? Could this perhaps be done directly in probe - I don't think this is actually parsing the device_node properties, right?
Overall this looks pretty nice to me.
Best Regards -- Matti
-- The Linux Kernel guy at ROHM Semiconductors
Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND
~~ this year is the year of a signature writers block ~~ [unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |