[RFC PATCH 0/4] Add "output-impedance" property to pinconf-generic

[RFC PATCH 0/4] Add "output-impedance" property to pinconf-generic

[Lad Prabhakar]

Hi All,

The first two patches of the series add add "output-impedance" property
to pinconf-generic and the last two patches add support to get/set
drive-strength and output-impedance for RZ/G2L SoC.

Cheers,
Prabhakar

Lad Prabhakar (4):
  dt-bindings: pincfg-node: Add "output-impedance" property
  pinctrl: pinconf-generic: Add support for "output-impedance" to be
    extracted from DT files
  dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Add output-impedance
    property
  pinctrl: renesas: pinctrl-rzg2l: Add support to get/set drive-strength
    and output-impedance

 .../bindings/pinctrl/pincfg-node.yaml         |   4 +
 .../pinctrl/renesas,rzg2l-pinctrl.yaml        |   2 +
 drivers/pinctrl/pinconf-generic.c             |   2 +
 drivers/pinctrl/renesas/pinctrl-rzg2l.c       | 181 ++++++++++++++----
 include/linux/pinctrl/pinconf-generic.h       |   3 +
 5 files changed, 159 insertions(+), 33 deletions(-)

-- 
2.17.1

[RFC PATCH 1/4] dt-bindings: pincfg-node: Add "output-impedance" property

[Lad Prabhakar]

On RZ/G2L SoC for Group-B pins, output impedance can be configured.
This patch documents "output-impedance" property in pincfg-node.yaml so
that other platforms requiring such feature can make use of this property.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
index 71ed0a9def84..cdcb23daeca2 100644
--- a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
@@ -114,6 +114,10 @@ properties:
     description: enable output on a pin without actively driving it
       (such as enabling an output buffer)
 
+  output-impedance:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: set the pins output impedance at most X ohm
+
   output-low:
     type: boolean
     description: set the pin to output mode with low level
-- 
2.17.1

Re: [RFC PATCH 1/4] dt-bindings: pincfg-node: Add "output-impedance" property

[Rob Herring]

On Thu, Sep 30, 2021 at 01:16:27PM +0100, Lad Prabhakar wrote:
> On RZ/G2L SoC for Group-B pins, output impedance can be configured.
> This patch documents "output-impedance" property in pincfg-node.yaml so
> that other platforms requiring such feature can make use of this property.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> index 71ed0a9def84..cdcb23daeca2 100644
> --- a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> @@ -114,6 +114,10 @@ properties:
>      description: enable output on a pin without actively driving it
>        (such as enabling an output buffer)
>  
> +  output-impedance:
> +    $ref: /schemas/types.yaml#/definitions/uint32

Use standard unit suffix and drop the type.

> +    description: set the pins output impedance at most X ohm
> +
>    output-low:
>      type: boolean
>      description: set the pin to output mode with low level
> -- 
> 2.17.1
> 
> 

Re: [RFC PATCH 1/4] dt-bindings: pincfg-node: Add "output-impedance" property

[Lad, Prabhakar]

Hi Rob,

Thank you for the review.

On Wed, Oct 6, 2021 at 10:14 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Sep 30, 2021 at 01:16:27PM +0100, Lad Prabhakar wrote:
> > On RZ/G2L SoC for Group-B pins, output impedance can be configured.
> > This patch documents "output-impedance" property in pincfg-node.yaml so
> > that other platforms requiring such feature can make use of this property.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> > index 71ed0a9def84..cdcb23daeca2 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> > @@ -114,6 +114,10 @@ properties:
> >      description: enable output on a pin without actively driving it
> >        (such as enabling an output buffer)
> >
> > +  output-impedance:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
>
> Use standard unit suffix and drop the type.
>
Sure will do in the next version.

Cheers,
Prabhakar

> > +    description: set the pins output impedance at most X ohm
> > +
> >    output-low:
> >      type: boolean
> >      description: set the pin to output mode with low level
> > --
> > 2.17.1
> >
> >

Re: [RFC PATCH 1/4] dt-bindings: pincfg-node: Add "output-impedance" property

[Geert Uytterhoeven]

Hi Prabhakar,

On Thu, Sep 30, 2021 at 2:17 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> On RZ/G2L SoC for Group-B pins, output impedance can be configured.
> This patch documents "output-impedance" property in pincfg-node.yaml so
> that other platforms requiring such feature can make use of this property.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> @@ -114,6 +114,10 @@ properties:
>      description: enable output on a pin without actively driving it
>        (such as enabling an output buffer)
>
> +  output-impedance:

