[PATCH v2 0/4] Renesas RZ/T2H pinctrl fixes and cleanups

[PATCH v2 0/4] Renesas RZ/T2H pinctrl fixes and cleanups

[Cosmin Tanislav]

This series fixes a couple of issues and then does some cleanups in the
pinctrl driver for Renesas RZ/T2H.

V2:
 * switch pm parameter in rzt2h_pin_write_pm() to unsigned int and
   remove the u16 cast
 * reuse irq_idx in rzt2h_gpio_irq_request_resources()
 * mention that irq_domain_free_irqs_common() is still wired up
 * remove the PFC_FUNC_INTERRUPT check in rzt2h_gpio_get()
 * pick up Geert's Reviewed-by
 * drop patches already queued up by Geert

Cosmin Tanislav (4):
  pinctrl: renesas: rzt2h: restore correct pin mode on IRQ free
  pinctrl: renesas: rzt2h: setup IRQ in
    .irq_{request,release}_resources()
  pinctrl: renesas: rzt2h: fix reading pin value in IRQ function
  pinctrl: renesas: rzt2h: reuse helpers

 drivers/pinctrl/renesas/pinctrl-rzt2h.c | 177 ++++++++++++++++--------
 1 file changed, 117 insertions(+), 60 deletions(-)

-- 
2.55.0

[PATCH v2 1/4] pinctrl: renesas: rzt2h: restore correct pin mode on IRQ free

[Cosmin Tanislav]

rzt2h_gpio_irq_domain_free() calls rzt2h_pinctrl_set_gpio_en() with
false leaving the pin in interrupt function instead of returning it to
GPIO mode.

Pass true to rzt2h_pinctrl_set_gpio_en() to take the pin out of
interrupt function after we're done using it as an IRQ.

rzt2h_pinctrl_set_pfc_mode() switches the pin to Hi-Z, losing the
previous PM value.

Save the PM value before switching to Hi-Z, and restore it after the
IRQ is freed.

Cc: stable@kernel.org
Fixes: 829dde3369a9 ("pinctrl: renesas: rzt2h: Add GPIO IRQ chip to handle interrupts")
Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * switch pm parameter in rzt2h_pin_write_pm() to unsigned int and
   remove the u16 cast

 drivers/pinctrl/renesas/pinctrl-rzt2h.c | 43 +++++++++++++++++++++----
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzt2h.c b/drivers/pinctrl/renesas/pinctrl-rzt2h.c
index d29ef107c2c1..9c1ac836ffa6 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzt2h.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzt2h.c
@@ -95,6 +95,7 @@ struct rzt2h_pinctrl {
 	struct device			*dev;
 	struct gpio_chip		gpio_chip;
 	DECLARE_BITMAP(used_irqs, RZT2H_INTERRUPTS_NUM);
+	u8				saved_pm[RZT2H_INTERRUPTS_NUM];
 	raw_spinlock_t			lock; /* lock read/write registers */
 	struct mutex			mutex; /* serialize adding groups and functions */
 	bool				safety_port_enabled;
@@ -169,6 +170,26 @@ static int rzt2h_validate_pin(struct rzt2h_pinctrl *pctrl, unsigned int offset)
 	return (pincfg & BIT(pin)) ? 0 : -EINVAL;
 }
 
