On Mon, 23 Jan 2023, Sebastian Reichel wrote: > Hi, > > On Fri, Jan 20, 2023 at 04:59:21PM +0000, Lee Jones wrote: > > On Mon, 09 Jan 2023, Sebastian Reichel wrote: > > > > > Add support for SPI connected rk806, which is used by the RK3588 > > > evaluation boards. The PMIC is advertised to support I2C and SPI, > > > but the evaluation boards all use SPI. Thus only SPI support is > > > added here. > > > > > > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> > > > --- > > > drivers/mfd/Kconfig | 14 ++ > > > drivers/mfd/Makefile | 1 + > > > drivers/mfd/rk8xx-core.c | 67 ++++++- > > > drivers/mfd/rk8xx-spi.c | 122 ++++++++++++ > > > include/linux/mfd/rk808.h | 409 ++++++++++++++++++++++++++++++++++++++ > > > 5 files changed, 611 insertions(+), 2 deletions(-) > > > create mode 100644 drivers/mfd/rk8xx-spi.c > > > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > > index 692e38283bda..13582ea5cb44 100644 > > > --- a/drivers/mfd/Kconfig > > > +++ b/drivers/mfd/Kconfig > > > @@ -1222,6 +1222,20 @@ config MFD_RK8XX_I2C > > > through I2C interface. The device supports multiple sub-devices > > > including interrupts, RTC, LDO & DCDC regulators, and onkey. > > > > > > +config MFD_RK8XX_SPI > > > + tristate "Rockchip RK806 Power Management Chip" > > > + depends on SPI && OF > > > + select MFD_CORE > > > + select REGMAP_SPI > > > + select REGMAP_IRQ > > > + select MFD_RK8XX > > > + help > > > + If you say yes here you get support for the RK806 Power Management > > > + chip. > > > + This driver provides common support for accessing the device > > > + through an SPI interface. The device supports multiple sub-devices > > > + including interrupts, LDO & DCDC regulators, and power on-key. > > > + > > > config MFD_RN5T618 > > > tristate "Ricoh RN5T567/618 PMIC" > > > depends on I2C > > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > > > index f65ef1bd0810..a88f27cd837b 100644 > > > --- a/drivers/mfd/Makefile > > > +++ b/drivers/mfd/Makefile > > > @@ -222,6 +222,7 @@ obj-$(CONFIG_MFD_NTXEC) += ntxec.o > > > obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o > > > obj-$(CONFIG_MFD_RK8XX) += rk8xx-core.o > > > obj-$(CONFIG_MFD_RK8XX_I2C) += rk8xx-i2c.o > > > +obj-$(CONFIG_MFD_RK8XX_SPI) += rk8xx-spi.o > > > obj-$(CONFIG_MFD_RN5T618) += rn5t618.o > > > obj-$(CONFIG_MFD_SEC_CORE) += sec-core.o sec-irq.o > > > obj-$(CONFIG_MFD_SYSCON) += syscon.o > > > diff --git a/drivers/mfd/rk8xx-core.c b/drivers/mfd/rk8xx-core.c > > > index c52f5fa1a4da..289f4c1f30c2 100644 > > > --- a/drivers/mfd/rk8xx-core.c > > > +++ b/drivers/mfd/rk8xx-core.c > > > @@ -37,6 +37,11 @@ static const struct resource rk805_key_resources[] = { > > > DEFINE_RES_IRQ(RK805_IRQ_PWRON_FALL), > > > }; > > > > > > +static struct resource rk806_pwrkey_resources[] = { > > > + DEFINE_RES_IRQ(RK806_IRQ_PWRON_FALL), > > > + DEFINE_RES_IRQ(RK806_IRQ_PWRON_RISE), > > > +}; > > > + > > > static const struct resource rk817_pwrkey_resources[] = { > > > DEFINE_RES_IRQ(RK817_IRQ_PWRON_RISE), > > > DEFINE_RES_IRQ(RK817_IRQ_PWRON_FALL), > > > @@ -64,6 +69,16 @@ static const struct mfd_cell rk805s[] = { > > > }, > > > }; > > > > > > +static const struct mfd_cell rk806s[] = { > > > + { .name = "rk805-pinctrl", }, > > > + { .name = "rk808-regulator", }, > > > + { > > > + .name = "rk805-pwrkey", > > > + .num_resources = ARRAY_SIZE(rk806_pwrkey_resources), > > > + .resources = &rk806_pwrkey_resources[0], > > > > My OCD-sense is tingling. > > > > Could you please add the resources *before* the num_resources please? > > > > Also: '.resources = rk806_pwrkey_resources' is fine > > Ack. > > > > > > + }, > > > +}; > > > + > > > static const struct mfd_cell rk808s[] = { > > > { .name = "rk808-clkout", .id = PLATFORM_DEVID_NONE, }, > > > { .name = "rk808-regulator", .id = PLATFORM_DEVID_NONE, }, > > > @@ -122,6 +137,12 @@ static const struct rk808_reg_data rk805_pre_init_reg[] = { > > > {RK805_THERMAL_REG, TEMP_HOTDIE_MSK, TEMP115C}, > > > }; > > > > > > +static const struct rk808_reg_data rk806_pre_init_reg[] = { > > > + { RK806_GPIO_INT_CONFIG, RK806_INT_POL_MSK, RK806_INT_POL_L }, > > > + { RK806_SYS_CFG3, RK806_SLAVE_RESTART_FUN_MSK, RK806_SLAVE_RESTART_FUN_EN }, > > > + { RK806_SYS_OPTION, RK806_SYS_ENB2_2M_MSK, RK806_SYS_ENB2_2M_EN }, > > > +}; > > > + > > > static const struct rk808_reg_data rk808_pre_init_reg[] = { > > > { RK808_BUCK3_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_150MA }, > > > { RK808_BUCK4_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_200MA }, > > > @@ -272,6 +293,27 @@ static const struct regmap_irq rk805_irqs[] = { > > > }, > > > }; > > > > > > +static const struct regmap_irq rk806_irqs[] = { > > > + /* INT_STS0 IRQs */ > > > + REGMAP_IRQ_REG(RK806_IRQ_PWRON_FALL, 0, RK806_INT_STS_PWRON_FALL), > > > + REGMAP_IRQ_REG(RK806_IRQ_PWRON_RISE, 0, RK806_INT_STS_PWRON_RISE), > > > + REGMAP_IRQ_REG(RK806_IRQ_PWRON, 0, RK806_INT_STS_PWRON), > > > + REGMAP_IRQ_REG(RK806_IRQ_PWRON_LP, 0, RK806_INT_STS_PWRON_LP), > > > + REGMAP_IRQ_REG(RK806_IRQ_HOTDIE, 0, RK806_INT_STS_HOTDIE), > > > + REGMAP_IRQ_REG(RK806_IRQ_VDC_RISE, 0, RK806_INT_STS_VDC_RISE), > > > + REGMAP_IRQ_REG(RK806_IRQ_VDC_FALL, 0, RK806_INT_STS_VDC_FALL), > > > + REGMAP_IRQ_REG(RK806_IRQ_VB_LO, 0, RK806_INT_STS_VB_LO), > > > + /* INT_STS1 IRQs */ > > > + REGMAP_IRQ_REG(RK806_IRQ_REV0, 1, RK806_INT_STS_REV0), > > > + REGMAP_IRQ_REG(RK806_IRQ_REV1, 1, RK806_INT_STS_REV1), > > > + REGMAP_IRQ_REG(RK806_IRQ_REV2, 1, RK806_INT_STS_REV2), > > > + REGMAP_IRQ_REG(RK806_IRQ_CRC_ERROR, 1, RK806_INT_STS_CRC_ERROR), > > > + REGMAP_IRQ_REG(RK806_IRQ_SLP3_GPIO, 1, RK806_INT_STS_SLP3_GPIO), > > > + REGMAP_IRQ_REG(RK806_IRQ_SLP2_GPIO, 1, RK806_INT_STS_SLP2_GPIO), > > > + REGMAP_IRQ_REG(RK806_IRQ_SLP1_GPIO, 1, RK806_INT_STS_SLP1_GPIO), > > > + REGMAP_IRQ_REG(RK806_IRQ_WDT, 1, RK806_INT_STS_WDT), > > > +}; > > > + > > > static const struct regmap_irq rk808_irqs[] = { > > > /* INT_STS */ > > > [RK808_IRQ_VOUT_LO] = { > > > @@ -422,6 +464,18 @@ static struct regmap_irq_chip rk805_irq_chip = { > > > .init_ack_masked = true, > > > }; > > > > > > +static struct regmap_irq_chip rk806_irq_chip = { > > > + .name = "rk806", > > > + .irqs = rk806_irqs, > > > + .num_irqs = ARRAY_SIZE(rk806_irqs), > > > + .num_regs = 2, > > > + .irq_reg_stride = 2, > > > + .mask_base = RK806_INT_MSK0, > > > + .status_base = RK806_INT_STS0, > > > + .ack_base = RK806_INT_STS0, > > > + .init_ack_masked = true, > > > +}; > > > + > > > static const struct regmap_irq_chip rk808_irq_chip = { > > > .name = "rk808", > > > .irqs = rk808_irqs, > > > @@ -548,6 +602,7 @@ int rk8xx_probe(struct device *dev, int variant, unsigned int irq, struct regmap > > > struct rk808 *rk808; > > > const struct rk808_reg_data *pre_init_reg; > > > const struct mfd_cell *cells; > > > + bool dual_support = false; > > > int nr_pre_init_regs; > > > int nr_cells; > > > int ret; > > > @@ -569,6 +624,14 @@ int rk8xx_probe(struct device *dev, int variant, unsigned int irq, struct regmap > > > cells = rk805s; > > > nr_cells = ARRAY_SIZE(rk805s); > > > break; > > > + case RK806_ID: > > > + rk808->regmap_irq_chip = &rk806_irq_chip; > > > + pre_init_reg = rk806_pre_init_reg; > > > + nr_pre_init_regs = ARRAY_SIZE(rk806_pre_init_reg); > > > + cells = rk806s; > > > + nr_cells = ARRAY_SIZE(rk806s); > > > + dual_support = true; > > > + break; > > > case RK808_ID: > > > rk808->regmap_irq_chip = &rk808_irq_chip; > > > pre_init_reg = rk808_pre_init_reg; > > > @@ -602,7 +665,7 @@ int rk8xx_probe(struct device *dev, int variant, unsigned int irq, struct regmap > > > return dev_err_probe(dev, -EINVAL, "No interrupt support, no core IRQ\n"); > > > > > > ret = devm_regmap_add_irq_chip(dev, rk808->regmap, irq, > > > - IRQF_ONESHOT, -1, > > > + IRQF_ONESHOT | (dual_support ? IRQF_SHARED : 0), -1, > > > > Why not 'dual_support = IRQF_SHARED', then | it regardless? > > Ack > > > > > > rk808->regmap_irq_chip, &rk808->irq_data); > > > if (ret) > > > return dev_err_probe(dev, ret, "Failed to add irq_chip\n"); > > > @@ -617,7 +680,7 @@ int rk8xx_probe(struct device *dev, int variant, unsigned int irq, struct regmap > > > pre_init_reg[i].addr); > > > } > > > > > > - ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, > > > + ret = devm_mfd_add_devices(dev, dual_support ? PLATFORM_DEVID_AUTO : PLATFORM_DEVID_NONE, > > > > Any reason why you can't use AUTO all the time? > > That would change device names (and thus sysfs paths) for already > supported devices. DEVID_AUTO appends a ".<number>" suffix compared > to DEVID_NONE. How about using cell->id instead of doing this little dance? -- Lee Jones [李琼斯]