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@xxxxxxxxxxx>
> > ---
> > 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
