[PATCH v2 0/2] pinctrl: renesas: trivial fixes and enhancements
Hi All,
This patch series includes trivial fixes and enhancements to renesas pfc driver.
Cheers, Prabhakar
v1->v2 * Patch 1/2 updated commit message * patch 2/2 unchanged
Lad Prabhakar (2): pinctrl: renesas: Make sure the pin type is updated after setting the MUX pinctrl: renesas: Implement get_pin_muxing() callback
drivers/pinctrl/renesas/pfc.c | 50 ++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-)
By default on startup all the pin types are configured to PINMUX_TYPE_NONE (in sh_pfc_map_pins()), when pin is set as GPIO the pin type is updated to PINMUX_TYPE_GPIO. But the type is not updated when the pin is set as a function in sh_pfc_pinctrl_pin_set() or sh_pfc_pinctrl_group_set() calls (these calls only set the MUX if the pin type is PINMUX_TYPE_NONE ie unused).
So with the current implementation pin functionality could be overwritten silently, for example if the same pin is added for SPI and serial.
This patch makes sure of updating pin type after every successful call to sh_pfc_config_mux() and thus fixing from pin functionality to be overwritten.
Signed-off-by: Lad Prabhakar prabhakar.mahadev-lad.rj@bp.renesas.com Reviewed-by: Biju Das biju.das.jz@bp.renesas.com --- drivers/pinctrl/renesas/pfc.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/renesas/pfc.c b/drivers/pinctrl/renesas/pfc.c index fb811a95bc..275702d13a 100644 --- a/drivers/pinctrl/renesas/pfc.c +++ b/drivers/pinctrl/renesas/pfc.c @@ -537,11 +537,18 @@ static int sh_pfc_pinctrl_pin_set(struct udevice *dev, unsigned pin_selector, const struct sh_pfc_pin *pin = &priv->pfc.info->pins[pin_selector]; int idx = sh_pfc_get_pin_index(pfc, pin->pin); struct sh_pfc_pin_config *cfg = &pmx->configs[idx]; + int ret;
if (cfg->type != PINMUX_TYPE_NONE) return -EBUSY;
- return sh_pfc_config_mux(pfc, pin->enum_id, PINMUX_TYPE_FUNCTION); + ret = sh_pfc_config_mux(pfc, pin->enum_id, PINMUX_TYPE_FUNCTION); + if (ret) + return ret; + + cfg->type = PINMUX_TYPE_FUNCTION; + + return 0; }
static int sh_pfc_pinctrl_group_set(struct udevice *dev, unsigned group_selector, @@ -551,12 +558,14 @@ static int sh_pfc_pinctrl_group_set(struct udevice *dev, unsigned group_selector struct sh_pfc_pinctrl *pmx = &priv->pmx; struct sh_pfc *pfc = &priv->pfc; const struct sh_pfc_pin_group *grp = &priv->pfc.info->groups[group_selector]; + struct sh_pfc_pin_config *cfg; unsigned int i; int ret = 0; + int idx;
for (i = 0; i < grp->nr_pins; ++i) { - int idx = sh_pfc_get_pin_index(pfc, grp->pins[i]); - struct sh_pfc_pin_config *cfg = &pmx->configs[idx]; + idx = sh_pfc_get_pin_index(pfc, grp->pins[i]); + cfg = &pmx->configs[idx];
if (cfg->type != PINMUX_TYPE_NONE) { ret = -EBUSY; @@ -568,6 +577,10 @@ static int sh_pfc_pinctrl_group_set(struct udevice *dev, unsigned group_selector ret = sh_pfc_config_mux(pfc, grp->mux[i], PINMUX_TYPE_FUNCTION); if (ret < 0) break; + + idx = sh_pfc_get_pin_index(pfc, grp->pins[i]); + cfg = &pmx->configs[idx]; + cfg->type = PINMUX_TYPE_FUNCTION; }
done:
On 11/6/20 7:01 PM, Lad Prabhakar wrote:
By default on startup all the pin types are configured to PINMUX_TYPE_NONE (in sh_pfc_map_pins()), when pin is set as GPIO the pin type is updated to PINMUX_TYPE_GPIO. But the type is not updated when the pin is set as a function in sh_pfc_pinctrl_pin_set() or sh_pfc_pinctrl_group_set() calls (these calls only set the MUX if the pin type is PINMUX_TYPE_NONE ie unused).
So with the current implementation pin functionality could be overwritten silently, for example if the same pin is added for SPI and serial.
Shouldn't the pinctrl core rather warn about such a collision and abort?
Hi Marek,
Thank you for the review.
-----Original Message----- From: Marek Vasut marex@denx.de Sent: 15 November 2020 13:18 To: Prabhakar Mahadev Lad prabhakar.mahadev-lad.rj@bp.renesas.com; Simon Glass sjg@chromium.org; Masahiro Yamada yamada.masahiro@socionext.com; u-boot@lists.denx.de Cc: Adam Ford aford173@gmail.com; Tom Rini trini@konsulko.com; Prabhakar prabhakar.csengg@gmail.com; Biju Das biju.das.jz@bp.renesas.com Subject: Re: [PATCH v2 1/2] pinctrl: renesas: Make sure the pin type is updated after setting the MUX
On 11/6/20 7:01 PM, Lad Prabhakar wrote:
By default on startup all the pin types are configured to PINMUX_TYPE_NONE (in sh_pfc_map_pins()), when pin is set as GPIO the pin type is updated to PINMUX_TYPE_GPIO. But the type is not updated when the pin is set as a function in sh_pfc_pinctrl_pin_set() or sh_pfc_pinctrl_group_set() calls (these calls only set the MUX if the pin type is PINMUX_TYPE_NONE ie unused).
So with the current implementation pin functionality could be overwritten silently, for example if the same pin is added for SPI and serial.
Shouldn't the pinctrl core rather warn about such a collision and abort?
As mentioned in the commit message this does fix the pin functionality from being overwritten (ie it aborts). Ill post a v3 adding a warning message.
Cheers, Prabhakar
Implement get_pin_muxing() callback so that pinmux status command can be used on Renesas platforms.
Signed-off-by: Lad Prabhakar prabhakar.mahadev-lad.rj@bp.renesas.com Reviewed-by: Biju Das biju.das.jz@bp.renesas.com --- drivers/pinctrl/renesas/pfc.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/pinctrl/renesas/pfc.c b/drivers/pinctrl/renesas/pfc.c index 275702d13a..a1da45db2e 100644 --- a/drivers/pinctrl/renesas/pfc.c +++ b/drivers/pinctrl/renesas/pfc.c @@ -44,6 +44,7 @@ enum sh_pfc_model {
struct sh_pfc_pin_config { u32 type; + const char *function_name; };
struct sh_pfc_pinctrl { @@ -448,6 +449,30 @@ static const char *sh_pfc_pinctrl_get_group_name(struct udevice *dev, return priv->pfc.info->groups[selector].name; }
+static int sh_pfc_pinctrl_get_pin_muxing(struct udevice *dev, + unsigned int selector, + char *buf, int size) +{ + struct sh_pfc_pinctrl_priv *priv = dev_get_priv(dev); + struct sh_pfc_pinctrl *pmx = &priv->pmx; + struct sh_pfc *pfc = &priv->pfc; + struct sh_pfc_pin_config *cfg; + const struct sh_pfc_pin *pin; + int idx; + + pin = &priv->pfc.info->pins[selector]; + if (!pin) { + snprintf(buf, size, "Unknown"); + return -EINVAL; + } + + idx = sh_pfc_get_pin_index(pfc, pin->pin); + cfg = &pmx->configs[idx]; + snprintf(buf, size, "%s", cfg->function_name); + + return 0; +} + static int sh_pfc_pinctrl_get_functions_count(struct udevice *dev) { struct sh_pfc_pinctrl_priv *priv = dev_get_priv(dev); @@ -495,6 +520,7 @@ static int sh_pfc_gpio_request_enable(struct udevice *dev, return ret;
cfg->type = PINMUX_TYPE_GPIO; + cfg->function_name = "gpio";
return 0; } @@ -524,6 +550,7 @@ static int sh_pfc_gpio_disable_free(struct udevice *dev, cfg = &pmx->configs[idx];
cfg->type = PINMUX_TYPE_NONE; + cfg->function_name = "none";
return 0; } @@ -547,6 +574,7 @@ static int sh_pfc_pinctrl_pin_set(struct udevice *dev, unsigned pin_selector, return ret;
cfg->type = PINMUX_TYPE_FUNCTION; + cfg->function_name = "function";
return 0; } @@ -581,6 +609,7 @@ static int sh_pfc_pinctrl_group_set(struct udevice *dev, unsigned group_selector idx = sh_pfc_get_pin_index(pfc, grp->pins[i]); cfg = &pmx->configs[idx]; cfg->type = PINMUX_TYPE_FUNCTION; + cfg->function_name = priv->pfc.info->groups[group_selector].name; }
done: @@ -787,6 +816,7 @@ static struct pinctrl_ops sh_pfc_pinctrl_ops = { .get_pin_name = sh_pfc_pinctrl_get_pin_name, .get_groups_count = sh_pfc_pinctrl_get_groups_count, .get_group_name = sh_pfc_pinctrl_get_group_name, + .get_pin_muxing = sh_pfc_pinctrl_get_pin_muxing, .get_functions_count = sh_pfc_pinctrl_get_functions_count, .get_function_name = sh_pfc_pinctrl_get_function_name,
@@ -817,6 +847,7 @@ static int sh_pfc_map_pins(struct sh_pfc *pfc, struct sh_pfc_pinctrl *pmx) for (i = 0; i < pfc->info->nr_pins; ++i) { struct sh_pfc_pin_config *cfg = &pmx->configs[i]; cfg->type = PINMUX_TYPE_NONE; + cfg->function_name = "none"; }
return 0;
participants (3)
-
Lad Prabhakar -
Marek Vasut -
Prabhakar Mahadev Lad