On 22/08/2023 09:51, Jiansheng Wu wrote: > There are three PMICs (UMP9620/21/22) on Unisoc's UMS9621 chip. > UMP9620 is a master PMIC, the others are slave ones. Slave PMICs > don't have irq functions, which is different from master device, > such as SC27xx series and UMP9620, etc. > > Signed-off-by: Jiansheng Wu <jiansheng.wu@xxxxxxxxxx> > --- > drivers/mfd/sprd-sc27xx-spi.c | 104 +++++++++++++++++++++++++--------- > 1 file changed, 77 insertions(+), 27 deletions(-) > > diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c > index d21f32cc784d..aa91301568a9 100644 > --- a/drivers/mfd/sprd-sc27xx-spi.c > +++ b/drivers/mfd/sprd-sc27xx-spi.c > @@ -24,6 +24,10 @@ > #define SPRD_SC2731_IRQ_BASE 0x140 > #define SPRD_SC2731_IRQ_NUMS 16 > #define SPRD_SC2731_CHG_DET 0xedc > +#define SPRD_UMP9620_IRQ_BASE 0x80 > +#define SPRD_UMP9620_IRQ_NUMS 11 > +#define SPRD_UMP9621_SLAVE_ID 0x8000 > +#define SPRD_UMP9622_SLAVE_ID 0xc000 > > /* PMIC charger detection definition */ > #define SPRD_PMIC_CHG_DET_DELAY_US 200000 > @@ -45,6 +49,7 @@ struct sprd_pmic { > }; > > struct sprd_pmic_data { > + u32 slave_id; See coding style about such wording. You know, it is not 2010 anymore... > u32 irq_base; > u32 num_irqs; > u32 charger_det; > @@ -67,6 +72,19 @@ static const struct sprd_pmic_data sc2731_data = { > .charger_det = SPRD_SC2731_CHG_DET, > }; > > +static const struct sprd_pmic_data ump9620_data = { > + .irq_base = SPRD_UMP9620_IRQ_BASE, > + .num_irqs = SPRD_UMP9620_IRQ_NUMS, > +}; > + > +static const struct sprd_pmic_data ump9621_data = { > + .slave_id = SPRD_UMP9621_SLAVE_ID, > +}; > + > +static const struct sprd_pmic_data ump9622_data = { > + .slave_id = SPRD_UMP9622_SLAVE_ID, > +}; > + > enum usb_charger_type sprd_pmic_detect_charger_type(struct device *dev) > { > struct spi_device *spi = to_spi_device(dev); > @@ -108,8 +126,27 @@ static int sprd_pmic_spi_write(void *context, const void *data, size_t count) > { > struct device *dev = context; > struct spi_device *spi = to_spi_device(dev); > + const struct sprd_pmic_data *pdata; > + int ret; > + u32 *pmdata; > + > + if (!pdata->slave_id) { > + ret = spi_write(spi, data, count); > + } else { > + pdata = ((struct sprd_pmic *)spi_get_drvdata(spi))->pdata; > + > + pmdata = kzalloc(count, GFP_KERNEL); > + if (!pmdata) > + return -ENOMEM; > + memcpy(pmdata, data, count); > + *pmdata += pdata->slave_id; > + ret = spi_write(spi, (const void *)pmdata, count); > + kfree(pmdata); > + } > + if (ret) > + pr_err("pmic mfd write failed!\n"); > > - return spi_write(spi, data, count); > + return ret; > } > > static int sprd_pmic_spi_read(void *context, > @@ -118,6 +155,7 @@ static int sprd_pmic_spi_read(void *context, > { > struct device *dev = context; > struct spi_device *spi = to_spi_device(dev); > + const struct sprd_pmic_data *pdata; > u32 rx_buf[2] = { 0 }; > int ret; > > @@ -125,11 +163,16 @@ static int sprd_pmic_spi_read(void *context, > if (reg_size != sizeof(u32) || val_size != sizeof(u32)) > return -EINVAL; > > + pdata = ((struct sprd_pmic *)spi_get_drvdata(spi))->pdata; > /* Copy address to read from into first element of SPI buffer. */ > memcpy(rx_buf, reg, sizeof(u32)); > + if (!pdata->slave_id) > + rx_buf[0] += pdata->slave_id; > ret = spi_read(spi, rx_buf, 1); > - if (ret < 0) > + if (ret < 0) { > + pr_err("pmic mfd read failed!\n"); Eh... drivers do not use pr_ but dev_. Which you can easily see within this driver, so before posting changes please look at the driver and its style, then learn from it and use similar coding convention. > return ret; > + } > > memcpy(val, rx_buf, val_size); > return 0; ... > > ret = devm_of_platform_populate(&spi->dev); > @@ -240,6 +284,9 @@ static DEFINE_SIMPLE_DEV_PM_OPS(sprd_pmic_pm_ops, > static const struct of_device_id sprd_pmic_match[] = { > { .compatible = "sprd,sc2730", .data = &sc2730_data }, > { .compatible = "sprd,sc2731", .data = &sc2731_data }, > + { .compatible = "sprd,ump9620", .data = &ump9620_data }, > + { .compatible = "sprd,ump9621", .data = &ump9621_data }, > + { .compatible = "sprd,ump9622", .data = &ump9622_data }, > {}, > }; > MODULE_DEVICE_TABLE(of, sprd_pmic_match); > @@ -247,6 +294,9 @@ MODULE_DEVICE_TABLE(of, sprd_pmic_match); > static const struct spi_device_id sprd_pmic_spi_ids[] = { > { .name = "sc2730", .driver_data = (unsigned long)&sc2730_data }, > { .name = "sc2731", .driver_data = (unsigned long)&sc2731_data }, > + { .name = "ump9620", .driver_data = (unsigned long)&ump9620_data }, > + { .name = "ump9621", .driver_data = (unsigned long)&ump9621_data }, > + { .name = "ump9622", .driver_data = (unsigned long)&ump9622_data }, So here you sneaked new compatibles... Sorry, adding new compatibles is not the same as converting old ones. Entirely separate patch. Best regards, Krzysztof