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

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

[Andy Shevchenko]

This is a series 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],
but I believe the Tested-by needs to be confirmed again.

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].

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 v1 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.

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 v1 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()]
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..339ff5921ec8 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_fnode_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 v1 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.

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 339ff5921ec8..30ef1750a9fa 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 v1 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.

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 30ef1750a9fa..30fa7c0a555e 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_slee_ptr(&tegra_hte_pm),
 		.of_match_table = tegra_hte_of_match,
 	},
 };
-- 
2.40.0.1.gaa8946217a0b

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

[Dipen Patel]

On 10/10/23 8:17 AM, Andy Shevchenko wrote:
> 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()]
> 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..339ff5921ec8 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_fnode_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;

Looks good to me, I will wait for others to comment and will test out (2,3,4
also) probably end of the day 11th Oct.

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

[Dipen Patel]

On 10/10/23 9:19 AM, Dipen Patel wrote:
> On 10/10/23 8:17 AM, Andy Shevchenko wrote:
>> 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()]
>> 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..339ff5921ec8 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_fnode_handle(gpio_ctrl));

I think there is typo for of_fnode*. Should it be of_fwnode*?

>>  			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;
> 
> Looks good to me, I will wait for others to comment and will test out (2,3,4
> also) probably end of the day 11th Oct.
> 

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

[Dipen Patel]

On 10/10/23 8:17 AM, Andy Shevchenko wrote:
> 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.
> 
> 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 30ef1750a9fa..30fa7c0a555e 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_slee_ptr(&tegra_hte_pm),

typo, pm_sleep_ptr instead?

>  		.of_match_table = tegra_hte_of_match,
>  	},
>  };

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

[Dipen Patel]

On 10/10/23 8:17 AM, Andy Shevchenko wrote:
> 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.
> 
> 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 339ff5921ec8..30ef1750a9fa 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);

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

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

[Andy Shevchenko]

On Tue, Oct 10, 2023 at 10:31 PM Dipen Patel <dipenp@nvidia.com> wrote:
> On 10/10/23 9:19 AM, Dipen Patel wrote:
> > On 10/10/23 8:17 AM, Andy Shevchenko wrote:

> >> +                    hte_dev->gdev =
> >> +                            gpio_device_find_by_fwnode(of_fnode_handle(gpio_ctrl));
>
> I think there is typo for of_fnode*. Should it be of_fwnode*?

Yep, it's a typo, can you. fix manually for the test, please?

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Tue, Oct 10, 2023 at 10:36 PM Dipen Patel <dipenp@nvidia.com> wrote:
> On 10/10/23 8:17 AM, Andy Shevchenko wrote:

> > +             .pm = pm_slee_ptr(&tegra_hte_pm),
>
> typo, pm_sleep_ptr instead?

Indeed. On my x86_64 this code slipped from compilation. I will try
harder to compile-test it next time.

-- 
With Best Regards,
Andy Shevchenko

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

[Bartosz Golaszewski]

On Tue, Oct 10, 2023 at 5:18 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> This is a series 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],
> but I believe the Tested-by needs to be confirmed again.
>
> 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].

The series looks good to me. I'd like to take patches 1 and 2 through
the GPIO tree once v2 is out. This way we could potentially remove
gpiochip_find() for v6.7 already.

Bart

>
> 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
>

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

[Andy Shevchenko]

On Wed, Oct 11, 2023 at 11:33:51AM +0200, Bartosz Golaszewski wrote:
> On Tue, Oct 10, 2023 at 5:18 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > This is a series 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],
> > but I believe the Tested-by needs to be confirmed again.
> >
> > 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].
> 
> The series looks good to me. I'd like to take patches 1 and 2 through
> the GPIO tree once v2 is out. This way we could potentially remove
> gpiochip_find() for v6.7 already.

It would be nice to see it being removed sooner than later!
I'm waiting for the test results by Dipen, I'll send the v2
ASAP if tests pass.

-- 
With Best Regards,
Andy Shevchenko

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

[Linus Walleij]

On Tue, Oct 10, 2023 at 5:18 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> This is a series 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],
> but I believe the Tested-by needs to be confirmed again.
>
> 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].
>
> 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>

This is good stuff. The series:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

[Dipen Patel]

On 10/11/23 3:06 AM, Andy Shevchenko wrote:
> On Wed, Oct 11, 2023 at 11:33:51AM +0200, Bartosz Golaszewski wrote:
>> On Tue, Oct 10, 2023 at 5:18 PM Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>>>
>>> This is a series 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],
>>> but I believe the Tested-by needs to be confirmed again.
>>>
>>> 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].
>>
>> The series looks good to me. I'd like to take patches 1 and 2 through
>> the GPIO tree once v2 is out. This way we could potentially remove
>> gpiochip_find() for v6.7 already.
> 
> It would be nice to see it being removed sooner than later!
> I'm waiting for the test results by Dipen, I'll send the v2
> ASAP if tests pass.
> 
Reviewed-by: Dipen Patel <dipenp@nvidia.com>
After correcting typo manually
Tested-by: Dipen Patel <dipenp@nvidia.com>

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

[Dipen Patel]

On 10/10/23 10:54 PM, Andy Shevchenko wrote:
> On Tue, Oct 10, 2023 at 10:31 PM Dipen Patel <dipenp@nvidia.com> wrote:
>> On 10/10/23 9:19 AM, Dipen Patel wrote:
>>> On 10/10/23 8:17 AM, Andy Shevchenko wrote:
> 
>>>> +                    hte_dev->gdev =
>>>> +                            gpio_device_find_by_fwnode(of_fnode_handle(gpio_ctrl));
>>
>> I think there is typo for of_fnode*. Should it be of_fwnode*?
> 
> Yep, it's a typo, can you. fix manually for the test, please?
> 
After manually correcting the typo...

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

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

[Dipen Patel]

On 10/10/23 10:55 PM, Andy Shevchenko wrote:
> On Tue, Oct 10, 2023 at 10:36 PM Dipen Patel <dipenp@nvidia.com> wrote:
>> On 10/10/23 8:17 AM, Andy Shevchenko wrote:
> 
>>> +             .pm = pm_slee_ptr(&tegra_hte_pm),
>>
>> typo, pm_sleep_ptr instead?
> 
> Indeed. On my x86_64 this code slipped from compilation. I will try
> harder to compile-test it next time.
> 
After manually correcting the error,

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

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

[Linus Walleij]

On Tue, Oct 10, 2023 at 5:18 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:


> 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.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

This makes sense to me! (But I haven't looked at the rest of the
patches in the series. Yet.)
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij