mbox series

Message ID 20211008012524.481877-1-dmitry.baryshkov@linaro.org
State Not Applicable, archived
Headers show
Series | expand

Pull-request

https://git.linaro.org/people/dmitry.baryshkov/kernel.git spmi-mpp

Checks

Context Check Description
robh/checkpatch warning total: 2 errors, 1 warnings, 0 lines checked

Message

Dmitry Baryshkov Oct. 8, 2021, 1:24 a.m. UTC
In 2019 (in kernel 5.4) spmi-gpio and ssbi-gpio drivers were converted
to hierarchical IRQ helpers, however MPP drivers were not converted at
that moment. Complete this by converting MPP drivers.

Changes since v2:
 - Add patches fixing/updating mpps nodes in the existing device trees

Changes since v1:
 - Drop the interrupt-controller from initial schema conversion
 - Add gpio-line-names to the qcom,pmic-mpp schema and to the example

The following changes since commit 6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f:

  Linux 5.15-rc1 (2021-09-12 16:28:37 -0700)

are available in the Git repository at:

  https://git.linaro.org/people/dmitry.baryshkov/kernel.git spmi-mpp

for you to fetch changes up to 9bccc31fc5cec98f26ca639a2ee21a9831efe1de:

  arm64: dts: qcom: pm8994: add interrupt controller properties (2021-10-08 04:19:33 +0300)

----------------------------------------------------------------
Dmitry Baryshkov (25):
      dt-bindings: pinctrl: qcom,pmic-mpp: Convert qcom pmic mpp bindings to YAML
      dt-bindings: mfd: qcom-pm8xxx: add missing child nodes
      ARM: dts: qcom-apq8064: add gpio-ranges to mpps nodes
      ARM: dts: qcom-msm8660: add gpio-ranges to mpps nodes
      ARM: dts: qcom-pm8841: add gpio-ranges to mpps nodes
      ARM: dts: qcom-pm8941: add gpio-ranges to mpps nodes
      ARM: dts: qcom-pma8084: add gpio-ranges to mpps nodes
      ARM: dts: qcom-mdm9615: add gpio-ranges to mpps node, fix its name
      ARM: dts: qcom-apq8060-dragonboard: fix mpps state names
      arm64: dts: qcom: pm8916: fix mpps device tree node
      arm64: dts: qcom: pm8994: fix mpps device tree node
      arm64: dts: qcom: apq8016-sbc: fix mpps state names
      pinctrl: qcom: ssbi-mpp: hardcode IRQ counts
      pinctrl: qcom: ssbi-mpp: add support for hierarchical IRQ chip
      pinctrl: qcom: spmi-mpp: hardcode IRQ counts
      pinctrl: qcom: spmi-mpp: add support for hierarchical IRQ chip
      dt-bindings: pinctrl: qcom,pmic-mpp: switch to #interrupt-cells
      ARM: dts: qcom-apq8064: add interrupt controller properties
      ARM: dts: qcom-mdm9615: add interrupt controller properties
      ARM: dts: qcom-msm8660: add interrupt controller properties
      ARM: dts: qcom-pm8841: add interrupt controller properties
      ARM: dts: qcom-pm8941: add interrupt controller properties
      ARM: dts: qcom-pma8084: add interrupt controller properties
      arm64: dts: qcom: pm8916: add interrupt controller properties
      arm64: dts: qcom: pm8994: add interrupt controller properties

 .../devicetree/bindings/mfd/qcom-pm8xxx.yaml       |  12 ++
 .../devicetree/bindings/pinctrl/qcom,pmic-mpp.txt  | 187 --------------------
 .../devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml | 188 +++++++++++++++++++++
 arch/arm/boot/dts/qcom-apq8060-dragonboard.dts     |   4 +-
 arch/arm/boot/dts/qcom-apq8064.dtsi                |  23 +--
 arch/arm/boot/dts/qcom-mdm9615.dtsi                |  12 +-
 arch/arm/boot/dts/qcom-msm8660.dtsi                |  17 +-
 arch/arm/boot/dts/qcom-pm8841.dtsi                 |   7 +-
 arch/arm/boot/dts/qcom-pm8941.dtsi                 |  11 +-
 arch/arm/boot/dts/qcom-pma8084.dtsi                |  11 +-
 arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi          |   4 +-
 arch/arm64/boot/dts/qcom/pm8916.dtsi               |   9 +-
 arch/arm64/boot/dts/qcom/pm8994.dtsi               |  13 +-
 drivers/pinctrl/qcom/pinctrl-spmi-mpp.c            | 111 ++++++++----
 drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c            | 133 +++++++++++----
 15 files changed, 414 insertions(+), 328 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.txt
 create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml

Comments

Linus Walleij Oct. 12, 2021, 11:54 p.m. UTC | #1
On Fri, Oct 8, 2021 at 3:25 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:

> Add gpio-ranges property to mpps device tree nodes, adding the mapping between
> pinctrl and GPIO pins.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Linus Walleij Oct. 12, 2021, 11:59 p.m. UTC | #2
On Fri, Oct 8, 2021 at 3:25 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:

> In 2019 (in kernel 5.4) spmi-gpio and ssbi-gpio drivers were converted
> to hierarchical IRQ helpers, however MPP drivers were not converted at
> that moment. Complete this by converting MPP drivers.
>
> Changes since v2:
>  - Add patches fixing/updating mpps nodes in the existing device trees

Thanks a *lot* for being thorough and fixing all this properly!

I am happy to apply the pinctrl portions to the pinctrl tree, I'm
uncertain about Rob's syntax checker robot here, are there real
problems? Sometimes it complains about things being changed
in the DTS files at the same time.

I could apply all of this (including DTS changes) to an immutable
branch and offer to Bjorn if he is fine with the patches and
the general approach.

Yours,
Linus Walleij
Dmitry Baryshkov Oct. 13, 2021, 3:46 a.m. UTC | #3
On Wed, 13 Oct 2021 at 02:59, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Fri, Oct 8, 2021 at 3:25 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>
> > In 2019 (in kernel 5.4) spmi-gpio and ssbi-gpio drivers were converted
> > to hierarchical IRQ helpers, however MPP drivers were not converted at
> > that moment. Complete this by converting MPP drivers.
> >
> > Changes since v2:
> >  - Add patches fixing/updating mpps nodes in the existing device trees
>
> Thanks a *lot* for being thorough and fixing all this properly!
>
> I am happy to apply the pinctrl portions to the pinctrl tree, I'm
> uncertain about Rob's syntax checker robot here, are there real
> problems? Sometimes it complains about things being changed
> in the DTS files at the same time.

Rob's checker reports issue that are being fixed by respective
patches. I think I've updated all dts entries for the mpp devices tree
nodes.

> I could apply all of this (including DTS changes) to an immutable
> branch and offer to Bjorn if he is fine with the patches and
> the general approach.

I'm fine with either approach.
Linus Walleij Oct. 13, 2021, 11:39 p.m. UTC | #4
On Wed, Oct 13, 2021 at 5:46 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
> On Wed, 13 Oct 2021 at 02:59, Linus Walleij <linus.walleij@linaro.org> wrote:

> > I am happy to apply the pinctrl portions to the pinctrl tree, I'm
> > uncertain about Rob's syntax checker robot here, are there real
> > problems? Sometimes it complains about things being changed
> > in the DTS files at the same time.
>
> Rob's checker reports issue that are being fixed by respective
> patches. I think I've updated all dts entries for the mpp devices tree
> nodes.
>
> > I could apply all of this (including DTS changes) to an immutable
> > branch and offer to Bjorn if he is fine with the patches and
> > the general approach.
>
> I'm fine with either approach.

Let's see what Bjorn says, if nothing happens poke me again and I'll
create an immutable branch and merge it.

Yours,
Linus Walleij
Bjorn Andersson Oct. 17, 2021, 4:52 p.m. UTC | #5
On Thu 07 Oct 20:25 CDT 2021, Dmitry Baryshkov wrote:

