[PATCH v2 0/4] hte: Improve GPIO handling and other cleanups

[PATCH v2 0/4] hte: Improve GPIO handling and other cleanups

[Andy Shevchenko]

This is a series that provides a new API to GPIO library (so far only
available in the GPIO tree), and respective update to the Tegra
HTE driver. On top a couple of other cleaups (patches 3 & 4, they
can be applied separately).

Patch 2 inherited tags from its respective discussion thread [1].

Due to dependencies this either should be applied to the GPIO tree,
or to the HTE when GPIO updates land the upstream (optionally with
the first patch be applied even now to the GPIO tree independently).

Another option is to have an immutable branch or tag, but I assume
that was discussed and rejected (?) in [1].

In v2:
- collected tags (Linus, Dipen) 
- fixed couple of typos (Dipen)

Link: https://lore.kernel.org/linux-gpio/20230905185309.131295-15-brgl@bgdev.pl/ [1]
Cc: Dipen Patel <dipenp@nvidia.com>
Cc: Linus Walleij <linus.walleij@linaro.org>

Andy Shevchenko (3):
  gpiolib: provide gpio_device_find_by_fwnode()
  hte: tegra194: Remove redundant dev_err()
  hte: tegra194: Switch to LATE_SIMPLE_DEV_PM_OPS()

Bartosz Golaszewski (1):
  hte: tegra194: don't access struct gpio_chip

 drivers/gpio/gpiolib.c      | 20 ++++++++++++++++
 drivers/hte/hte-tegra194.c  | 46 +++++++++++++++++++------------------
 include/linux/gpio/driver.h |  1 +
 3 files changed, 45 insertions(+), 22 deletions(-)

-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 1/4] gpiolib: provide gpio_device_find_by_fwnode()

[Andy Shevchenko]

One of the ways of looking up GPIO devices is using their fwnode.
Provide a helper for that to avoid every user implementing their
own matching function.

Reviewed-by: Dipen Patel <dipenp@nvidia.com>
Tested-by: Dipen Patel <dipenp@nvidia.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20231010151709.4104747-2-andriy.shevchenko@linux.intel.com
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c      | 20 ++++++++++++++++++++
 include/linux/gpio/driver.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 7b4d12b714a3..31c06a32cb8a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1142,6 +1142,26 @@ struct gpio_device *gpio_device_find_by_label(const char *label)
 }
 EXPORT_SYMBOL_GPL(gpio_device_find_by_label);
 
