2021-10-29 12:46:59

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v2 0/5] RZ/G2L: pinctrl: Support to get/set drive-strength and output-impedance-ohms

Hi All,

This patch series add support to get/set drive-strength and
output-impedance for RZ/G2L SoC. Along with some macro renames
and code cleanup.

Cheers,
Prabhakar

Changes for v2:
* Fixed review comments pointed by Geert, split up patch 4 from series [1]

Note: This patch series is dependent on first two patches of series [1]

[1] https://patchwork.kernel.org/project/linux-renesas-soc/cover/
[email protected]/

Lad Prabhakar (5):
dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Add output-impedance-ohms
property
pinctrl: renesas: pinctrl-rzg2l: Add helper functions to read/write
pin config
pinctrl: renesas: pinctrl-rzg2l: Add support to get/set pin config for
GPIO port pins
pinctrl: renesas: pinctrl-rzg2l: Rename PIN_CFG_* macros to match HW
manual
pinctrl: renesas: pinctrl-rzg2l: Add support to get/set drive-strength
and output-impedance-ohms

.../pinctrl/renesas,rzg2l-pinctrl.yaml | 2 +
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 316 ++++++++++++------
2 files changed, 222 insertions(+), 96 deletions(-)

--
2.17.1


2021-10-29 12:47:28

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v2 2/5] pinctrl: renesas: pinctrl-rzg2l: Add helper functions to read/write pin config

Add helper functions to read/read modify write pin config.

Switch to use helper functions for pins supporting PIN_CONFIG_INPUT_ENABLE
capabilities.

Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Biju Das <[email protected]>
---
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 78 +++++++++++++++++--------
1 file changed, 54 insertions(+), 24 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 20b2af889ca9..f294ae7b8b5a 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -91,6 +91,8 @@
#define SD_CH(n) (0x3000 + (n) * 4)
#define QSPI (0x3008)

+#define PORT_PIN_CFG_OFFSET 0x80
+
#define PVDD_1800 1 /* I/O domain voltage <= 1.8V */
#define PVDD_3300 0 /* I/O domain voltage >= 3.3V */

@@ -424,6 +426,52 @@ static int rzg2l_dt_node_to_map(struct pinctrl_dev *pctldev,
return ret;
}

+static u32 rzg2l_read_pin_config(struct rzg2l_pinctrl *pctrl, bool port_pin,
+ u32 offset, u8 bit, u32 mask)
+{
+ void __iomem *addr = pctrl->base + offset;
+ unsigned long flags;
+ u32 reg;
+
+ if (port_pin)
+ addr += PORT_PIN_CFG_OFFSET;
+
+ /* handle _L/_H for 32-bit register read/write */
+ if (bit >= 4) {
+ bit -= 4;
+ addr += 4;
+ }
+
+ spin_lock_irqsave(&pctrl->lock, flags);
+ reg = readl(addr) & (mask << (bit * 8));
+ spin_unlock_irqrestore(&pctrl->lock, flags);
+ reg = (reg >> (bit * 8)) & mask;
+
+ return reg;
+}
+
+static void rzg2l_rmw_pin_config(struct rzg2l_pinctrl *pctrl, bool port_pin,
+ u32 offset, u8 bit, u32 mask, u32 val)
+{
+ void __iomem *addr = pctrl->base + offset;
+ unsigned long flags;
+ u32 reg;
+
+ if (port_pin)
+ addr += PORT_PIN_CFG_OFFSET;
+
+ /* handle _L/_H for 32-bit register read/write */
+ if (bit >= 4) {
+ bit -= 4;
+ addr += 4;
+ }
+
+ spin_lock_irqsave(&pctrl->lock, flags);
+ reg = readl(addr) & ~(mask << (bit * 8));
+ writel(reg | (val << (bit * 8)), addr);
+ spin_unlock_irqrestore(&pctrl->lock, flags);
+}
+
static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
unsigned int _pin,
unsigned long *config)
@@ -432,10 +480,11 @@ 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 port_pin = false;
unsigned int arg = 0;
unsigned long flags;
void __iomem *addr;
- u32 port = 0, reg;
+ u32 port = 0;
u32 cfg = 0;
u8 bit = 0;

@@ -452,17 +501,8 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
case PIN_CONFIG_INPUT_ENABLE:
if (!(cfg & PIN_CFG_IEN))
return -EINVAL;
- spin_lock_irqsave(&pctrl->lock, flags);
- /* handle _L/_H for 32-bit register read/write */
- addr = pctrl->base + IEN(port);
- if (bit >= 4) {
- bit -= 4;
- addr += 4;
- }

- reg = readl(addr) & (IEN_MASK << (bit * 8));
- arg = (reg >> (bit * 8)) & 0x1;
- spin_unlock_irqrestore(&pctrl->lock, flags);
+ arg = rzg2l_read_pin_config(pctrl, port_pin, IEN(port), bit, IEN_MASK);
break;

case PIN_CONFIG_POWER_SOURCE: {
@@ -502,10 +542,11 @@ 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 port_pin = false;
unsigned long flags;
void __iomem *addr;
- u32 port = 0, reg;
unsigned int i;
+ u32 port = 0;
u32 cfg = 0;
u8 bit = 0;

@@ -524,21 +565,10 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
case PIN_CONFIG_INPUT_ENABLE: {
unsigned int arg =
pinconf_to_config_argument(_configs[i]);
-
if (!(cfg & PIN_CFG_IEN))
return -EINVAL;

- /* handle _L/_H for 32-bit register read/write */
- addr = pctrl->base + IEN(port);
- if (bit >= 4) {
- bit -= 4;
- addr += 4;
- }
-
- spin_lock_irqsave(&pctrl->lock, flags);
- reg = readl(addr) & ~(IEN_MASK << (bit * 8));
- writel(reg | (arg << (bit * 8)), addr);
- spin_unlock_irqrestore(&pctrl->lock, flags);
+ rzg2l_rmw_pin_config(pctrl, port_pin, IEN(port), bit, IEN_MASK, !!arg);
break;
}

--
2.17.1

2021-10-29 12:47:38

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v2 3/5] pinctrl: renesas: pinctrl-rzg2l: Add support to get/set pin config for GPIO port pins

Add support to get/set pin config for GPIO port pins.

Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Biju Das <[email protected]>
---
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 33 +++++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index f294ae7b8b5a..bc34c63bbb36 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -426,6 +426,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 u32 rzg2l_read_pin_config(struct rzg2l_pinctrl *pctrl, bool port_pin,
u32 offset, u8 bit, u32 mask)
{
@@ -495,6 +512,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);
+ } else {
+ cfg = RZG2L_GPIO_PORT_GET_CFGS(*pin_data);
+ port = RZG2L_PIN_ID_TO_PORT(_pin);
+ bit = RZG2L_PIN_ID_TO_PIN(_pin);
+ port_pin = true;
+
+ if (rzg2l_validate_gpio_pin(pctrl, *pin_data, port, bit))
+ return -EINVAL;
}

switch (param) {
@@ -557,6 +582,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);
+ } else {
+ cfg = RZG2L_GPIO_PORT_GET_CFGS(*pin_data);
+ port = RZG2L_PIN_ID_TO_PORT(_pin);
+ bit = RZG2L_PIN_ID_TO_PIN(_pin);
+ port_pin = true;
+
+ if (rzg2l_validate_gpio_pin(pctrl, *pin_data, port, bit))
+ return -EINVAL;
}

for (i = 0; i < num_configs; i++) {
--
2.17.1

2021-10-29 12:48:14

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v2 4/5] pinctrl: renesas: pinctrl-rzg2l: Rename PIN_CFG_* macros to match HW manual

