* [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver
@ 2024-10-11 14:48 Emil Renner Berthing
2024-10-11 14:48 ` [PATCH v1 1/3] pinctrl: th1520: Fix pinconf return values Emil Renner Berthing
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Emil Renner Berthing @ 2024-10-11 14:48 UTC (permalink / raw)
To: linux-kernel, linux-gpio, devicetree, linux-riscv
Cc: Linus Walleij, Jisheng Zhang, Guo Ren, Fu Wei, Paul Walmsley,
Palmer Dabbelt, Thomas Bonnefille
Hi,
Here are 2 important fixes and a code improvement to the T-Head TH1520
pinctrl driver that was either introduced or missed when Drew took over
upstreaming it.
It is based on Linus' pinctrl/for-next:
6dbd1577b7dc ("Merge branch 'devel' into for-next")
Emil Renner Berthing (3):
pinctrl: th1520: Fix pinconf return values
pinctrl: th1520: Update pinmux tables
pinctrl: th1520: Factor out casts
drivers/pinctrl/pinctrl-th1520.c | 52 ++++++++++++++++++++------------
1 file changed, 32 insertions(+), 20 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 1/3] pinctrl: th1520: Fix pinconf return values
2024-10-11 14:48 [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver Emil Renner Berthing
@ 2024-10-11 14:48 ` Emil Renner Berthing
2024-10-11 15:29 ` Drew Fustini
2024-10-11 14:48 ` [PATCH v1 2/3] pinctrl: th1520: Update pinmux tables Emil Renner Berthing
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Emil Renner Berthing @ 2024-10-11 14:48 UTC (permalink / raw)
To: linux-kernel, linux-gpio, devicetree, linux-riscv
Cc: Linus Walleij, Jisheng Zhang, Guo Ren, Fu Wei, Paul Walmsley,
Palmer Dabbelt, Thomas Bonnefille
When Drew took over the pinctrl driver he must have changed
all the -ENOTSUPP returns into -EOPNOTSUPP. This subtle change
was most likely not spotted because it was never mentioned in the
changelog of the patchset, but it breaks all the places in the
pin control and GPIO frameworks where -ENOTSUPP is expected.
Fixes: bed5cd6f8a98 ("pinctrl: Add driver for the T-Head TH1520 SoC")
Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
---
drivers/pinctrl/pinctrl-th1520.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-th1520.c b/drivers/pinctrl/pinctrl-th1520.c
index c8d2ee6defa7..03326df69668 100644
--- a/drivers/pinctrl/pinctrl-th1520.c
+++ b/drivers/pinctrl/pinctrl-th1520.c
@@ -591,7 +591,7 @@ static int th1520_pinconf_get(struct pinctrl_dev *pctldev,
u32 arg;
if ((uintptr_t)desc->drv_data & TH1520_PAD_NO_PADCFG)
- return -EOPNOTSUPP;
+ return -ENOTSUPP;
value = readl_relaxed(th1520_padcfg(thp, pin));
value = (value >> th1520_padcfg_shift(pin)) & GENMASK(9, 0);
@@ -636,7 +636,7 @@ static int th1520_pinconf_get(struct pinctrl_dev *pctldev,
arg = enabled ? 1 : 0;
break;
default:
- return -EOPNOTSUPP;
+ return -ENOTSUPP;
}
*config = pinconf_to_config_packed(param, arg);
@@ -661,7 +661,7 @@ static int th1520_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
u16 mask, value;
if ((uintptr_t)desc->drv_data & TH1520_PAD_NO_PADCFG)
- return -EOPNOTSUPP;
+ return -ENOTSUPP;
mask = 0;
value = 0;
@@ -676,14 +676,14 @@ static int th1520_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
break;
case PIN_CONFIG_BIAS_PULL_DOWN:
if (arg == 0)
- return -EOPNOTSUPP;
+ return -ENOTSUPP;
mask |= TH1520_PADCFG_BIAS;
value &= ~TH1520_PADCFG_BIAS;
value |= TH1520_PADCFG_PE;
break;
case PIN_CONFIG_BIAS_PULL_UP:
if (arg == 0)
- return -EOPNOTSUPP;
+ return -ENOTSUPP;
mask |= TH1520_PADCFG_BIAS;
value &= ~TH1520_PADCFG_BIAS;
if (arg == TH1520_PULL_STRONG_OHM)
@@ -718,7 +718,7 @@ static int th1520_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
value &= ~TH1520_PADCFG_SL;
break;
default:
- return -EOPNOTSUPP;
+ return -ENOTSUPP;
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 2/3] pinctrl: th1520: Update pinmux tables
2024-10-11 14:48 [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver Emil Renner Berthing
2024-10-11 14:48 ` [PATCH v1 1/3] pinctrl: th1520: Fix pinconf return values Emil Renner Berthing
@ 2024-10-11 14:48 ` Emil Renner Berthing
2024-10-11 16:31 ` Drew Fustini
2024-10-11 14:48 ` [PATCH v1 3/3] pinctrl: th1520: Factor out casts Emil Renner Berthing
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Emil Renner Berthing @ 2024-10-11 14:48 UTC (permalink / raw)
To: linux-kernel, linux-gpio, devicetree, linux-riscv
Cc: Linus Walleij, Jisheng Zhang, Guo Ren, Fu Wei, Paul Walmsley,
Palmer Dabbelt, Thomas Bonnefille
When Drew took over the pinctrl driver it seems like he didn't use the
git tree I pointed him at and thus missed some important fixes to the
tables describing valid pinmux settings.
The documentation has a nice overview table of these settings but
unfortunately it doesn't fully match the register descriptions, which
seem to be the correct version.
Fixes: bed5cd6f8a98 ("pinctrl: Add driver for the T-Head TH1520 SoC")
Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
---
drivers/pinctrl/pinctrl-th1520.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-th1520.c b/drivers/pinctrl/pinctrl-th1520.c
index 03326df69668..8bd40cb2f013 100644
--- a/drivers/pinctrl/pinctrl-th1520.c
+++ b/drivers/pinctrl/pinctrl-th1520.c
@@ -221,9 +221,9 @@ static const struct pinctrl_pin_desc th1520_group2_pins[] = {
TH1520_PAD(15, UART4_RTSN, UART, ____, ____, GPIO, ____, ____, 0),
TH1520_PAD(16, UART3_TXD, DBG, UART, ____, GPIO, ____, ____, 0),
TH1520_PAD(17, UART3_RXD, DBG, UART, ____, GPIO, ____, ____, 0),
- TH1520_PAD(18, GPIO0_18, GPIO, I2C, ____, ____, ____, ____, 0),
- TH1520_PAD(19, GPIO0_19, GPIO, I2C, ____, ____, ____, ____, 0),
- TH1520_PAD(20, GPIO0_20, GPIO, UART, IR, ____, ____, ____, 0),
+ TH1520_PAD(18, GPIO0_18, GPIO, I2C, ____, ____, DPU0, DPU1, 0),
+ TH1520_PAD(19, GPIO0_19, GPIO, I2C, ____, ____, DPU0, DPU1, 0),
+ TH1520_PAD(20, GPIO0_20, GPIO, UART, IR, ____, DPU0, DPU1, 0),
TH1520_PAD(21, GPIO0_21, GPIO, UART, IR, ____, DPU0, DPU1, 0),
TH1520_PAD(22, GPIO0_22, GPIO, JTAG, I2C, ____, DPU0, DPU1, 0),
TH1520_PAD(23, GPIO0_23, GPIO, JTAG, I2C, ____, DPU0, DPU1, 0),
@@ -241,7 +241,7 @@ static const struct pinctrl_pin_desc th1520_group2_pins[] = {
TH1520_PAD(35, GPIO1_3, GPIO, JTAG, ____, ____, DPU0, DPU1, 0),
TH1520_PAD(36, GPIO1_4, GPIO, JTAG, ____, ____, DPU0, DPU1, 0),
TH1520_PAD(37, GPIO1_5, GPIO, ____, ____, ____, DPU0, DPU1, 0),
- TH1520_PAD(38, GPIO1_6, GPIO, ____, ____, ____, DPU0, DPU1, 0),
+ TH1520_PAD(38, GPIO1_6, GPIO, QSPI, ____, ____, DPU0, DPU1, 0),
TH1520_PAD(39, GPIO1_7, GPIO, QSPI, ____, ____, DPU0, DPU1, 0),
TH1520_PAD(40, GPIO1_8, GPIO, QSPI, ____, ____, DPU0, DPU1, 0),
TH1520_PAD(41, GPIO1_9, GPIO, QSPI, ____, ____, DPU0, DPU1, 0),
@@ -256,11 +256,11 @@ static const struct pinctrl_pin_desc th1520_group2_pins[] = {
TH1520_PAD(50, CLK_OUT_1, BSEL, CLK, ____, GPIO, ____, ____, 0),
TH1520_PAD(51, CLK_OUT_2, BSEL, CLK, ____, GPIO, ____, ____, 0),
TH1520_PAD(52, CLK_OUT_3, BSEL, CLK, ____, GPIO, ____, ____, 0),
- TH1520_PAD(53, GPIO1_21, GPIO, ____, ISP, ____, ____, ____, 0),
- TH1520_PAD(54, GPIO1_22, GPIO, ____, ISP, ____, ____, ____, 0),
- TH1520_PAD(55, GPIO1_23, GPIO, ____, ISP, ____, ____, ____, 0),
- TH1520_PAD(56, GPIO1_24, GPIO, ____, ISP, ____, ____, ____, 0),
- TH1520_PAD(57, GPIO1_25, GPIO, ____, ISP, ____, ____, ____, 0),
+ TH1520_PAD(53, GPIO1_21, JTAG, ____, ISP, GPIO, ____, ____, 0),
+ TH1520_PAD(54, GPIO1_22, JTAG, ____, ISP, GPIO, ____, ____, 0),
+ TH1520_PAD(55, GPIO1_23, JTAG, ____, ISP, GPIO, ____, ____, 0),
+ TH1520_PAD(56, GPIO1_24, JTAG, ____, ISP, GPIO, ____, ____, 0),
+ TH1520_PAD(57, GPIO1_25, JTAG, ____, ISP, GPIO, ____, ____, 0),
TH1520_PAD(58, GPIO1_26, GPIO, ____, ISP, ____, ____, ____, 0),
TH1520_PAD(59, GPIO1_27, GPIO, ____, ISP, ____, ____, ____, 0),
TH1520_PAD(60, GPIO1_28, GPIO, ____, ISP, ____, ____, ____, 0),
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 3/3] pinctrl: th1520: Factor out casts
2024-10-11 14:48 [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver Emil Renner Berthing
2024-10-11 14:48 ` [PATCH v1 1/3] pinctrl: th1520: Fix pinconf return values Emil Renner Berthing
2024-10-11 14:48 ` [PATCH v1 2/3] pinctrl: th1520: Update pinmux tables Emil Renner Berthing
@ 2024-10-11 14:48 ` Emil Renner Berthing
2024-10-11 16:35 ` Drew Fustini
2024-10-16 18:45 ` Kees Bakker
2024-10-11 16:15 ` [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver Drew Fustini
2024-10-11 19:28 ` Linus Walleij
4 siblings, 2 replies; 11+ messages in thread
From: Emil Renner Berthing @ 2024-10-11 14:48 UTC (permalink / raw)
To: linux-kernel, linux-gpio, devicetree, linux-riscv
Cc: Linus Walleij, Jisheng Zhang, Guo Ren, Fu Wei, Paul Walmsley,
Palmer Dabbelt, Thomas Bonnefille
Limit the casts to get the mux data and flags from the driver data
pointer with each pin to two inline functions as requested by Andy
during review.
Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
---
drivers/pinctrl/pinctrl-th1520.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-th1520.c b/drivers/pinctrl/pinctrl-th1520.c
index 8bd40cb2f013..7474d8da32f9 100644
--- a/drivers/pinctrl/pinctrl-th1520.c
+++ b/drivers/pinctrl/pinctrl-th1520.c
@@ -152,6 +152,16 @@ static enum th1520_muxtype th1520_muxtype_get(const char *str)
(TH1520_MUX_##m0 << 0) | (TH1520_MUX_##m1 << 5) | (TH1520_MUX_##m2 << 10) | \
(TH1520_MUX_##m3 << 15) | (TH1520_MUX_##m4 << 20) | (TH1520_MUX_##m5 << 25)) }
+static unsigned long th1520_pad_muxdata(void *drv_data)
+{
+ return (uintptr_t)drv_data & TH1520_PAD_MUXDATA;
+}
+
+static bool th1520_pad_no_padcfg(void *drv_data)
+{
+ return (uintptr_t)drv_data & TH1520_PAD_NO_PADCFG;
+}
+
static const struct pinctrl_pin_desc th1520_group1_pins[] = {
TH1520_PAD(0, OSC_CLK_IN, ____, ____, ____, ____, ____, ____, TH1520_PAD_NO_PADCFG),
TH1520_PAD(1, OSC_CLK_OUT, ____, ____, ____, ____, ____, ____, TH1520_PAD_NO_PADCFG),
@@ -590,7 +600,7 @@ static int th1520_pinconf_get(struct pinctrl_dev *pctldev,
u32 value;
u32 arg;
- if ((uintptr_t)desc->drv_data & TH1520_PAD_NO_PADCFG)
+ if (th1520_pad_no_padcfg(desc->drv_data))
return -ENOTSUPP;
value = readl_relaxed(th1520_padcfg(thp, pin));
@@ -660,7 +670,7 @@ static int th1520_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
unsigned int i;
u16 mask, value;
- if ((uintptr_t)desc->drv_data & TH1520_PAD_NO_PADCFG)
+ if (th1520_pad_no_padcfg(desc->drv_data))
return -ENOTSUPP;
mask = 0;
@@ -793,12 +803,14 @@ static int th1520_pinmux_set_mux(struct pinctrl_dev *pctldev,
{
struct th1520_pinctrl *thp = pinctrl_dev_get_drvdata(pctldev);
const struct function_desc *func = pinmux_generic_get_function(pctldev, fsel);
+ enum th1520_muxtype muxtype = (uintptr_t)func->data;
if (!func)
return -EINVAL;
+
return th1520_pinmux_set(thp, thp->desc.pins[gsel].number,
- (uintptr_t)thp->desc.pins[gsel].drv_data & TH1520_PAD_MUXDATA,
- (uintptr_t)func->data);
+ th1520_pad_muxdata(thp->desc.pins[gsel].drv_data),
+ muxtype);
}
static int th1520_gpio_request_enable(struct pinctrl_dev *pctldev,
@@ -809,7 +821,7 @@ static int th1520_gpio_request_enable(struct pinctrl_dev *pctldev,
const struct pin_desc *desc = pin_desc_get(pctldev, offset);
return th1520_pinmux_set(thp, offset,
- (uintptr_t)desc->drv_data & TH1520_PAD_MUXDATA,
+ th1520_pad_muxdata(desc->drv_data),
TH1520_MUX_GPIO);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/3] pinctrl: th1520: Fix pinconf return values
2024-10-11 14:48 ` [PATCH v1 1/3] pinctrl: th1520: Fix pinconf return values Emil Renner Berthing
@ 2024-10-11 15:29 ` Drew Fustini
0 siblings, 0 replies; 11+ messages in thread
From: Drew Fustini @ 2024-10-11 15:29 UTC (permalink / raw)
To: Emil Renner Berthing
Cc: linux-kernel, linux-gpio, devicetree, linux-riscv, Linus Walleij,
Jisheng Zhang, Guo Ren, Fu Wei, Paul Walmsley, Palmer Dabbelt,
Thomas Bonnefille
On Fri, Oct 11, 2024 at 04:48:23PM +0200, Emil Renner Berthing wrote:
> When Drew took over the pinctrl driver he must have changed
> all the -ENOTSUPP returns into -EOPNOTSUPP. This subtle change
> was most likely not spotted because it was never mentioned in the
> changelog of the patchset, but it breaks all the places in the
> pin control and GPIO frameworks where -ENOTSUPP is expected.
>
> Fixes: bed5cd6f8a98 ("pinctrl: Add driver for the T-Head TH1520 SoC")
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> ---
> drivers/pinctrl/pinctrl-th1520.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-th1520.c b/drivers/pinctrl/pinctrl-th1520.c
> index c8d2ee6defa7..03326df69668 100644
> --- a/drivers/pinctrl/pinctrl-th1520.c
> +++ b/drivers/pinctrl/pinctrl-th1520.c
> @@ -591,7 +591,7 @@ static int th1520_pinconf_get(struct pinctrl_dev *pctldev,
> u32 arg;
>
> if ((uintptr_t)desc->drv_data & TH1520_PAD_NO_PADCFG)
> - return -EOPNOTSUPP;
> + return -ENOTSUPP;
>
> value = readl_relaxed(th1520_padcfg(thp, pin));
> value = (value >> th1520_padcfg_shift(pin)) & GENMASK(9, 0);
> @@ -636,7 +636,7 @@ static int th1520_pinconf_get(struct pinctrl_dev *pctldev,
> arg = enabled ? 1 : 0;
> break;
> default:
> - return -EOPNOTSUPP;
> + return -ENOTSUPP;
> }
>
> *config = pinconf_to_config_packed(param, arg);
> @@ -661,7 +661,7 @@ static int th1520_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> u16 mask, value;
>
> if ((uintptr_t)desc->drv_data & TH1520_PAD_NO_PADCFG)
> - return -EOPNOTSUPP;
> + return -ENOTSUPP;
>
> mask = 0;
> value = 0;
> @@ -676,14 +676,14 @@ static int th1520_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> break;
> case PIN_CONFIG_BIAS_PULL_DOWN:
> if (arg == 0)
> - return -EOPNOTSUPP;
> + return -ENOTSUPP;
> mask |= TH1520_PADCFG_BIAS;
> value &= ~TH1520_PADCFG_BIAS;
> value |= TH1520_PADCFG_PE;
> break;
> case PIN_CONFIG_BIAS_PULL_UP:
> if (arg == 0)
> - return -EOPNOTSUPP;
> + return -ENOTSUPP;
> mask |= TH1520_PADCFG_BIAS;
> value &= ~TH1520_PADCFG_BIAS;
> if (arg == TH1520_PULL_STRONG_OHM)
> @@ -718,7 +718,7 @@ static int th1520_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> value &= ~TH1520_PADCFG_SL;
> break;
> default:
> - return -EOPNOTSUPP;
> + return -ENOTSUPP;
> }
> }
>
> --
> 2.43.0
>
Reviewed-by: Drew Fustini <dfustini@tenstorrent.com>
Thanks for the fix. This was something I changed due to checkpatch
("ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP") wihtout
realizing the implication.
-Drew
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver
2024-10-11 14:48 [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver Emil Renner Berthing
` (2 preceding siblings ...)
2024-10-11 14:48 ` [PATCH v1 3/3] pinctrl: th1520: Factor out casts Emil Renner Berthing
@ 2024-10-11 16:15 ` Drew Fustini
2024-10-11 19:28 ` Linus Walleij
4 siblings, 0 replies; 11+ messages in thread
From: Drew Fustini @ 2024-10-11 16:15 UTC (permalink / raw)
To: Emil Renner Berthing
Cc: linux-kernel, linux-gpio, devicetree, linux-riscv, Linus Walleij,
Jisheng Zhang, Guo Ren, Fu Wei, Paul Walmsley, Palmer Dabbelt,
Thomas Bonnefille
On Fri, Oct 11, 2024 at 04:48:22PM +0200, Emil Renner Berthing wrote:
> Hi,
>
> Here are 2 important fixes and a code improvement to the T-Head TH1520
> pinctrl driver that was either introduced or missed when Drew took over
> upstreaming it.
>
> It is based on Linus' pinctrl/for-next:
>
> 6dbd1577b7dc ("Merge branch 'devel' into for-next")
>
> Emil Renner Berthing (3):
> pinctrl: th1520: Fix pinconf return values
> pinctrl: th1520: Update pinmux tables
> pinctrl: th1520: Factor out casts
>
> drivers/pinctrl/pinctrl-th1520.c | 52 ++++++++++++++++++++------------
> 1 file changed, 32 insertions(+), 20 deletions(-)
>
> --
> 2.43.0
Emil informed me that the out-of-tree USB driver is broken when trying
to use the pinctrl-th1520 driver that I submitted. This is because I
had changed -ENOTSUPP to -EOPNOTSUPP to silence a checkpatch warning
without realizing the implication. I've just been working on the
dwmac etherenet series [1] on top of mainline and I didn't realize there
was a problem with gpio.
I've just rebuilt and booted okay on lpi4a and beaglev ahead with this
series. For the whole series:
Tested-by: Drew Fustini <dfustini@tenstorrent.com>
[1] https://lore.kernel.org/linux-riscv/20240930-th1520-dwmac-v3-0-ae3e03c225ab@tenstorrent.com/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 2/3] pinctrl: th1520: Update pinmux tables
2024-10-11 14:48 ` [PATCH v1 2/3] pinctrl: th1520: Update pinmux tables Emil Renner Berthing
@ 2024-10-11 16:31 ` Drew Fustini
0 siblings, 0 replies; 11+ messages in thread
From: Drew Fustini @ 2024-10-11 16:31 UTC (permalink / raw)
To: Emil Renner Berthing
Cc: linux-kernel, linux-gpio, devicetree, linux-riscv, Linus Walleij,
Jisheng Zhang, Guo Ren, Fu Wei, Paul Walmsley, Palmer Dabbelt,
Thomas Bonnefille
On Fri, Oct 11, 2024 at 04:48:24PM +0200, Emil Renner Berthing wrote:
> When Drew took over the pinctrl driver it seems like he didn't use the
> git tree I pointed him at and thus missed some important fixes to the
> tables describing valid pinmux settings.
>
> The documentation has a nice overview table of these settings but
> unfortunately it doesn't fully match the register descriptions, which
> seem to be the correct version.
>
> Fixes: bed5cd6f8a98 ("pinctrl: Add driver for the T-Head TH1520 SoC")
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> ---
> drivers/pinctrl/pinctrl-th1520.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-th1520.c b/drivers/pinctrl/pinctrl-th1520.c
> index 03326df69668..8bd40cb2f013 100644
> --- a/drivers/pinctrl/pinctrl-th1520.c
> +++ b/drivers/pinctrl/pinctrl-th1520.c
> @@ -221,9 +221,9 @@ static const struct pinctrl_pin_desc th1520_group2_pins[] = {
> TH1520_PAD(15, UART4_RTSN, UART, ____, ____, GPIO, ____, ____, 0),
> TH1520_PAD(16, UART3_TXD, DBG, UART, ____, GPIO, ____, ____, 0),
> TH1520_PAD(17, UART3_RXD, DBG, UART, ____, GPIO, ____, ____, 0),
> - TH1520_PAD(18, GPIO0_18, GPIO, I2C, ____, ____, ____, ____, 0),
> - TH1520_PAD(19, GPIO0_19, GPIO, I2C, ____, ____, ____, ____, 0),
> - TH1520_PAD(20, GPIO0_20, GPIO, UART, IR, ____, ____, ____, 0),
> + TH1520_PAD(18, GPIO0_18, GPIO, I2C, ____, ____, DPU0, DPU1, 0),
> + TH1520_PAD(19, GPIO0_19, GPIO, I2C, ____, ____, DPU0, DPU1, 0),
> + TH1520_PAD(20, GPIO0_20, GPIO, UART, IR, ____, DPU0, DPU1, 0),
> TH1520_PAD(21, GPIO0_21, GPIO, UART, IR, ____, DPU0, DPU1, 0),
> TH1520_PAD(22, GPIO0_22, GPIO, JTAG, I2C, ____, DPU0, DPU1, 0),
> TH1520_PAD(23, GPIO0_23, GPIO, JTAG, I2C, ____, DPU0, DPU1, 0),
> @@ -241,7 +241,7 @@ static const struct pinctrl_pin_desc th1520_group2_pins[] = {
> TH1520_PAD(35, GPIO1_3, GPIO, JTAG, ____, ____, DPU0, DPU1, 0),
> TH1520_PAD(36, GPIO1_4, GPIO, JTAG, ____, ____, DPU0, DPU1, 0),
> TH1520_PAD(37, GPIO1_5, GPIO, ____, ____, ____, DPU0, DPU1, 0),
> - TH1520_PAD(38, GPIO1_6, GPIO, ____, ____, ____, DPU0, DPU1, 0),
> + TH1520_PAD(38, GPIO1_6, GPIO, QSPI, ____, ____, DPU0, DPU1, 0),
> TH1520_PAD(39, GPIO1_7, GPIO, QSPI, ____, ____, DPU0, DPU1, 0),
> TH1520_PAD(40, GPIO1_8, GPIO, QSPI, ____, ____, DPU0, DPU1, 0),
> TH1520_PAD(41, GPIO1_9, GPIO, QSPI, ____, ____, DPU0, DPU1, 0),
> @@ -256,11 +256,11 @@ static const struct pinctrl_pin_desc th1520_group2_pins[] = {
> TH1520_PAD(50, CLK_OUT_1, BSEL, CLK, ____, GPIO, ____, ____, 0),
> TH1520_PAD(51, CLK_OUT_2, BSEL, CLK, ____, GPIO, ____, ____, 0),
> TH1520_PAD(52, CLK_OUT_3, BSEL, CLK, ____, GPIO, ____, ____, 0),
> - TH1520_PAD(53, GPIO1_21, GPIO, ____, ISP, ____, ____, ____, 0),
> - TH1520_PAD(54, GPIO1_22, GPIO, ____, ISP, ____, ____, ____, 0),
> - TH1520_PAD(55, GPIO1_23, GPIO, ____, ISP, ____, ____, ____, 0),
> - TH1520_PAD(56, GPIO1_24, GPIO, ____, ISP, ____, ____, ____, 0),
> - TH1520_PAD(57, GPIO1_25, GPIO, ____, ISP, ____, ____, ____, 0),
> + TH1520_PAD(53, GPIO1_21, JTAG, ____, ISP, GPIO, ____, ____, 0),
> + TH1520_PAD(54, GPIO1_22, JTAG, ____, ISP, GPIO, ____, ____, 0),
> + TH1520_PAD(55, GPIO1_23, JTAG, ____, ISP, GPIO, ____, ____, 0),
> + TH1520_PAD(56, GPIO1_24, JTAG, ____, ISP, GPIO, ____, ____, 0),
> + TH1520_PAD(57, GPIO1_25, JTAG, ____, ISP, GPIO, ____, ____, 0),
> TH1520_PAD(58, GPIO1_26, GPIO, ____, ISP, ____, ____, ____, 0),
> TH1520_PAD(59, GPIO1_27, GPIO, ____, ISP, ____, ____, ____, 0),
> TH1520_PAD(60, GPIO1_28, GPIO, ____, ISP, ____, ____, ____, 0),
> --
> 2.43.0
>
Reviewed-by: Drew Fustini <dfustini@tenstorrent.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 3/3] pinctrl: th1520: Factor out casts
2024-10-11 14:48 ` [PATCH v1 3/3] pinctrl: th1520: Factor out casts Emil Renner Berthing
@ 2024-10-11 16:35 ` Drew Fustini
2024-10-16 18:45 ` Kees Bakker
1 sibling, 0 replies; 11+ messages in thread
From: Drew Fustini @ 2024-10-11 16:35 UTC (permalink / raw)
To: Emil Renner Berthing
Cc: linux-kernel, linux-gpio, devicetree, linux-riscv, Linus Walleij,
Jisheng Zhang, Guo Ren, Fu Wei, Paul Walmsley, Palmer Dabbelt,
Thomas Bonnefille
On Fri, Oct 11, 2024 at 04:48:25PM +0200, Emil Renner Berthing wrote:
> Limit the casts to get the mux data and flags from the driver data
> pointer with each pin to two inline functions as requested by Andy
> during review.
>
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> ---
> drivers/pinctrl/pinctrl-th1520.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-th1520.c b/drivers/pinctrl/pinctrl-th1520.c
> index 8bd40cb2f013..7474d8da32f9 100644
> --- a/drivers/pinctrl/pinctrl-th1520.c
> +++ b/drivers/pinctrl/pinctrl-th1520.c
> @@ -152,6 +152,16 @@ static enum th1520_muxtype th1520_muxtype_get(const char *str)
> (TH1520_MUX_##m0 << 0) | (TH1520_MUX_##m1 << 5) | (TH1520_MUX_##m2 << 10) | \
> (TH1520_MUX_##m3 << 15) | (TH1520_MUX_##m4 << 20) | (TH1520_MUX_##m5 << 25)) }
>
> +static unsigned long th1520_pad_muxdata(void *drv_data)
> +{
> + return (uintptr_t)drv_data & TH1520_PAD_MUXDATA;
> +}
> +
> +static bool th1520_pad_no_padcfg(void *drv_data)
> +{
> + return (uintptr_t)drv_data & TH1520_PAD_NO_PADCFG;
> +}
> +
> static const struct pinctrl_pin_desc th1520_group1_pins[] = {
> TH1520_PAD(0, OSC_CLK_IN, ____, ____, ____, ____, ____, ____, TH1520_PAD_NO_PADCFG),
> TH1520_PAD(1, OSC_CLK_OUT, ____, ____, ____, ____, ____, ____, TH1520_PAD_NO_PADCFG),
> @@ -590,7 +600,7 @@ static int th1520_pinconf_get(struct pinctrl_dev *pctldev,
> u32 value;
> u32 arg;
>
> - if ((uintptr_t)desc->drv_data & TH1520_PAD_NO_PADCFG)
> + if (th1520_pad_no_padcfg(desc->drv_data))
> return -ENOTSUPP;
>
> value = readl_relaxed(th1520_padcfg(thp, pin));
> @@ -660,7 +670,7 @@ static int th1520_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> unsigned int i;
> u16 mask, value;
>
> - if ((uintptr_t)desc->drv_data & TH1520_PAD_NO_PADCFG)
> + if (th1520_pad_no_padcfg(desc->drv_data))
> return -ENOTSUPP;
>
> mask = 0;
> @@ -793,12 +803,14 @@ static int th1520_pinmux_set_mux(struct pinctrl_dev *pctldev,
> {
> struct th1520_pinctrl *thp = pinctrl_dev_get_drvdata(pctldev);
> const struct function_desc *func = pinmux_generic_get_function(pctldev, fsel);
> + enum th1520_muxtype muxtype = (uintptr_t)func->data;
>
> if (!func)
> return -EINVAL;
> +
> return th1520_pinmux_set(thp, thp->desc.pins[gsel].number,
> - (uintptr_t)thp->desc.pins[gsel].drv_data & TH1520_PAD_MUXDATA,
> - (uintptr_t)func->data);
> + th1520_pad_muxdata(thp->desc.pins[gsel].drv_data),
> + muxtype);
> }
>
> static int th1520_gpio_request_enable(struct pinctrl_dev *pctldev,
> @@ -809,7 +821,7 @@ static int th1520_gpio_request_enable(struct pinctrl_dev *pctldev,
> const struct pin_desc *desc = pin_desc_get(pctldev, offset);
>
> return th1520_pinmux_set(thp, offset,
> - (uintptr_t)desc->drv_data & TH1520_PAD_MUXDATA,
> + th1520_pad_muxdata(desc->drv_data),
> TH1520_MUX_GPIO);
> }
>
> --
> 2.43.0
>
Reviewed-by: Drew Fustini <dfustini@tenstorrent.com>
Thanks for improving this. I see the feedback from Andy [1] on your v2
now that you mention it.
-Drew
[1] https://lore.kernel.org/linux-gpio/Zj8K_0zpI_IAY66R@surfacebook.localdomain/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver
2024-10-11 14:48 [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver Emil Renner Berthing
` (3 preceding siblings ...)
2024-10-11 16:15 ` [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver Drew Fustini
@ 2024-10-11 19:28 ` Linus Walleij
2024-10-15 16:52 ` Drew Fustini
4 siblings, 1 reply; 11+ messages in thread
From: Linus Walleij @ 2024-10-11 19:28 UTC (permalink / raw)
To: Emil Renner Berthing
Cc: linux-kernel, linux-gpio, devicetree, linux-riscv, Jisheng Zhang,
Guo Ren, Fu Wei, Paul Walmsley, Palmer Dabbelt, Thomas Bonnefille
On Fri, Oct 11, 2024 at 4:48 PM Emil Renner Berthing
<emil.renner.berthing@canonical.com> wrote:
> Here are 2 important fixes and a code improvement to the T-Head TH1520
> pinctrl driver that was either introduced or missed when Drew took over
> upstreaming it.
>
> It is based on Linus' pinctrl/for-next:
>
> 6dbd1577b7dc ("Merge branch 'devel' into for-next")
All patches applied!
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver
2024-10-11 19:28 ` Linus Walleij
@ 2024-10-15 16:52 ` Drew Fustini
0 siblings, 0 replies; 11+ messages in thread
From: Drew Fustini @ 2024-10-15 16:52 UTC (permalink / raw)
To: Linus Walleij
Cc: Emil Renner Berthing, linux-kernel, linux-gpio, devicetree,
linux-riscv, Jisheng Zhang, Guo Ren, Fu Wei, Paul Walmsley,
Palmer Dabbelt, Thomas Bonnefille
On Fri, Oct 11, 2024 at 09:28:10PM +0200, Linus Walleij wrote:
> On Fri, Oct 11, 2024 at 4:48 PM Emil Renner Berthing
> <emil.renner.berthing@canonical.com> wrote:
>
> > Here are 2 important fixes and a code improvement to the T-Head TH1520
> > pinctrl driver that was either introduced or missed when Drew took over
> > upstreaming it.
> >
> > It is based on Linus' pinctrl/for-next:
> >
> > 6dbd1577b7dc ("Merge branch 'devel' into for-next")
>
> All patches applied!
>
> Yours,
> Linus Walleij
Hi Linus,
Which branch did you apply these patches to?
I don't see them in your devel branch [1].
Thanks,
Drew
[1] https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 3/3] pinctrl: th1520: Factor out casts
2024-10-11 14:48 ` [PATCH v1 3/3] pinctrl: th1520: Factor out casts Emil Renner Berthing
2024-10-11 16:35 ` Drew Fustini
@ 2024-10-16 18:45 ` Kees Bakker
1 sibling, 0 replies; 11+ messages in thread
From: Kees Bakker @ 2024-10-16 18:45 UTC (permalink / raw)
To: Emil Renner Berthing, linux-kernel, linux-gpio, devicetree,
linux-riscv
Cc: Linus Walleij, Jisheng Zhang, Guo Ren, Fu Wei, Paul Walmsley,
Palmer Dabbelt, Thomas Bonnefille
Op 11-10-2024 om 16:48 schreef Emil Renner Berthing:
> Limit the casts to get the mux data and flags from the driver data
> pointer with each pin to two inline functions as requested by Andy
> during review.
>
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> ---
> drivers/pinctrl/pinctrl-th1520.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-th1520.c b/drivers/pinctrl/pinctrl-th1520.c
> index 8bd40cb2f013..7474d8da32f9 100644
> --- a/drivers/pinctrl/pinctrl-th1520.c
> +++ b/drivers/pinctrl/pinctrl-th1520.c
> @@ -152,6 +152,16 @@ static enum th1520_muxtype th1520_muxtype_get(const char *str)
> (TH1520_MUX_##m0 << 0) | (TH1520_MUX_##m1 << 5) | (TH1520_MUX_##m2 << 10) | \
> (TH1520_MUX_##m3 << 15) | (TH1520_MUX_##m4 << 20) | (TH1520_MUX_##m5 << 25)) }
>
> +static unsigned long th1520_pad_muxdata(void *drv_data)
> +{
> + return (uintptr_t)drv_data & TH1520_PAD_MUXDATA;
> +}
> +
> +static bool th1520_pad_no_padcfg(void *drv_data)
> +{
> + return (uintptr_t)drv_data & TH1520_PAD_NO_PADCFG;
> +}
> +
> static const struct pinctrl_pin_desc th1520_group1_pins[] = {
> TH1520_PAD(0, OSC_CLK_IN, ____, ____, ____, ____, ____, ____, TH1520_PAD_NO_PADCFG),
> TH1520_PAD(1, OSC_CLK_OUT, ____, ____, ____, ____, ____, ____, TH1520_PAD_NO_PADCFG),
> @@ -590,7 +600,7 @@ static int th1520_pinconf_get(struct pinctrl_dev *pctldev,
> u32 value;
> u32 arg;
>
> - if ((uintptr_t)desc->drv_data & TH1520_PAD_NO_PADCFG)
> + if (th1520_pad_no_padcfg(desc->drv_data))
> return -ENOTSUPP;
>
> value = readl_relaxed(th1520_padcfg(thp, pin));
> @@ -660,7 +670,7 @@ static int th1520_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> unsigned int i;
> u16 mask, value;
>
> - if ((uintptr_t)desc->drv_data & TH1520_PAD_NO_PADCFG)
> + if (th1520_pad_no_padcfg(desc->drv_data))
> return -ENOTSUPP;
>
> mask = 0;
> @@ -793,12 +803,14 @@ static int th1520_pinmux_set_mux(struct pinctrl_dev *pctldev,
> {
> struct th1520_pinctrl *thp = pinctrl_dev_get_drvdata(pctldev);
> const struct function_desc *func = pinmux_generic_get_function(pctldev, fsel);
> + enum th1520_muxtype muxtype = (uintptr_t)func->data;
You cannot use func before checking for NULL (see if statement below)
>
> if (!func)
> return -EINVAL;
> +
> return th1520_pinmux_set(thp, thp->desc.pins[gsel].number,
> - (uintptr_t)thp->desc.pins[gsel].drv_data & TH1520_PAD_MUXDATA,
> - (uintptr_t)func->data);
> + th1520_pad_muxdata(thp->desc.pins[gsel].drv_data),
> + muxtype);
> }
>
> static int th1520_gpio_request_enable(struct pinctrl_dev *pctldev,
> @@ -809,7 +821,7 @@ static int th1520_gpio_request_enable(struct pinctrl_dev *pctldev,
> const struct pin_desc *desc = pin_desc_get(pctldev, offset);
>
> return th1520_pinmux_set(thp, offset,
> - (uintptr_t)desc->drv_data & TH1520_PAD_MUXDATA,
> + th1520_pad_muxdata(desc->drv_data),
> TH1520_MUX_GPIO);
> }
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-10-16 18:45 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-11 14:48 [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver Emil Renner Berthing
2024-10-11 14:48 ` [PATCH v1 1/3] pinctrl: th1520: Fix pinconf return values Emil Renner Berthing
2024-10-11 15:29 ` Drew Fustini
2024-10-11 14:48 ` [PATCH v1 2/3] pinctrl: th1520: Update pinmux tables Emil Renner Berthing
2024-10-11 16:31 ` Drew Fustini
2024-10-11 14:48 ` [PATCH v1 3/3] pinctrl: th1520: Factor out casts Emil Renner Berthing
2024-10-11 16:35 ` Drew Fustini
2024-10-16 18:45 ` Kees Bakker
2024-10-11 16:15 ` [PATCH v1 0/3] pinctrl: th1520: Unbreak the driver Drew Fustini
2024-10-11 19:28 ` Linus Walleij
2024-10-15 16:52 ` Drew Fustini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).