+static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, void *fwnode)
+{
+	return device_match_fwnode(&gc->gpiodev->dev, fwnode);
+}
+
+/**
+ * gpio_device_find_by_fwnode() - wrapper around gpio_device_find() finding
+ *                                the GPIO device by its fwnode
+ * @fwnode: Firmware node to lookup
+ *
+ * Returns:
+ * Reference to the GPIO device or NULL. Reference must be released with
+ * gpio_device_put().
+ */
+struct gpio_device *gpio_device_find_by_fwnode(const struct fwnode_handle *fwnode)
+{
+	return gpio_device_find((void *)fwnode, gpio_chip_match_by_fwnode);
+}
+EXPORT_SYMBOL_GPL(gpio_device_find_by_fwnode);
+
 /**
  * gpio_device_get() - Increase the reference count of this GPIO device
  * @gdev: GPIO device to increase the refcount for
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index f8ad7f40100c..ae4162d3f1d3 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -611,6 +611,7 @@ struct gpio_chip *gpiochip_find(void *data,
 struct gpio_device *gpio_device_find(void *data,
 				int (*match)(struct gpio_chip *gc, void *data));
 struct gpio_device *gpio_device_find_by_label(const char *label);
+struct gpio_device *gpio_device_find_by_fwnode(const struct fwnode_handle *fwnode);
 
 struct gpio_device *gpio_device_get(struct gpio_device *gdev);
 void gpio_device_put(struct gpio_device *gdev);
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 2/4] hte: tegra194: don't access struct gpio_chip

[Andy Shevchenko]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Using struct gpio_chip is not safe as it will disappear if the
underlying driver is unbound for any reason. Switch to using reference
counted struct gpio_device and its dedicated accessors.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Tested-by: Dipen Patel <dipenp@nvidia.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
[andy: used gpio_device_find_by_fwnode()]
Reviewed-by: Dipen Patel <dipenp@nvidia.com>
Link: https://lore.kernel.org/r/20231010151709.4104747-3-andriy.shevchenko@linux.intel.com
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/hte/hte-tegra194.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/hte/hte-tegra194.c b/drivers/hte/hte-tegra194.c
index 9fd3c00ff695..f6c2dbcd0aca 100644
--- a/drivers/hte/hte-tegra194.c
+++ b/drivers/hte/hte-tegra194.c
@@ -132,7 +132,7 @@ struct tegra_hte_soc {
 	const struct tegra_hte_data *prov_data;
 	struct tegra_hte_line_data *line_data;
 	struct hte_chip *chip;
-	struct gpio_chip *c;
+	struct gpio_device *gdev;
 	void __iomem *regs;
 };
 
@@ -421,7 +421,7 @@ static int tegra_hte_line_xlate(struct hte_chip *gc,
 	 * HTE/GTE namespace.
 	 */
 	if (gs->prov_data->type == HTE_TEGRA_TYPE_GPIO && !args) {
-		line_id = desc->attr.line_id - gs->c->base;
+		line_id = desc->attr.line_id - gpio_device_get_base(gs->gdev);
 		map = gs->prov_data->map;
 		map_sz = gs->prov_data->map_sz;
 	} else if (gs->prov_data->type == HTE_TEGRA_TYPE_GPIO && args) {
@@ -643,12 +643,15 @@ static irqreturn_t tegra_hte_isr(int irq, void *dev_id)
 static bool tegra_hte_match_from_linedata(const struct hte_chip *chip,
 					  const struct hte_ts_desc *hdesc)
 {
+	struct gpio_device *gdev __free(gpio_device_put) = NULL;
 	struct tegra_hte_soc *hte_dev = chip->data;
 
 	if (!hte_dev || (hte_dev->prov_data->type != HTE_TEGRA_TYPE_GPIO))
 		return false;
 
-	return hte_dev->c == gpiod_to_chip(hdesc->attr.line_data);
+	gdev = gpiod_to_device(hdesc->attr.line_data);
+
+	return hte_dev->gdev == gdev;
 }
 
 static const struct of_device_id tegra_hte_of_match[] = {
@@ -676,14 +679,11 @@ static void tegra_gte_disable(void *data)
 	tegra_hte_writel(gs, HTE_TECTRL, 0);
 }
 
-static int tegra_get_gpiochip_from_name(struct gpio_chip *chip, void *data)
+static void tegra_hte_put_gpio_device(void *data)
 {
-	return !strcmp(chip->label, data);
-}
+	struct gpio_device *gdev = data;
 
-static int tegra_gpiochip_match(struct gpio_chip *chip, void *data)
-{
-	return chip->fwnode == of_node_to_fwnode(data);
+	gpio_device_put(gdev);
 }
 
 static int tegra_hte_probe(struct platform_device *pdev)
@@ -763,8 +763,8 @@ static int tegra_hte_probe(struct platform_device *pdev)
 
 		if (of_device_is_compatible(dev->of_node,
 					    "nvidia,tegra194-gte-aon")) {
-			hte_dev->c = gpiochip_find("tegra194-gpio-aon",
-						tegra_get_gpiochip_from_name);
+			hte_dev->gdev =
+				gpio_device_find_by_label("tegra194-gpio-aon");
 		} else {
 			gpio_ctrl = of_parse_phandle(dev->of_node,
 						     "nvidia,gpio-controller",
@@ -775,14 +775,19 @@ static int tegra_hte_probe(struct platform_device *pdev)
 				return -ENODEV;
 			}
 
-			hte_dev->c = gpiochip_find(gpio_ctrl,
-						   tegra_gpiochip_match);
+			hte_dev->gdev =
+				gpio_device_find_by_fwnode(of_fwnode_handle(gpio_ctrl));
 			of_node_put(gpio_ctrl);
 		}
 
-		if (!hte_dev->c)
+		if (!hte_dev->gdev)
 			return dev_err_probe(dev, -EPROBE_DEFER,
 					     "wait for gpio controller\n");
+
+		ret = devm_add_action_or_reset(dev, tegra_hte_put_gpio_device,
+					       hte_dev->gdev);
+		if (ret)
+			return ret;
 	}
 
 	hte_dev->chip = gc;
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 3/4] hte: tegra194: Remove redundant dev_err()

[Andy Shevchenko]

There is no need to call the dev_err() function directly to print a custom
message when handling an error from platform_get_irq() function as it is
going to display an appropriate error message in case of a failure.

Reviewed-by: Dipen Patel <dipenp@nvidia.com>
Tested-by: Dipen Patel <dipenp@nvidia.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20231010151709.4104747-4-andriy.shevchenko@linux.intel.com
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/hte/hte-tegra194.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hte/hte-tegra194.c b/drivers/hte/hte-tegra194.c
index f6c2dbcd0aca..cf6058193403 100644
--- a/drivers/hte/hte-tegra194.c
+++ b/drivers/hte/hte-tegra194.c
@@ -731,10 +731,8 @@ static int tegra_hte_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	ret = platform_get_irq(pdev, 0);
-	if (ret < 0) {
-		dev_err_probe(dev, ret, "failed to get irq\n");
+	if (ret < 0)
 		return ret;
-	}
 	hte_dev->hte_irq = ret;
 	ret = devm_request_irq(dev, hte_dev->hte_irq, tegra_hte_isr, 0,
 			       dev_name(dev), hte_dev);
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 4/4] hte: tegra194: Switch to LATE_SIMPLE_DEV_PM_OPS()