Rename the below macros to match the HW manual (Rev.1.00):
PIN_CFG_IOLH_SD0 -> PIN_CFG_IO_VMC_SD0
PIN_CFG_IOLH_SD1 -> PIN_CFG_IO_VMC_SD1
PIN_CFG_IOLH_QSPI -> PIN_CFG_IO_VMC_QSPI
PIN_CFG_IOLH_ETH0 -> PIN_CFG_IO_VMC_ETH0
PIN_CFG_IOLH_ETH1 -> PIN_CFG_IO_VMC_ETH1

Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Biju Das <[email protected]>
---
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 122 ++++++++++++------------
1 file changed, 61 insertions(+), 61 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index bc34c63bbb36..348fc8dd74e6 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -39,11 +39,11 @@
#define PIN_CFG_SR BIT(1)
#define PIN_CFG_IEN BIT(2)
#define PIN_CFG_PUPD BIT(3)
-#define PIN_CFG_IOLH_SD0 BIT(4)
-#define PIN_CFG_IOLH_SD1 BIT(5)
-#define PIN_CFG_IOLH_QSPI BIT(6)
-#define PIN_CFG_IOLH_ETH0 BIT(7)
-#define PIN_CFG_IOLH_ETH1 BIT(8)
+#define PIN_CFG_IO_VMC_SD0 BIT(4)
+#define PIN_CFG_IO_VMC_SD1 BIT(5)
+#define PIN_CFG_IO_VMC_QSPI BIT(6)
+#define PIN_CFG_IO_VMC_ETH0 BIT(7)
+#define PIN_CFG_IO_VMC_ETH1 BIT(8)
#define PIN_CFG_FILONOFF BIT(9)
#define PIN_CFG_FILNUM BIT(10)
#define PIN_CFG_FILCLKSEL BIT(11)
@@ -533,11 +533,11 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
case PIN_CONFIG_POWER_SOURCE: {
u32 pwr_reg = 0x0;

- if (cfg & PIN_CFG_IOLH_SD0)
+ if (cfg & PIN_CFG_IO_VMC_SD0)
pwr_reg = SD_CH(0);
- else if (cfg & PIN_CFG_IOLH_SD1)
+ else if (cfg & PIN_CFG_IO_VMC_SD1)
pwr_reg = SD_CH(1);
- else if (cfg & PIN_CFG_IOLH_QSPI)
+ else if (cfg & PIN_CFG_IO_VMC_QSPI)
pwr_reg = QSPI;
else
return -EINVAL;
@@ -612,11 +612,11 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
if (mV != 1800 && mV != 3300)
return -EINVAL;

- if (cfg & PIN_CFG_IOLH_SD0)
+ if (cfg & PIN_CFG_IO_VMC_SD0)
pwr_reg = SD_CH(0);
- else if (cfg & PIN_CFG_IOLH_SD1)
+ else if (cfg & PIN_CFG_IO_VMC_SD1)
pwr_reg = SD_CH(1);
- else if (cfg & PIN_CFG_IOLH_QSPI)
+ else if (cfg & PIN_CFG_IO_VMC_QSPI)
pwr_reg = QSPI;
else
return -EINVAL;
@@ -918,24 +918,24 @@ static const u32 rzg2l_gpio_configs[] = {
RZG2L_GPIO_PORT_PACK(3, 0x21, RZG2L_MPXED_PIN_FUNCS),
RZG2L_GPIO_PORT_PACK(2, 0x22, RZG2L_MPXED_PIN_FUNCS),
RZG2L_GPIO_PORT_PACK(2, 0x23, RZG2L_MPXED_PIN_FUNCS),
- RZG2L_GPIO_PORT_PACK(3, 0x24, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH0)),
- RZG2L_GPIO_PORT_PACK(2, 0x25, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH0)),
- RZG2L_GPIO_PORT_PACK(2, 0x26, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH0)),
- RZG2L_GPIO_PORT_PACK(2, 0x27, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH0)),
- RZG2L_GPIO_PORT_PACK(2, 0x28, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH0)),
- RZG2L_GPIO_PORT_PACK(2, 0x29, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH0)),
- RZG2L_GPIO_PORT_PACK(2, 0x2a, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH0)),
- RZG2L_GPIO_PORT_PACK(2, 0x2b, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH0)),
- RZG2L_GPIO_PORT_PACK(2, 0x2c, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH0)),
- RZG2L_GPIO_PORT_PACK(2, 0x2d, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH1)),
- RZG2L_GPIO_PORT_PACK(2, 0x2e, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH1)),
- RZG2L_GPIO_PORT_PACK(2, 0x2f, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH1)),
- RZG2L_GPIO_PORT_PACK(2, 0x30, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH1)),
- RZG2L_GPIO_PORT_PACK(2, 0x31, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH1)),
- RZG2L_GPIO_PORT_PACK(2, 0x32, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH1)),
- RZG2L_GPIO_PORT_PACK(2, 0x33, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH1)),
- RZG2L_GPIO_PORT_PACK(2, 0x34, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH1)),
- RZG2L_GPIO_PORT_PACK(3, 0x35, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_ETH1)),
+ RZG2L_GPIO_PORT_PACK(3, 0x24, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH0)),
+ RZG2L_GPIO_PORT_PACK(2, 0x25, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH0)),
+ RZG2L_GPIO_PORT_PACK(2, 0x26, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH0)),
+ RZG2L_GPIO_PORT_PACK(2, 0x27, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH0)),
+ RZG2L_GPIO_PORT_PACK(2, 0x28, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH0)),
+ RZG2L_GPIO_PORT_PACK(2, 0x29, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH0)),
+ RZG2L_GPIO_PORT_PACK(2, 0x2a, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH0)),
+ RZG2L_GPIO_PORT_PACK(2, 0x2b, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH0)),
+ RZG2L_GPIO_PORT_PACK(2, 0x2c, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH0)),
+ RZG2L_GPIO_PORT_PACK(2, 0x2d, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH1)),
+ RZG2L_GPIO_PORT_PACK(2, 0x2e, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH1)),
+ RZG2L_GPIO_PORT_PACK(2, 0x2f, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH1)),
+ RZG2L_GPIO_PORT_PACK(2, 0x30, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH1)),
+ RZG2L_GPIO_PORT_PACK(2, 0x31, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH1)),
+ RZG2L_GPIO_PORT_PACK(2, 0x32, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH1)),
+ RZG2L_GPIO_PORT_PACK(2, 0x33, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH1)),
+ RZG2L_GPIO_PORT_PACK(2, 0x34, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH1)),
+ RZG2L_GPIO_PORT_PACK(3, 0x35, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IO_VMC_ETH1)),
RZG2L_GPIO_PORT_PACK(2, 0x36, RZG2L_MPXED_PIN_FUNCS),
RZG2L_GPIO_PORT_PACK(3, 0x37, RZG2L_MPXED_PIN_FUNCS),
RZG2L_GPIO_PORT_PACK(3, 0x38, RZG2L_MPXED_PIN_FUNCS),
@@ -959,68 +959,68 @@ static struct rzg2l_dedicated_configs rzg2l_dedicated_pins[] = {
{ "AUDIO_CLK1", RZG2L_SINGLE_PIN_PACK(0x4, 0, PIN_CFG_IEN) },
{ "AUDIO_CLK2", RZG2L_SINGLE_PIN_PACK(0x4, 1, PIN_CFG_IEN) },
{ "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_IO_VMC_SD0)) },
{ "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_IO_VMC_SD0)) },
{ "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_IO_VMC_SD0)) },
{ "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_IO_VMC_SD0)) },
{ "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_IO_VMC_SD0)) },
{ "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_IO_VMC_SD0)) },
{ "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_IO_VMC_SD0)) },
{ "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_IO_VMC_SD0)) },
{ "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_IO_VMC_SD0)) },
{ "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_IO_VMC_SD0)) },
{ "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_IO_VMC_SD0)) },
{ "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_IO_VMC_SD1))},
{ "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_IO_VMC_SD1)) },
{ "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_IO_VMC_SD1)) },
{ "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_IO_VMC_SD1)) },
{ "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_IO_VMC_SD1)) },
{ "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_IO_VMC_SD1)) },
{ "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_IO_VMC_QSPI)) },
{ "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_IO_VMC_QSPI)) },
{ "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_IO_VMC_QSPI)) },
{ "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_IO_VMC_QSPI)) },
{ "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_IO_VMC_QSPI)) },
{ "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_IO_VMC_QSPI)) },
{ "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_IO_VMC_QSPI)) },
{ "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_IO_VMC_QSPI)) },
{ "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_IO_VMC_QSPI)) },
{ "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_IO_VMC_QSPI)) },
{ "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_IO_VMC_QSPI)) },
{ "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_IO_VMC_QSPI)) },
{ "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_IO_VMC_QSPI)) },
{ "QSPI_WP#", RZG2L_SINGLE_PIN_PACK(0xc, 1,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
- { "QSPI_INT#", RZG2L_SINGLE_PIN_PACK(0xc, 2, (PIN_CFG_SR | PIN_CFG_IOLH_QSPI)) },
+ (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ { "QSPI_INT#", RZG2L_SINGLE_PIN_PACK(0xc, 2, (PIN_CFG_SR | PIN_CFG_IO_VMC_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) },
{ "RIIC0_SCL", RZG2L_SINGLE_PIN_PACK(0xe, 1, PIN_CFG_IEN) },
--
2.17.1

2021-10-29 12:48:31

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v2 1/5] dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Add output-impedance-ohms property

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 <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
Reviewed-by: Biju Das <[email protected]>
---
.../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..3a66fd214c17 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-ohms:
+ enum: [ 33, 50, 66, 100 ]
power-source:
enum: [ 1800, 2500, 3300 ]
slew-rate: true
--
2.17.1

2021-10-29 12:50:42

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v2 5/5] pinctrl: renesas: pinctrl-rzg2l: Add support to get/set drive-strength and output-impedance-ohms

RZ/G2L supports two groups of pins Group-A and Group-B. For Group-A
pins drive-strength can be configured and for Group-B output-impedance
can be configured.

This patch splits PIN_CFG_IOLH macro to PIN_CFG_IOLH_A/B and adds
support to get/set drive-strength and output-impedance-ohms for the
supported pins.

Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Biju Das <[email protected]>
---
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 159 ++++++++++++++++--------
1 file changed, 110 insertions(+), 49 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 348fc8dd74e6..f02d76c4966b 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -35,20 +35,21 @@
#define MUX_FUNC(pinconf) (((pinconf) & MUX_FUNC_MASK) >> MUX_FUNC_OFFS)

/* PIN capabilities */
-#define PIN_CFG_IOLH BIT(0)
-#define PIN_CFG_SR BIT(1)
-#define PIN_CFG_IEN BIT(2)
-#define PIN_CFG_PUPD BIT(3)
-#define PIN_CFG_IO_VMC_SD0 BIT(4)
-#define PIN_CFG_IO_VMC_SD1 BIT(5)
-#define PIN_CFG_IO_VMC_QSPI BIT(6)
-#define PIN_CFG_IO_VMC_ETH0 BIT(7)
-#define PIN_CFG_IO_VMC_ETH1 BIT(8)
-#define PIN_CFG_FILONOFF BIT(9)
-#define PIN_CFG_FILNUM BIT(10)
-#define PIN_CFG_FILCLKSEL BIT(11)
-
-#define RZG2L_MPXED_PIN_FUNCS (PIN_CFG_IOLH | \
+#define PIN_CFG_IOLH_A BIT(0)
+#define PIN_CFG_IOLH_B BIT(1)
+#define PIN_CFG_SR BIT(2)
+#define PIN_CFG_IEN BIT(3)
+#define PIN_CFG_PUPD BIT(4)
+#define PIN_CFG_IO_VMC_SD0 BIT(5)
+#define PIN_CFG_IO_VMC_SD1 BIT(6)
+#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 RZG2L_MPXED_PIN_FUNCS (PIN_CFG_IOLH_A | \
PIN_CFG_SR | \
PIN_CFG_PUPD | \
PIN_CFG_FILONOFF | \
@@ -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)
@@ -103,6 +105,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
@@ -139,6 +142,9 @@ struct rzg2l_pinctrl {
spinlock_t lock;
};

+static const unsigned int iolh_groupa_mA[] = { 2, 4, 8, 12 };
+static const unsigned int iolh_groupb_oi[] = { 100, 66, 50, 33 };
+
static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
u8 port, u8 pin, u8 func)
{
@@ -501,7 +507,7 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
unsigned int arg = 0;
unsigned long flags;
void __iomem *addr;
- u32 port = 0;
+ u32 port = 0, reg;
u32 cfg = 0;
u8 bit = 0;

@@ -549,6 +555,24 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
break;
}

+ case PIN_CONFIG_DRIVE_STRENGTH: {
+ if (!(cfg & PIN_CFG_IOLH_A))
+ return -EINVAL;
+
+ reg = rzg2l_read_pin_config(pctrl, port_pin, IOLH(port), bit, IOLH_MASK);
+ arg = iolh_groupa_mA[reg];
+ break;
+ }
+
+ case PIN_CONFIG_OUTPUT_IMPEDANCE_OHMS: {
+ if (!(cfg & PIN_CFG_IOLH_B))
+ return -EINVAL;
+
+ reg = rzg2l_read_pin_config(pctrl, port_pin, IOLH(port), bit, IOLH_MASK);
+ arg = iolh_groupb_oi[reg];
+ break;
+ }
+
default:
return -ENOTSUPP;
}
@@ -627,6 +651,43 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
spin_unlock_irqrestore(&pctrl->lock, flags);
break;
}
+
+ case PIN_CONFIG_DRIVE_STRENGTH: {
+ unsigned int arg = pinconf_to_config_argument(_configs[i]);
+ unsigned int index;
+
+ if (!(cfg & PIN_CFG_IOLH_A))
+ return -EINVAL;
+
+ for (index = 0; index < ARRAY_SIZE(iolh_groupa_mA); index++) {
+ if (arg == iolh_groupa_mA[index])
+ break;
+ }
+ if (index >= ARRAY_SIZE(iolh_groupa_mA))
+ return -EINVAL;
+
+ rzg2l_rmw_pin_config(pctrl, port_pin, IOLH(port), bit, IOLH_MASK, index);
+ break;
+ }
+
+ case PIN_CONFIG_OUTPUT_IMPEDANCE_OHMS: {
+ unsigned int arg = pinconf_to_config_argument(_configs[i]);
+ unsigned int index;
+
+ if (!(cfg & PIN_CFG_IOLH_B))
+ return -EINVAL;
+
+ for (index = 0; index < ARRAY_SIZE(iolh_groupb_oi); index++) {
+ if (arg == iolh_groupb_oi[index])
+ break;
+ }
+ if (index >= ARRAY_SIZE(iolh_groupb_oi))
+ return -EINVAL;
+
+ rzg2l_rmw_pin_config(pctrl, port_pin, IOLH(port), bit, IOLH_MASK, index);
+ break;
+ }
+
default:
return -EOPNOTSUPP;
}
@@ -953,75 +1014,75 @@ static struct rzg2l_dedicated_configs rzg2l_dedicated_pins[] = {
{ "NMI", RZG2L_SINGLE_PIN_PACK(0x1, 0,
(PIN_CFG_FILONOFF | PIN_CFG_FILNUM | PIN_CFG_FILCLKSEL)) },
{ "TMS/SWDIO", RZG2L_SINGLE_PIN_PACK(0x2, 0,
- (PIN_CFG_SR | PIN_CFG_IOLH | PIN_CFG_IEN)) },
+ (PIN_CFG_SR | PIN_CFG_IOLH_A | PIN_CFG_IEN)) },
{ "TDO", RZG2L_SINGLE_PIN_PACK(0x3, 0,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN)) },
+ (PIN_CFG_IOLH_A | 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) },
{ "SD0_CLK", RZG2L_SINGLE_PIN_PACK(0x6, 0,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_SD0)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_SD0)) },
{ "SD0_CMD", RZG2L_SINGLE_PIN_PACK(0x6, 1,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
{ "SD0_RST#", RZG2L_SINGLE_PIN_PACK(0x6, 2,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_SD0)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_SD0)) },
{ "SD0_DATA0", RZG2L_SINGLE_PIN_PACK(0x7, 0,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
{ "SD0_DATA1", RZG2L_SINGLE_PIN_PACK(0x7, 1,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
{ "SD0_DATA2", RZG2L_SINGLE_PIN_PACK(0x7, 2,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
{ "SD0_DATA3", RZG2L_SINGLE_PIN_PACK(0x7, 3,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
{ "SD0_DATA4", RZG2L_SINGLE_PIN_PACK(0x7, 4,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
{ "SD0_DATA5", RZG2L_SINGLE_PIN_PACK(0x7, 5,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
{ "SD0_DATA6", RZG2L_SINGLE_PIN_PACK(0x7, 6,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
{ "SD0_DATA7", RZG2L_SINGLE_PIN_PACK(0x7, 7,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD0)) },
{ "SD1_CLK", RZG2L_SINGLE_PIN_PACK(0x8, 0,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_SD1))},
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_SD1)) },
{ "SD1_CMD", RZG2L_SINGLE_PIN_PACK(0x8, 1,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD1)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD1)) },
{ "SD1_DATA0", RZG2L_SINGLE_PIN_PACK(0x9, 0,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD1)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD1)) },
{ "SD1_DATA1", RZG2L_SINGLE_PIN_PACK(0x9, 1,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD1)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD1)) },
{ "SD1_DATA2", RZG2L_SINGLE_PIN_PACK(0x9, 2,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD1)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD1)) },
{ "SD1_DATA3", RZG2L_SINGLE_PIN_PACK(0x9, 3,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD1)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_IO_VMC_SD1)) },
{ "QSPI0_SPCLK", RZG2L_SINGLE_PIN_PACK(0xa, 0,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI0_IO0", RZG2L_SINGLE_PIN_PACK(0xa, 1,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI0_IO1", RZG2L_SINGLE_PIN_PACK(0xa, 2,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI0_IO2", RZG2L_SINGLE_PIN_PACK(0xa, 3,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI0_IO3", RZG2L_SINGLE_PIN_PACK(0xa, 4,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI0_SSL", RZG2L_SINGLE_PIN_PACK(0xa, 5,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI1_SPCLK", RZG2L_SINGLE_PIN_PACK(0xb, 0,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI1_IO0", RZG2L_SINGLE_PIN_PACK(0xb, 1,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI1_IO1", RZG2L_SINGLE_PIN_PACK(0xb, 2,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI1_IO2", RZG2L_SINGLE_PIN_PACK(0xb, 3,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI1_IO3", RZG2L_SINGLE_PIN_PACK(0xb, 4,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI1_SSL", RZG2L_SINGLE_PIN_PACK(0xb, 5,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI_RESET#", RZG2L_SINGLE_PIN_PACK(0xc, 0,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI_WP#", RZG2L_SINGLE_PIN_PACK(0xc, 1,
- (PIN_CFG_IOLH | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
+ (PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
{ "QSPI_INT#", RZG2L_SINGLE_PIN_PACK(0xc, 2, (PIN_CFG_SR | PIN_CFG_IO_VMC_QSPI)) },
- { "WDTOVF_PERROUT#", RZG2L_SINGLE_PIN_PACK(0xd, 0, (PIN_CFG_IOLH | PIN_CFG_SR)) },
+ { "WDTOVF_PERROUT#", RZG2L_SINGLE_PIN_PACK(0xd, 0, (PIN_CFG_IOLH_A | PIN_CFG_SR)) },
{ "RIIC0_SDA", RZG2L_SINGLE_PIN_PACK(0xe, 0, PIN_CFG_IEN) },
{ "RIIC0_SCL", RZG2L_SINGLE_PIN_PACK(0xe, 1, PIN_CFG_IEN) },
{ "RIIC1_SDA", RZG2L_SINGLE_PIN_PACK(0xe, 2, PIN_CFG_IEN) },
--
2.17.1

2021-11-08 16:52:13

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] pinctrl: renesas: pinctrl-rzg2l: Rename PIN_CFG_* macros to match HW manual

On Fri, Oct 29, 2021 at 2:44 PM Lad Prabhakar
<[email protected]> wrote:
> Rename the below macros to match the HW manual (Rev.1.00):
> PIN_CFG_IOLH_SD0 -> PIN_CFG_IO_VMC_SD0
> PIN_CFG_IOLH_SD1 -> PIN_CFG_IO_VMC_SD1
> PIN_CFG_IOLH_QSPI -> PIN_CFG_IO_VMC_QSPI
> PIN_CFG_IOLH_ETH0 -> PIN_CFG_IO_VMC_ETH0
> PIN_CFG_IOLH_ETH1 -> PIN_CFG_IO_VMC_ETH1
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> Reviewed-by: Biju Das <[email protected]>

Reviewed-by: Geert Uytterhoeven <[email protected]>
i.e. will queue in renesas-pinctrl-for-v5.17.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2021-11-08 17:42:29

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] pinctrl: renesas: pinctrl-rzg2l: Add support to get/set drive-strength and output-impedance-ohms

Hi Prabhakar,

On Fri, Oct 29, 2021 at 2:45 PM Lad Prabhakar
<[email protected]> wrote:
> RZ/G2L supports two groups of pins Group-A and Group-B. For Group-A
> pins drive-strength can be configured and for Group-B output-impedance
> can be configured.
>
> This patch splits PIN_CFG_IOLH macro to PIN_CFG_IOLH_A/B and adds
> support to get/set drive-strength and output-impedance-ohms for the
> supported pins.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> Reviewed-by: Biju Das <[email protected]>

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c

> @@ -501,7 +507,7 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> unsigned int arg = 0;
> unsigned long flags;
> void __iomem *addr;
> - u32 port = 0;
> + u32 port = 0, reg;

"unsigned int index", for symmetry with rzg2l_pinctrl_pinconf_set()?

> u32 cfg = 0;
> u8 bit = 0;
>
> @@ -549,6 +555,24 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> break;
> }
>
> + case PIN_CONFIG_DRIVE_STRENGTH: {
> + if (!(cfg & PIN_CFG_IOLH_A))
> + return -EINVAL;
> +
> + reg = rzg2l_read_pin_config(pctrl, port_pin, IOLH(port), bit, IOLH_MASK);

port_pin still under discussion, cfr. my comments for the other
patches in this series.

The rest looks good to me, so
Reviewed-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2021-11-08 20:29:58

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] pinctrl: renesas: pinctrl-rzg2l: Add helper functions to read/write pin config

Hi Prabhakar,

On Fri, Oct 29, 2021 at 2:44 PM Lad Prabhakar
<[email protected]> wrote:
> Add helper functions to read/read modify write pin config.
>
> Switch to use helper functions for pins supporting PIN_CONFIG_INPUT_ENABLE
> capabilities.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> Reviewed-by: Biju Das <[email protected]>

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -91,6 +91,8 @@
> #define SD_CH(n) (0x3000 + (n) * 4)
> #define QSPI (0x3008)
>
> +#define PORT_PIN_CFG_OFFSET 0x80

This definition belongs in [PATCH v2 5/5].

> +
> #define PVDD_1800 1 /* I/O domain voltage <= 1.8V */
> #define PVDD_3300 0 /* I/O domain voltage >= 3.3V */
>
> @@ -424,6 +426,52 @@ static int rzg2l_dt_node_to_map(struct pinctrl_dev *pctldev,
> return ret;
> }
>
> +static u32 rzg2l_read_pin_config(struct rzg2l_pinctrl *pctrl, bool port_pin,
> + u32 offset, u8 bit, u32 mask)
> +{
> + void __iomem *addr = pctrl->base + offset;
> + unsigned long flags;
> + u32 reg;
> +
> + if (port_pin)
> + addr += PORT_PIN_CFG_OFFSET;

I'm wondering if it would be better to handle this in the caller,
by passing an adjusted offset?
Same for rzg2l_rmw_pin_config().

> +
> + /* handle _L/_H for 32-bit register read/write */
> + if (bit >= 4) {
> + bit -= 4;
> + addr += 4;
> + }
> +
> + spin_lock_irqsave(&pctrl->lock, flags);
> + reg = readl(addr) & (mask << (bit * 8));

The masking is not needed here, as it is done below.

> + spin_unlock_irqrestore(&pctrl->lock, flags);

I still think you don't need that spinlock here, as reading a MMIO
register is an atomic operation.

(/me fixes drivers/pinctrl/renesas/pinctrl.c you referred to before)

> + reg = (reg >> (bit * 8)) & mask;
> +
> + return reg;

return (reg >> (bit * 8)) & mask;

> +}

> @@ -432,10 +480,11 @@ 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 port_pin = false;

Do you really need this?

> unsigned int arg = 0;
> unsigned long flags;
> void __iomem *addr;
> - u32 port = 0, reg;
> + u32 port = 0;
> u32 cfg = 0;
> u8 bit = 0;
>
> @@ -452,17 +501,8 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> case PIN_CONFIG_INPUT_ENABLE:
> if (!(cfg & PIN_CFG_IEN))
> return -EINVAL;
> - spin_lock_irqsave(&pctrl->lock, flags);
> - /* handle _L/_H for 32-bit register read/write */
> - addr = pctrl->base + IEN(port);
> - if (bit >= 4) {
> - bit -= 4;
> - addr += 4;
> - }
>
> - reg = readl(addr) & (IEN_MASK << (bit * 8));
> - arg = (reg >> (bit * 8)) & 0x1;
> - spin_unlock_irqrestore(&pctrl->lock, flags);
> + arg = rzg2l_read_pin_config(pctrl, port_pin, IEN(port), bit, IEN_MASK);

port_pin is always false here, as PIN_CFG_IEN is only ever set for
dedicated pins.

Same comments for rzg2l_pinctrl_pinconf_set().

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2021-11-08 21:19:23

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] pinctrl: renesas: pinctrl-rzg2l: Add support to get/set pin config for GPIO port pins

Hi Prabhakar,

On Fri, Oct 29, 2021 at 2:44 PM Lad Prabhakar
<[email protected]> wrote:
> Add support to get/set pin config for GPIO port pins.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> Reviewed-by: Biju Das <[email protected]>

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c

> @@ -495,6 +512,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);
> + } else {
> + cfg = RZG2L_GPIO_PORT_GET_CFGS(*pin_data);
> + port = RZG2L_PIN_ID_TO_PORT(_pin);
> + bit = RZG2L_PIN_ID_TO_PIN(_pin);
> + port_pin = true;

Instead of setting this flag, perhaps port should be adjusted?
Then rzg2l_r{ead,mw}_pin_config() don't have to care about that
anymore.

> +
> + if (rzg2l_validate_gpio_pin(pctrl, *pin_data, port, bit))
> + return -EINVAL;
> }
>
> switch (param) {
> @@ -557,6 +582,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);
> + } else {
> + cfg = RZG2L_GPIO_PORT_GET_CFGS(*pin_data);
> + port = RZG2L_PIN_ID_TO_PORT(_pin);
> + bit = RZG2L_PIN_ID_TO_PIN(_pin);
> + port_pin = true;

Likewise.

> +
> + if (rzg2l_validate_gpio_pin(pctrl, *pin_data, port, bit))
> + return -EINVAL;
> }
>
> for (i = 0; i < num_configs; i++) {

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2021-11-09 01:03:29

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Add output-impedance-ohms property

On Fri, 29 Oct 2021 13:44:33 +0100, Lad Prabhakar 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 <[email protected]>
> Reviewed-by: Geert Uytterhoeven <[email protected]>
> Reviewed-by: Biju Das <[email protected]>
> ---
> .../devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>

Reviewed-by: Rob Herring <[email protected]>

2021-11-09 23:56:19

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] pinctrl: renesas: pinctrl-rzg2l: Add support to get/set pin config for GPIO port pins

Hi Geert,

Thank you for the review.

On Mon, Nov 8, 2021 at 3:36 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Fri, Oct 29, 2021 at 2:44 PM Lad Prabhakar
> <[email protected]> wrote:
> > Add support to get/set pin config for GPIO port pins.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > Reviewed-by: Biju Das <[email protected]>
>
> Thanks for your patch!
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
>
> > @@ -495,6 +512,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);
> > + } else {
> > + cfg = RZG2L_GPIO_PORT_GET_CFGS(*pin_data);
> > + port = RZG2L_PIN_ID_TO_PORT(_pin);
> > + bit = RZG2L_PIN_ID_TO_PIN(_pin);
> > + port_pin = true;
>
> Instead of setting this flag, perhaps port should be adjusted?

Something like below?

#define RZG2L_PORT_START_OFFSET 0x10

port = RZG2L_PIN_ID_TO_PORT_pin) + RZG2L_PORT_START_OFFSET;
rzg2l_validate_gpio_pin(pctrl, *pin_data, port - RZG2L_PORT_START_OFFSET, bit)

and rename port -> port_offset in rzg2l_pinctrl_pinconf_get/set Or
would you prefer to change the RZG2L_PIN_ID_TO_PORT macro and adjust
the entire file?

> Then rzg2l_r{ead,mw}_pin_config() don't have to care about that
> anymore.
>
Agreed.

Cheers,
Prabhakar

> > +
> > + if (rzg2l_validate_gpio_pin(pctrl, *pin_data, port, bit))
> > + return -EINVAL;
> > }
> >
> > switch (param) {
> > @@ -557,6 +582,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);
> > + } else {
> > + cfg = RZG2L_GPIO_PORT_GET_CFGS(*pin_data);
> > + port = RZG2L_PIN_ID_TO_PORT(_pin);
> > + bit = RZG2L_PIN_ID_TO_PIN(_pin);
> > + port_pin = true;
>
> Likewise.
>
> > +
> > + if (rzg2l_validate_gpio_pin(pctrl, *pin_data, port, bit))
> > + return -EINVAL;
> > }
> >
> > for (i = 0; i < num_configs; i++) {
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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

2021-11-09 23:58:51

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] pinctrl: renesas: pinctrl-rzg2l: Add support to get/set pin config for GPIO port pins

Hi Prabhakar,

On Tue, Nov 9, 2021 at 3:31 PM Lad, Prabhakar
<[email protected]> wrote:
> On Mon, Nov 8, 2021 at 3:36 PM Geert Uytterhoeven <[email protected]> wrote:
> > On Fri, Oct 29, 2021 at 2:44 PM Lad Prabhakar
> > <[email protected]> wrote:
> > > Add support to get/set pin config for GPIO port pins.
> > >
> > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > Reviewed-by: Biju Das <[email protected]>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> >
> > > @@ -495,6 +512,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);
> > > + } else {
> > > + cfg = RZG2L_GPIO_PORT_GET_CFGS(*pin_data);
> > > + port = RZG2L_PIN_ID_TO_PORT(_pin);
> > > + bit = RZG2L_PIN_ID_TO_PIN(_pin);
> > > + port_pin = true;
> >
> > Instead of setting this flag, perhaps port should be adjusted?
>
> Something like below?
>
> #define RZG2L_PORT_START_OFFSET 0x10
>
> port = RZG2L_PIN_ID_TO_PORT_pin) + RZG2L_PORT_START_OFFSET;
> rzg2l_validate_gpio_pin(pctrl, *pin_data, port - RZG2L_PORT_START_OFFSET, bit)

Or adjust port after the call to rzg2l_validate_gpio_pin(), to avoid adding
the offset first, and subtracting it again for calling the latter?

> and rename port -> port_offset in rzg2l_pinctrl_pinconf_get/set

That makes sense. Currently "port" has two meanings: it can mean
either the GPIO port index, or the global register index covering both
single function pin groups and GPIO port indices.
RZG2L_SINGLE_PIN_GET_PORT() returns the latter.
RZG2L_PIN_ID_TO_PORT() returns the former, thus needing an extra offset
to convert to the global register index.

> Or
> would you prefer to change the RZG2L_PIN_ID_TO_PORT macro and adjust
> the entire file?

Changing RZG2L_PIN_ID_TO_PORT() would imply changing all macros
accessing GPIO registers, and is thus quite intrusive.

> > Then rzg2l_r{ead,mw}_pin_config() don't have to care about that
> > anymore.
> >
> Agreed.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2021-11-10 00:01:28

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] pinctrl: renesas: pinctrl-rzg2l: Add support to get/set pin config for GPIO port pins

Hi Geert,

On Tue, Nov 9, 2021 at 3:00 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Tue, Nov 9, 2021 at 3:31 PM Lad, Prabhakar
> <[email protected]> wrote:
> > On Mon, Nov 8, 2021 at 3:36 PM Geert Uytterhoeven <[email protected]> wrote:
> > > On Fri, Oct 29, 2021 at 2:44 PM Lad Prabhakar
> > > <[email protected]> wrote:
> > > > Add support to get/set pin config for GPIO port pins.
> > > >
> > > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > > Reviewed-by: Biju Das <[email protected]>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > >
> > > > @@ -495,6 +512,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);
> > > > + } else {
> > > > + cfg = RZG2L_GPIO_PORT_GET_CFGS(*pin_data);
> > > > + port = RZG2L_PIN_ID_TO_PORT(_pin);
> > > > + bit = RZG2L_PIN_ID_TO_PIN(_pin);
> > > > + port_pin = true;
> > >
> > > Instead of setting this flag, perhaps port should be adjusted?
> >
> > Something like below?
> >
> > #define RZG2L_PORT_START_OFFSET 0x10
> >
> > port = RZG2L_PIN_ID_TO_PORT_pin) + RZG2L_PORT_START_OFFSET;
> > rzg2l_validate_gpio_pin(pctrl, *pin_data, port - RZG2L_PORT_START_OFFSET, bit)
>
> Or adjust port after the call to rzg2l_validate_gpio_pin(), to avoid adding
> the offset first, and subtracting it again for calling the latter?
>
> > and rename port -> port_offset in rzg2l_pinctrl_pinconf_get/set
>
> That makes sense. Currently "port" has two meanings: it can mean
> either the GPIO port index, or the global register index covering both
> single function pin groups and GPIO port indices.
> RZG2L_SINGLE_PIN_GET_PORT() returns the latter.
> RZG2L_PIN_ID_TO_PORT() returns the former, thus needing an extra offset
> to convert to the global register index.
>
for symmetry will rename the below:
RZG2L_SINGLE_PIN_GET_PORT -> RZG2L_SINGLE_PIN_GET_PORT_OFFSET

Introduce a new macros:
#define RZG2L_PORT_START_OFFSET 0x10
#define RZG2L_PIN_ID_TO_PORT_OFFSET(id) (((id) / RZG2L_PINS_PER_PORT)
+ RZG2L_PORT_START_OFFSET)

And use the above two in rzg2l_pinctrl_pinconf_get/set along with
renaming port -> port_offset

And for rzg2l_validate_gpio_pin() will use below instead:
rzg2l_validate_gpio_pin(pctrl, *pin_data, RZG2L_PIN_ID_TO_PORT(_pin), bit);

> > Or
> > would you prefer to change the RZG2L_PIN_ID_TO_PORT macro and adjust
> > the entire file?
>
> Changing RZG2L_PIN_ID_TO_PORT() would imply changing all macros
> accessing GPIO registers, and is thus quite intrusive.
>
Agreed, I will drop this option.

Cheers,
Prabhakar
> > > Then rzg2l_r{ead,mw}_pin_config() don't have to care about that
> > > anymore.
> > >
> > Agreed.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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

2021-11-10 13:42:48

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] pinctrl: renesas: pinctrl-rzg2l: Add support to get/set drive-strength and output-impedance-ohms

Hi Geert,

Thank you for the review.

On Mon, Nov 8, 2021 at 3:40 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Fri, Oct 29, 2021 at 2:45 PM Lad Prabhakar
> <[email protected]> wrote:
> > RZ/G2L supports two groups of pins Group-A and Group-B. For Group-A
> > pins drive-strength can be configured and for Group-B output-impedance
> > can be configured.
> >
> > This patch splits PIN_CFG_IOLH macro to PIN_CFG_IOLH_A/B and adds
> > support to get/set drive-strength and output-impedance-ohms for the
> > supported pins.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > Reviewed-by: Biju Das <[email protected]>
>
> Thanks for your patch!
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
>
> > @@ -501,7 +507,7 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> > unsigned int arg = 0;
> > unsigned long flags;
> > void __iomem *addr;
> > - u32 port = 0;
> > + u32 port = 0, reg;
>
> "unsigned int index", for symmetry with rzg2l_pinctrl_pinconf_set()?
>
Agreed.

> > u32 cfg = 0;
> > u8 bit = 0;
> >
> > @@ -549,6 +555,24 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> > break;
> > }
> >
> > + case PIN_CONFIG_DRIVE_STRENGTH: {
> > + if (!(cfg & PIN_CFG_IOLH_A))
> > + return -EINVAL;
> > +
> > + reg = rzg2l_read_pin_config(pctrl, port_pin, IOLH(port), bit, IOLH_MASK);
>
> port_pin still under discussion, cfr. my comments for the other
> patches in this series.
>
As discussed in patch 3/5 will drop this.

Cheers,
Prabhakar

> The rest looks good to me, so
> Reviewed-by: Geert Uytterhoeven <[email protected]>
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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