+static u8 rzt2h_pin_read_pm(struct rzt2h_pinctrl *pctrl, u8 port, u8 pin)
+{
+	u16 reg = rzt2h_pinctrl_readw(pctrl, port, PM(port));
+
+	return field_get(PM_PIN_MASK(pin), reg);
+}
+
+static void rzt2h_pin_write_pm(struct rzt2h_pinctrl *pctrl, u8 port, u8 pin,
+			       unsigned int pm)
+{
+	u16 reg;
+
+	guard(raw_spinlock_irqsave)(&pctrl->lock);
+
+	reg = rzt2h_pinctrl_readw(pctrl, port, PM(port));
+	reg &= ~PM_PIN_MASK(pin);
+	reg |= pm << (pin * 2);
+	rzt2h_pinctrl_writew(pctrl, port, reg, PM(port));
+}
+
 static void rzt2h_pinctrl_set_gpio_en(struct rzt2h_pinctrl *pctrl,
 				      u8 port, u8 pin, bool en)
 {
@@ -1022,16 +1043,23 @@ static int rzt2h_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
 	struct rzt2h_pinctrl *pctrl = gpiochip_get_data(gc);
 	u8 port = RZT2H_PIN_ID_TO_PORT(child);
 	u8 pin = RZT2H_PIN_ID_TO_PIN(child);
-	u8 parent_irq;
+	u8 parent_irq, irq_idx;
 
 	parent_irq = rzt2h_gpio_irq_map[child];
 	if (parent_irq < RZT2H_INTERRUPTS_START)
 		return -EINVAL;
 
-	if (test_and_set_bit(parent_irq - RZT2H_INTERRUPTS_START,
-			     pctrl->used_irqs))
+	irq_idx = parent_irq - RZT2H_INTERRUPTS_START;
+	if (test_and_set_bit(irq_idx, pctrl->used_irqs))
 		return -EBUSY;
 
+	/*
+	 * rzt2h_pinctrl_set_pfc_mode() sets PM to Hi-Z before switching to the
+	 * interrupt function, losing the previous PM value.
+	 * Save it so it can be restored when the IRQ is freed.
+	 */
+	pctrl->saved_pm[irq_idx] = rzt2h_pin_read_pm(pctrl, port, pin);
+
 	rzt2h_pinctrl_set_pfc_mode(pctrl, port, pin, PFC_FUNC_INTERRUPT);
 
 	*parent = parent_irq;
@@ -1049,14 +1077,17 @@ static void rzt2h_gpio_irq_domain_free(struct irq_domain *domain, unsigned int v
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 	u8 port = RZT2H_PIN_ID_TO_PORT(hwirq);
 	u8 pin = RZT2H_PIN_ID_TO_PIN(hwirq);
-	u8 parent_irq;
+	u8 parent_irq, irq_idx;
 
 	parent_irq = rzt2h_gpio_irq_map[hwirq];
 	if (parent_irq < RZT2H_INTERRUPTS_START)
 		return;
 
-	if (test_and_clear_bit(parent_irq - RZT2H_INTERRUPTS_START, pctrl->used_irqs))
-		rzt2h_pinctrl_set_gpio_en(pctrl, port, pin, false);
+	irq_idx = parent_irq - RZT2H_INTERRUPTS_START;
+	if (test_and_clear_bit(irq_idx, pctrl->used_irqs)) {
+		rzt2h_pin_write_pm(pctrl, port, pin, pctrl->saved_pm[irq_idx]);
+		rzt2h_pinctrl_set_gpio_en(pctrl, port, pin, true);
+	}
 
 	irq_domain_free_irqs_common(domain, virq, nr_irqs);
 }
-- 
2.55.0

[PATCH v2 2/4] pinctrl: renesas: rzt2h: setup IRQ in .irq_{request,release}_resources()

[Cosmin Tanislav]

When exporting a GPIO using the deprecated sysfs GPIO API,
gpio_is_visible() calls gpiod_to_irq() to determine whether a GPIO is
IRQ-capable, which ends up using
struct gpio_irq_chip::child_to_parent_hwirq().

When using the GPIO character device, it is possible to request a line
and to dynamically switch between using it as an IRQ or as a GPIO, by
updating the edge detector.

The current struct gpio_irq_chip::child_to_parent_hwirq() implementation
makes the assumption that it will only be called with the intention of
setting up the IRQ permanently, and that it will only be released when
the IRQ domain is freed.

Since we now know that this is not the case, take the following steps in
order to properly handle that case.

Implement struct irq_chip::irq_{request,release}_resources() based on
the default GPIOCHIP_IRQ_RESOURCE_HELPERS implementations.

Move the IRQ setup and release logic inside them.

Keep the mapping logic inside ::child_to_parent_hwirq() to be able to
provide a proper HW IRQ number, but do not actually setup the IRQ.

Remove rzt2h_gpio_irq_domain_free() as its only purpose was to free the
acquired IRQs. irq_domain_free_irqs_common() will still be set up as
struct gpio_irq_chip::child_irq_domain_ops::free by
gpiochip_hierarchy_setup_domain_ops().

Cc: stable@kernel.org
Fixes: 829dde3369a9 ("pinctrl: renesas: rzt2h: Add GPIO IRQ chip to handle interrupts")
Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * reuse irq_idx in rzt2h_gpio_irq_request_resources()
 * mention that irq_domain_free_irqs_common() is still wired up

 drivers/pinctrl/renesas/pinctrl-rzt2h.c | 79 +++++++++++++++----------
 1 file changed, 49 insertions(+), 30 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzt2h.c b/drivers/pinctrl/renesas/pinctrl-rzt2h.c
index 9c1ac836ffa6..796120e70bc4 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzt2h.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzt2h.c
@@ -1020,32 +1020,17 @@ static int rzt2h_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
 	return 0;
 }
 
