Messages in this thread | | | From | Jernej Škrabec <> | Subject | Re: [PATCH v2] mfd: axp20x: Generalise handling without interrupt | Date | Tue, 05 Sep 2023 23:03:46 +0200 |
| |
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]);
| |