output-impedance-ohms (ugh, the standard suffix is plural)

Yes, I know only one other property has a standard unit suffix from
dt-schema/schemas/property-units.yaml. Should we add properties
including standard unit suffixes, and deprecate the old ones?

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: set the pins output impedance at most X ohm
> +
>    output-low:
>      type: boolean
>      description: set the pin to output mode with low level

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

Re: [RFC PATCH 1/4] dt-bindings: pincfg-node: Add "output-impedance" property

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Thu, Oct 7, 2021 at 5:57 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Thu, Sep 30, 2021 at 2:17 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > On RZ/G2L SoC for Group-B pins, output impedance can be configured.
> > This patch documents "output-impedance" property in pincfg-node.yaml so
> > that other platforms requiring such feature can make use of this property.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> > @@ -114,6 +114,10 @@ properties:
> >      description: enable output on a pin without actively driving it
> >        (such as enabling an output buffer)
> >
> > +  output-impedance:
>
> output-impedance-ohms (ugh, the standard suffix is plural)
>
> Yes, I know only one other property has a standard unit suffix from
> dt-schema/schemas/property-units.yaml. Should we add properties
> including standard unit suffixes, and deprecate the old ones?
>
Yes, that should be the way to go forward.

Cheers,
Prabhakar

> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: set the pins output impedance at most X ohm
> > +
> >    output-low:
> >      type: boolean
> >      description: set the pin to output mode with low level
>
> 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

[RFC PATCH 2/4] pinctrl: pinconf-generic: Add support for "output-impedance" to be extracted from DT files

[Lad Prabhakar]

