Re: [PATCH] pinctrl: mtmips: do not log when repeating the same pinctrl request

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,
Thank you for your review.

On Sun, 23 Jul 2023 21:49:52 +0200 Linus Walleij wrote:

>On Tue, Jul 18, 2023 at 5:16 PM Shiji Yang <yangshiji66@xxxxxxxxxxx> wrote:
>
>> Sometimes when driver fails to probe a device, the kernel will retry
>> it later. However, this will result in duplicate requests for the
>> same pinctrl configuration. In this case, we should not throw error
>> logs. This patch adds extra check for the pin group function. Now the
>> pinctrl driver only prints error log when attempting to configure the
>> same group as different functions.
>>
>> Signed-off-by: Shiji Yang <yangshiji66@xxxxxxxxxxx>
>> ---
>>  drivers/pinctrl/mediatek/pinctrl-mtmips.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtmips.c b/drivers/pinctrl/mediatek/pinctrl-mtmips.c
>> index efd77b6c5..8f5493220 100644
>> --- a/drivers/pinctrl/mediatek/pinctrl-mtmips.c
>> +++ b/drivers/pinctrl/mediatek/pinctrl-mtmips.c
>> @@ -125,8 +125,9 @@ static int mtmips_pmx_group_enable(struct pinctrl_dev *pctrldev,
>>
>>         /* dont allow double use */
>>         if (p->groups[group].enabled) {
>> -               dev_err(p->dev, "%s is already enabled\n",
>> -                       p->groups[group].name);
>> +               if (!p->func[func]->enabled)
>> +                       dev_err(p->dev, "%s is already enabled\n",
>> +                               p->groups[group].name);
>
>Why is the driver not backing out properly and setting this .enabled back
>to false when probing fails for some requesting driver?
>
>Or am I getting something wrong here?
>
>Yours,
>Linus Walleij
>

We use pinctrl_select_state() to request the pinctrl and set the pin
function. After searching "include/linux/pinctrl/consumer.h", I don't
find a relevant API to reverse this operation so we can't set .enabled
back to the false state. If I'm wrong please correct me, I don't know
much about the pinctrl architecture.

At least I can sure pinctrl-mtmips doesn't have an implementation to
unset func[]->enabled and groups[].enabled status.

ref:
https://elixir.bootlin.com/linux/latest/source/include/linux/pinctrl/consumer.h

Regards,
Shiji Yang



[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux