[v2,1/5] spmi: pmic-arb: hardcode IRQ counts

The probing of this driver calls platform_irq_count, which will setup
all of the IRQs that are available. This is a problem since some of
these IRQs may be setup in an IRQ hierarchy later in the boot process
by spmi-gpio. This will cause these hwirqs to be associated with
multiple Linux virqs and interrupts will not work as expected. This
patch changes the pmic-arb driver so that the IRQ counts are hard
coded in the data field of the of_device_id structure so that IRQs
are setup on an as-needed basis.

This patch also removes the generic qcom,spmi-gpio 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: Brian Masney <masneyb@onstation.org>
---
Note: The qcom,pms405-gpio looks suspicious to me. The existing code
will start at gpio1, which is supposed to be a hole according to the
comment. I can fix this in a later patch if there is a desire. I didn't
do it now to try to keep this series as small as possible. Its no
worse than what was there before.

 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 37 ++++++++++++------------
 1 file changed, 19 insertions(+), 18 deletions(-)

Hi Brian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on pinctrl/devel]
[also build test WARNING on v5.0-rc1 next-20190103]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Brian-Masney/qcom-spmi-add-support-for-hierarchical-IRQ-chip/20190107-110503
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All warnings (new ones prefixed by >>):

   drivers/pinctrl//qcom/pinctrl-spmi-gpio.c: In function 'pmic_gpio_probe':
>> drivers/pinctrl//qcom/pinctrl-spmi-gpio.c:974:10: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     npins = (int) of_id->data;
             ^

vim +974 drivers/pinctrl//qcom/pinctrl-spmi-gpio.c

   952	
   953	static int pmic_gpio_probe(struct platform_device *pdev)
   954	{
   955		const struct of_device_id *of_id;
   956		struct device *dev = &pdev->dev;
   957		struct pinctrl_pin_desc *pindesc;
   958		struct pinctrl_desc *pctrldesc;
   959		struct pmic_gpio_pad *pad, *pads;
   960		struct pmic_gpio_state *state;
   961		int ret, npins, i;
   962		u32 reg;
   963	
   964		ret = of_property_read_u32(dev->of_node, "reg", &reg);
   965		if (ret < 0) {
   966			dev_err(dev, "missing base address");
   967			return ret;
   968		}
   969	
   970		of_id = of_match_device(pmic_gpio_of_match, &pdev->dev);
   971		if (!of_id)
   972			return -EINVAL;
   973	
 > 974		npins = (int) of_id->data;
   975	
   976		state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
   977		if (!state)
   978			return -ENOMEM;
   979	
   980		platform_set_drvdata(pdev, state);
   981	
   982		state->dev = &pdev->dev;
   983		state->map = dev_get_regmap(dev->parent, NULL);
   984	
   985		pindesc = devm_kcalloc(dev, npins, sizeof(*pindesc), GFP_KERNEL);
   986		if (!pindesc)
   987			return -ENOMEM;
   988	
   989		pads = devm_kcalloc(dev, npins, sizeof(*pads), GFP_KERNEL);
   990		if (!pads)
   991			return -ENOMEM;
   992	
   993		pctrldesc = devm_kzalloc(dev, sizeof(*pctrldesc), GFP_KERNEL);
   994		if (!pctrldesc)
   995			return -ENOMEM;
   996	
   997		pctrldesc->pctlops = &pmic_gpio_pinctrl_ops;
   998		pctrldesc->pmxops = &pmic_gpio_pinmux_ops;
   999		pctrldesc->confops = &pmic_gpio_pinconf_ops;
  1000		pctrldesc->owner = THIS_MODULE;
  1001		pctrldesc->name = dev_name(dev);
  1002		pctrldesc->pins = pindesc;
  1003		pctrldesc->npins = npins;
  1004		pctrldesc->num_custom_params = ARRAY_SIZE(pmic_gpio_bindings);
  1005		pctrldesc->custom_params = pmic_gpio_bindings;
  1006	#ifdef CONFIG_DEBUG_FS
  1007		pctrldesc->custom_conf_items = pmic_conf_items;
  1008	#endif
  1009	
  1010		for (i = 0; i < npins; i++, pindesc++) {
  1011			pad = &pads[i];
  1012			pindesc->drv_data = pad;
  1013			pindesc->number = i;
  1014			pindesc->name = pmic_gpio_groups[i];
  1015	
  1016			pad->irq = platform_get_irq(pdev, i);
  1017			if (pad->irq < 0)
  1018				return pad->irq;
  1019	
  1020			pad->base = reg + i * PMIC_GPIO_ADDRESS_RANGE;
  1021	
  1022			ret = pmic_gpio_populate(state, pad);
  1023			if (ret < 0)
  1024				return ret;
  1025		}
  1026	
  1027		state->chip = pmic_gpio_gpio_template;
  1028		state->chip.parent = dev;
  1029		state->chip.base = -1;
  1030		state->chip.ngpio = npins;
  1031		state->chip.label = dev_name(dev);
  1032		state->chip.of_gpio_n_cells = 2;
  1033		state->chip.can_sleep = false;
  1034	
  1035		state->ctrl = devm_pinctrl_register(dev, pctrldesc, state);
  1036		if (IS_ERR(state->ctrl))
  1037			return PTR_ERR(state->ctrl);
  1038	
  1039		ret = gpiochip_add_data(&state->chip, state);
  1040		if (ret) {
  1041			dev_err(state->dev, "can't add gpio chip\n");
  1042			return ret;
  1043		}
  1044	
  1045		/*
  1046		 * For DeviceTree-supported systems, the gpio core checks the
  1047		 * pinctrl's device node for the "gpio-ranges" property.
  1048		 * If it is present, it takes care of adding the pin ranges
  1049		 * for the driver. In this case the driver can skip ahead.
  1050		 *
  1051		 * In order to remain compatible with older, existing DeviceTree
  1052		 * files which don't set the "gpio-ranges" property or systems that
  1053		 * utilize ACPI the driver has to call gpiochip_add_pin_range().
  1054		 */
  1055		if (!of_property_read_bool(dev->of_node, "gpio-ranges")) {
  1056			ret = gpiochip_add_pin_range(&state->chip, dev_name(dev), 0, 0,
  1057						     npins);
  1058			if (ret) {
  1059				dev_err(dev, "failed to add pin range\n");
  1060				goto err_range;
  1061			}
  1062		}
  1063	
  1064		return 0;
  1065	
  1066	err_range:
  1067		gpiochip_remove(&state->chip);
  1068		return ret;
  1069	}
  1070	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

