* [PATCH 0/2] pinctrl: renesas: rzg2l: Simplify noise filter cfg macro
@ 2024-08-27 13:17 Prabhakar
2024-08-27 13:17 ` [PATCH 1/2] pinctrl: renesas: rzg2l: Introduce single macro for digital noise filter configuration Prabhakar
2024-08-27 13:17 ` [PATCH 2/2] pinctrl: renesas: rzg2l: Move pinconf_to_config_argument() call outside of switch cases Prabhakar
0 siblings, 2 replies; 7+ messages in thread
From: Prabhakar @ 2024-08-27 13:17 UTC (permalink / raw)
To: Geert Uytterhoeven, Linus Walleij
Cc: linux-renesas-soc, linux-gpio, linux-kernel, Prabhakar, Biju Das,
Fabrizio Castro, Lad Prabhakar
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Hi All,
This patch series simplifies noise cfg macro, by just adding single macro
for configuration and while at it simplified rzg2l_pinctrl_pinconf_set()
function.
Cheers,
Prabhakar
Lad Prabhakar (2):
pinctrl: renesas: rzg2l: Introduce single macro for digital noise
filter configuration
pinctrl: renesas: rzg2l: Move pinconf_to_config_argument() call
outside of switch cases
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 62 +++++++++----------------
1 file changed, 21 insertions(+), 41 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] pinctrl: renesas: rzg2l: Introduce single macro for digital noise filter configuration
2024-08-27 13:17 [PATCH 0/2] pinctrl: renesas: rzg2l: Simplify noise filter cfg macro Prabhakar
@ 2024-08-27 13:17 ` Prabhakar
2024-08-29 13:09 ` Geert Uytterhoeven
2024-08-27 13:17 ` [PATCH 2/2] pinctrl: renesas: rzg2l: Move pinconf_to_config_argument() call outside of switch cases Prabhakar
1 sibling, 1 reply; 7+ messages in thread
From: Prabhakar @ 2024-08-27 13:17 UTC (permalink / raw)
To: Geert Uytterhoeven, Linus Walleij
Cc: linux-renesas-soc, linux-gpio, linux-kernel, Prabhakar, Biju Das,
Fabrizio Castro, Lad Prabhakar
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
When enabling the digital noise filter for the pins, it is necessary to
configure both the noise filter stages (via the FILNUM register) and the
sampling interval (via the FILCLKSEL register). To simplify this process,
we introduce a single macro for configuring the digital noise filter.
This patch removes the PIN_CFG_FILNUM and PIN_CFG_FILCLKSEL configuration
macros and renames PIN_CFG_FILONOFF to PIN_CFG_NF.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 52 ++++++++++---------------
1 file changed, 20 insertions(+), 32 deletions(-)
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 2a73a8c374b3..8fc1f28d02d1 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -51,17 +51,15 @@
#define PIN_CFG_IO_VMC_QSPI BIT(7)
#define PIN_CFG_IO_VMC_ETH0 BIT(8)
#define PIN_CFG_IO_VMC_ETH1 BIT(9)
-#define PIN_CFG_FILONOFF BIT(10)
-#define PIN_CFG_FILNUM BIT(11)
-#define PIN_CFG_FILCLKSEL BIT(12)
-#define PIN_CFG_IOLH_C BIT(13)
-#define PIN_CFG_SOFT_PS BIT(14)
-#define PIN_CFG_OEN BIT(15)
-#define PIN_CFG_NOGPIO_INT BIT(16)
-#define PIN_CFG_NOD BIT(17) /* N-ch Open Drain */
-#define PIN_CFG_SMT BIT(18) /* Schmitt-trigger input control */
-#define PIN_CFG_ELC BIT(19)
-#define PIN_CFG_IOLH_RZV2H BIT(20)
+#define PIN_CFG_NF BIT(10) /* Digital noise filter */
+#define PIN_CFG_IOLH_C BIT(11)
+#define PIN_CFG_SOFT_PS BIT(12)
+#define PIN_CFG_OEN BIT(13)
+#define PIN_CFG_NOGPIO_INT BIT(14)
+#define PIN_CFG_NOD BIT(15) /* N-ch Open Drain */
+#define PIN_CFG_SMT BIT(16) /* Schmitt-trigger input control */
+#define PIN_CFG_ELC BIT(17)
+#define PIN_CFG_IOLH_RZV2H BIT(18)
#define RZG2L_SINGLE_PIN BIT_ULL(63) /* Dedicated pin */
#define RZG2L_VARIABLE_CFG BIT_ULL(62) /* Variable cfg for port pins */
@@ -69,9 +67,7 @@
#define RZG2L_MPXED_COMMON_PIN_FUNCS(group) \
(PIN_CFG_IOLH_##group | \
PIN_CFG_PUPD | \
- PIN_CFG_FILONOFF | \
- PIN_CFG_FILNUM | \
- PIN_CFG_FILCLKSEL)
+ PIN_CFG_NF)
#define RZG2L_MPXED_PIN_FUNCS (RZG2L_MPXED_COMMON_PIN_FUNCS(A) | \
PIN_CFG_SR)
@@ -84,10 +80,7 @@
PIN_CFG_SR | \
PIN_CFG_SMT)
-#define RZG2L_MPXED_ETH_PIN_FUNCS(x) ((x) | \
- PIN_CFG_FILONOFF | \
- PIN_CFG_FILNUM | \
- PIN_CFG_FILCLKSEL)
+#define RZG2L_MPXED_ETH_PIN_FUNCS(x) ((x) | PIN_CFG_NF)
#define PIN_CFG_PIN_MAP_MASK GENMASK_ULL(61, 54)
#define PIN_CFG_PIN_REG_MASK GENMASK_ULL(53, 46)
@@ -394,13 +387,13 @@ static const u64 r9a09g057_variable_pin_cfg[] = {
#ifdef CONFIG_RISCV
static const u64 r9a07g043f_variable_pin_cfg[] = {
RZG2L_VARIABLE_PIN_CFG_PACK(20, 0, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
- PIN_CFG_FILONOFF | PIN_CFG_FILNUM | PIN_CFG_FILCLKSEL |
+ PIN_CFG_NF |
PIN_CFG_IEN | PIN_CFG_NOGPIO_INT),
RZG2L_VARIABLE_PIN_CFG_PACK(20, 1, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
- PIN_CFG_FILONOFF | PIN_CFG_FILNUM | PIN_CFG_FILCLKSEL |
+ PIN_CFG_NF |
PIN_CFG_IEN | PIN_CFG_NOGPIO_INT),
RZG2L_VARIABLE_PIN_CFG_PACK(20, 2, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
- PIN_CFG_FILONOFF | PIN_CFG_FILNUM | PIN_CFG_FILCLKSEL |
+ PIN_CFG_NF |
PIN_CFG_IEN | PIN_CFG_NOGPIO_INT),
RZG2L_VARIABLE_PIN_CFG_PACK(20, 3, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
PIN_CFG_IEN | PIN_CFG_NOGPIO_INT),
@@ -431,7 +424,7 @@ static const u64 r9a07g043f_variable_pin_cfg[] = {
RZG2L_VARIABLE_PIN_CFG_PACK(24, 4, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
PIN_CFG_NOGPIO_INT),
RZG2L_VARIABLE_PIN_CFG_PACK(24, 5, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
- PIN_CFG_FILONOFF | PIN_CFG_FILNUM | PIN_CFG_FILCLKSEL |
+ PIN_CFG_NF |
PIN_CFG_NOGPIO_INT),
};
#endif
@@ -1886,8 +1879,7 @@ static const u64 r9a07g043_gpio_configs[] = {
#ifdef CONFIG_RISCV
/* Below additional port pins (P19 - P28) are exclusively available on RZ/Five SoC only */
RZG2L_GPIO_PORT_SPARSE_PACK(0x2, 0x06, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
- PIN_CFG_FILONOFF | PIN_CFG_FILNUM | PIN_CFG_FILCLKSEL |
- PIN_CFG_IEN | PIN_CFG_NOGPIO_INT), /* P19 */
+ PIN_CFG_NF | PIN_CFG_IEN | PIN_CFG_NOGPIO_INT), /* P19 */
RZG2L_GPIO_PORT_PACK_VARIABLE(8, 0x07), /* P20 */
RZG2L_GPIO_PORT_SPARSE_PACK(0x2, 0x08, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
PIN_CFG_IEN | PIN_CFG_NOGPIO_INT), /* P21 */
@@ -1895,8 +1887,7 @@ static const u64 r9a07g043_gpio_configs[] = {
PIN_CFG_IEN | PIN_CFG_NOGPIO_INT), /* P22 */
RZG2L_GPIO_PORT_SPARSE_PACK_VARIABLE(0x3e, 0x0a), /* P23 */
RZG2L_GPIO_PORT_PACK_VARIABLE(6, 0x0b), /* P24 */
- RZG2L_GPIO_PORT_SPARSE_PACK(0x2, 0x0c, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_FILONOFF |
- PIN_CFG_FILNUM | PIN_CFG_FILCLKSEL |
+ RZG2L_GPIO_PORT_SPARSE_PACK(0x2, 0x0c, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_NF |
PIN_CFG_NOGPIO_INT), /* P25 */
0x0, /* P26 */
0x0, /* P27 */
@@ -1974,8 +1965,7 @@ static const struct {
struct rzg2l_dedicated_configs rzg2l_pins[7];
} rzg2l_dedicated_pins = {
.common = {
- { "NMI", RZG2L_SINGLE_PIN_PACK(0x1, 0,
- (PIN_CFG_FILONOFF | PIN_CFG_FILNUM | PIN_CFG_FILCLKSEL)) },
+ { "NMI", RZG2L_SINGLE_PIN_PACK(0x1, 0, PIN_CFG_NF) },
{ "TMS/SWDIO", RZG2L_SINGLE_PIN_PACK(0x2, 0,
(PIN_CFG_IOLH_A | PIN_CFG_SR | PIN_CFG_IEN)) },
{ "TDO", RZG2L_SINGLE_PIN_PACK(0x3, 0,
@@ -2056,8 +2046,7 @@ static const struct {
};
static const struct rzg2l_dedicated_configs rzg3s_dedicated_pins[] = {
- { "NMI", RZG2L_SINGLE_PIN_PACK(0x0, 0, (PIN_CFG_FILONOFF | PIN_CFG_FILNUM |
- PIN_CFG_FILCLKSEL)) },
+ { "NMI", RZG2L_SINGLE_PIN_PACK(0x0, 0, PIN_CFG_NF) },
{ "TMS/SWDIO", RZG2L_SINGLE_PIN_PACK(0x1, 0, (PIN_CFG_IOLH_A | PIN_CFG_IEN |
PIN_CFG_SOFT_PS)) },
{ "TDO", RZG2L_SINGLE_PIN_PACK(0x1, 1, (PIN_CFG_IOLH_A | PIN_CFG_SOFT_PS)) },
@@ -2096,8 +2085,7 @@ static const struct rzg2l_dedicated_configs rzg3s_dedicated_pins[] = {
};
static struct rzg2l_dedicated_configs rzv2h_dedicated_pins[] = {
- { "NMI", RZG2L_SINGLE_PIN_PACK(0x1, 0, (PIN_CFG_FILONOFF | PIN_CFG_FILNUM |
- PIN_CFG_FILCLKSEL)) },
+ { "NMI", RZG2L_SINGLE_PIN_PACK(0x1, 0, PIN_CFG_NF) },
{ "TMS_SWDIO", RZG2L_SINGLE_PIN_PACK(0x3, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
PIN_CFG_IEN)) },
{ "TDO", RZG2L_SINGLE_PIN_PACK(0x3, 2, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] pinctrl: renesas: rzg2l: Move pinconf_to_config_argument() call outside of switch cases
2024-08-27 13:17 [PATCH 0/2] pinctrl: renesas: rzg2l: Simplify noise filter cfg macro Prabhakar
2024-08-27 13:17 ` [PATCH 1/2] pinctrl: renesas: rzg2l: Introduce single macro for digital noise filter configuration Prabhakar
@ 2024-08-27 13:17 ` Prabhakar
2024-08-29 13:15 ` Geert Uytterhoeven
1 sibling, 1 reply; 7+ messages in thread
From: Prabhakar @ 2024-08-27 13:17 UTC (permalink / raw)
To: Geert Uytterhoeven, Linus Walleij
Cc: linux-renesas-soc, linux-gpio, linux-kernel, Prabhakar, Biju Das,
Fabrizio Castro, Lad Prabhakar
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Refactor the `rzg2l_pinctrl_pinconf_set()` function by moving the call to
`arg = pinconf_to_config_argument(_configs[i])` to the beginning of the
loop. Previously, this call was redundantly made in each case of the
switch statement.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 8fc1f28d02d1..e742a4e3eb4a 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -1384,9 +1384,9 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
for (i = 0; i < num_configs; i++) {
param = pinconf_to_config_param(_configs[i]);
+ arg = pinconf_to_config_argument(_configs[i]);
switch (param) {
case PIN_CONFIG_INPUT_ENABLE:
- arg = pinconf_to_config_argument(_configs[i]);
if (!(cfg & PIN_CFG_IEN))
return -EINVAL;
@@ -1395,7 +1395,6 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
break;
case PIN_CONFIG_OUTPUT_ENABLE:
- arg = pinconf_to_config_argument(_configs[i]);
if (!(cfg & PIN_CFG_OEN))
return -EINVAL;
if (!pctrl->data->oen_write)
@@ -1410,8 +1409,6 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
break;
case PIN_CONFIG_SLEW_RATE:
- arg = pinconf_to_config_argument(_configs[i]);
-
if (!(cfg & PIN_CFG_SR) || arg > 1)
return -EINVAL;
@@ -1432,8 +1429,6 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
break;
case PIN_CONFIG_DRIVE_STRENGTH:
- arg = pinconf_to_config_argument(_configs[i]);
-
if (!(cfg & PIN_CFG_IOLH_A) || hwcfg->drive_strength_ua)
return -EINVAL;
@@ -1457,8 +1452,6 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
break;
case PIN_CONFIG_OUTPUT_IMPEDANCE_OHMS:
- arg = pinconf_to_config_argument(_configs[i]);
-
if (!(cfg & PIN_CFG_IOLH_B) || !hwcfg->iolh_groupb_oi[0])
return -EINVAL;
@@ -1476,7 +1469,6 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
if (!(cfg & PIN_CFG_IOLH_RZV2H))
return -EINVAL;
- arg = pinconf_to_config_argument(_configs[i]);
if (arg > 3)
return -EINVAL;
rzg2l_rmw_pin_config(pctrl, IOLH(off), bit, IOLH_MASK, arg);
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] pinctrl: renesas: rzg2l: Introduce single macro for digital noise filter configuration
2024-08-27 13:17 ` [PATCH 1/2] pinctrl: renesas: rzg2l: Introduce single macro for digital noise filter configuration Prabhakar
@ 2024-08-29 13:09 ` Geert Uytterhoeven
2024-08-29 19:15 ` Lad, Prabhakar
0 siblings, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2024-08-29 13:09 UTC (permalink / raw)
To: Prabhakar
Cc: Linus Walleij, linux-renesas-soc, linux-gpio, linux-kernel,
Biju Das, Fabrizio Castro, Lad Prabhakar
Hi Prabhakar,
Thanks for your patch!
On Tue, Aug 27, 2024 at 3:17 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> When enabling the digital noise filter for the pins, it is necessary to
> configure both the noise filter stages (via the FILNUM register) and the
> sampling interval (via the FILCLKSEL register). To simplify this process,
> we introduce a single macro for configuring the digital noise filter.
Currently the pin control tables just declare which pins support
digital noise filter configuration, but the driver does not support
configuring the digital noise filters yet, right?
So I'd reword the paragraph above to something like:
Support for enabling the digital noise filter, and support for
configuring the noise filter stages (via the FILNUM register) and the
sampling interval (via the FILCLKSEL register) are related: a pin
supports either all or none of them. Hence simplify declaring digital
noise filter support for a pin by using a single feature flag instead of
three separate flags.
> This patch removes the PIN_CFG_FILNUM and PIN_CFG_FILCLKSEL configuration
> macros and renames PIN_CFG_FILONOFF to PIN_CFG_NF.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
For the actual patch contents:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] pinctrl: renesas: rzg2l: Move pinconf_to_config_argument() call outside of switch cases
2024-08-27 13:17 ` [PATCH 2/2] pinctrl: renesas: rzg2l: Move pinconf_to_config_argument() call outside of switch cases Prabhakar
@ 2024-08-29 13:15 ` Geert Uytterhoeven
2024-08-29 19:17 ` Lad, Prabhakar
0 siblings, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2024-08-29 13:15 UTC (permalink / raw)
To: Prabhakar
Cc: Linus Walleij, linux-renesas-soc, linux-gpio, linux-kernel,
Biju Das, Fabrizio Castro, Lad Prabhakar
Hi Prabhakar,
Thanks for your patch!
On Tue, Aug 27, 2024 at 3:17 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Refactor the `rzg2l_pinctrl_pinconf_set()` function by moving the call to
> `arg = pinconf_to_config_argument(_configs[i])` to the beginning of the
> loop. Previously, this call was redundantly made in each case of the
> switch statement.
This is not 100% true: the PIN_CONFIG_BIAS_* cases do not
call pinconf_to_config_argument(). But I agree that calling it
unconditionally doesn't harm.
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -1395,7 +1395,6 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
> break;
>
> case PIN_CONFIG_OUTPUT_ENABLE:
> - arg = pinconf_to_config_argument(_configs[i]);
> if (!(cfg & PIN_CFG_OEN))
> return -EINVAL;
> if (!pctrl->data->oen_write)
Missed opportunity for simplification:
case PIN_CONFIG_POWER_SOURCE:
- settings.power_source =
pinconf_to_config_argument(_configs[i]);
+ settings.power_source = arg;
break;
> @@ -1432,8 +1429,6 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
> break;
>
> case PIN_CONFIG_DRIVE_STRENGTH:
> - arg = pinconf_to_config_argument(_configs[i]);
> -
> if (!(cfg & PIN_CFG_IOLH_A) || hwcfg->drive_strength_ua)
> return -EINVAL;
>
case PIN_CONFIG_DRIVE_STRENGTH_UA:
...
- settings.drive_strength_ua =
pinconf_to_config_argument(_configs[i]);
+ settings.drive_strength_ua = arg;
break;
The rest LGTM.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] pinctrl: renesas: rzg2l: Introduce single macro for digital noise filter configuration
2024-08-29 13:09 ` Geert Uytterhoeven
@ 2024-08-29 19:15 ` Lad, Prabhakar
0 siblings, 0 replies; 7+ messages in thread
From: Lad, Prabhakar @ 2024-08-29 19:15 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, linux-renesas-soc, linux-gpio, linux-kernel,
Biju Das, Fabrizio Castro, Lad Prabhakar
Hi Geert,
Thank you for the review.
On Thu, Aug 29, 2024 at 2:09 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> Thanks for your patch!
>
> On Tue, Aug 27, 2024 at 3:17 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > When enabling the digital noise filter for the pins, it is necessary to
> > configure both the noise filter stages (via the FILNUM register) and the
> > sampling interval (via the FILCLKSEL register). To simplify this process,
> > we introduce a single macro for configuring the digital noise filter.
>
> Currently the pin control tables just declare which pins support
> digital noise filter configuration, but the driver does not support
> configuring the digital noise filters yet, right?
>
Yes that's right.
> So I'd reword the paragraph above to something like:
>
> Support for enabling the digital noise filter, and support for
> configuring the noise filter stages (via the FILNUM register) and the
> sampling interval (via the FILCLKSEL register) are related: a pin
> supports either all or none of them. Hence simplify declaring digital
> noise filter support for a pin by using a single feature flag instead of
> three separate flags.
>
Ok, I'll update the commit description as above.
Cheers,
Prabhakar
> > This patch removes the PIN_CFG_FILNUM and PIN_CFG_FILCLKSEL configuration
> > macros and renames PIN_CFG_FILONOFF to PIN_CFG_NF.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> For the actual patch contents:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] pinctrl: renesas: rzg2l: Move pinconf_to_config_argument() call outside of switch cases
2024-08-29 13:15 ` Geert Uytterhoeven
@ 2024-08-29 19:17 ` Lad, Prabhakar
0 siblings, 0 replies; 7+ messages in thread
From: Lad, Prabhakar @ 2024-08-29 19:17 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, linux-renesas-soc, linux-gpio, linux-kernel,
Biju Das, Fabrizio Castro, Lad Prabhakar
Hi Geert,
Thank you for the review.
On Thu, Aug 29, 2024 at 2:15 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> Thanks for your patch!
>
> On Tue, Aug 27, 2024 at 3:17 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Refactor the `rzg2l_pinctrl_pinconf_set()` function by moving the call to
> > `arg = pinconf_to_config_argument(_configs[i])` to the beginning of the
> > loop. Previously, this call was redundantly made in each case of the
> > switch statement.
>
> This is not 100% true: the PIN_CONFIG_BIAS_* cases do not
> call pinconf_to_config_argument(). But I agree that calling it
> unconditionally doesn't harm.
>
Ok, I'll update the commit description to below:
Refactor the `rzg2l_pinctrl_pinconf_set()` function by moving the call to
`arg = pinconf_to_config_argument(_configs[i])` to the beginning of the
loop. Previously, this call was redundantly made in most cases within the
switch statement.
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -1395,7 +1395,6 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
> > break;
> >
> > case PIN_CONFIG_OUTPUT_ENABLE:
> > - arg = pinconf_to_config_argument(_configs[i]);
> > if (!(cfg & PIN_CFG_OEN))
> > return -EINVAL;
> > if (!pctrl->data->oen_write)
>
> Missed opportunity for simplification:
>
> case PIN_CONFIG_POWER_SOURCE:
> - settings.power_source =
> pinconf_to_config_argument(_configs[i]);
> + settings.power_source = arg;
> break;
>
And while at it I'll replace as above and below in the v2.
Cheers,
Prabhakar
> > @@ -1432,8 +1429,6 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
> > break;
> >
> > case PIN_CONFIG_DRIVE_STRENGTH:
> > - arg = pinconf_to_config_argument(_configs[i]);
> > -
> > if (!(cfg & PIN_CFG_IOLH_A) || hwcfg->drive_strength_ua)
> > return -EINVAL;
> >
>
> case PIN_CONFIG_DRIVE_STRENGTH_UA:
> ...
> - settings.drive_strength_ua =
> pinconf_to_config_argument(_configs[i]);
> + settings.drive_strength_ua = arg;
> break;
>
> The rest LGTM.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-08-29 19:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-27 13:17 [PATCH 0/2] pinctrl: renesas: rzg2l: Simplify noise filter cfg macro Prabhakar
2024-08-27 13:17 ` [PATCH 1/2] pinctrl: renesas: rzg2l: Introduce single macro for digital noise filter configuration Prabhakar
2024-08-29 13:09 ` Geert Uytterhoeven
2024-08-29 19:15 ` Lad, Prabhakar
2024-08-27 13:17 ` [PATCH 2/2] pinctrl: renesas: rzg2l: Move pinconf_to_config_argument() call outside of switch cases Prabhakar
2024-08-29 13:15 ` Geert Uytterhoeven
2024-08-29 19:17 ` Lad, Prabhakar
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).