Add "output-impedance" property to generic options used for DT parsing
files. This enables drivers, which use generic pin configurations, to
get the value passed to this property.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/pinctrl/pinconf-generic.c       | 2 ++
 include/linux/pinctrl/pinconf-generic.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 22e8d4c4040e..3bff4e4340b9 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -46,6 +46,7 @@ static const struct pin_config_item conf_items[] = {
 	PCONFDUMP(PIN_CONFIG_MODE_LOW_POWER, "pin low power", "mode", true),
 	PCONFDUMP(PIN_CONFIG_OUTPUT_ENABLE, "output enabled", NULL, false),
 	PCONFDUMP(PIN_CONFIG_OUTPUT, "pin output", "level", true),
+	PCONFDUMP(PIN_CONFIG_OUTPUT_IMPEDANCE, "output impedance", "ohm", true),
 	PCONFDUMP(PIN_CONFIG_POWER_SOURCE, "pin power source", "selector", true),
 	PCONFDUMP(PIN_CONFIG_SLEEP_HARDWARE_STATE, "sleep hardware state", NULL, false),
 	PCONFDUMP(PIN_CONFIG_SLEW_RATE, "slew rate", NULL, true),
@@ -179,6 +180,7 @@ static const struct pinconf_generic_params dt_params[] = {
 	{ "output-disable", PIN_CONFIG_OUTPUT_ENABLE, 0 },
 	{ "output-enable", PIN_CONFIG_OUTPUT_ENABLE, 1 },
 	{ "output-high", PIN_CONFIG_OUTPUT, 1, },
+	{ "output-impedance", PIN_CONFIG_OUTPUT_IMPEDANCE, 0 },
 	{ "output-low", PIN_CONFIG_OUTPUT, 0, },
 	{ "power-source", PIN_CONFIG_POWER_SOURCE, 0 },
 	{ "sleep-hardware-state", PIN_CONFIG_SLEEP_HARDWARE_STATE, 0 },
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index eee0e3948537..e6293ebd4052 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -91,6 +91,8 @@ struct pinctrl_map;
  * 	configuration (eg. the currently selected mux function) drive values on
  * 	the line. Use argument 1 to enable output mode, argument 0 to disable
  * 	it.
+ * @PIN_CONFIG_OUTPUT_IMPEDANCE: this will configure the pins output impedance with
+ *	 the value passed as argument. The argument is in ohm.
  * @PIN_CONFIG_PERSIST_STATE: retain pin state across sleep or controller reset
  * @PIN_CONFIG_POWER_SOURCE: if the pin can select between different power
  *	supplies, the argument to this parameter (on a custom format) tells
@@ -129,6 +131,7 @@ enum pin_config_param {
 	PIN_CONFIG_MODE_PWM,
 	PIN_CONFIG_OUTPUT,
 	PIN_CONFIG_OUTPUT_ENABLE,
+	PIN_CONFIG_OUTPUT_IMPEDANCE,
 	PIN_CONFIG_PERSIST_STATE,
 	PIN_CONFIG_POWER_SOURCE,
 	PIN_CONFIG_SKEW_DELAY,
-- 
2.17.1

Re: [RFC PATCH 2/4] pinctrl: pinconf-generic: Add support for "output-impedance" to be extracted from DT files

[Geert Uytterhoeven]

Hi Prabhakar,

On Thu, Sep 30, 2021 at 2:17 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add "output-impedance" property to generic options used for DT parsing
> files. This enables drivers, which use generic pin configurations, to
> get the value passed to this property.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/pinctrl/pinconf-generic.c
> +++ b/drivers/pinctrl/pinconf-generic.c
> @@ -46,6 +46,7 @@ static const struct pin_config_item conf_items[] = {
>         PCONFDUMP(PIN_CONFIG_MODE_LOW_POWER, "pin low power", "mode", true),
>         PCONFDUMP(PIN_CONFIG_OUTPUT_ENABLE, "output enabled", NULL, false),
>         PCONFDUMP(PIN_CONFIG_OUTPUT, "pin output", "level", true),
> +       PCONFDUMP(PIN_CONFIG_OUTPUT_IMPEDANCE, "output impedance", "ohm", true),
>         PCONFDUMP(PIN_CONFIG_POWER_SOURCE, "pin power source", "selector", true),
>         PCONFDUMP(PIN_CONFIG_SLEEP_HARDWARE_STATE, "sleep hardware state", NULL, false),
>         PCONFDUMP(PIN_CONFIG_SLEW_RATE, "slew rate", NULL, true),
> @@ -179,6 +180,7 @@ static const struct pinconf_generic_params dt_params[] = {
>         { "output-disable", PIN_CONFIG_OUTPUT_ENABLE, 0 },
>         { "output-enable", PIN_CONFIG_OUTPUT_ENABLE, 1 },
>         { "output-high", PIN_CONFIG_OUTPUT, 1, },
> +       { "output-impedance", PIN_CONFIG_OUTPUT_IMPEDANCE, 0 },

"output-impedance-ohms", as per DT standardized property units.

>         { "output-low", PIN_CONFIG_OUTPUT, 0, },
>         { "power-source", PIN_CONFIG_POWER_SOURCE, 0 },
>         { "sleep-hardware-state", PIN_CONFIG_SLEEP_HARDWARE_STATE, 0 },

With the above fixed:
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

Re: [RFC PATCH 2/4] pinctrl: pinconf-generic: Add support for "output-impedance" to be extracted from DT files

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Thu, Oct 7, 2021 at 6:02 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Thu, Sep 30, 2021 at 2:17 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Add "output-impedance" property to generic options used for DT parsing
> > files. This enables drivers, which use generic pin configurations, to
> > get the value passed to this property.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/pinctrl/pinconf-generic.c
> > +++ b/drivers/pinctrl/pinconf-generic.c
> > @@ -46,6 +46,7 @@ static const struct pin_config_item conf_items[] = {
> >         PCONFDUMP(PIN_CONFIG_MODE_LOW_POWER, "pin low power", "mode", true),
> >         PCONFDUMP(PIN_CONFIG_OUTPUT_ENABLE, "output enabled", NULL, false),
> >         PCONFDUMP(PIN_CONFIG_OUTPUT, "pin output", "level", true),
> > +       PCONFDUMP(PIN_CONFIG_OUTPUT_IMPEDANCE, "output impedance", "ohm", true),
> >         PCONFDUMP(PIN_CONFIG_POWER_SOURCE, "pin power source", "selector", true),
> >         PCONFDUMP(PIN_CONFIG_SLEEP_HARDWARE_STATE, "sleep hardware state", NULL, false),
> >         PCONFDUMP(PIN_CONFIG_SLEW_RATE, "slew rate", NULL, true),
> > @@ -179,6 +180,7 @@ static const struct pinconf_generic_params dt_params[] = {
> >         { "output-disable", PIN_CONFIG_OUTPUT_ENABLE, 0 },
> >         { "output-enable", PIN_CONFIG_OUTPUT_ENABLE, 1 },
> >         { "output-high", PIN_CONFIG_OUTPUT, 1, },
> > +       { "output-impedance", PIN_CONFIG_OUTPUT_IMPEDANCE, 0 },
>
> "output-impedance-ohms", as per DT standardized property units.
>
Sure will replace that with output-impedance-ohms and to chime in Ill
re-name the macro to PIN_CONFIG_OUTPUT_IMPEDANCE_OHMS

Cheers,
Prabhakar

> >         { "output-low", PIN_CONFIG_OUTPUT, 0, },
> >         { "power-source", PIN_CONFIG_POWER_SOURCE, 0 },
> >         { "sleep-hardware-state", PIN_CONFIG_SLEEP_HARDWARE_STATE, 0 },
>
> With the above fixed:
> 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

[RFC PATCH 3/4] dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Add output-impedance property

[Lad Prabhakar]

RZ/G2L SoC has two groups of pins, Group-A and Group-B. RZ/G2L SoC supports
configuring Output Impedance for Group-B pins (valid values 33/50/66/100).

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 .../devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml      | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
index ef68dabcf4dc..8abdf1e8153f 100644
--- a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
@@ -73,6 +73,8 @@ additionalProperties:
         pins: true
         drive-strength:
           enum: [ 2, 4, 8, 12 ]
+        output-impedance:
+          enum: [ 33, 50, 66, 100 ]
         power-source:
           enum: [ 1800, 2500, 3300 ]
         slew-rate: true
-- 
2.17.1

Re: [RFC PATCH 3/4] dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Add output-impedance property

[Geert Uytterhoeven]

Hi Prabhakar,

On Thu, Sep 30, 2021 at 2:17 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> RZ/G2L SoC has two groups of pins, Group-A and Group-B. RZ/G2L SoC supports
> configuring Output Impedance for Group-B pins (valid values 33/50/66/100).
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
> @@ -73,6 +73,8 @@ additionalProperties:
>          pins: true
>          drive-strength:
>            enum: [ 2, 4, 8, 12 ]
> +        output-impedance:

"output-impedance-ohms", as per DT standardized property units.

> +          enum: [ 33, 50, 66, 100 ]
>          power-source:
>            enum: [ 1800, 2500, 3300 ]
>          slew-rate: true

With the above fixed:
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

[RFC PATCH 4/4] pinctrl: renesas: pinctrl-rzg2l: Add support to get/set drive-strength and output-impedance

[Lad Prabhakar]

Add support to get/set drive-strength and output-impedance of the pins.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 181 +++++++++++++++++++-----
 1 file changed, 148 insertions(+), 33 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 20b2af889ca9..d75b31ca33ab 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -47,6 +47,7 @@
 #define PIN_CFG_FILONOFF		BIT(9)
 #define PIN_CFG_FILNUM			BIT(10)
 #define PIN_CFG_FILCLKSEL		BIT(11)
+#define PIN_CFG_GROUP_B			BIT(12)
 
 #define RZG2L_MPXED_PIN_FUNCS		(PIN_CFG_IOLH | \
 					 PIN_CFG_SR | \
@@ -86,6 +87,7 @@
 #define PMC(n)			(0x0200 + 0x10 + (n))
 #define PFC(n)			(0x0400 + 0x40 + (n) * 4)
 #define PIN(n)			(0x0800 + 0x10 + (n))
+#define IOLH(n)			(0x1010 + (n) * 8 - 0x10)
 #define IEN(n)			(0x1800 + (n) * 8)
 #define PWPR			(0x3014)
 #define SD_CH(n)		(0x3000 + (n) * 4)
@@ -101,6 +103,7 @@
 #define PVDD_MASK		0x01
 #define PFC_MASK		0x07
 #define IEN_MASK		0x01
+#define IOLH_MASK		0x03
 
 #define PM_INPUT		0x1
 #define PM_OUTPUT		0x2
@@ -424,6 +427,23 @@ static int rzg2l_dt_node_to_map(struct pinctrl_dev *pctldev,
 	return ret;
 }
 
+static int rzg2l_validate_gpio_pin(struct rzg2l_pinctrl *pctrl,
+				   u32 cfg, u32 port, u8 bit)
+{
+	u8 pincount = RZG2L_GPIO_PORT_GET_PINCNT(cfg);
+	u32 port_index = RZG2L_GPIO_PORT_GET_INDEX(cfg);
+	u32 data;
+
+	if (bit >= pincount || port >= pctrl->data->n_port_pins)
+		return -EINVAL;
+
+	data = pctrl->data->port_pin_configs[port];
+	if (port_index != RZG2L_GPIO_PORT_GET_INDEX(data))
+		return -EINVAL;
+
+	return 0;
+}
+
 static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
 				     unsigned int _pin,
 				     unsigned long *config)
@@ -432,6 +452,7 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
 	enum pin_config_param param = pinconf_to_config_param(*config);
 	const struct pinctrl_pin_desc *pin = &pctrl->desc.pins[_pin];
 	unsigned int *pin_data = pin->drv_data;
+	bool groupb_pin = false;
 	unsigned int arg = 0;
 	unsigned long flags;
 	void __iomem *addr;
@@ -446,6 +467,14 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
 		port = RZG2L_SINGLE_PIN_GET_PORT(*pin_data);
 		cfg = RZG2L_SINGLE_PIN_GET_CFGS(*pin_data);
 		bit = RZG2L_SINGLE_PIN_GET_BIT(*pin_data);
+		groupb_pin = cfg & PIN_CFG_GROUP_B;
+	} else {
+		cfg = RZG2L_GPIO_PORT_GET_CFGS(*pin_data);
+		port = RZG2L_PIN_ID_TO_PORT(_pin);
+		bit = RZG2L_PIN_ID_TO_PIN(_pin);
+
+		if (rzg2l_validate_gpio_pin(pctrl, *pin_data, port, bit))
+			return -EINVAL;
 	}
 
 	switch (param) {
@@ -484,6 +513,38 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
 		break;
 	}
 
+	case PIN_CONFIG_OUTPUT_IMPEDANCE:
+	case PIN_CONFIG_DRIVE_STRENGTH: {
+		unsigned int mA[4] = { 2, 4, 8, 12 };
+		unsigned int oi[4] = { 100, 66, 50, 33 };
+
+		if (param == PIN_CONFIG_DRIVE_STRENGTH) {
+			if (!(cfg & PIN_CFG_IOLH) || groupb_pin)
+				return -EINVAL;
+		} else {
+			if (!(cfg & PIN_CFG_IOLH) || !groupb_pin)
+				return -EINVAL;
+		}
+
+		spin_lock_irqsave(&pctrl->lock, flags);
+
+		/* handle _L/_H for 32-bit register read/write */
+		addr = pctrl->base + IOLH(port);
+		if (bit >= 4) {
+			bit -= 4;
+			addr += 4;
+		}
+
+		reg = readl(addr) & (IOLH_MASK << (bit * 8));
+		reg = reg >> (bit * 8);
+		if (param == PIN_CONFIG_DRIVE_STRENGTH)
+			arg = mA[reg];
+		else
+			arg = oi[reg];
+		spin_unlock_irqrestore(&pctrl->lock, flags);
+		break;
+	}
+
 	default:
 		return -ENOTSUPP;
 	}
@@ -502,6 +563,7 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
 	const struct pinctrl_pin_desc *pin = &pctrl->desc.pins[_pin];
 	unsigned int *pin_data = pin->drv_data;
 	enum pin_config_param param;
+	bool groupb_pin = false;
 	unsigned long flags;
 	void __iomem *addr;
 	u32 port = 0, reg;
@@ -516,6 +578,14 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
 		port = RZG2L_SINGLE_PIN_GET_PORT(*pin_data);
 		cfg = RZG2L_SINGLE_PIN_GET_CFGS(*pin_data);
 		bit = RZG2L_SINGLE_PIN_GET_BIT(*pin_data);
+		groupb_pin = cfg & PIN_CFG_GROUP_B;
+	} else {
+		cfg = RZG2L_GPIO_PORT_GET_CFGS(*pin_data);
+		port = RZG2L_PIN_ID_TO_PORT(_pin);
+		bit = RZG2L_PIN_ID_TO_PIN(_pin);
+
+		if (rzg2l_validate_gpio_pin(pctrl, *pin_data, port, bit))
+			return -EINVAL;
 	}
 
 	for (i = 0; i < num_configs; i++) {
@@ -564,6 +634,51 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
 			spin_unlock_irqrestore(&pctrl->lock, flags);
 			break;
 		}
+
+		case PIN_CONFIG_OUTPUT_IMPEDANCE:
+		case PIN_CONFIG_DRIVE_STRENGTH: {
+			unsigned int arg = pinconf_to_config_argument(_configs[i]);
+			unsigned int mA[4] = { 2, 4, 8, 12 };
+			unsigned int oi[4] = { 100, 66, 50, 33 };
+
+			if (param == PIN_CONFIG_DRIVE_STRENGTH) {
+				if (!(cfg & PIN_CFG_IOLH) || groupb_pin)
+					return -EINVAL;
+
+				for (i = 0; i < ARRAY_SIZE(mA); i++) {
+					if (arg == mA[i])
+						break;
+				}
+
+				if (i >= ARRAY_SIZE(mA))
+					return -EINVAL;
+			} else {
+				if (!(cfg & PIN_CFG_IOLH) || !groupb_pin)
+					return -EINVAL;
+
+				for (i = 0; i < ARRAY_SIZE(oi); i++) {
+					if (arg == oi[i])
+						break;
+				}
+				if (i >= ARRAY_SIZE(oi))
+					return -EINVAL;
+			}
+
+			spin_lock_irqsave(&pctrl->lock, flags);
+
+			/* handle _L/_H for 32-bit register read/write */
+			addr = pctrl->base + IOLH(port);
+			if (bit >= 4) {
+				bit -= 4;
+				addr += 4;
+			}
+
+			reg = readl(addr) & ~(IOLH_MASK << (bit * 8));
+			writel(reg | (i << (bit * 8)), addr);
+			spin_unlock_irqrestore(&pctrl->lock, flags);
+			break;
+		}
+
 		default:
 			return -EOPNOTSUPP;
 		}
@@ -893,70 +1008,70 @@ static  struct rzg2l_dedicated_configs rzg2l_dedicated_pins[] = {
 	 (PIN_CFG_SR | PIN_CFG_IOLH | PIN_CFG_IEN)) },
 	{ "TDO", RZG2L_SINGLE_PIN_PACK(0x3, 0,
 	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN)) },
-	{ "AUDIO_CLK1", RZG2L_SINGLE_PIN_PACK(0x4, 0, PIN_CFG_IEN) },
-	{ "AUDIO_CLK2", RZG2L_SINGLE_PIN_PACK(0x4, 1, PIN_CFG_IEN) },
+	{ "AUDIO_CLK1", RZG2L_SINGLE_PIN_PACK(0x4, 0, PIN_CFG_IEN | PIN_CFG_GROUP_B) },
+	{ "AUDIO_CLK2", RZG2L_SINGLE_PIN_PACK(0x4, 1, PIN_CFG_IEN | PIN_CFG_GROUP_B) },
 	{ "SD0_CLK", RZG2L_SINGLE_PIN_PACK(0x6, 0,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_SD0)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_SD0 | PIN_CFG_GROUP_B)) },
 	{ "SD0_CMD", RZG2L_SINGLE_PIN_PACK(0x6, 1,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0 | PIN_CFG_GROUP_B)) },
 	{ "SD0_RST#", RZG2L_SINGLE_PIN_PACK(0x6, 2,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_SD0)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_SD0 | PIN_CFG_GROUP_B)) },
 	{ "SD0_DATA0", RZG2L_SINGLE_PIN_PACK(0x7, 0,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0 | PIN_CFG_GROUP_B)) },
 	{ "SD0_DATA1", RZG2L_SINGLE_PIN_PACK(0x7, 1,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0 | PIN_CFG_GROUP_B)) },
 	{ "SD0_DATA2", RZG2L_SINGLE_PIN_PACK(0x7, 2,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0 | PIN_CFG_GROUP_B)) },
 	{ "SD0_DATA3", RZG2L_SINGLE_PIN_PACK(0x7, 3,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0 | PIN_CFG_GROUP_B)) },
 	{ "SD0_DATA4", RZG2L_SINGLE_PIN_PACK(0x7, 4,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0 | PIN_CFG_GROUP_B)) },
 	{ "SD0_DATA5", RZG2L_SINGLE_PIN_PACK(0x7, 5,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0 | PIN_CFG_GROUP_B)) },
 	{ "SD0_DATA6", RZG2L_SINGLE_PIN_PACK(0x7, 6,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0 | PIN_CFG_GROUP_B)) },
 	{ "SD0_DATA7", RZG2L_SINGLE_PIN_PACK(0x7, 7,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD0 | PIN_CFG_GROUP_B)) },
 	{ "SD1_CLK", RZG2L_SINGLE_PIN_PACK(0x8, 0,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_SD1))},
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_SD1 | PIN_CFG_GROUP_B)) },
 	{ "SD1_CMD", RZG2L_SINGLE_PIN_PACK(0x8, 1,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD1)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD1 | PIN_CFG_GROUP_B)) },
 	{ "SD1_DATA0", RZG2L_SINGLE_PIN_PACK(0x9, 0,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD1)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD1 | PIN_CFG_GROUP_B)) },
 	{ "SD1_DATA1", RZG2L_SINGLE_PIN_PACK(0x9, 1,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD1)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD1 | PIN_CFG_GROUP_B)) },
 	{ "SD1_DATA2", RZG2L_SINGLE_PIN_PACK(0x9, 2,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD1)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD1 | PIN_CFG_GROUP_B)) },
 	{ "SD1_DATA3", RZG2L_SINGLE_PIN_PACK(0x9, 3,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD1)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IOLH_SD1 | PIN_CFG_GROUP_B)) },
 	{ "QSPI0_SPCLK", RZG2L_SINGLE_PIN_PACK(0xa, 0,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI0_IO0", RZG2L_SINGLE_PIN_PACK(0xa, 1,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI0_IO1", RZG2L_SINGLE_PIN_PACK(0xa, 2,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI0_IO2", RZG2L_SINGLE_PIN_PACK(0xa, 3,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI0_IO3", RZG2L_SINGLE_PIN_PACK(0xa, 4,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI0_SSL", RZG2L_SINGLE_PIN_PACK(0xa, 5,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI1_SPCLK", RZG2L_SINGLE_PIN_PACK(0xb, 0,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI1_IO0", RZG2L_SINGLE_PIN_PACK(0xb, 1,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI1_IO1", RZG2L_SINGLE_PIN_PACK(0xb, 2,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI1_IO2", RZG2L_SINGLE_PIN_PACK(0xb, 3,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI1_IO3", RZG2L_SINGLE_PIN_PACK(0xb, 4,
-	 (PIN_CFG_IOLH | PIN_CFG_SR  | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR  | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI1_SSL", RZG2L_SINGLE_PIN_PACK(0xb, 5,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI_RESET#", RZG2L_SINGLE_PIN_PACK(0xc, 0,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI_WP#", RZG2L_SINGLE_PIN_PACK(0xc, 1,
-	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+	 (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI | PIN_CFG_GROUP_B)) },
 	{ "QSPI_INT#", RZG2L_SINGLE_PIN_PACK(0xc, 2, (PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
 	{ "WDTOVF_PERROUT#", RZG2L_SINGLE_PIN_PACK(0xd, 0, (PIN_CFG_IOLH | PIN_CFG_SR)) },
 	{ "RIIC0_SDA", RZG2L_SINGLE_PIN_PACK(0xe, 0, PIN_CFG_IEN) },
-- 
2.17.1

Re: [RFC PATCH 4/4] pinctrl: renesas: pinctrl-rzg2l: Add support to get/set drive-strength and output-impedance

[Geert Uytterhoeven]

Hi Prabhakar,

On Thu, Sep 30, 2021 at 2:17 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add support to get/set drive-strength and output-impedance of the pins.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -47,6 +47,7 @@
>  #define PIN_CFG_FILONOFF               BIT(9)
>  #define PIN_CFG_FILNUM                 BIT(10)
>  #define PIN_CFG_FILCLKSEL              BIT(11)
> +#define PIN_CFG_GROUP_B                        BIT(12)

Perhaps it would be easier to have separate PIN_CFG_IOLH_A and
PIN_CFG_IOLH_B flags, instead of a PIN_CFG_IOLH flag and a
PIN_CFG_GROUP_B modifier flag?

>
>  #define RZG2L_MPXED_PIN_FUNCS          (PIN_CFG_IOLH | \
>                                          PIN_CFG_SR | \

> @@ -484,6 +513,38 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
>                 break;
>         }
>
> +       case PIN_CONFIG_OUTPUT_IMPEDANCE:
> +       case PIN_CONFIG_DRIVE_STRENGTH: {
> +               unsigned int mA[4] = { 2, 4, 8, 12 };
> +               unsigned int oi[4] = { 100, 66, 50, 33 };

static const

> +
> +               if (param == PIN_CONFIG_DRIVE_STRENGTH) {
> +                       if (!(cfg & PIN_CFG_IOLH) || groupb_pin)
> +                               return -EINVAL;
> +               } else {
> +                       if (!(cfg & PIN_CFG_IOLH) || !groupb_pin)
> +                               return -EINVAL;
> +               }
> +
> +               spin_lock_irqsave(&pctrl->lock, flags);
> +
> +               /* handle _L/_H for 32-bit register read/write */
> +               addr = pctrl->base + IOLH(port);
> +               if (bit >= 4) {
> +                       bit -= 4;
> +                       addr += 4;
> +               }
> +
> +               reg = readl(addr) & (IOLH_MASK << (bit * 8));
> +               reg = reg >> (bit * 8);
> +               if (param == PIN_CONFIG_DRIVE_STRENGTH)
> +                       arg = mA[reg];
> +               else
> +                       arg = oi[reg];
> +               spin_unlock_irqrestore(&pctrl->lock, flags);

I think you've reached the point where it starts to make sense to
have helper functions to read and modify these sub-register fields
that may be located into the current or next register.

And after that, you can split it in two smaller separate cases for
drive strength and output impedance.

> +               break;
> +       }
> +
>         default:
>                 return -ENOTSUPP;
>         }

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

Re: [RFC PATCH 4/4] pinctrl: renesas: pinctrl-rzg2l: Add support to get/set drive-strength and output-impedance

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Thu, Oct 7, 2021 at 6:23 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Thu, Sep 30, 2021 at 2:17 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Add support to get/set drive-strength and output-impedance of the pins.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -47,6 +47,7 @@
> >  #define PIN_CFG_FILONOFF               BIT(9)
> >  #define PIN_CFG_FILNUM                 BIT(10)
> >  #define PIN_CFG_FILCLKSEL              BIT(11)
> > +#define PIN_CFG_GROUP_B                        BIT(12)
>
> Perhaps it would be easier to have separate PIN_CFG_IOLH_A and
> PIN_CFG_IOLH_B flags, instead of a PIN_CFG_IOLH flag and a
> PIN_CFG_GROUP_B modifier flag?
>
Agreed will do that.

> >
> >  #define RZG2L_MPXED_PIN_FUNCS          (PIN_CFG_IOLH | \
> >                                          PIN_CFG_SR | \
>
> > @@ -484,6 +513,38 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> >                 break;
> >         }
> >
> > +       case PIN_CONFIG_OUTPUT_IMPEDANCE:
> > +       case PIN_CONFIG_DRIVE_STRENGTH: {
> > +               unsigned int mA[4] = { 2, 4, 8, 12 };
> > +               unsigned int oi[4] = { 100, 66, 50, 33 };
>
> static const
>
agreed.

> > +
> > +               if (param == PIN_CONFIG_DRIVE_STRENGTH) {
> > +                       if (!(cfg & PIN_CFG_IOLH) || groupb_pin)
> > +                               return -EINVAL;
> > +               } else {
> > +                       if (!(cfg & PIN_CFG_IOLH) || !groupb_pin)
> > +                               return -EINVAL;
> > +               }
> > +
> > +               spin_lock_irqsave(&pctrl->lock, flags);
> > +
> > +               /* handle _L/_H for 32-bit register read/write */
> > +               addr = pctrl->base + IOLH(port);
> > +               if (bit >= 4) {
> > +                       bit -= 4;
> > +                       addr += 4;
> > +               }
> > +
> > +               reg = readl(addr) & (IOLH_MASK << (bit * 8));
> > +               reg = reg >> (bit * 8);
> > +               if (param == PIN_CONFIG_DRIVE_STRENGTH)
> > +                       arg = mA[reg];
> > +               else
> > +                       arg = oi[reg];
> > +               spin_unlock_irqrestore(&pctrl->lock, flags);
>
> I think you've reached the point where it starts to make sense to
> have helper functions to read and modify these sub-register fields
> that may be located into the current or next register.
>
Ok will add helpers to read and rmw.

> And after that, you can split it in two smaller separate cases for
> drive strength and output impedance.
>
Agreed.

Cheers,
Prabhakar

> > +               break;
> > +       }
> > +
> >         default:
> >                 return -ENOTSUPP;
> >         }
>
> 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