drivers/mfd/axp20x.c | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-)
At the moment we allow the AXP15060 and the AXP806 PMICs to omit the
interrupt line to the SoC, and we skip registering the PEK (power key)
driver in this case, since that crashes when no IRQ is described in the
DT node.
The IRQ pin potentially not being connected to anything does affect more
PMICs, though, and the PEK driver is not the only one requiring an
interrupt: at least the AC power supply driver crashes in a similar
fashion.
Generalise the handling of AXP MFD devices when the platform tables
describe no interrupt, by allowing each device to specify an alternative
MFD list for this case. If no specific alternative is specified, we go
with the safe default of "just the regulators", which matches the current
situation.
This enables new devices using the AXP313a PMIC, but not connecting the
IRQ pin.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
Changelog v2 .. v1:
- drop reordering approach, use separate cell lists
drivers/mfd/axp20x.c | 44 ++++++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 20 deletions(-)
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index c03bc5cda080a..239e7f18956ae 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -1133,6 +1133,8 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
struct device *dev = axp20x->dev;
const struct acpi_device_id *acpi_id;
const struct of_device_id *of_id;
+ const struct mfd_cell *cells_no_irq = NULL;
+ int nr_cells_no_irq = 0;
if (dev->of_node) {
of_id = of_match_device(dev->driver->of_match_table, dev);
@@ -1207,14 +1209,15 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
* if there is no interrupt line.
*/
if (of_property_read_bool(axp20x->dev->of_node,
- "x-powers,self-working-mode") &&
- axp20x->irq > 0) {
+ "x-powers,self-working-mode")) {
axp20x->nr_cells = ARRAY_SIZE(axp806_self_working_cells);
axp20x->cells = axp806_self_working_cells;
} else {
axp20x->nr_cells = ARRAY_SIZE(axp806_cells);
axp20x->cells = axp806_cells;
}
+ nr_cells_no_irq = ARRAY_SIZE(axp806_cells);
+ cells_no_irq = axp806_cells;
axp20x->regmap_cfg = &axp806_regmap_config;
axp20x->regmap_irq_chip = &axp806_regmap_irq_chip;
break;
@@ -1238,24 +1241,8 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
axp20x->regmap_irq_chip = &axp803_regmap_irq_chip;
break;
case AXP15060_ID:
- /*
- * Don't register the power key part if there is no interrupt
- * line.
- *
- * Since most use cases of AXP PMICs are Allwinner SOCs, board
- * designers follow Allwinner's reference design and connects
- * IRQ line to SOC, there's no need for those variants to deal
- * with cases that IRQ isn't connected. However, AXP15660 is
- * used by some other vendors' SOCs that didn't connect IRQ
- * line, we need to deal with this case.
- */
- if (axp20x->irq > 0) {
- axp20x->nr_cells = ARRAY_SIZE(axp15060_cells);
- axp20x->cells = axp15060_cells;
- } else {
- axp20x->nr_cells = ARRAY_SIZE(axp_regulator_only_cells);
- axp20x->cells = axp_regulator_only_cells;
- }
+ axp20x->nr_cells = ARRAY_SIZE(axp15060_cells);
+ axp20x->cells = axp15060_cells;
axp20x->regmap_cfg = &axp15060_regmap_config;
axp20x->regmap_irq_chip = &axp15060_regmap_irq_chip;
break;
@@ -1263,6 +1250,23 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
dev_err(dev, "unsupported AXP20X ID %lu\n", axp20x->variant);
return -EINVAL;
}
+
+ /*
+ * Use an alternative cell array when no interrupt line is connected,
+ * since IRQs are required by some drivers.
+ * The default is the safe "regulator-only", as this works fine without
+ * an interrupt specified.
+ */
+ if (axp20x->irq <= 0) {
+ if (cells_no_irq) {
+ axp20x->nr_cells = nr_cells_no_irq;
+ axp20x->cells = cells_no_irq;
+ } else {
+ axp20x->nr_cells = ARRAY_SIZE(axp_regulator_only_cells);
+ axp20x->cells = axp_regulator_only_cells;
+ }
+ }
+
dev_info(dev, "AXP20x variant %s found\n",
axp20x_model_names[axp20x->variant]);
--
2.35.8
On Mon, 28 Aug 2023 22:32:29 +0100, Andre Przywara wrote:
> At the moment we allow the AXP15060 and the AXP806 PMICs to omit the
> interrupt line to the SoC, and we skip registering the PEK (power key)
> driver in this case, since that crashes when no IRQ is described in the
> DT node.
> The IRQ pin potentially not being connected to anything does affect more
> PMICs, though, and the PEK driver is not the only one requiring an
> interrupt: at least the AC power supply driver crashes in a similar
> fashion.
>
> [...]
Applied, thanks!
[1/1] mfd: axp20x: Generalise handling without interrupt
commit: e928aaf249755fd3c1659dece4a23e089534f66f
--
Lee Jones [李琼斯]
On Monday, August 28, 2023 11:32:29 PM CEST Andre Przywara wrote:
> At the moment we allow the AXP15060 and the AXP806 PMICs to omit the
> interrupt line to the SoC, and we skip registering the PEK (power key)
> driver in this case, since that crashes when no IRQ is described in the
> DT node.
> The IRQ pin potentially not being connected to anything does affect more
> PMICs, though, and the PEK driver is not the only one requiring an
> interrupt: at least the AC power supply driver crashes in a similar
> fashion.
>
> Generalise the handling of AXP MFD devices when the platform tables
> describe no interrupt, by allowing each device to specify an alternative
> MFD list for this case. If no specific alternative is specified, we go
> with the safe default of "just the regulators", which matches the current
> situation.
>
> This enables new devices using the AXP313a PMIC, but not connecting the
> IRQ pin.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> Changelog v2 .. v1:
> - drop reordering approach, use separate cell lists
>
> drivers/mfd/axp20x.c | 44 ++++++++++++++++++++++++--------------------
> 1 file changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index c03bc5cda080a..239e7f18956ae 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -1133,6 +1133,8 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
> struct device *dev = axp20x->dev;
> const struct acpi_device_id *acpi_id;
> const struct of_device_id *of_id;
> + const struct mfd_cell *cells_no_irq = NULL;
> + int nr_cells_no_irq = 0;
>
> if (dev->of_node) {
> of_id = of_match_device(dev->driver->of_match_table,
dev);
> @@ -1207,14 +1209,15 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
> * if there is no interrupt line.
> */
> if (of_property_read_bool(axp20x->dev->of_node,
> - "x-powers,self-
working-mode") &&
> - axp20x->irq > 0) {
> + "x-powers,self-
working-mode")) {
> axp20x->nr_cells =
ARRAY_SIZE(axp806_self_working_cells);
> axp20x->cells = axp806_self_working_cells;
> } else {
> axp20x->nr_cells = ARRAY_SIZE(axp806_cells);
> axp20x->cells = axp806_cells;
> }
> + nr_cells_no_irq = ARRAY_SIZE(axp806_cells);
> + cells_no_irq = axp806_cells;
> axp20x->regmap_cfg = &axp806_regmap_config;
> axp20x->regmap_irq_chip = &axp806_regmap_irq_chip;
> break;
> @@ -1238,24 +1241,8 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
> axp20x->regmap_irq_chip = &axp803_regmap_irq_chip;
> break;
> case AXP15060_ID:
> - /*
> - * Don't register the power key part if there is no
interrupt
> - * line.
> - *
> - * Since most use cases of AXP PMICs are Allwinner
SOCs, board
> - * designers follow Allwinner's reference design and
connects
> - * IRQ line to SOC, there's no need for those variants
to deal
> - * with cases that IRQ isn't connected. However,
AXP15660 is
> - * used by some other vendors' SOCs that didn't connect
IRQ
> - * line, we need to deal with this case.
> - */
> - if (axp20x->irq > 0) {
> - axp20x->nr_cells =
ARRAY_SIZE(axp15060_cells);
> - axp20x->cells = axp15060_cells;
> - } else {
> - axp20x->nr_cells =
ARRAY_SIZE(axp_regulator_only_cells);
> - axp20x->cells = axp_regulator_only_cells;
> - }
> + axp20x->nr_cells = ARRAY_SIZE(axp15060_cells);
> + axp20x->cells = axp15060_cells;
> axp20x->regmap_cfg = &axp15060_regmap_config;
> axp20x->regmap_irq_chip = &axp15060_regmap_irq_chip;
> break;
> @@ -1263,6 +1250,23 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
> dev_err(dev, "unsupported AXP20X ID %lu\n", axp20x-
>variant);
> return -EINVAL;
> }
> +
> + /*
> + * Use an alternative cell array when no interrupt line is
connected,
> + * since IRQs are required by some drivers.
> + * The default is the safe "regulator-only", as this works fine
without
> + * an interrupt specified.
> + */
> + if (axp20x->irq <= 0) {
> + if (cells_no_irq) {
> + axp20x->nr_cells = nr_cells_no_irq;
> + axp20x->cells = cells_no_irq;
> + } else {
> + axp20x->nr_cells =
ARRAY_SIZE(axp_regulator_only_cells);
> + axp20x->cells = axp_regulator_only_cells;
axp806_cells (old value for AXP806_ID without irq) and
axp_regulator_only_cells differs in id field. Is that an issue?
Best regards,
Jernej
> + }
> + }
> +
> dev_info(dev, "AXP20x variant %s found\n",
> axp20x_model_names[axp20x->variant]);
On Tue, 05 Sep 2023 22:28:10 +0200
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
Hi Jernej,
> On Monday, August 28, 2023 11:32:29 PM CEST Andre Przywara wrote:
> > At the moment we allow the AXP15060 and the AXP806 PMICs to omit the
> > interrupt line to the SoC, and we skip registering the PEK (power key)
> > driver in this case, since that crashes when no IRQ is described in the
> > DT node.
> > The IRQ pin potentially not being connected to anything does affect more
> > PMICs, though, and the PEK driver is not the only one requiring an
> > interrupt: at least the AC power supply driver crashes in a similar
> > fashion.
> >
> > Generalise the handling of AXP MFD devices when the platform tables
> > describe no interrupt, by allowing each device to specify an alternative
> > MFD list for this case. If no specific alternative is specified, we go
> > with the safe default of "just the regulators", which matches the current
> > situation.
> >
> > This enables new devices using the AXP313a PMIC, but not connecting the
> > IRQ pin.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> > Changelog v2 .. v1:
> > - drop reordering approach, use separate cell lists
> >
> > drivers/mfd/axp20x.c | 44 ++++++++++++++++++++++++--------------------
> > 1 file changed, 24 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> > index c03bc5cda080a..239e7f18956ae 100644
> > --- a/drivers/mfd/axp20x.c
> > +++ b/drivers/mfd/axp20x.c
> > @@ -1133,6 +1133,8 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
> > struct device *dev = axp20x->dev;
> > const struct acpi_device_id *acpi_id;
> > const struct of_device_id *of_id;
> > + const struct mfd_cell *cells_no_irq = NULL;
> > + int nr_cells_no_irq = 0;
> >
> > if (dev->of_node) {
> > of_id = of_match_device(dev->driver->of_match_table,
> dev);
> > @@ -1207,14 +1209,15 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
> > * if there is no interrupt line.
> > */
> > if (of_property_read_bool(axp20x->dev->of_node,
> > - "x-powers,self-
> working-mode") &&
> > - axp20x->irq > 0) {
> > + "x-powers,self-
> working-mode")) {
> > axp20x->nr_cells =
> ARRAY_SIZE(axp806_self_working_cells);
> > axp20x->cells = axp806_self_working_cells;
> > } else {
> > axp20x->nr_cells = ARRAY_SIZE(axp806_cells);
> > axp20x->cells = axp806_cells;
> > }
> > + nr_cells_no_irq = ARRAY_SIZE(axp806_cells);
> > + cells_no_irq = axp806_cells;
> > axp20x->regmap_cfg = &axp806_regmap_config;
> > axp20x->regmap_irq_chip = &axp806_regmap_irq_chip;
> > break;
> > @@ -1238,24 +1241,8 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
> > axp20x->regmap_irq_chip = &axp803_regmap_irq_chip;
> > break;
> > case AXP15060_ID:
> > - /*
> > - * Don't register the power key part if there is no
> interrupt
> > - * line.
> > - *
> > - * Since most use cases of AXP PMICs are Allwinner
> SOCs, board
> > - * designers follow Allwinner's reference design and
> connects
> > - * IRQ line to SOC, there's no need for those variants
> to deal
> > - * with cases that IRQ isn't connected. However,
> AXP15660 is
> > - * used by some other vendors' SOCs that didn't connect
> IRQ
> > - * line, we need to deal with this case.
> > - */
> > - if (axp20x->irq > 0) {
> > - axp20x->nr_cells =
> ARRAY_SIZE(axp15060_cells);
> > - axp20x->cells = axp15060_cells;
> > - } else {
> > - axp20x->nr_cells =
> ARRAY_SIZE(axp_regulator_only_cells);
> > - axp20x->cells = axp_regulator_only_cells;
> > - }
> > + axp20x->nr_cells = ARRAY_SIZE(axp15060_cells);
> > + axp20x->cells = axp15060_cells;
> > axp20x->regmap_cfg = &axp15060_regmap_config;
> > axp20x->regmap_irq_chip = &axp15060_regmap_irq_chip;
> > break;
> > @@ -1263,6 +1250,23 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
> > dev_err(dev, "unsupported AXP20X ID %lu\n", axp20x-
> >variant);
> > return -EINVAL;
> > }
> > +
> > + /*
> > + * Use an alternative cell array when no interrupt line is
> connected,
> > + * since IRQs are required by some drivers.
> > + * The default is the safe "regulator-only", as this works fine
> without
> > + * an interrupt specified.
> > + */
> > + if (axp20x->irq <= 0) {
> > + if (cells_no_irq) {
> > + axp20x->nr_cells = nr_cells_no_irq;
> > + axp20x->cells = cells_no_irq;
> > + } else {
> > + axp20x->nr_cells =
> ARRAY_SIZE(axp_regulator_only_cells);
> > + axp20x->cells = axp_regulator_only_cells;
>
> axp806_cells (old value for AXP806_ID without irq) and
> axp_regulator_only_cells differs in id field. Is that an issue?
Yes, an annoying one, which led me to introduce the ability to let each
PMIC override this default. So the case AXP806_ID: above sets the local
variable cells_no_irq to axp806_cells, and the check above sees that it
has been changed from the NULL initialisation and uses that instead of
the default. At least that was my plan, let me know if I messed that
up somehow, or if it deserves more comments.
Cheers,
Andre
>
> Best regards,
> Jernej
>
> > + }
> > + }
> > +
> > dev_info(dev, "AXP20x variant %s found\n",
> > axp20x_model_names[axp20x->variant]);
>
>
>
>
>
Dne torek, 05. september 2023 ob 22:46:42 CEST je Andre Przywara napisal(a):
> On Tue, 05 Sep 2023 22:28:10 +0200
> Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> Hi Jernej,
>
> > On Monday, August 28, 2023 11:32:29 PM CEST Andre Przywara wrote:
> > > At the moment we allow the AXP15060 and the AXP806 PMICs to omit the
> > > interrupt line to the SoC, and we skip registering the PEK (power key)
> > > driver in this case, since that crashes when no IRQ is described in the
> > > DT node.
> > > The IRQ pin potentially not being connected to anything does affect more
> > > PMICs, though, and the PEK driver is not the only one requiring an
> > > interrupt: at least the AC power supply driver crashes in a similar
> > > fashion.
> > >
> > > Generalise the handling of AXP MFD devices when the platform tables
> > > describe no interrupt, by allowing each device to specify an alternative
> > > MFD list for this case. If no specific alternative is specified, we go
> > > with the safe default of "just the regulators", which matches the
> > > current
> > > situation.
> > >
> > > This enables new devices using the AXP313a PMIC, but not connecting the
> > > IRQ pin.
> > >
> > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > ---
> > > Changelog v2 .. v1:
> > > - drop reordering approach, use separate cell lists
> > >
> > > drivers/mfd/axp20x.c | 44 ++++++++++++++++++++++++--------------------
> > > 1 file changed, 24 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> > > index c03bc5cda080a..239e7f18956ae 100644
> > > --- a/drivers/mfd/axp20x.c
> > > +++ b/drivers/mfd/axp20x.c
> > > @@ -1133,6 +1133,8 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
> > >
> > > struct device *dev = axp20x->dev;
> > > const struct acpi_device_id *acpi_id;
> > > const struct of_device_id *of_id;
> > >
> > > + const struct mfd_cell *cells_no_irq = NULL;
> > > + int nr_cells_no_irq = 0;
> > >
> > > if (dev->of_node) {
> > >
> > > of_id = of_match_device(dev->driver->of_match_table,
> >
> > dev);
> >
> > > @@ -1207,14 +1209,15 @@ int axp20x_match_device(struct axp20x_dev
> > > *axp20x)
> > >
> > > * if there is no interrupt line.
> > > */
> > >
> > > if (of_property_read_bool(axp20x->dev->of_node,
> > >
> > > - "x-powers,self-
> >
> > working-mode") &&
> >
> > > - axp20x->irq > 0) {
> > > + "x-powers,self-
> >
> > working-mode")) {
> >
> > > axp20x->nr_cells =
> >
> > ARRAY_SIZE(axp806_self_working_cells);
> >
> > > axp20x->cells = axp806_self_working_cells;
> > >
> > > } else {
> > >
> > > axp20x->nr_cells =
ARRAY_SIZE(axp806_cells);
> > > axp20x->cells = axp806_cells;
> > >
> > > }
> > >
> > > + nr_cells_no_irq = ARRAY_SIZE(axp806_cells);
> > > + cells_no_irq = axp806_cells;
> > >
> > > axp20x->regmap_cfg = &axp806_regmap_config;
> > > axp20x->regmap_irq_chip = &axp806_regmap_irq_chip;
> > > break;
> > >
> > > @@ -1238,24 +1241,8 @@ int axp20x_match_device(struct axp20x_dev
> > > *axp20x)
> > >
> > > axp20x->regmap_irq_chip = &axp803_regmap_irq_chip;
> > > break;
> > >
> > > case AXP15060_ID:
> > > - /*
> > > - * Don't register the power key part if there is no
> >
> > interrupt
> >
> > > - * line.
> > > - *
> > > - * Since most use cases of AXP PMICs are Allwinner
> >
> > SOCs, board
> >
> > > - * designers follow Allwinner's reference design and
> >
> > connects
> >
> > > - * IRQ line to SOC, there's no need for those variants
> >
> > to deal
> >
> > > - * with cases that IRQ isn't connected. However,
> >
> > AXP15660 is
> >
> > > - * used by some other vendors' SOCs that didn't
connect
> >
> > IRQ
> >
> > > - * line, we need to deal with this case.
> > > - */
> > > - if (axp20x->irq > 0) {
> > > - axp20x->nr_cells =
> >
> > ARRAY_SIZE(axp15060_cells);
> >
> > > - axp20x->cells = axp15060_cells;
> > > - } else {
> > > - axp20x->nr_cells =
> >
> > ARRAY_SIZE(axp_regulator_only_cells);
> >
> > > - axp20x->cells = axp_regulator_only_cells;
> > > - }
> > > + axp20x->nr_cells = ARRAY_SIZE(axp15060_cells);
> > > + axp20x->cells = axp15060_cells;
> > >
> > > axp20x->regmap_cfg = &axp15060_regmap_config;
> > > axp20x->regmap_irq_chip = &axp15060_regmap_irq_chip;
> > > break;
> > >
> > > @@ -1263,6 +1250,23 @@ int axp20x_match_device(struct axp20x_dev
> > > *axp20x)
> > >
> > > dev_err(dev, "unsupported AXP20X ID %lu\n", axp20x-
> > >
> > >variant);
> > >
> > > return -EINVAL;
> > >
> > > }
> > >
> > > +
> > > + /*
> > > + * Use an alternative cell array when no interrupt line is
> >
> > connected,
> >
> > > + * since IRQs are required by some drivers.
> > > + * The default is the safe "regulator-only", as this works fine
> >
> > without
> >
> > > + * an interrupt specified.
> > > + */
> > > + if (axp20x->irq <= 0) {
> > > + if (cells_no_irq) {
> > > + axp20x->nr_cells = nr_cells_no_irq;
> > > + axp20x->cells = cells_no_irq;
> > > + } else {
> > > + axp20x->nr_cells =
> >
> > ARRAY_SIZE(axp_regulator_only_cells);
> >
> > > + axp20x->cells = axp_regulator_only_cells;
> >
> > axp806_cells (old value for AXP806_ID without irq) and
> > axp_regulator_only_cells differs in id field. Is that an issue?
>
> Yes, an annoying one, which led me to introduce the ability to let each
> PMIC override this default. So the case AXP806_ID: above sets the local
> variable cells_no_irq to axp806_cells, and the check above sees that it
> has been changed from the NULL initialisation and uses that instead of
> the default. At least that was my plan, let me know if I messed that
> up somehow, or if it deserves more comments.
Ah, yes. It looks ok, but it's not obvious. It's fine.
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Best regards,
Jernej
>
> Cheers,
> Andre
>
> > Best regards,
> > Jernej
> >
> > > + }
> > > + }
> > > +
> > >
> > > dev_info(dev, "AXP20x variant %s found\n",
> > >
> > > axp20x_model_names[axp20x->variant]);
© 2016 - 2026 Red Hat, Inc.