On Mon, Jan 7, 2019 at 3:11 AM Brian Masney <masneyb@onstation.org> wrote:

> Note: The qcom,pms405-gpio looks suspicious to me. The existing code
> will start at gpio1, which is supposed to be a hole according to the
> comment. I can fix this in a later patch if there is a desire. I didn't
> do it now to try to keep this series as small as possible. Its no
> worse than what was there before.

If the range has holes the valid_mask should be set up to mask
them off for this variant.

That would be a separate patch though and hardly your problem
as you say.

Yours,
Linus Walleij

Quoting Brian Masney (2019-01-06 18:11:41)
> @@ -1062,19 +1076,6 @@ static int pmic_gpio_remove(struct platform_device *pdev)
>         return 0;
>  }
>  
> -static const struct of_device_id pmic_gpio_of_match[] = {
> -       { .compatible = "qcom,pm8916-gpio" },   /* 4 GPIO's */
> -       { .compatible = "qcom,pm8941-gpio" },   /* 36 GPIO's */
> -       { .compatible = "qcom,pm8994-gpio" },   /* 22 GPIO's */
> -       { .compatible = "qcom,pmi8994-gpio" },  /* 10 GPIO's */
> -       { .compatible = "qcom,pma8084-gpio" },  /* 22 GPIO's */
> -       { .compatible = "qcom,pms405-gpio" },   /* 12 GPIO's, holes on 1 9 10 */
> -       { .compatible = "qcom,spmi-gpio" }, /* Generic */
> -       { },
> -};
> -
> -MODULE_DEVICE_TABLE(of, pmic_gpio_of_match);
> -

Please don't move the array. It isn't necessary if you use
device_get_match_data() instead of of_match_device().