| Message ID | TYAP286MB0315A9671B4BA0347E70D9E0BC00A@TYAP286MB0315.JPNP286.PROD.OUTLOOK.COM (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | pinctrl: mtmips: support requesting different functions for same group | expand |
On 26.07.2023 03:48, Shiji Yang wrote: > Sometimes pinctrl consumers may request different functions for the > same pin group in different situations. This patch can help to reset > the group function flag when requesting a different function. > > Signed-off-by: Shiji Yang <yangshiji66@outlook.com> > --- > drivers/pinctrl/mediatek/pinctrl-mtmips.c | 21 +++++++++++++++++---- > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtmips.c b/drivers/pinctrl/mediatek/pinctrl-mtmips.c > index efd77b6c5..e5e085915 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtmips.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mtmips.c > @@ -123,11 +123,24 @@ static int mtmips_pmx_group_enable(struct pinctrl_dev *pctrldev, > int i; > int shift; > > - /* dont allow double use */ > + /* > + * for the same pin group, if request a different function, > + * then clear the group function flag and continue, else exit. > + */ > if (p->groups[group].enabled) { > - dev_err(p->dev, "%s is already enabled\n", > - p->groups[group].name); > - return 0; > + for (i = 0; i < p->groups[group].func_count; i++) { > + if (p->groups[group].func[i].enabled == 1) { > + if (!strcmp(p->func[func]->name, > + p->groups[group].func[i].name)) > + return 0; > + p->groups[group].func[i].enabled = 0; > + break; > + } > + } > + > + /* exit if request the "gpio" function again */ > + if (i == p->groups[group].func_count && func == 0) > + return 0; Could you help me understand why? The @gpio_request_enable operation is not properly implemented on this driver so this check would never be true, no? Even if it was, this makes it so that if a pin group is already given a function (meaning the pin group is enabled), it will never be given the gpio function when requested, unless I understand it wrong. Arınç
On Sat, 29 Jul 2023 19:39:34 +0300, Arınç ÜNAL wrote: > On 26.07.2023 03:48, Shiji Yang wrote: > > Sometimes pinctrl consumers may request different functions for the > > same pin group in different situations. This patch can help to reset > > the group function flag when requesting a different function. > > > > Signed-off-by: Shiji Yang <yangshiji66@outlook.com> > > --- > > drivers/pinctrl/mediatek/pinctrl-mtmips.c | 21 +++++++++++++++++---- > > 1 file changed, 17 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtmips.c b/drivers/pinctrl/mediatek/pinctrl-mtmips.c > > index efd77b6c5..e5e085915 100644 > > --- a/drivers/pinctrl/mediatek/pinctrl-mtmips.c > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtmips.c > > @@ -123,11 +123,24 @@ static int mtmips_pmx_group_enable(struct pinctrl_dev *pctrldev, > > int i; > > int shift; > > > > - /* dont allow double use */ > > + /* > > + * for the same pin group, if request a different function, > > + * then clear the group function flag and continue, else exit. > > + */ > > if (p->groups[group].enabled) { > > - dev_err(p->dev, "%s is already enabled\n", > > - p->groups[group].name); > > - return 0; > > + for (i = 0; i < p->groups[group].func_count; i++) { > > + if (p->groups[group].func[i].enabled == 1) { > > + if (!strcmp(p->func[func]->name, > > + p->groups[group].func[i].name)) > > + return 0; > > + p->groups[group].func[i].enabled = 0; > > + break; > > + } > > + } > > + > > + /* exit if request the "gpio" function again */ > > + if (i == p->groups[group].func_count && func == 0) > > + return 0; > > Could you help me understand why? The @gpio_request_enable operation is > not properly implemented on this driver so this check would never be > true, no? '.func_count' is the function number of a pin group. I will use MT7620's 'pa' group as an example to explain it. 'pa' group only has 1 function 'pa' ('gpio' function not included). When this group is first requested as a gpio function, groups[pa].enabled will be set to 1, However, groups[pa].func[i].enabled will still be 0 because 'gpio' is not a member of groups[pa]. In this case, when we request gpio function again, for() loop will do nothing but just increase 'i' to 1 (func_count). This will make 'if (i == p->groups[group].func_count && func == 0)' to be true. > > Even if it was, this makes it so that if a pin group is already given a > function (meaning the pin group is enabled), it will never be given the > gpio function when requested, unless I understand it wrong. > > Arınç When current function is pa, and we want to request a gpio function, 'if (p->groups[group].func[i].enabled == 1)' check will break the for() loop and continue the pin configuration code. If this 'if (p->groups[group].enabled)' check doesn't return, the pinmux mode register will be reset and reconfigured. p->groups[group].enabled = 1; p->func[func]->enabled = 1; shift = p->groups[group].shift; if (shift >= 32) { shift -= 32; reg = SYSC_REG_GPIO_MODE2; } mode = rt_sysc_r32(reg); mode &= ~(p->groups[group].mask << shift); /* mark the pins as gpio */ for (i = 0; i < p->groups[group].func[0].pin_count; i++) p->gpio[p->groups[group].func[0].pins[i]] = 1; /* function 0 is gpio and needs special handling */ if (func == 0) { mode |= p->groups[group].gpio << shift; } else { for (i = 0; i < p->func[func]->pin_count; i++) p->gpio[p->func[func]->pins[i]] = 0; mode |= p->func[func]->value << shift; } rt_sysc_w32(mode, reg); Ref: MT7620 pa group switches between PA and GPIO functions during wireless calibration. https://github.com/openwrt/openwrt/blob/main/package/kernel/mac80211/patches/rt2x00/994-rt2x00-import-support-for-external-LNA-on-MT7620.patch Regards, Shiji Yang
diff --git a/drivers/pinctrl/mediatek/pinctrl-mtmips.c b/drivers/pinctrl/mediatek/pinctrl-mtmips.c index efd77b6c5..e5e085915 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mtmips.c +++ b/drivers/pinctrl/mediatek/pinctrl-mtmips.c @@ -123,11 +123,24 @@ static int mtmips_pmx_group_enable(struct pinctrl_dev *pctrldev, int i; int shift; - /* dont allow double use */ + /* + * for the same pin group, if request a different function, + * then clear the group function flag and continue, else exit. + */ if (p->groups[group].enabled) { - dev_err(p->dev, "%s is already enabled\n", - p->groups[group].name); - return 0; + for (i = 0; i < p->groups[group].func_count; i++) { + if (p->groups[group].func[i].enabled == 1) { + if (!strcmp(p->func[func]->name, + p->groups[group].func[i].name)) + return 0; + p->groups[group].func[i].enabled = 0; + break; + } + } + + /* exit if request the "gpio" function again */ + if (i == p->groups[group].func_count && func == 0) + return 0; } p->groups[group].enabled = 1;
Sometimes pinctrl consumers may request different functions for the same pin group in different situations. This patch can help to reset the group function flag when requesting a different function. Signed-off-by: Shiji Yang <yangshiji66@outlook.com> --- drivers/pinctrl/mediatek/pinctrl-mtmips.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)