-static const struct irq_chip rzt2h_gpio_irqchip = {
-	.name = "rzt2h-gpio",
-	.irq_disable = rzt2h_gpio_irq_disable,
-	.irq_enable = rzt2h_gpio_irq_enable,
-	.irq_mask = irq_chip_mask_parent,
-	.irq_unmask = irq_chip_unmask_parent,
-	.irq_set_type = irq_chip_set_type_parent,
-	.irq_set_wake = rzt2h_gpio_irq_set_wake,
-	.irq_eoi = irq_chip_eoi_parent,
-	.irq_set_affinity = irq_chip_set_affinity_parent,
-	.flags = IRQCHIP_IMMUTABLE,
-	GPIOCHIP_IRQ_RESOURCE_HELPERS,
-};
-
-static int rzt2h_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
-					    unsigned int child,
-					    unsigned int child_type,
-					    unsigned int *parent,
-					    unsigned int *parent_type)
+static int rzt2h_gpio_irq_request_resources(struct irq_data *d)
 {
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct rzt2h_pinctrl *pctrl = gpiochip_get_data(gc);
-	u8 port = RZT2H_PIN_ID_TO_PORT(child);
-	u8 pin = RZT2H_PIN_ID_TO_PIN(child);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	u8 port = RZT2H_PIN_ID_TO_PORT(hwirq);
+	u8 pin = RZT2H_PIN_ID_TO_PIN(hwirq);
 	u8 parent_irq, irq_idx;
+	int ret;
 
-	parent_irq = rzt2h_gpio_irq_map[child];
+	parent_irq = rzt2h_gpio_irq_map[hwirq];
 	if (parent_irq < RZT2H_INTERRUPTS_START)
 		return -EINVAL;
 
@@ -1062,18 +1047,21 @@ static int rzt2h_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
 
 	rzt2h_pinctrl_set_pfc_mode(pctrl, port, pin, PFC_FUNC_INTERRUPT);
 
-	*parent = parent_irq;
-	*parent_type = child_type;
+	ret = gpiochip_lock_as_irq(gc, hwirq);
+	if (ret) {
+		clear_bit(irq_idx, pctrl->used_irqs);
+		rzt2h_pin_write_pm(pctrl, port, pin, pctrl->saved_pm[irq_idx]);
+		rzt2h_pinctrl_set_gpio_en(pctrl, port, pin, true);
+		return ret;
+	}
 
 	return 0;
 }
 
