On Mon, 11 Mar 2024, Karel Balej wrote: > From: Karel Balej <balejk@xxxxxxxxx> > > Marvell 88PM886 is a PMIC which provides various functions such as > onkey, battery, charger and regulators. It is found for instance in the > samsung,coreprimevelte smartphone with which this was tested. Implement > basic support to allow for the use of regulators and onkey. > > Signed-off-by: Karel Balej <balejk@xxxxxxxxx> > --- > > Notes: > RFC v4: > - Use MFD_CELL_* macros. > - Address Lee's feedback: > - Do not define regmap_config.val_bits and .reg_bits. > - Drop everything regulator related except mfd_cell (regmap > initialization, IDs enum etc.). Drop pm886_initialize_subregmaps. > - Do not store regmap pointers as an array as there is now only one > regmap. Also drop the corresponding enum. > - Move regmap_config to the header as it is needed in the regulators > driver. > - pm886_chip.whoami -> chip_id > - Reword chip ID mismatch error message and print the ID as > hexadecimal. > - Fix includes in include/linux/88pm886.h. > - Drop the pm886_irq_number enum and define the (for the moment) only > IRQ explicitly. > - Have only one MFD cell for all regulators as they are now registered > all at once in the regulators driver. > - Reword commit message. > - Make device table static and remove comma after the sentinel to signal > that nothing should come after it. > RFC v3: > - Drop onkey cell .of_compatible. > - Rename LDO page offset and regmap to REGULATORS. > RFC v2: > - Remove some abstraction. > - Sort includes alphabetically and add linux/of.h. > - Depend on OF, remove of_match_ptr and add MODULE_DEVICE_TABLE. > - Use more temporaries and break long lines. > - Do not initialize ret in probe. > - Use the wakeup-source DT property. > - Rename ret to err. > - Address Lee's comments: > - Drop patched in presets for base regmap and related defines. > - Use full sentences in comments. > - Remove IRQ comment. > - Define regmap_config member values. > - Rename data to sys_off_data. > - Add _PMIC suffix to Kconfig. > - Use dev_err_probe. > - Do not store irq_data. > - s/WHOAMI/CHIP_ID > - Drop LINUX part of include guard name. > - Merge in the regulator series modifications in order to have more > devices and modify the commit message accordingly. Changes with > respect to the original regulator series patches: > - ret -> err > - Add temporary for dev in pm88x_initialize_subregmaps. > - Drop of_compatible for the regulators. > - Do not duplicate LDO regmap for bucks. > - Rewrite commit message. > > drivers/mfd/88pm886.c | 149 ++++++++++++++++++++++++++++++++++++ > drivers/mfd/Kconfig | 12 +++ > drivers/mfd/Makefile | 1 + > include/linux/mfd/88pm886.h | 38 +++++++++ > 4 files changed, 200 insertions(+) > create mode 100644 drivers/mfd/88pm886.c > create mode 100644 include/linux/mfd/88pm886.h Looks mostly okay. > diff --git a/include/linux/mfd/88pm886.h b/include/linux/mfd/88pm886.h > new file mode 100644 > index 000000000000..a5e6524bb19d > --- /dev/null > +++ b/include/linux/mfd/88pm886.h > @@ -0,0 +1,38 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +#ifndef __MFD_88PM886_H > +#define __MFD_88PM886_H > + > +#include <linux/i2c.h> > +#include <linux/regmap.h> > + > +#define PM886_A1_CHIP_ID 0xa1 > + > +#define PM886_REGMAP_CONF_MAX_REG 0xfe > + > +#define PM886_REG_ID 0x00 > + > +#define PM886_REG_STATUS1 0x01 > +#define PM886_ONKEY_STS1 BIT(0) > + > +#define PM886_REG_MISC_CONFIG1 0x14 > +#define PM886_SW_PDOWN BIT(5) > + > +#define PM886_REG_MISC_CONFIG2 0x15 > +#define PM886_INT_INV BIT(0) > +#define PM886_INT_CLEAR BIT(1) > +#define PM886_INT_RC 0x00 > +#define PM886_INT_WC BIT(1) > +#define PM886_INT_MASK_MODE BIT(2) > + > +struct pm886_chip { > + struct i2c_client *client; > + unsigned int chip_id; > + struct regmap *regmap; > +}; > + > +static const struct regmap_config pm886_i2c_regmap = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = PM886_REGMAP_CONF_MAX_REG, > +}; Why is this in here? > +#endif /* __MFD_88PM886_H */ What would you like me to do with this RFC patch? -- Lee Jones [李琼斯]