2023-10-11 20:27:17

by Andy Shevchenko

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

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/[email protected]/ [1]
Cc: Dipen Patel <[email protected]>
Cc: Linus Walleij <[email protected]>

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


2023-10-11 20:27:18

by Andy Shevchenko

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

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 <[email protected]>
Tested-by: Dipen Patel <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Andy Shevchenko <[email protected]>
---
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

2023-10-11 20:27:33

by Andy Shevchenko

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

From: Bartosz Golaszewski <[email protected]>

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 <[email protected]>
Tested-by: Dipen Patel <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
[andy: used gpio_device_find_by_fwnode()]
Reviewed-by: Dipen Patel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Andy Shevchenko <[email protected]>
---
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

2023-10-11 22:59:41

by Dipen Patel

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

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/[email protected]/ [1]
> Cc: Dipen Patel <[email protected]>
> Cc: Linus Walleij <[email protected]>
>
> 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 <[email protected]>
Tested-by: Dipen Patel <[email protected]>

2023-10-12 08:21:08

by Bartosz Golaszewski

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

On Thu, Oct 12, 2023 at 12:59 AM Dipen Patel <[email protected]> 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/[email protected]/ [1]
> > Cc: Dipen Patel <[email protected]>
> > Cc: Linus Walleij <[email protected]>
> >
> > 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 <[email protected]>
> Tested-by: Dipen Patel <[email protected]>
>

Yes, let me queue them right away.

Bart

2023-10-12 17:38:42

by Dipen Patel

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

On 10/12/23 1:20 AM, Bartosz Golaszewski wrote:
> On Thu, Oct 12, 2023 at 12:59 AM Dipen Patel <[email protected]> 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/[email protected]/ [1]
>>> Cc: Dipen Patel <[email protected]>
>>> Cc: Linus Walleij <[email protected]>
>>>
>>> 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 <[email protected]>
>> Tested-by: Dipen Patel <[email protected]>
>>
>
> 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