[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250320165220.GB1750245@google.com>
Date: Thu, 20 Mar 2025 16:52:20 +0000
From: Lee Jones <lee@...nel.org>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/14] mfd: rohm-bd96801: Add chip info
On Thu, 13 Mar 2025, Matti Vaittinen wrote:
> Prepare for adding support for BD96802 which is very similar to BD96801.
> Separate chip specific data into own structure which can be picked to be
> used by the device-tree.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@...il.com>
> ---
> drivers/mfd/rohm-bd96801.c | 83 ++++++++++++++++++++++++++------------
> 1 file changed, 57 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/mfd/rohm-bd96801.c b/drivers/mfd/rohm-bd96801.c
> index 60ec8db790a7..1232f571e4b1 100644
> --- a/drivers/mfd/rohm-bd96801.c
> +++ b/drivers/mfd/rohm-bd96801.c
> @@ -40,7 +40,21 @@
> #include <linux/mfd/rohm-bd96801.h>
> #include <linux/mfd/rohm-generic.h>
>
> -static const struct resource regulator_errb_irqs[] = {
> +struct bd968xx_chip_data {
> + const struct resource *errb_irqs;
> + const struct resource *intb_irqs;
> + int num_errb_irqs;
> + int num_intb_irqs;
> + const struct regmap_irq_chip *errb_irq_chip;
> + const struct regmap_irq_chip *intb_irq_chip;
> + const struct regmap_config *regmap_config;
> + struct mfd_cell *cells;
We're not passing MFD data through OF to be fed back through MFD APIs.
It's generally considered better to device_get_match_data() on an enum,
then populate MFD cells using that as a differentiator.
git grep compatible -- drivers/mfd | grep data
> + int num_cells;
> + int unlock_reg;
> + int unlock_val;
> +};
> +
> +static const struct resource bd96801_reg_errb_irqs[] = {
> DEFINE_RES_IRQ_NAMED(BD96801_OTP_ERR_STAT, "bd96801-otp-err"),
> DEFINE_RES_IRQ_NAMED(BD96801_DBIST_ERR_STAT, "bd96801-dbist-err"),
> DEFINE_RES_IRQ_NAMED(BD96801_EEP_ERR_STAT, "bd96801-eep-err"),
> @@ -98,7 +112,7 @@ static const struct resource regulator_errb_irqs[] = {
> DEFINE_RES_IRQ_NAMED(BD96801_LDO7_SHDN_ERR_STAT, "bd96801-ldo7-shdn-err"),
> };
>
> -static const struct resource regulator_intb_irqs[] = {
> +static const struct resource bd96801_reg_intb_irqs[] = {
> DEFINE_RES_IRQ_NAMED(BD96801_TW_STAT, "bd96801-core-thermal"),
>
> DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OCPH_STAT, "bd96801-buck1-overcurr-h"),
> @@ -345,18 +359,37 @@ static const struct regmap_config bd96801_regmap_config = {
> .cache_type = REGCACHE_MAPLE,
> };
>
> +static const struct bd968xx_chip_data bd96801_chip_data = {
Just call it 'struct bd968xx' then below instead of cd, use ddata.
git grep "cc =" -- drivers/mfd
VS
git grep "ddata =" -- drivers/mfd
Conforrrrrrmmm ... =;-)
> + .errb_irqs = bd96801_reg_errb_irqs,
> + .intb_irqs = bd96801_reg_intb_irqs,
> + .num_errb_irqs = ARRAY_SIZE(bd96801_reg_errb_irqs),
> + .num_intb_irqs = ARRAY_SIZE(bd96801_reg_intb_irqs),
> + .errb_irq_chip = &bd96801_irq_chip_errb,
> + .intb_irq_chip = &bd96801_irq_chip_intb,
> + .regmap_config = &bd96801_regmap_config,
> + .cells = bd96801_cells,
> + .num_cells = ARRAY_SIZE(bd96801_cells),
> + .unlock_reg = BD96801_LOCK_REG,
> + .unlock_val = BD96801_UNLOCK,
> +};
> +
> static int bd96801_i2c_probe(struct i2c_client *i2c)
> {
> struct regmap_irq_chip_data *intb_irq_data, *errb_irq_data;
> struct irq_domain *intb_domain, *errb_domain;
> + static const struct bd968xx_chip_data *cd;
> const struct fwnode_handle *fwnode;
> struct resource *regulator_res;
> struct resource wdg_irq;
> struct regmap *regmap;
> - int intb_irq, errb_irq, num_intb, num_errb = 0;
> + int intb_irq, errb_irq, num_errb = 0;
> int num_regu_irqs, wdg_irq_no;
> int i, ret;
>
> + cd = device_get_match_data(&i2c->dev);
> + if (!cd)
> + return -ENODEV;
> +
> fwnode = dev_fwnode(&i2c->dev);
> if (!fwnode)
> return dev_err_probe(&i2c->dev, -EINVAL, "Failed to find fwnode\n");
> @@ -365,34 +398,32 @@ static int bd96801_i2c_probe(struct i2c_client *i2c)
> if (intb_irq < 0)
> return dev_err_probe(&i2c->dev, intb_irq, "INTB IRQ not configured\n");
>
> - num_intb = ARRAY_SIZE(regulator_intb_irqs);
> -
> /* ERRB may be omitted if processor is powered by the PMIC */
> errb_irq = fwnode_irq_get_byname(fwnode, "errb");
> - if (errb_irq < 0)
> - errb_irq = 0;
> + if (errb_irq == -EPROBE_DEFER)
> + return errb_irq;
>
> - if (errb_irq)
> - num_errb = ARRAY_SIZE(regulator_errb_irqs);
> + if (errb_irq > 0)
> + num_errb = cd->num_errb_irqs;
>
> - num_regu_irqs = num_intb + num_errb;
> + num_regu_irqs = cd->num_intb_irqs + num_errb;
>
> regulator_res = devm_kcalloc(&i2c->dev, num_regu_irqs,
> sizeof(*regulator_res), GFP_KERNEL);
> if (!regulator_res)
> return -ENOMEM;
>
> - regmap = devm_regmap_init_i2c(i2c, &bd96801_regmap_config);
> + regmap = devm_regmap_init_i2c(i2c, cd->regmap_config);
> if (IS_ERR(regmap))
> return dev_err_probe(&i2c->dev, PTR_ERR(regmap),
> "Regmap initialization failed\n");
>
> - ret = regmap_write(regmap, BD96801_LOCK_REG, BD96801_UNLOCK);
> + ret = regmap_write(regmap, cd->unlock_reg, cd->unlock_val);
> if (ret)
> return dev_err_probe(&i2c->dev, ret, "Failed to unlock PMIC\n");
>
> ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, intb_irq,
> - IRQF_ONESHOT, 0, &bd96801_irq_chip_intb,
> + IRQF_ONESHOT, 0, cd->intb_irq_chip,
> &intb_irq_data);
> if (ret)
> return dev_err_probe(&i2c->dev, ret, "Failed to add INTB IRQ chip\n");
> @@ -404,24 +435,25 @@ static int bd96801_i2c_probe(struct i2c_client *i2c)
> * has two domains so we do IRQ mapping here and provide the
> * already mapped IRQ numbers to sub-devices.
> */
> - for (i = 0; i < num_intb; i++) {
> + for (i = 0; i < cd->num_intb_irqs; i++) {
> struct resource *res = ®ulator_res[i];
>
> - *res = regulator_intb_irqs[i];
> + *res = cd->intb_irqs[i];
> res->start = res->end = irq_create_mapping(intb_domain,
> res->start);
> }
>
> wdg_irq_no = irq_create_mapping(intb_domain, BD96801_WDT_ERR_STAT);
> wdg_irq = DEFINE_RES_IRQ_NAMED(wdg_irq_no, "bd96801-wdg");
> - bd96801_cells[WDG_CELL].resources = &wdg_irq;
> - bd96801_cells[WDG_CELL].num_resources = 1;
> +
> + cd->cells[WDG_CELL].resources = &wdg_irq;
> + cd->cells[WDG_CELL].num_resources = 1;
>
> if (!num_errb)
> goto skip_errb;
>
> ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, errb_irq, IRQF_ONESHOT,
> - 0, &bd96801_irq_chip_errb, &errb_irq_data);
> + 0, cd->errb_irq_chip, &errb_irq_data);
> if (ret)
> return dev_err_probe(&i2c->dev, ret,
> "Failed to add ERRB IRQ chip\n");
> @@ -429,18 +461,17 @@ static int bd96801_i2c_probe(struct i2c_client *i2c)
> errb_domain = regmap_irq_get_domain(errb_irq_data);
>
> for (i = 0; i < num_errb; i++) {
> - struct resource *res = ®ulator_res[num_intb + i];
> + struct resource *res = ®ulator_res[cd->num_intb_irqs + i];
>
> - *res = regulator_errb_irqs[i];
> + *res = cd->errb_irqs[i];
> res->start = res->end = irq_create_mapping(errb_domain, res->start);
> }
>
> skip_errb:
> - bd96801_cells[REGULATOR_CELL].resources = regulator_res;
> - bd96801_cells[REGULATOR_CELL].num_resources = num_regu_irqs;
> -
> - ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, bd96801_cells,
> - ARRAY_SIZE(bd96801_cells), NULL, 0, NULL);
> + cd->cells[REGULATOR_CELL].resources = regulator_res;
> + cd->cells[REGULATOR_CELL].num_resources = num_regu_irqs;
> + ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO,
> + cd->cells, cd->num_cells, NULL, 0, NULL);
> if (ret)
> dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
>
> @@ -448,7 +479,7 @@ static int bd96801_i2c_probe(struct i2c_client *i2c)
> }
>
> static const struct of_device_id bd96801_of_match[] = {
> - { .compatible = "rohm,bd96801", },
> + { .compatible = "rohm,bd96801", .data = &bd96801_chip_data, },
> { }
> };
> MODULE_DEVICE_TABLE(of, bd96801_of_match);
> --
> 2.48.1
>
--
Lee Jones [李琼斯]
Powered by blists - more mailing lists