> The probing of this driver calls platform_irq_count, which will
> setup all of the IRQs that are configured in device tree. In
> preparation for converting this driver to be a hierarchical IRQ
> chip, hardcode the IRQ count based on the hardware type so that all
> the IRQs are not configured immediately and are configured on an
> as-needed basis later in the boot process.
> 
> This change will also allow for the removal of the interrupts property
> later in this patch series once the hierarchical IRQ chip support is in.
> 
> This patch also removes the generic qcom,ssbi-mpp OF match since we
> don't know the number of pins. All of the existing upstream bindings
> already include the more-specific binding.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c b/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
> index 92e7f2602847..a90cada1d657 100644
> --- a/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
> +++ b/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
> @@ -733,13 +733,12 @@ static int pm8xxx_pin_populate(struct pm8xxx_mpp *pctrl,
>  }
>  
>  static const struct of_device_id pm8xxx_mpp_of_match[] = {
> -	{ .compatible = "qcom,pm8018-mpp" },
> -	{ .compatible = "qcom,pm8038-mpp" },
> -	{ .compatible = "qcom,pm8058-mpp" },
> -	{ .compatible = "qcom,pm8917-mpp" },
> -	{ .compatible = "qcom,pm8821-mpp" },
> -	{ .compatible = "qcom,pm8921-mpp" },
> -	{ .compatible = "qcom,ssbi-mpp" },
> +	{ .compatible = "qcom,pm8018-mpp", .data = (void *) 6 },
> +	{ .compatible = "qcom,pm8038-mpp", .data = (void *) 6 },
> +	{ .compatible = "qcom,pm8058-mpp", .data = (void *) 12 },
> +	{ .compatible = "qcom,pm8821-mpp", .data = (void *) 4 },
> +	{ .compatible = "qcom,pm8917-mpp", .data = (void *) 10 },
> +	{ .compatible = "qcom,pm8921-mpp", .data = (void *) 12 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, pm8xxx_mpp_of_match);
> @@ -750,19 +749,14 @@ static int pm8xxx_mpp_probe(struct platform_device *pdev)
>  	struct pinctrl_pin_desc *pins;
>  	struct pm8xxx_mpp *pctrl;
>  	int ret;
> -	int i, npins;
> +	int i;
>  
>  	pctrl = devm_kzalloc(&pdev->dev, sizeof(*pctrl), GFP_KERNEL);
>  	if (!pctrl)
>  		return -ENOMEM;
>  
>  	pctrl->dev = &pdev->dev;
> -	npins = platform_irq_count(pdev);
> -	if (!npins)
> -		return -EINVAL;
> -	if (npins < 0)
> -		return npins;
> -	pctrl->npins = npins;
> +	pctrl->npins = (uintptr_t) device_get_match_data(&pdev->dev);
>  
>  	pctrl->regmap = dev_get_regmap(pdev->dev.parent, NULL);
>  	if (!pctrl->regmap) {
> -- 
> 2.30.2
>
Bjorn Andersson Oct. 17, 2021, 4:53 p.m. UTC | #6
On Thu 07 Oct 20:25 CDT 2021, Dmitry Baryshkov wrote:

> ssbi-mpp did not have any irqchip support so consumers of this in
> device tree would need to call gpio[d]_to_irq() in order to get the
> proper IRQ on the underlying PMIC. IRQ chips in device tree should be
> usable from the start without the consumer having to make an additional
> call to get the proper IRQ on the parent. This patch adds hierarchical
> IRQ chip support to the ssbi-mpp code to correct this issue.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c | 111 ++++++++++++++++++++----
>  1 file changed, 93 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c b/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
> index a90cada1d657..842940594c4a 100644
> --- a/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
> +++ b/drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c
> @@ -87,7 +87,6 @@
>  /**
>   * struct pm8xxx_pin_data - dynamic configuration for a pin
>   * @reg:		address of the control register
> - * @irq:		IRQ from the PMIC interrupt controller
>   * @mode:		operating mode for the pin (digital, analog or current sink)
>   * @input:		pin is input
>   * @output:		pin is output
> @@ -103,7 +102,6 @@
>   */
>  struct pm8xxx_pin_data {
>  	unsigned reg;
> -	int irq;
>  
>  	u8 mode;
>  
> @@ -126,6 +124,7 @@ struct pm8xxx_mpp {
>  	struct regmap *regmap;
>  	struct pinctrl_dev *pctrl;
>  	struct gpio_chip chip;
> +	struct irq_chip irq;
>  
>  	struct pinctrl_desc desc;
>  	unsigned npins;
> @@ -148,6 +147,8 @@ static const struct pin_config_item pm8xxx_conf_items[] = {
>  #endif
>  
>  #define PM8XXX_MAX_MPPS	12
> +#define PM8XXX_MPP_PHYSICAL_OFFSET    1
> +
>  static const char * const pm8xxx_groups[PM8XXX_MAX_MPPS] = {
>  	"mpp1", "mpp2", "mpp3", "mpp4", "mpp5", "mpp6", "mpp7", "mpp8",
>  	"mpp9", "mpp10", "mpp11", "mpp12",
> @@ -492,12 +493,16 @@ static int pm8xxx_mpp_get(struct gpio_chip *chip, unsigned offset)
>  	struct pm8xxx_mpp *pctrl = gpiochip_get_data(chip);
>  	struct pm8xxx_pin_data *pin = pctrl->desc.pins[offset].drv_data;
>  	bool state;
> -	int ret;
> +	int ret, irq;
>  
>  	if (!pin->input)
>  		return !!pin->output_value;
>  
> -	ret = irq_get_irqchip_state(pin->irq, IRQCHIP_STATE_LINE_LEVEL, &state);
> +	irq = chip->to_irq(chip, offset);
> +	if (irq < 0)
> +		return irq;
> +
> +	ret = irq_get_irqchip_state(irq, IRQCHIP_STATE_LINE_LEVEL, &state);
>  	if (!ret)
>  		ret = !!state;
>  
> @@ -524,18 +529,10 @@ static int pm8xxx_mpp_of_xlate(struct gpio_chip *chip,
>  	if (flags)
>  		*flags = gpio_desc->args[1];
>  
> -	return gpio_desc->args[0] - 1;
> +	return gpio_desc->args[0] - PM8XXX_MPP_PHYSICAL_OFFSET;
>  }
>  
>  
> -static int pm8xxx_mpp_to_irq(struct gpio_chip *chip, unsigned offset)
> -{
> -	struct pm8xxx_mpp *pctrl = gpiochip_get_data(chip);
> -	struct pm8xxx_pin_data *pin = pctrl->desc.pins[offset].drv_data;
> -
> -	return pin->irq;
> -}
> -
>  #ifdef CONFIG_DEBUG_FS
>  #include <linux/seq_file.h>
>  
> @@ -558,7 +555,7 @@ static void pm8xxx_mpp_dbg_show_one(struct seq_file *s,
>  		"abus3",
>  	};
>  
> -	seq_printf(s, " mpp%-2d:", offset + 1);
> +	seq_printf(s, " mpp%-2d:", offset + PM8XXX_MPP_PHYSICAL_OFFSET);
>  
>  	switch (pin->mode) {
>  	case PM8XXX_MPP_DIGITAL:
> @@ -640,7 +637,6 @@ static const struct gpio_chip pm8xxx_mpp_template = {
>  	.get = pm8xxx_mpp_get,
>  	.set = pm8xxx_mpp_set,
>  	.of_xlate = pm8xxx_mpp_of_xlate,
> -	.to_irq = pm8xxx_mpp_to_irq,
>  	.dbg_show = pm8xxx_mpp_dbg_show,
>  	.owner = THIS_MODULE,
>  };
> @@ -732,6 +728,55 @@ static int pm8xxx_pin_populate(struct pm8xxx_mpp *pctrl,
>  	return 0;
>  }
>  
> +static int pm8xxx_mpp_domain_translate(struct irq_domain *domain,
> +				   struct irq_fwspec *fwspec,
> +				   unsigned long *hwirq,
> +				   unsigned int *type)
> +{
> +	struct pm8xxx_mpp *pctrl = container_of(domain->host_data,
> +						 struct pm8xxx_mpp, chip);
> +
> +	if (fwspec->param_count != 2 ||
> +	    fwspec->param[0] < PM8XXX_MPP_PHYSICAL_OFFSET ||
> +	    fwspec->param[0] > pctrl->chip.ngpio)
> +		return -EINVAL;
> +
> +	*hwirq = fwspec->param[0] - PM8XXX_MPP_PHYSICAL_OFFSET;
> +	*type = fwspec->param[1];
> +
> +	return 0;
> +}
> +
> +static unsigned int pm8xxx_mpp_child_offset_to_irq(struct gpio_chip *chip,
> +						   unsigned int offset)
> +{
> +	return offset + PM8XXX_MPP_PHYSICAL_OFFSET;
> +}
> +
> +static int pm8821_mpp_child_to_parent_hwirq(struct gpio_chip *chip,
> +					    unsigned int child_hwirq,
> +					    unsigned int child_type,
> +					    unsigned int *parent_hwirq,
> +					    unsigned int *parent_type)
> +{
> +	*parent_hwirq = child_hwirq + 24;
> +	*parent_type = child_type;
> +
> +	return 0;
> +}
> +
> +static int pm8xxx_mpp_child_to_parent_hwirq(struct gpio_chip *chip,
> +					    unsigned int child_hwirq,
> +					    unsigned int child_type,
> +					    unsigned int *parent_hwirq,
> +					    unsigned int *parent_type)
> +{
> +	*parent_hwirq = child_hwirq + 0x80;
> +	*parent_type = child_type;
> +
> +	return 0;
> +}
> +
>  static const struct of_device_id pm8xxx_mpp_of_match[] = {
>  	{ .compatible = "qcom,pm8018-mpp", .data = (void *) 6 },
>  	{ .compatible = "qcom,pm8038-mpp", .data = (void *) 6 },
> @@ -746,7 +791,10 @@ MODULE_DEVICE_TABLE(of, pm8xxx_mpp_of_match);
>  static int pm8xxx_mpp_probe(struct platform_device *pdev)
>  {
>  	struct pm8xxx_pin_data *pin_data;
> +	struct irq_domain *parent_domain;
> +	struct device_node *parent_node;
>  	struct pinctrl_pin_desc *pins;
> +	struct gpio_irq_chip *girq;
>  	struct pm8xxx_mpp *pctrl;
>  	int ret;
>  	int i;
> @@ -783,9 +831,6 @@ static int pm8xxx_mpp_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < pctrl->desc.npins; i++) {
>  		pin_data[i].reg = SSBI_REG_ADDR_MPP(i);
> -		pin_data[i].irq = platform_get_irq(pdev, i);
> -		if (pin_data[i].irq < 0)
> -			return pin_data[i].irq;
>  
>  		ret = pm8xxx_pin_populate(pctrl, &pin_data[i]);
>  		if (ret)
> @@ -816,6 +861,36 @@ static int pm8xxx_mpp_probe(struct platform_device *pdev)
>  	pctrl->chip.of_gpio_n_cells = 2;
>  	pctrl->chip.label = dev_name(pctrl->dev);
>  	pctrl->chip.ngpio = pctrl->npins;
> +
> +	parent_node = of_irq_find_parent(pctrl->dev->of_node);
> +	if (!parent_node)
> +		return -ENXIO;
> +
> +	parent_domain = irq_find_host(parent_node);
> +	of_node_put(parent_node);
> +	if (!parent_domain)
> +		return -ENXIO;
> +
> +	pctrl->irq.name = "ssbi-mpp";
> +	pctrl->irq.irq_mask_ack = irq_chip_mask_ack_parent;
> +	pctrl->irq.irq_unmask = irq_chip_unmask_parent;
> +	pctrl->irq.irq_set_type = irq_chip_set_type_parent;
> +	pctrl->irq.flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_SKIP_SET_WAKE;
> +
> +	girq = &pctrl->chip.irq;
> +	girq->chip = &pctrl->irq;
> +	girq->default_type = IRQ_TYPE_NONE;
> +	girq->handler = handle_level_irq;
> +	girq->fwnode = of_node_to_fwnode(pctrl->dev->of_node);
> +	girq->parent_domain = parent_domain;
> +	if (of_device_is_compatible(pdev->dev.of_node, "qcom,pm8821-mpp"))
> +		girq->child_to_parent_hwirq = pm8821_mpp_child_to_parent_hwirq;
> +	else
> +		girq->child_to_parent_hwirq = pm8xxx_mpp_child_to_parent_hwirq;
> +	girq->populate_parent_alloc_arg = gpiochip_populate_parent_fwspec_twocell;
> +	girq->child_offset_to_irq = pm8xxx_mpp_child_offset_to_irq;
> +	girq->child_irq_domain_ops.translate = pm8xxx_mpp_domain_translate;
> +
>  	ret = gpiochip_add_data(&pctrl->chip, pctrl);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed register gpiochip\n");
> -- 
> 2.30.2
>
Bjorn Andersson Oct. 17, 2021, 4:53 p.m. UTC | #7
On Thu 07 Oct 20:25 CDT 2021, Dmitry Baryshkov wrote:

> The probing of this driver calls platform_irq_count, which will
> setup all of the IRQs that are configured in device tree. In
> preparation for converting this driver to be a hierarchical IRQ
> chip, hardcode the IRQ count based on the hardware type so that all
> the IRQs are not configured immediately and are configured on an
> as-needed basis later in the boot process.
> 
> This change will also allow for the removal of the interrupts property
> later in this patch series once the hierarchical IRQ chip support is in.
> 
> This patch also removes the generic qcom,spmi-mpp OF match since we
> don't know the number of pins. All of the existing upstream bindings
> already include the more-specific binding.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  drivers/pinctrl/qcom/pinctrl-spmi-mpp.c | 25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c b/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
> index 2da9b5f68f3f..a9f994863126 100644
> --- a/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
> @@ -812,11 +812,7 @@ static int pmic_mpp_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	npins = platform_irq_count(pdev);
> -	if (!npins)
> -		return -EINVAL;
> -	if (npins < 0)
> -		return npins;
> +	npins = (uintptr_t) device_get_match_data(&pdev->dev);
>  
>  	BUG_ON(npins > ARRAY_SIZE(pmic_mpp_groups));
>  
> @@ -912,16 +908,15 @@ static int pmic_mpp_remove(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id pmic_mpp_of_match[] = {
> -	{ .compatible = "qcom,pm8019-mpp" },	/* 6 MPP's */
> -	{ .compatible = "qcom,pm8841-mpp" },	/* 4 MPP's */
> -	{ .compatible = "qcom,pm8916-mpp" },	/* 4 MPP's */
> -	{ .compatible = "qcom,pm8941-mpp" },	/* 8 MPP's */
> -	{ .compatible = "qcom,pm8950-mpp" },	/* 4 MPP's */
> -	{ .compatible = "qcom,pmi8950-mpp" },	/* 4 MPP's */
> -	{ .compatible = "qcom,pm8994-mpp" },	/* 8 MPP's */
> -	{ .compatible = "qcom,pma8084-mpp" },	/* 8 MPP's */
> -	{ .compatible = "qcom,pmi8994-mpp" },	/* 4 MPP's */
> -	{ .compatible = "qcom,spmi-mpp" },	/* Generic */
> +	{ .compatible = "qcom,pm8019-mpp", .data = (void *) 6 },
> +	{ .compatible = "qcom,pm8841-mpp", .data = (void *) 4 },
> +	{ .compatible = "qcom,pm8916-mpp", .data = (void *) 4 },
> +	{ .compatible = "qcom,pm8941-mpp", .data = (void *) 8 },
> +	{ .compatible = "qcom,pm8950-mpp", .data = (void *) 4 },
> +	{ .compatible = "qcom,pmi8950-mpp", .data = (void *) 4 },
> +	{ .compatible = "qcom,pm8994-mpp", .data = (void *) 8 },
> +	{ .compatible = "qcom,pma8084-mpp", .data = (void *) 8 },
> +	{ .compatible = "qcom,pmi8994-mpp", .data = (void *) 4 },
>  	{ },
>  };
>  
> -- 
> 2.30.2
>
Bjorn Andersson Oct. 17, 2021, 4:53 p.m. UTC | #8
On Thu 07 Oct 20:25 CDT 2021, Dmitry Baryshkov wrote:

> spmi-mpp did not have any irqchip support so consumers of this in
> device tree would need to call gpio[d]_to_irq() in order to get the
> proper IRQ on the underlying PMIC. IRQ chips in device tree should be
> usable from the start without the consumer having to make an additional
> call to get the proper IRQ on the parent. This patch adds hierarchical
> IRQ chip support to the spmi-mpp code to correct this issue.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  drivers/pinctrl/qcom/pinctrl-spmi-mpp.c | 86 ++++++++++++++++++++-----
>  1 file changed, 69 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c b/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
> index a9f994863126..b80723928b7e 100644
> --- a/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
> @@ -103,7 +103,6 @@
>  /**
>   * struct pmic_mpp_pad - keep current MPP settings
>   * @base: Address base in SPMI device.
> - * @irq: IRQ number which this MPP generate.
>   * @is_enabled: Set to false when MPP should be put in high Z state.
>   * @out_value: Cached pin output value.
>   * @output_enabled: Set to true if MPP output logic is enabled.
> @@ -121,7 +120,6 @@
>   */
>  struct pmic_mpp_pad {
>  	u16		base;
> -	int		irq;
>  	bool		is_enabled;
>  	bool		out_value;
>  	bool		output_enabled;
> @@ -143,6 +141,7 @@ struct pmic_mpp_state {
>  	struct regmap	*map;
>  	struct pinctrl_dev *ctrl;
>  	struct gpio_chip chip;
> +	struct irq_chip irq;
>  };
>  
>  static const struct pinconf_generic_params pmic_mpp_bindings[] = {
> @@ -622,16 +621,6 @@ static int pmic_mpp_of_xlate(struct gpio_chip *chip,
>  	return gpio_desc->args[0] - PMIC_MPP_PHYSICAL_OFFSET;
>  }
>  
> -static int pmic_mpp_to_irq(struct gpio_chip *chip, unsigned pin)
> -{
> -	struct pmic_mpp_state *state = gpiochip_get_data(chip);
> -	struct pmic_mpp_pad *pad;
> -
> -	pad = state->ctrl->desc->pins[pin].drv_data;
> -
> -	return pad->irq;
> -}
> -
>  static void pmic_mpp_dbg_show(struct seq_file *s, struct gpio_chip *chip)
>  {
>  	struct pmic_mpp_state *state = gpiochip_get_data(chip);
> @@ -651,7 +640,6 @@ static const struct gpio_chip pmic_mpp_gpio_template = {
>  	.request		= gpiochip_generic_request,
>  	.free			= gpiochip_generic_free,
>  	.of_xlate		= pmic_mpp_of_xlate,
> -	.to_irq			= pmic_mpp_to_irq,
>  	.dbg_show		= pmic_mpp_dbg_show,
>  };
>  
> @@ -796,13 +784,53 @@ static int pmic_mpp_populate(struct pmic_mpp_state *state,
>  	return 0;
>  }
>  
> +static int pmic_mpp_domain_translate(struct irq_domain *domain,
> +				      struct irq_fwspec *fwspec,
> +				      unsigned long *hwirq,
> +				      unsigned int *type)
> +{
> +	struct pmic_mpp_state *state = container_of(domain->host_data,
> +						     struct pmic_mpp_state,
> +						     chip);
> +
> +	if (fwspec->param_count != 2 ||
> +	    fwspec->param[0] < 1 || fwspec->param[0] > state->chip.ngpio)
> +		return -EINVAL;
> +
> +	*hwirq = fwspec->param[0] - PMIC_MPP_PHYSICAL_OFFSET;
> +	*type = fwspec->param[1];
> +
> +	return 0;
> +}
> +
> +static unsigned int pmic_mpp_child_offset_to_irq(struct gpio_chip *chip,
> +						  unsigned int offset)
> +{
> +	return offset + PMIC_MPP_PHYSICAL_OFFSET;
> +}
> +
> +static int pmic_mpp_child_to_parent_hwirq(struct gpio_chip *chip,
> +					   unsigned int child_hwirq,
> +					   unsigned int child_type,
> +					   unsigned int *parent_hwirq,
> +					   unsigned int *parent_type)
> +{
> +	*parent_hwirq = child_hwirq + 0xc0;
> +	*parent_type = child_type;
> +
> +	return 0;
> +}
> +
>  static int pmic_mpp_probe(struct platform_device *pdev)
>  {
> +	struct irq_domain *parent_domain;
> +	struct device_node *parent_node;
>  	struct device *dev = &pdev->dev;
>  	struct pinctrl_pin_desc *pindesc;
>  	struct pinctrl_desc *pctrldesc;
>  	struct pmic_mpp_pad *pad, *pads;
>  	struct pmic_mpp_state *state;
> +	struct gpio_irq_chip *girq;
>  	int ret, npins, i;
>  	u32 reg;
>  
> @@ -857,10 +885,6 @@ static int pmic_mpp_probe(struct platform_device *pdev)
>  		pindesc->number = i;
>  		pindesc->name = pmic_mpp_groups[i];
>  
> -		pad->irq = platform_get_irq(pdev, i);
> -		if (pad->irq < 0)
> -			return pad->irq;
> -
>  		pad->base = reg + i * PMIC_MPP_ADDRESS_RANGE;
>  
>  		ret = pmic_mpp_populate(state, pad);
> @@ -880,6 +904,34 @@ static int pmic_mpp_probe(struct platform_device *pdev)
>  	if (IS_ERR(state->ctrl))
>  		return PTR_ERR(state->ctrl);
>  
> +	parent_node = of_irq_find_parent(state->dev->of_node);
> +	if (!parent_node)
> +		return -ENXIO;
> +
> +	parent_domain = irq_find_host(parent_node);
> +	of_node_put(parent_node);
> +	if (!parent_domain)
> +		return -ENXIO;
> +
> +	state->irq.name = "spmi-mpp",
> +	state->irq.irq_ack = irq_chip_ack_parent,
> +	state->irq.irq_mask = irq_chip_mask_parent,
> +	state->irq.irq_unmask = irq_chip_unmask_parent,
> +	state->irq.irq_set_type = irq_chip_set_type_parent,
> +	state->irq.irq_set_wake = irq_chip_set_wake_parent,
> +	state->irq.flags = IRQCHIP_MASK_ON_SUSPEND,
> +
> +	girq = &state->chip.irq;
> +	girq->chip = &state->irq;
> +	girq->default_type = IRQ_TYPE_NONE;
> +	girq->handler = handle_level_irq;
> +	girq->fwnode = of_node_to_fwnode(state->dev->of_node);
> +	girq->parent_domain = parent_domain;
> +	girq->child_to_parent_hwirq = pmic_mpp_child_to_parent_hwirq;
> +	girq->populate_parent_alloc_arg = gpiochip_populate_parent_fwspec_fourcell;
> +	girq->child_offset_to_irq = pmic_mpp_child_offset_to_irq;
> +	girq->child_irq_domain_ops.translate = pmic_mpp_domain_translate;
> +
>  	ret = gpiochip_add_data(&state->chip, state);
>  	if (ret) {
>  		dev_err(state->dev, "can't add gpio chip\n");
> -- 
> 2.30.2
>
Bjorn Andersson Oct. 17, 2021, 4:54 p.m. UTC | #9
On Tue 12 Oct 18:59 CDT 2021, Linus Walleij wrote:

> On Fri, Oct 8, 2021 at 3:25 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> 
> > In 2019 (in kernel 5.4) spmi-gpio and ssbi-gpio drivers were converted
> > to hierarchical IRQ helpers, however MPP drivers were not converted at
> > that moment. Complete this by converting MPP drivers.
> >
> > Changes since v2:
> >  - Add patches fixing/updating mpps nodes in the existing device trees
> 
> Thanks a *lot* for being thorough and fixing all this properly!
> 
> I am happy to apply the pinctrl portions to the pinctrl tree, I'm
> uncertain about Rob's syntax checker robot here, are there real
> problems? Sometimes it complains about things being changed
> in the DTS files at the same time.
> 
> I could apply all of this (including DTS changes) to an immutable
> branch and offer to Bjorn if he is fine with the patches and
> the general approach.
> 

I like the driver changes and I'm wrapping up a second pull for the dts
pieces in the coming few days. So if you're happy to take the driver
patches I'll include the DT changes for 5.16 as well.

Thanks,
Bjorn
Linus Walleij Oct. 17, 2021, 9:31 p.m. UTC | #10
On Sun, Oct 17, 2021 at 6:54 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:

> I like the driver changes and I'm wrapping up a second pull for the dts
> pieces in the coming few days. So if you're happy to take the driver
> patches I'll include the DT changes for 5.16 as well.

OK let's do like that. I'll queue the binding changes and driver
changes so we finally get this fixed up.

Yours,
Linus Walleij
Linus Walleij Oct. 17, 2021, 9:35 p.m. UTC | #11
I queued thes patches in the pinctrl tree for v5.16:

On Fri, Oct 8, 2021 at 3:25 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:

>       dt-bindings: pinctrl: qcom,pmic-mpp: Convert qcom pmic mpp bindings to YAML
>       pinctrl: qcom: ssbi-mpp: hardcode IRQ counts
>       pinctrl: qcom: ssbi-mpp: add support for hierarchical IRQ chip
>       pinctrl: qcom: spmi-mpp: hardcode IRQ counts
>       pinctrl: qcom: spmi-mpp: add support for hierarchical IRQ chip
>       dt-bindings: pinctrl: qcom,pmic-mpp: switch to #interrupt-cells

Any breakages will be fixed when Bjorn applies the DTS changes to his
tree.

I wonder about the MFD patch, maybe Lee can expedite merging that too
or ACK it for Bjorn to merge with the remainders.

Yours,
Linus Walleij
Bjorn Andersson Oct. 18, 2021, 12:13 a.m. UTC | #12
On Fri, 8 Oct 2021 04:24:59 +0300, Dmitry Baryshkov wrote:
> In 2019 (in kernel 5.4) spmi-gpio and ssbi-gpio drivers were converted
> to hierarchical IRQ helpers, however MPP drivers were not converted at
> that moment. Complete this by converting MPP drivers.
> 
> Changes since v2:
>  - Add patches fixing/updating mpps nodes in the existing device trees
> 
> [...]

Applied, thanks!

[03/25] ARM: dts: qcom-apq8064: add gpio-ranges to mpps nodes
        commit: 9be51f0b16ef83208fbfdc42fe59a622b6beee4c
[04/25] ARM: dts: qcom-msm8660: add gpio-ranges to mpps nodes
        commit: cd1049b631d05ad25b7976cf67144277598e72f2
[05/25] ARM: dts: qcom-pm8841: add gpio-ranges to mpps nodes
        commit: 6a91e584a3a0a247f836f063cbd3d99b1babaf4c
[06/25] ARM: dts: qcom-pm8941: add gpio-ranges to mpps nodes
        commit: 72af8d006b68cb88ae618d812b1053e59b06fe56
[07/25] ARM: dts: qcom-pma8084: add gpio-ranges to mpps nodes
        commit: 50ec4abed12cd0d5d34656330bb82192d607b3b7
[08/25] ARM: dts: qcom-mdm9615: add gpio-ranges to mpps node, fix its name
        commit: 7cf05e3b457b4d0eea385ad0acec327ee0adc5a1
[09/25] ARM: dts: qcom-apq8060-dragonboard: fix mpps state names
        commit: 636396efe303345cba6b0084b3228cf861d22e36
[18/25] ARM: dts: qcom-apq8064: add interrupt controller properties
        commit: 216f41938d669e7949964c181350cb61b4fdda03
[19/25] ARM: dts: qcom-mdm9615: add interrupt controller properties
        commit: f574aa0b12403dd0f4bef366199bfba860188086
[20/25] ARM: dts: qcom-msm8660: add interrupt controller properties
        commit: 789a247a3f10985ddae58a975e2550a35388ca52
[21/25] ARM: dts: qcom-pm8841: add interrupt controller properties
        commit: 3dca61a70c0453ea02089059d9d435a7b9b104ce
[22/25] ARM: dts: qcom-pm8941: add interrupt controller properties
        commit: 9fb04774f3436f93075b80870fd94e2e68f8bf04
[23/25] ARM: dts: qcom-pma8084: add interrupt controller properties
        commit: a7fe01561e6cda173b1fffb1c8552040933e7588

Best regards,