| Message ID | 20230209105144.9351-1-cniedermaier@dh-electronics.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V2,1/2] mfd: da9062: Remove IRQ requirement | expand |
On 2/9/23 11:51, Christoph Niedermaier wrote: > This patch removes the requirement for an IRQ, because for the core > functionality IRQ isn't needed. So this makes the DA9061/62 chip > useable for designs which haven't connected the IRQ pin. > > Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> > --- > Cc: Support Opensource <support.opensource@diasemi.com> > Cc: Lee Jones <lee@kernel.org> > Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: Marek Vasut <marex@denx.de> > Cc: kernel@dh-electronics.com > Cc: linux-kernel@vger.kernel.org > To: linux-arm-kernel@lists.infradead.org > --- > V2: - Rebase on current next 20230209 > - Add Lee Jones to Cc list Reviewed-by: Marek Vasut <marex@denx.de>
On 2/9/23 11:51, Christoph Niedermaier wrote: > This patch removes the requirement for an IRQ, because for the core > functionality IRQ isn't needed. So this makes the DA9061/62 chip > useable for designs which haven't connected the IRQ pin. > > Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> > --- > Cc: Support Opensource <support.opensource@diasemi.com> > Cc: Lee Jones <lee@kernel.org> > Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: Marek Vasut <marex@denx.de> > Cc: kernel@dh-electronics.com > Cc: linux-kernel@vger.kernel.org > To: linux-arm-kernel@lists.infradead.org > --- > V2: - Rebase on current next 20230209 > - Add Lee Jones to Cc list Thanks! Reviewed-by: Adam Ward <Adam.Ward.Opensouce@diasemi.com>
On Thu, 09 Feb 2023, Christoph Niedermaier wrote: > This patch removes the requirement for an IRQ, because for the core > functionality IRQ isn't needed. So this makes the DA9061/62 chip > useable for designs which haven't connected the IRQ pin. > > Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> > --- > Cc: Support Opensource <support.opensource@diasemi.com> > Cc: Lee Jones <lee@kernel.org> > Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: Marek Vasut <marex@denx.de> > Cc: kernel@dh-electronics.com > Cc: linux-kernel@vger.kernel.org > To: linux-arm-kernel@lists.infradead.org > --- > V2: - Rebase on current next 20230209 > - Add Lee Jones to Cc list > --- > drivers/mfd/da9062-core.c | 98 +++++++++++++++++++++++++++++++++++------------ > 1 file changed, 73 insertions(+), 25 deletions(-) > > diff --git a/drivers/mfd/da9062-core.c b/drivers/mfd/da9062-core.c > index 40cde51e5719..caa597400dd1 100644 > --- a/drivers/mfd/da9062-core.c > +++ b/drivers/mfd/da9062-core.c > @@ -212,6 +212,27 @@ static const struct mfd_cell da9061_devs[] = { > }, > }; > > +static const struct mfd_cell da9061_devs_without_irq[] = { "_noirq" > + { > + .name = "da9061-core", > + }, > + { > + .name = "da9062-regulators", > + }, Place the one line entries on one line please. Even better, use MFD_CELL_NAME() > + { > + .name = "da9061-watchdog", > + .of_compatible = "dlg,da9061-watchdog", > + }, MFD_CELL_OF(<name>, NULL, NULL, NULL, 0, <compatible>); > + { > + .name = "da9061-thermal", > + .of_compatible = "dlg,da9061-thermal", > + }, > + { > + .name = "da9061-onkey", > + .of_compatible = "dlg,da9061-onkey", > + }, > +}; > + > static const struct resource da9062_core_resources[] = { > DEFINE_RES_NAMED(DA9062_IRQ_VDD_WARN, 1, "VDD_WARN", IORESOURCE_IRQ), > }; > @@ -288,6 +309,35 @@ static const struct mfd_cell da9062_devs[] = { > }, > }; > > +static const struct mfd_cell da9062_devs_without_irq[] = { > + { > + .name = "da9062-core", > + }, > + { > + .name = "da9062-regulators", > + }, > + { > + .name = "da9062-watchdog", > + .of_compatible = "dlg,da9062-watchdog", > + }, > + { > + .name = "da9062-thermal", > + .of_compatible = "dlg,da9062-thermal", > + }, > + { > + .name = "da9062-rtc", > + .of_compatible = "dlg,da9062-rtc", > + }, > + { > + .name = "da9062-onkey", > + .of_compatible = "dlg,da9062-onkey", > + }, > + { > + .name = "da9062-gpio", > + .of_compatible = "dlg,da9062-gpio", > + }, > +}; As above. > static int da9062_clear_fault_log(struct da9062 *chip) > { > int ret; > @@ -625,7 +675,7 @@ static int da9062_i2c_probe(struct i2c_client *i2c) > { > const struct i2c_device_id *id = i2c_client_get_device_id(i2c); > struct da9062 *chip; > - unsigned int irq_base; > + unsigned int irq_base = 0; > const struct mfd_cell *cell; > const struct regmap_irq_chip *irq_chip; > const struct regmap_config *config; > @@ -645,21 +695,16 @@ static int da9062_i2c_probe(struct i2c_client *i2c) > i2c_set_clientdata(i2c, chip); > chip->dev = &i2c->dev; > > - if (!i2c->irq) { > - dev_err(chip->dev, "No IRQ configured\n"); > - return -EINVAL; > - } > - > switch (chip->chip_type) { > case COMPAT_TYPE_DA9061: > - cell = da9061_devs; > - cell_num = ARRAY_SIZE(da9061_devs); > + cell = i2c->irq ? da9061_devs : da9061_devs_without_irq; > + cell_num = i2c->irq ? ARRAY_SIZE(da9061_devs) : ARRAY_SIZE(da9061_devs_without_irq); This is hideous. Why not just NULLify the resources below instead? > irq_chip = &da9061_irq_chip; > config = &da9061_regmap_config; > break; > case COMPAT_TYPE_DA9062: > - cell = da9062_devs; > - cell_num = ARRAY_SIZE(da9062_devs); > + cell = i2c->irq ? da9062_devs : da9062_devs_without_irq; > + cell_num = i2c->irq ? ARRAY_SIZE(da9062_devs) : ARRAY_SIZE(da9062_devs_without_irq); > irq_chip = &da9062_irq_chip; Still setting this despite no IRQs? > config = &da9062_regmap_config; > break; __ _||_ \ / \/ [...] if (i2c->irq <= 0) cell->resources = NULL; cell->num_resources = 0; > @@ -695,29 +740,32 @@ static int da9062_i2c_probe(struct i2c_client *i2c) > if (ret) > return ret; > > - ret = da9062_configure_irq_type(chip, i2c->irq, &trigger_type); > - if (ret < 0) { > - dev_err(chip->dev, "Failed to configure IRQ type\n"); > - return ret; > - } > + if (i2c->irq) { > + ret = da9062_configure_irq_type(chip, i2c->irq, &trigger_type); > + if (ret < 0) { > + dev_err(chip->dev, "Failed to configure IRQ type\n"); > + return ret; > + } > > - ret = regmap_add_irq_chip(chip->regmap, i2c->irq, > - trigger_type | IRQF_SHARED | IRQF_ONESHOT, > - -1, irq_chip, &chip->regmap_irq); > - if (ret) { > - dev_err(chip->dev, "Failed to request IRQ %d: %d\n", > - i2c->irq, ret); > - return ret; > - } > + ret = regmap_add_irq_chip(chip->regmap, i2c->irq, > + trigger_type | IRQF_SHARED | IRQF_ONESHOT, > + -1, irq_chip, &chip->regmap_irq); > + if (ret) { > + dev_err(chip->dev, "Failed to request IRQ %d: %d\n", > + i2c->irq, ret); > + return ret; > + } > > - irq_base = regmap_irq_chip_get_base(chip->regmap_irq); > + irq_base = regmap_irq_chip_get_base(chip->regmap_irq); > + } > > ret = mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE, cell, > cell_num, NULL, irq_base, > NULL); > if (ret) { > dev_err(chip->dev, "Cannot register child devices\n"); > - regmap_del_irq_chip(i2c->irq, chip->regmap_irq); > + if (i2c->irq) > + regmap_del_irq_chip(i2c->irq, chip->regmap_irq); > return ret; > } > > -- > 2.11.0 >
From: Lee Jones [mailto:lee@kernel.org] Sent: Friday, March 3, 2023 9:42 AM > > On Thu, 09 Feb 2023, Christoph Niedermaier wrote: > >> This patch removes the requirement for an IRQ, because for the core >> functionality IRQ isn't needed. So this makes the DA9061/62 chip >> useable for designs which haven't connected the IRQ pin. >> >> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> >> --- >> Cc: Support Opensource <support.opensource@diasemi.com> >> Cc: Lee Jones <lee@kernel.org> >> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> >> Cc: Liam Girdwood <lgirdwood@gmail.com> >> Cc: Mark Brown <broonie@kernel.org> >> Cc: Marek Vasut <marex@denx.de> >> Cc: kernel@dh-electronics.com >> Cc: linux-kernel@vger.kernel.org >> To: linux-arm-kernel@lists.infradead.org >> --- >> V2: - Rebase on current next 20230209 >> - Add Lee Jones to Cc list >> --- >> drivers/mfd/da9062-core.c | 98 +++++++++++++++++++++++++++++++++++------------ >> 1 file changed, 73 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/mfd/da9062-core.c b/drivers/mfd/da9062-core.c >> index 40cde51e5719..caa597400dd1 100644 >> --- a/drivers/mfd/da9062-core.c >> +++ b/drivers/mfd/da9062-core.c >> @@ -212,6 +212,27 @@ static const struct mfd_cell da9061_devs[] = { >> }, >> }; >> >> +static const struct mfd_cell da9061_devs_without_irq[] = { > > "_noirq" > >> + { >> + .name = "da9061-core", >> + }, >> + { >> + .name = "da9062-regulators", >> + }, > > Place the one line entries on one line please. > > Even better, use MFD_CELL_NAME() > >> + { >> + .name = "da9061-watchdog", >> + .of_compatible = "dlg,da9061-watchdog", >> + }, > > MFD_CELL_OF(<name>, NULL, NULL, NULL, 0, <compatible>); > I will convert all to MFD_CELL_NAME() in version 3. >> + { >> + .name = "da9061-thermal", >> + .of_compatible = "dlg,da9061-thermal", >> + }, >> + { >> + .name = "da9061-onkey", >> + .of_compatible = "dlg,da9061-onkey", >> + }, >> +}; >> + >> static const struct resource da9062_core_resources[] = { >> DEFINE_RES_NAMED(DA9062_IRQ_VDD_WARN, 1, "VDD_WARN", IORESOURCE_IRQ), >> }; >> @@ -288,6 +309,35 @@ static const struct mfd_cell da9062_devs[] = { >> }, >> }; >> >> +static const struct mfd_cell da9062_devs_without_irq[] = { >> + { >> + .name = "da9062-core", >> + }, >> + { >> + .name = "da9062-regulators", >> + }, >> + { >> + .name = "da9062-watchdog", >> + .of_compatible = "dlg,da9062-watchdog", >> + }, >> + { >> + .name = "da9062-thermal", >> + .of_compatible = "dlg,da9062-thermal", >> + }, >> + { >> + .name = "da9062-rtc", >> + .of_compatible = "dlg,da9062-rtc", >> + }, >> + { >> + .name = "da9062-onkey", >> + .of_compatible = "dlg,da9062-onkey", >> + }, >> + { >> + .name = "da9062-gpio", >> + .of_compatible = "dlg,da9062-gpio", >> + }, >> +}; > > As above. > I will convert all to MFD_CELL_NAME() in version 3. >> static int da9062_clear_fault_log(struct da9062 *chip) >> { >> int ret; >> @@ -625,7 +675,7 @@ static int da9062_i2c_probe(struct i2c_client *i2c) >> { >> const struct i2c_device_id *id = i2c_client_get_device_id(i2c); >> struct da9062 *chip; >> - unsigned int irq_base; >> + unsigned int irq_base = 0; >> const struct mfd_cell *cell; >> const struct regmap_irq_chip *irq_chip; >> const struct regmap_config *config; >> @@ -645,21 +695,16 @@ static int da9062_i2c_probe(struct i2c_client *i2c) >> i2c_set_clientdata(i2c, chip); >> chip->dev = &i2c->dev; >> >> - if (!i2c->irq) { >> - dev_err(chip->dev, "No IRQ configured\n"); >> - return -EINVAL; >> - } >> - >> switch (chip->chip_type) { >> case COMPAT_TYPE_DA9061: >> - cell = da9061_devs; >> - cell_num = ARRAY_SIZE(da9061_devs); >> + cell = i2c->irq ? da9061_devs : da9061_devs_without_irq; >> + cell_num = i2c->irq ? ARRAY_SIZE(da9061_devs) : ARRAY_SIZE(da9061_devs_without_irq); > > This is hideous. > > Why not just NULLify the resources below instead? > >> irq_chip = &da9061_irq_chip; >> config = &da9061_regmap_config; >> break; >> case COMPAT_TYPE_DA9062: >> - cell = da9062_devs; >> - cell_num = ARRAY_SIZE(da9062_devs); >> + cell = i2c->irq ? da9062_devs : da9062_devs_without_irq; >> + cell_num = i2c->irq ? ARRAY_SIZE(da9062_devs) : ARRAY_SIZE(da9062_devs_without_irq); > > irq_chip = &da9062_irq_chip; > > Still setting this despite no IRQs? I will take it into account in version 3. > >> config = &da9062_regmap_config; >> break; > __ > _||_ > \ / > \/ > > [...] > > if (i2c->irq <= 0) > cell->resources = NULL; > cell->num_resources = 0; But it is an array. I then have to go through it completely and check if it is related to IRQ. I will try to refactor the my code to be less hideous and send a version 3. > >> @@ -695,29 +740,32 @@ static int da9062_i2c_probe(struct i2c_client *i2c) >> if (ret) >> return ret; >> >> - ret = da9062_configure_irq_type(chip, i2c->irq, &trigger_type); >> - if (ret < 0) { >> - dev_err(chip->dev, "Failed to configure IRQ type\n"); >> - return ret; >> - } >> + if (i2c->irq) { >> + ret = da9062_configure_irq_type(chip, i2c->irq, &trigger_type); >> + if (ret < 0) { >> + dev_err(chip->dev, "Failed to configure IRQ type\n"); >> + return ret; >> + } >> >> - ret = regmap_add_irq_chip(chip->regmap, i2c->irq, >> - trigger_type | IRQF_SHARED | IRQF_ONESHOT, >> - -1, irq_chip, &chip->regmap_irq); >> - if (ret) { >> - dev_err(chip->dev, "Failed to request IRQ %d: %d\n", >> - i2c->irq, ret); >> - return ret; >> - } >> + ret = regmap_add_irq_chip(chip->regmap, i2c->irq, >> + trigger_type | IRQF_SHARED | IRQF_ONESHOT, >> + -1, irq_chip, &chip->regmap_irq); >> + if (ret) { >> + dev_err(chip->dev, "Failed to request IRQ %d: %d\n", >> + i2c->irq, ret); >> + return ret; >> + } >> >> - irq_base = regmap_irq_chip_get_base(chip->regmap_irq); >> + irq_base = regmap_irq_chip_get_base(chip->regmap_irq); >> + } >> >> ret = mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE, cell, >> cell_num, NULL, irq_base, >> NULL); >> if (ret) { >> dev_err(chip->dev, "Cannot register child devices\n"); >> - regmap_del_irq_chip(i2c->irq, chip->regmap_irq); >> + if (i2c->irq) >> + regmap_del_irq_chip(i2c->irq, chip->regmap_irq); >> return ret; >> } >> Regards Christoph
diff --git a/drivers/mfd/da9062-core.c b/drivers/mfd/da9062-core.c index 40cde51e5719..caa597400dd1 100644 --- a/drivers/mfd/da9062-core.c +++ b/drivers/mfd/da9062-core.c @@ -212,6 +212,27 @@ static const struct mfd_cell da9061_devs[] = { }, }; +static const struct mfd_cell da9061_devs_without_irq[] = { + { + .name = "da9061-core", + }, + { + .name = "da9062-regulators", + }, + { + .name = "da9061-watchdog", + .of_compatible = "dlg,da9061-watchdog", + }, + { + .name = "da9061-thermal", + .of_compatible = "dlg,da9061-thermal", + }, + { + .name = "da9061-onkey", + .of_compatible = "dlg,da9061-onkey", + }, +}; + static const struct resource da9062_core_resources[] = { DEFINE_RES_NAMED(DA9062_IRQ_VDD_WARN, 1, "VDD_WARN", IORESOURCE_IRQ), }; @@ -288,6 +309,35 @@ static const struct mfd_cell da9062_devs[] = { }, }; +static const struct mfd_cell da9062_devs_without_irq[] = { + { + .name = "da9062-core", + }, + { + .name = "da9062-regulators", + }, + { + .name = "da9062-watchdog", + .of_compatible = "dlg,da9062-watchdog", + }, + { + .name = "da9062-thermal", + .of_compatible = "dlg,da9062-thermal", + }, + { + .name = "da9062-rtc", + .of_compatible = "dlg,da9062-rtc", + }, + { + .name = "da9062-onkey", + .of_compatible = "dlg,da9062-onkey", + }, + { + .name = "da9062-gpio", + .of_compatible = "dlg,da9062-gpio", + }, +}; + static int da9062_clear_fault_log(struct da9062 *chip) { int ret; @@ -625,7 +675,7 @@ static int da9062_i2c_probe(struct i2c_client *i2c) { const struct i2c_device_id *id = i2c_client_get_device_id(i2c); struct da9062 *chip; - unsigned int irq_base; + unsigned int irq_base = 0; const struct mfd_cell *cell; const struct regmap_irq_chip *irq_chip; const struct regmap_config *config; @@ -645,21 +695,16 @@ static int da9062_i2c_probe(struct i2c_client *i2c) i2c_set_clientdata(i2c, chip); chip->dev = &i2c->dev; - if (!i2c->irq) { - dev_err(chip->dev, "No IRQ configured\n"); - return -EINVAL; - } - switch (chip->chip_type) { case COMPAT_TYPE_DA9061: - cell = da9061_devs; - cell_num = ARRAY_SIZE(da9061_devs); + cell = i2c->irq ? da9061_devs : da9061_devs_without_irq; + cell_num = i2c->irq ? ARRAY_SIZE(da9061_devs) : ARRAY_SIZE(da9061_devs_without_irq); irq_chip = &da9061_irq_chip; config = &da9061_regmap_config; break; case COMPAT_TYPE_DA9062: - cell = da9062_devs; - cell_num = ARRAY_SIZE(da9062_devs); + cell = i2c->irq ? da9062_devs : da9062_devs_without_irq; + cell_num = i2c->irq ? ARRAY_SIZE(da9062_devs) : ARRAY_SIZE(da9062_devs_without_irq); irq_chip = &da9062_irq_chip; config = &da9062_regmap_config; break; @@ -695,29 +740,32 @@ static int da9062_i2c_probe(struct i2c_client *i2c) if (ret) return ret; - ret = da9062_configure_irq_type(chip, i2c->irq, &trigger_type); - if (ret < 0) { - dev_err(chip->dev, "Failed to configure IRQ type\n"); - return ret; - } + if (i2c->irq) { + ret = da9062_configure_irq_type(chip, i2c->irq, &trigger_type); + if (ret < 0) { + dev_err(chip->dev, "Failed to configure IRQ type\n"); + return ret; + } - ret = regmap_add_irq_chip(chip->regmap, i2c->irq, - trigger_type | IRQF_SHARED | IRQF_ONESHOT, - -1, irq_chip, &chip->regmap_irq); - if (ret) { - dev_err(chip->dev, "Failed to request IRQ %d: %d\n", - i2c->irq, ret); - return ret; - } + ret = regmap_add_irq_chip(chip->regmap, i2c->irq, + trigger_type | IRQF_SHARED | IRQF_ONESHOT, + -1, irq_chip, &chip->regmap_irq); + if (ret) { + dev_err(chip->dev, "Failed to request IRQ %d: %d\n", + i2c->irq, ret); + return ret; + } - irq_base = regmap_irq_chip_get_base(chip->regmap_irq); + irq_base = regmap_irq_chip_get_base(chip->regmap_irq); + } ret = mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE, cell, cell_num, NULL, irq_base, NULL); if (ret) { dev_err(chip->dev, "Cannot register child devices\n"); - regmap_del_irq_chip(i2c->irq, chip->regmap_irq); + if (i2c->irq) + regmap_del_irq_chip(i2c->irq, chip->regmap_irq); return ret; }
This patch removes the requirement for an IRQ, because for the core functionality IRQ isn't needed. So this makes the DA9061/62 chip useable for designs which haven't connected the IRQ pin. Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> --- Cc: Support Opensource <support.opensource@diasemi.com> Cc: Lee Jones <lee@kernel.org> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> Cc: Liam Girdwood <lgirdwood@gmail.com> Cc: Mark Brown <broonie@kernel.org> Cc: Marek Vasut <marex@denx.de> Cc: kernel@dh-electronics.com Cc: linux-kernel@vger.kernel.org To: linux-arm-kernel@lists.infradead.org --- V2: - Rebase on current next 20230209 - Add Lee Jones to Cc list --- drivers/mfd/da9062-core.c | 98 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 73 insertions(+), 25 deletions(-)