Hello Lee, On Tue, 2021-02-09 at 14:55 +0000, Lee Jones wrote: > On Fri, 22 Jan 2021, Matti Vaittinen wrote: > > > Add core support for ROHM BD9576MUF and BD9573MUF PMICs which are > > mainly used to power the R-Car series processors. > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx> > > --- > > + > > +static struct mfd_cell bd9573_mfd_cells[] = { > > + { .name = "bd9573-pmic", }, > > + { .name = "bd9576-wdt", }, > > +}; > > + > > +static struct mfd_cell bd9576_mfd_cells[] = { > > + { .name = "bd9576-pmic", }, > > + { .name = "bd9576-wdt", }, > > +}; > > What is a PMIC in this context? > > To me a PMIC is a bunch of devices. What is this probing? I agree. PMIC is the IC as a whole. This name was not the best one. > > Maybe this is *-regulator? That would be more descriptive and I can change this. However, it means I need to change the already applied regulator part too. Furthermore, all other ROHM PMIC drivers I've written use the <part-name>-pmic for regulators and so does a few other drivers at least for ICs from Maxim, Samsung and TI. That's why I don't think the <partname>-pmic is that confusing. If it was my decision, I would stick with the pmic for the sake of the consistency. + > > + ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, > > cells, > > This nomenclature is confusing. > > cells and num_cells would clear it up. I can change it. + > > +#define BD957X_MAX_REGISTER 0x61 > > Nit: Can you tab these out for improved readability please? Sure, no problem. Thanks for the review! Best Regards --Matti -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. "non cogito me" dixit Rene Descarte, deinde evanescavit (Thanks for the translation Simon)