On 18/04/2023 17:28, nick.hawkins@xxxxxxx wrote:
> From: Nick Hawkins <nick.hawkins@xxxxxxx>
>
> The GXP SoC supports GPIO on multiple interfaces: Host, CPLD and Soc
> pins. The interface from CPLD and Host are interruptable from vic0
> and vic1. This driver allows support for both of these interfaces
> through the use of different compatible bindings.
>
> Signed-off-by: Nick Hawkins <nick.hawkins@xxxxxxx>
> ---
> drivers/gpio/Kconfig | 9 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-gxp.c | 1056 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1066 insertions(+)
> create mode 100644 drivers/gpio/gpio-gxp.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 13be729710f2..47435307698c 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1235,6 +1235,15 @@ config HTC_EGPIO
> several HTC phones. It provides basic support for input
> pins, output pins, and IRQs.
>
> +config GPIO_GXP
> + tristate "GXP GPIO support"
> + depends on ARCH_HPE_GXP
|| COMPILE_TEST
(...)
> +
> + drvdata->chip.parent = &pdev->dev;
> + ret = devm_gpiochip_add_data(&pdev->dev, &drvdata->chip, NULL);
> + if (ret < 0)
> + dev_err(&pdev->dev,
> + "Could not register gpiochip for fn2, %d\n", ret);
> + dev_info(&pdev->dev, "HPE GXP FN2 driver loaded.\n");
Drop non-informative probe successes. They are not needed.
> +
> + return 0;
> +}
> +
> +static int gxp_gpio_pl_init(struct platform_device *pdev)
> +{
> + struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(&pdev->dev);
> + struct gpio_irq_chip *girq;
> + int ret;
> + unsigned int val;
> +
> + drvdata->pl_led = gxp_gpio_init_regmap(pdev, "pl-led", 8);
> + if (IS_ERR(drvdata->pl_led))
> + return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->pl_int),
> + "failed to map pl_led\n");
> +
> + drvdata->pl_health = gxp_gpio_init_regmap(pdev, "pl-health", 8);
> + if (IS_ERR(drvdata->pl_health))
> + return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->pl_int),
> + "failed to map pl_health\n");
> +
> + drvdata->pl_int = gxp_gpio_init_regmap(pdev, "pl-int", 8);
> + if (IS_ERR(drvdata->pl_int))
> + return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->pl_int),
> + "failed to map pl_int\n");
> +
> + drvdata->chip = plreg_chip;
> + drvdata->chip.ngpio = 100;
> + drvdata->chip.parent = &pdev->dev;
> +
> + girq = &drvdata->chip.irq;
> + girq->chip = &gxp_plreg_irqchip;
> + girq->parent_handler = NULL;
> + girq->num_parents = 0;
> + girq->parents = NULL;
> + girq->default_type = IRQ_TYPE_NONE;
> + girq->handler = handle_edge_irq;
> +
> + girq->init_hw = gxp_gpio_irq_init_hw;
> +
> + ret = devm_gpiochip_add_data(&pdev->dev, &drvdata->chip, drvdata);
> + if (ret < 0)
> + dev_err(&pdev->dev, "Could not register gpiochip for plreg, %d\n",
> + ret);
> +
> + regmap_update_bits(drvdata->pl_int, PLREG_INT_HI_PRI_EN,
> + BIT(4) | BIT(5), BIT(4) | BIT(5));
> + regmap_update_bits(drvdata->pl_int, PLREG_INT_GRP_STAT_MASK,
> + BIT(4) | BIT(5), 0x00);
> + regmap_read(drvdata->pl_int, PLREG_INT_HI_PRI_EN, &val);
> + regmap_read(drvdata->pl_int, PLREG_INT_GRP_STAT_MASK, &val);
> +
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Get irq from platform fail - %d\n", ret);
> + return ret;
return dev_err_probe
> + }
> +
> + drvdata->irq = ret;
> +
> + ret = devm_request_irq(&pdev->dev, drvdata->irq, gxp_gpio_pl_irq_handle,
> + IRQF_SHARED, "gxp-pl", drvdata);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "IRQ handler failed - %d\n", ret);
return dev_err_probe
Actually other applicable places as well...
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id gxp_gpio_of_match[] = {
> + { .compatible = "hpe,gxp-gpio"},
> + { .compatible = "hpe,gxp-gpio-pl"},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, gxp_gpio_of_match);
> +
> +static int gxp_gpio_probe(struct platform_device *pdev)
> +{
> + int ret;
> + struct gxp_gpio_drvdata *drvdata;
> + struct device *dev = &pdev->dev;
> + struct device *parent;
> + const struct of_device_id *match = of_match_device(gxp_gpio_of_match, dev);
> +
> + if (!match) {
> + dev_err(dev, "device is not compatible");
It is not possible.
> + return -EINVAL;
> + }
> +
> + parent = dev->parent;
> + if (!parent) {
> + dev_err(dev, "no parent\n");
Why do you need this check?
> + return -ENODEV;
> + }
> +
> + drvdata = devm_kzalloc(&pdev->dev, sizeof(struct gxp_gpio_drvdata),
sizeof(*)
> + GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, drvdata);
> +
> + if (strcmp(match->compatible, "hpe,gxp-gpio-pl") == 0)
No, use match data. Anyway this is open-coding of OF functions...
> + ret = gxp_gpio_pl_init(pdev);
> + else
> + ret = gxp_gpio_init(pdev);
> +
> + return ret;
Best regards,
Krzysztof