-static void rzt2h_gpio_irq_domain_free(struct irq_domain *domain, unsigned int virq,
-				       unsigned int nr_irqs)
+static void rzt2h_gpio_irq_release_resources(struct irq_data *d)
 {
-	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct rzt2h_pinctrl *pctrl = container_of(gc, struct rzt2h_pinctrl, gpio_chip);
+	struct rzt2h_pinctrl *pctrl = gpiochip_get_data(gc);
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 	u8 port = RZT2H_PIN_ID_TO_PORT(hwirq);
 	u8 pin = RZT2H_PIN_ID_TO_PIN(hwirq);
@@ -1089,7 +1077,39 @@ static void rzt2h_gpio_irq_domain_free(struct irq_domain *domain, unsigned int v
 		rzt2h_pinctrl_set_gpio_en(pctrl, port, pin, true);
 	}
 
-	irq_domain_free_irqs_common(domain, virq, nr_irqs);
+	gpiochip_unlock_as_irq(gc, hwirq);
+}
+
+static const struct irq_chip rzt2h_gpio_irqchip = {
+	.name = "rzt2h-gpio",
+	.irq_disable = rzt2h_gpio_irq_disable,
+	.irq_enable = rzt2h_gpio_irq_enable,
+	.irq_mask = irq_chip_mask_parent,
+	.irq_unmask = irq_chip_unmask_parent,
+	.irq_set_type = irq_chip_set_type_parent,
+	.irq_set_wake = rzt2h_gpio_irq_set_wake,
+	.irq_eoi = irq_chip_eoi_parent,
+	.irq_request_resources = rzt2h_gpio_irq_request_resources,
+	.irq_release_resources = rzt2h_gpio_irq_release_resources,
+	.irq_set_affinity = irq_chip_set_affinity_parent,
+	.flags = IRQCHIP_IMMUTABLE,
+};
+
+static int rzt2h_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
+					    unsigned int child,
+					    unsigned int child_type,
+					    unsigned int *parent,
+					    unsigned int *parent_type)
+{
+	u8 parent_irq = rzt2h_gpio_irq_map[child];
+
+	if (parent_irq < RZT2H_INTERRUPTS_START)
+		return -EINVAL;
+
+	*parent = parent_irq;
+	*parent_type = child_type;
+
+	return 0;
 }
 
 static void rzt2h_gpio_init_irq_valid_mask(struct gpio_chip *gc,
@@ -1157,7 +1177,6 @@ static int rzt2h_gpio_register(struct rzt2h_pinctrl *pctrl)
 		girq->parent_domain = parent_domain;
 		girq->child_to_parent_hwirq = rzt2h_gpio_child_to_parent_hwirq;
 		girq->populate_parent_alloc_arg = gpiochip_populate_parent_fwspec_twocell;
-		girq->child_irq_domain_ops.free = rzt2h_gpio_irq_domain_free;
 		girq->init_valid_mask = rzt2h_gpio_init_irq_valid_mask;
 	}
 
-- 
2.55.0

[PATCH v2 3/4] pinctrl: renesas: rzt2h: fix reading pin value in IRQ function

[Cosmin Tanislav]

rzt2h_gpio_get() only reports a level for pins in input or output mode
and returns -EINVAL otherwise. When a pin is requested as an interrupt
it is switched to IRQ function and its I/O mode is set to Hi-Z, so its
value can no longer be read.

gpiolib calls gpiod_get_value_cansleep() to determine the edge's
direction when a line is watched with GPIO_V2_LINE_FLAG_EDGE_BOTH.
Non-zero values are translated to a rising edge, zero to a falling edge.

Since gpiod_get_value_cansleep() ends up calling rzt2h_gpio_get() which
returns -EINVAL when the pin is in IRQ function, every edge is reported
as rising.

When using the IRQ function, the input buffers are enabled and the PINm
registers reflect the live state of the input.

Report the input level even when the pin is used as an IRQ.

Cc: stable@kernel.org
Fixes: 829dde3369a9 ("pinctrl: renesas: rzt2h: Add GPIO IRQ chip to handle interrupts")
Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * remove the PFC_FUNC_INTERRUPT check in rzt2h_gpio_get()

 drivers/pinctrl/renesas/pinctrl-rzt2h.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzt2h.c b/drivers/pinctrl/renesas/pinctrl-rzt2h.c