[Andy Shevchenko]

SET_LATE_SYSTEM_SLEEP_PM_OPS is deprecated, replace it with
LATE_SYSTEM_SLEEP_PM_OPS() and use pm_sleep_ptr() for setting
the driver's pm routines. We can now remove the __maybe_unused
qualifier in the suspend and resume functions.

Tested-by: Dipen Patel <dipenp@nvidia.com>
Reviewed-by: Dipen Patel <dipenp@nvidia.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20231010151709.4104747-5-andriy.shevchenko@linux.intel.com
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/hte/hte-tegra194.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/hte/hte-tegra194.c b/drivers/hte/hte-tegra194.c
index cf6058193403..76e1b774c55e 100644
--- a/drivers/hte/hte-tegra194.c
+++ b/drivers/hte/hte-tegra194.c
@@ -815,7 +815,7 @@ static int tegra_hte_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int __maybe_unused tegra_hte_resume_early(struct device *dev)
+static int tegra_hte_resume_early(struct device *dev)
 {
 	u32 i;
 	struct tegra_hte_soc *gs = dev_get_drvdata(dev);
@@ -836,7 +836,7 @@ static int __maybe_unused tegra_hte_resume_early(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused tegra_hte_suspend_late(struct device *dev)
+static int tegra_hte_suspend_late(struct device *dev)
 {
 	u32 i;
 	struct tegra_hte_soc *gs = dev_get_drvdata(dev);
@@ -856,15 +856,14 @@ static int __maybe_unused tegra_hte_suspend_late(struct device *dev)
 }
 
 static const struct dev_pm_ops tegra_hte_pm = {
-	SET_LATE_SYSTEM_SLEEP_PM_OPS(tegra_hte_suspend_late,
-				     tegra_hte_resume_early)
+	LATE_SYSTEM_SLEEP_PM_OPS(tegra_hte_suspend_late, tegra_hte_resume_early)
 };
 
 static struct platform_driver tegra_hte_driver = {
 	.probe = tegra_hte_probe,
 	.driver = {
 		.name = "tegra_hte",
-		.pm = &tegra_hte_pm,
+		.pm = pm_sleep_ptr(&tegra_hte_pm),
 		.of_match_table = tegra_hte_of_match,
 	},
 };
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v2 0/4] hte: Improve GPIO handling and other cleanups

[Dipen Patel]

On 10/11/23 1:26 PM, Andy Shevchenko wrote:
> This is a series that provides a new API to GPIO library (so far only
> available in the GPIO tree), and respective update to the Tegra
> HTE driver. On top a couple of other cleaups (patches 3 & 4, they
> can be applied separately).
> 
> Patch 2 inherited tags from its respective discussion thread [1].
> 
> Due to dependencies this either should be applied to the GPIO tree,
> or to the HTE when GPIO updates land the upstream (optionally with
> the first patch be applied even now to the GPIO tree independently).
> 
> Another option is to have an immutable branch or tag, but I assume
> that was discussed and rejected (?) in [1].
> 
> In v2:
> - collected tags (Linus, Dipen) 
> - fixed couple of typos (Dipen)
> 
> Link: https://lore.kernel.org/linux-gpio/20230905185309.131295-15-brgl@bgdev.pl/ [1]
> Cc: Dipen Patel <dipenp@nvidia.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> 
> Andy Shevchenko (3):
>   gpiolib: provide gpio_device_find_by_fwnode()
>   hte: tegra194: Remove redundant dev_err()
>   hte: tegra194: Switch to LATE_SIMPLE_DEV_PM_OPS()
> 
> Bartosz Golaszewski (1):
>   hte: tegra194: don't access struct gpio_chip
> 
>  drivers/gpio/gpiolib.c      | 20 ++++++++++++++++
>  drivers/hte/hte-tegra194.c  | 46 +++++++++++++++++++------------------
>  include/linux/gpio/driver.h |  1 +
>  3 files changed, 45 insertions(+), 22 deletions(-)
> 
Looks great...I am going to assume you are going to push patches 1 and 2 through
gpio subsystem and rest through HTE, right?

Reviewed-by: Dipen Patel <dipenp@nvidia.com>
Tested-by: Dipen Patel <dipenp@nvidia.com>

Re: [PATCH v2 0/4] hte: Improve GPIO handling and other cleanups

[Bartosz Golaszewski]

On Thu, Oct 12, 2023 at 12:59 AM Dipen Patel <dipenp@nvidia.com> wrote:
>
> On 10/11/23 1:26 PM, Andy Shevchenko wrote:
> > This is a series that provides a new API to GPIO library (so far only
> > available in the GPIO tree), and respective update to the Tegra
> > HTE driver. On top a couple of other cleaups (patches 3 & 4, they
> > can be applied separately).
> >
> > Patch 2 inherited tags from its respective discussion thread [1].
> >
> > Due to dependencies this either should be applied to the GPIO tree,
> > or to the HTE when GPIO updates land the upstream (optionally with
> > the first patch be applied even now to the GPIO tree independently).
> >
> > Another option is to have an immutable branch or tag, but I assume
> > that was discussed and rejected (?) in [1].
> >
> > In v2:
> > - collected tags (Linus, Dipen)
> > - fixed couple of typos (Dipen)
> >
> > Link: https://lore.kernel.org/linux-gpio/20230905185309.131295-15-brgl@bgdev.pl/ [1]
> > Cc: Dipen Patel <dipenp@nvidia.com>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> >
> > Andy Shevchenko (3):
> >   gpiolib: provide gpio_device_find_by_fwnode()
> >   hte: tegra194: Remove redundant dev_err()
> >   hte: tegra194: Switch to LATE_SIMPLE_DEV_PM_OPS()
> >
> > Bartosz Golaszewski (1):
> >   hte: tegra194: don't access struct gpio_chip
> >
> >  drivers/gpio/gpiolib.c      | 20 ++++++++++++++++
> >  drivers/hte/hte-tegra194.c  | 46 +++++++++++++++++++------------------
> >  include/linux/gpio/driver.h |  1 +
> >  3 files changed, 45 insertions(+), 22 deletions(-)
> >
> Looks great...I am going to assume you are going to push patches 1 and 2 through
> gpio subsystem and rest through HTE, right?
>
> Reviewed-by: Dipen Patel <dipenp@nvidia.com>
> Tested-by: Dipen Patel <dipenp@nvidia.com>
>

Yes, let me queue them right away.

Bart

Re: [PATCH v2 0/4] hte: Improve GPIO handling and other cleanups

[Dipen Patel]

On 10/12/23 1:20 AM, Bartosz Golaszewski wrote:
> On Thu, Oct 12, 2023 at 12:59 AM Dipen Patel <dipenp@nvidia.com> wrote:
>>
>> On 10/11/23 1:26 PM, Andy Shevchenko wrote:
>>> This is a series that provides a new API to GPIO library (so far only
>>> available in the GPIO tree), and respective update to the Tegra
>>> HTE driver. On top a couple of other cleaups (patches 3 & 4, they
>>> can be applied separately).
>>>
>>> Patch 2 inherited tags from its respective discussion thread [1].
>>>
>>> Due to dependencies this either should be applied to the GPIO tree,
>>> or to the HTE when GPIO updates land the upstream (optionally with
>>> the first patch be applied even now to the GPIO tree independently).
>>>
>>> Another option is to have an immutable branch or tag, but I assume
>>> that was discussed and rejected (?) in [1].
>>>
>>> In v2:
>>> - collected tags (Linus, Dipen)
>>> - fixed couple of typos (Dipen)
>>>
>>> Link: https://lore.kernel.org/linux-gpio/20230905185309.131295-15-brgl@bgdev.pl/ [1]
>>> Cc: Dipen Patel <dipenp@nvidia.com>
>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> Andy Shevchenko (3):
>>>   gpiolib: provide gpio_device_find_by_fwnode()
>>>   hte: tegra194: Remove redundant dev_err()
>>>   hte: tegra194: Switch to LATE_SIMPLE_DEV_PM_OPS()
>>>
>>> Bartosz Golaszewski (1):
>>>   hte: tegra194: don't access struct gpio_chip
>>>
>>>  drivers/gpio/gpiolib.c      | 20 ++++++++++++++++
>>>  drivers/hte/hte-tegra194.c  | 46 +++++++++++++++++++------------------
>>>  include/linux/gpio/driver.h |  1 +
>>>  3 files changed, 45 insertions(+), 22 deletions(-)
>>>
>> Looks great...I am going to assume you are going to push patches 1 and 2 through
>> gpio subsystem and rest through HTE, right?
>>
>> Reviewed-by: Dipen Patel <dipenp@nvidia.com>
>> Tested-by: Dipen Patel <dipenp@nvidia.com>
>>
> 
> Yes, let me queue them right away.
>
Thanks, I just pushed patches 3 and 4 to hte subsys tree. Thanks everyone for
the quick turnaround to resolve this issue.

> Bart