index 796120e70bc4..8cf4b7f48e7e 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzt2h.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzt2h.c
@@ -170,6 +170,23 @@ static int rzt2h_validate_pin(struct rzt2h_pinctrl *pctrl, unsigned int offset)
 	return (pincfg & BIT(pin)) ? 0 : -EINVAL;
 }
 
+static bool rzt2h_pin_mode_is_peripheral(struct rzt2h_pinctrl *pctrl, u8 port, u8 bit)
+{
+	return rzt2h_pinctrl_readb(pctrl, port, PMC(port)) & BIT(bit);
+}
+
+static u8 rzt2h_pin_read_pfc(struct rzt2h_pinctrl *pctrl, u8 port, u8 pin)
+{
+	u64 reg64 = rzt2h_pinctrl_readq(pctrl, port, PFC(port));
+
+	return field_get(PFC_PIN_MASK(pin), reg64);
+}
+
+static bool rzt2h_pin_read_input(struct rzt2h_pinctrl *pctrl, u8 port, u8 bit)
+{
+	return rzt2h_pinctrl_readb(pctrl, port, PIN(port)) & BIT(bit);
+}
+
 static u8 rzt2h_pin_read_pm(struct rzt2h_pinctrl *pctrl, u8 port, u8 pin)
 {
 	u16 reg = rzt2h_pinctrl_readw(pctrl, port, PM(port));
@@ -847,6 +864,9 @@ static int rzt2h_gpio_get(struct gpio_chip *chip, unsigned int offset)
 	u8 bit = RZT2H_PIN_ID_TO_PIN(offset);
 	u16 reg;
 
+	if (rzt2h_pin_mode_is_peripheral(pctrl, port, bit))
+		return rzt2h_pin_read_input(pctrl, port, bit);
+
 	reg = rzt2h_pinctrl_readw(pctrl, port, PM(port));
 	reg = (reg >> (bit * 2)) & PM_MASK;
 	if (reg & PM_INPUT)
-- 
2.55.0

[PATCH v2 4/4] pinctrl: renesas: rzt2h: reuse helpers

[Cosmin Tanislav]

The PMCm peripheral-mode test, the PFCm function read, the PINm input
read, the PMm write, and the PMm read are open-coded in several places.

Reuse the already existing helpers.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * pick up Geert's Reviewed-by

 drivers/pinctrl/renesas/pinctrl-rzt2h.c | 39 +++++++++----------------
 1 file changed, 13 insertions(+), 26 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzt2h.c b/drivers/pinctrl/renesas/pinctrl-rzt2h.c
index 8cf4b7f48e7e..4872a208e0c3 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzt2h.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzt2h.c
@@ -230,7 +230,7 @@ static void rzt2h_pinctrl_set_pfc_mode(struct rzt2h_pinctrl *pctrl,
 
 	reg64 = rzt2h_pinctrl_readq(pctrl, port, PFC(port));
 	/* Check if pin is already configured to the desired function */
-	if ((rzt2h_pinctrl_readb(pctrl, port, PMC(port)) & BIT(pin)) &&
+	if (rzt2h_pin_mode_is_peripheral(pctrl, port, pin) &&
 	    field_get(PFC_PIN_MASK(pin), reg64) == func)
 		return;
 
@@ -784,15 +784,7 @@ static int rzt2h_gpio_request(struct gpio_chip *chip, unsigned int offset)
 static void rzt2h_gpio_set_direction(struct rzt2h_pinctrl *pctrl, u32 port,
 				     u8 bit, bool output)
 {
-	u16 reg;
-
-	guard(raw_spinlock_irqsave)(&pctrl->lock);
-
-	reg = rzt2h_pinctrl_readw(pctrl, port, PM(port));
-	reg &= ~PM_PIN_MASK(bit);
-
-	reg |= (output ? PM_OUTPUT : PM_INPUT) << (bit * 2);
-	rzt2h_pinctrl_writew(pctrl, port, reg, PM(port));
+	rzt2h_pin_write_pm(pctrl, port, bit, output ? PM_OUTPUT : PM_INPUT);
 }
 
 static int rzt2h_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
@@ -800,9 +792,8 @@ static int rzt2h_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
 	struct rzt2h_pinctrl *pctrl = gpiochip_get_data(chip);
 	u8 port = RZT2H_PIN_ID_TO_PORT(offset);
 	u8 bit = RZT2H_PIN_ID_TO_PIN(offset);
-	u64 reg64;
-	u16 reg;
 	int ret;
+	u8 pm;
 
 	ret = rzt2h_validate_pin(pctrl, offset);
 	if (ret)
@@ -810,7 +801,7 @@ static int rzt2h_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
 
 	guard(raw_spinlock_irqsave)(&pctrl->lock);
 
-	if (rzt2h_pinctrl_readb(pctrl, port, PMC(port)) & BIT(bit)) {
+	if (rzt2h_pin_mode_is_peripheral(pctrl, port, bit)) {
 		/*
 		 * When a GPIO is being requested as an IRQ, the pinctrl
 		 * framework expects to be able to read the GPIO's direction.
@@ -820,19 +811,16 @@ static int rzt2h_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
 		 * called to enable the IRQ function.
 		 * Default to input direction for IRQ function.
 		 */
-		reg64 = rzt2h_pinctrl_readq(pctrl, port, PFC(port));
-		reg64 = (reg64 >> (bit * 8)) & PFC_MASK;
-		if (reg64 == PFC_FUNC_INTERRUPT)
+		if (rzt2h_pin_read_pfc(pctrl, port, bit) == PFC_FUNC_INTERRUPT)
 			return GPIO_LINE_DIRECTION_IN;
 
 		return -EINVAL;
 	}
 
-	reg = rzt2h_pinctrl_readw(pctrl, port, PM(port));
-	reg = (reg >> (bit * 2)) & PM_MASK;
-	if (reg & PM_OUTPUT)
+	pm = rzt2h_pin_read_pm(pctrl, port, bit);
+	if (pm & PM_OUTPUT)
 		return GPIO_LINE_DIRECTION_OUT;
-	if (reg & PM_INPUT)
+	if (pm & PM_INPUT)
 		return GPIO_LINE_DIRECTION_IN;
 
 	return -EINVAL;
@@ -862,16 +850,15 @@ static int rzt2h_gpio_get(struct gpio_chip *chip, unsigned int offset)
 	struct rzt2h_pinctrl *pctrl = gpiochip_get_data(chip);
 	u8 port = RZT2H_PIN_ID_TO_PORT(offset);
 	u8 bit = RZT2H_PIN_ID_TO_PIN(offset);
-	u16 reg;
+	u8 pm;
 
 	if (rzt2h_pin_mode_is_peripheral(pctrl, port, bit))
 		return rzt2h_pin_read_input(pctrl, port, bit);
 
-	reg = rzt2h_pinctrl_readw(pctrl, port, PM(port));
-	reg = (reg >> (bit * 2)) & PM_MASK;
-	if (reg & PM_INPUT)
-		return !!(rzt2h_pinctrl_readb(pctrl, port, PIN(port)) & BIT(bit));
-	if (reg & PM_OUTPUT)
+	pm = rzt2h_pin_read_pm(pctrl, port, bit);
+	if (pm & PM_INPUT)
+		return rzt2h_pin_read_input(pctrl, port, bit);
+	if (pm & PM_OUTPUT)
 		return !!(rzt2h_pinctrl_readb(pctrl, port, P(port)) & BIT(bit));
 
 	return -EINVAL;
-- 
2.55.0

Re: [PATCH v2 3/4] pinctrl: renesas: rzt2h: fix reading pin value in IRQ function

[Andy Shevchenko]

On Thu, Sep 10, 2026 at 07:02:25PM +0300, Cosmin Tanislav wrote:
> rzt2h_gpio_get() only reports a level for pins in input or output mode
> and returns -EINVAL otherwise. When a pin is requested as an interrupt
> it is switched to IRQ function and its I/O mode is set to Hi-Z, so its
> value can no longer be read.
> 
> gpiolib calls gpiod_get_value_cansleep() to determine the edge's
> direction when a line is watched with GPIO_V2_LINE_FLAG_EDGE_BOTH.
> Non-zero values are translated to a rising edge, zero to a falling edge.
> 
> Since gpiod_get_value_cansleep() ends up calling rzt2h_gpio_get() which
> returns -EINVAL when the pin is in IRQ function, every edge is reported
> as rising.
> 
> When using the IRQ function, the input buffers are enabled and the PINm
> registers reflect the live state of the input.
> 
> Report the input level even when the pin is used as an IRQ.

...

> +static u8 rzt2h_pin_read_pfc(struct rzt2h_pinctrl *pctrl, u8 port, u8 pin)
> +{
> +	u64 reg64 = rzt2h_pinctrl_readq(pctrl, port, PFC(port));
> +
> +	return field_get(PFC_PIN_MASK(pin), reg64);
> +}

This is a dead code. Add it when it has users.
Especially this is weird to have in the patch with Fixes tag and Cc to stable@.
Bad rebase?

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v2 3/4] pinctrl: renesas: rzt2h: fix reading pin value in IRQ function

[Cosmin-Gabriel Tanislav]

> From: Andy Shevchenko <andriy.shevchenko@intel.com>
> Sent: Sunday, September 13, 2026 10:52 AM
> 
> [You don't often get email from andriy.shevchenko@intel.com. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Thu, Sep 10, 2026 at 07:02:25PM +0300, Cosmin Tanislav wrote:
> > rzt2h_gpio_get() only reports a level for pins in input or output mode
> > and returns -EINVAL otherwise. When a pin is requested as an interrupt
> > it is switched to IRQ function and its I/O mode is set to Hi-Z, so its
> > value can no longer be read.
> >
> > gpiolib calls gpiod_get_value_cansleep() to determine the edge's
> > direction when a line is watched with GPIO_V2_LINE_FLAG_EDGE_BOTH.
> > Non-zero values are translated to a rising edge, zero to a falling edge.
> >
> > Since gpiod_get_value_cansleep() ends up calling rzt2h_gpio_get() which
> > returns -EINVAL when the pin is in IRQ function, every edge is reported
> > as rising.
> >
> > When using the IRQ function, the input buffers are enabled and the PINm
> > registers reflect the live state of the input.
> >
> > Report the input level even when the pin is used as an IRQ.
> 
> ...
> 
> > +static u8 rzt2h_pin_read_pfc(struct rzt2h_pinctrl *pctrl, u8 port, u8 pin)
> > +{
> > +     u64 reg64 = rzt2h_pinctrl_readq(pctrl, port, PFC(port));
> > +
> > +     return field_get(PFC_PIN_MASK(pin), reg64);
> > +}
> 
> This is a dead code. Add it when it has users.
> Especially this is weird to have in the patch with Fixes tag and Cc to stable@.
> Bad rebase?
> 

I forgot to move it further in the chain when the original usage (which
used to be in this patch) was removed...

Sorry about that, I'll send a new version to fix it.

> --
> With Best Regards,
> Andy Shevchenko
> 

Re: [PATCH v2 4/4] pinctrl: renesas: rzt2h: reuse helpers

[Andy Shevchenko]

On Thu, Sep 10, 2026 at 07:02:26PM +0300, Cosmin Tanislav wrote:
> The PMCm peripheral-mode test, the PFCm function read, the PINm input
> read, the PMm write, and the PMm read are open-coded in several places.

> Reuse the already existing helpers.

...

> -		reg64 = rzt2h_pinctrl_readq(pctrl, port, PFC(port));
> -		reg64 = (reg64 >> (bit * 8)) & PFC_MASK;
> -		if (reg64 == PFC_FUNC_INTERRUPT)
> +		if (rzt2h_pin_read_pfc(pctrl, port, bit) == PFC_FUNC_INTERRUPT)
>  			return GPIO_LINE_DIRECTION_IN;

Basically this part with the helper should be split into a separate patch w/o
Fixes tag.

-- 
With Best Regards,
Andy Shevchenko