mbox series

[RFC,0/2] gpiolib: of: Introduce hook for missing gpio-ranges

Message ID 1646855026-9132-1-git-send-email-stefan.wahren@i2se.com
Headers show
Series gpiolib: of: Introduce hook for missing gpio-ranges | expand

Message

Stefan Wahren March 9, 2022, 7:43 p.m. UTC
This patch series tries to provide backward compatibility for DTB which
lacks the gpio-ranges property.

The commit ("pinctrl: msm: fix gpio-hog related boot issues") by Christian
Lamparter already contains a fallback in case the gpio-ranges property
is missing. But this approach doesn't work on BCM2835 with a gpio-hog
defined for the SoC GPIOs.

Based Christian's on explanation i conclude that the fallback must happen
during the gpiochip_add() call and not afterwards. So the approach is to
call an optional hook, which can be implemented in the platform driver.

This series has been tested on Raspberry Pi 3 B Plus.

Stefan Wahren (2):
  gpiolib: of: Introduce hook for missing gpio-ranges
  pinctrl: bcm2835: implement hook for missing gpio-ranges

 drivers/gpio/gpiolib-of.c             |  5 +++++
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 18 ++++++++++++++++++
 include/linux/gpio/driver.h           | 12 ++++++++++++
 3 files changed, 35 insertions(+)

Comments

Linus Walleij March 17, 2022, 1:15 a.m. UTC | #1
On Wed, Mar 9, 2022 at 8:44 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:

> This patch series tries to provide backward compatibility for DTB which
> lacks the gpio-ranges property.
>
> The commit ("pinctrl: msm: fix gpio-hog related boot issues") by Christian
> Lamparter already contains a fallback in case the gpio-ranges property
> is missing. But this approach doesn't work on BCM2835 with a gpio-hog
> defined for the SoC GPIOs.
>
> Based Christian's on explanation i conclude that the fallback must happen
> during the gpiochip_add() call and not afterwards. So the approach is to
> call an optional hook, which can be implemented in the platform driver.
>
> This series has been tested on Raspberry Pi 3 B Plus.
>
> Stefan Wahren (2):
>   gpiolib: of: Introduce hook for missing gpio-ranges
>   pinctrl: bcm2835: implement hook for missing gpio-ranges

Looks good to me, is this something I should apply to the pinctrl
tree or should I wait for a non-RFC version?

Yours,
Linus Walleij
Florian Fainelli March 17, 2022, 2:02 a.m. UTC | #2
On 3/16/2022 6:15 PM, Linus Walleij wrote:
> On Wed, Mar 9, 2022 at 8:44 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
> 
>> This patch series tries to provide backward compatibility for DTB which
>> lacks the gpio-ranges property.
>>
>> The commit ("pinctrl: msm: fix gpio-hog related boot issues") by Christian
>> Lamparter already contains a fallback in case the gpio-ranges property
>> is missing. But this approach doesn't work on BCM2835 with a gpio-hog
>> defined for the SoC GPIOs.
>>
>> Based Christian's on explanation i conclude that the fallback must happen
>> during the gpiochip_add() call and not afterwards. So the approach is to
>> call an optional hook, which can be implemented in the platform driver.
>>
>> This series has been tested on Raspberry Pi 3 B Plus.
>>
>> Stefan Wahren (2):
>>    gpiolib: of: Introduce hook for missing gpio-ranges
>>    pinctrl: bcm2835: implement hook for missing gpio-ranges
> 
> Looks good to me, is this something I should apply to the pinctrl
> tree or should I wait for a non-RFC version?

I would be inclined to slap a couple of different Fixes tag to each 
commit because breaking older DTBs should IMHO be considered a 
regression. So for the first patch I would add:

Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges")

and for the second patch:

Fixes: 266423e60ea1 ("pinctrl: bcm2835: Change init order for gpio hogs")

WDYT?
Stefan Wahren March 17, 2022, 11:48 a.m. UTC | #3
Hi,

Am 17.03.22 um 03:02 schrieb Florian Fainelli:
>
>
> On 3/16/2022 6:15 PM, Linus Walleij wrote:
>> On Wed, Mar 9, 2022 at 8:44 PM Stefan Wahren <stefan.wahren@i2se.com> 
>> wrote:
>>
>>> This patch series tries to provide backward compatibility for DTB which
>>> lacks the gpio-ranges property.
>>>
>>> The commit ("pinctrl: msm: fix gpio-hog related boot issues") by 
>>> Christian
>>> Lamparter already contains a fallback in case the gpio-ranges property
>>> is missing. But this approach doesn't work on BCM2835 with a gpio-hog
>>> defined for the SoC GPIOs.
>>>
>>> Based Christian's on explanation i conclude that the fallback must 
>>> happen
>>> during the gpiochip_add() call and not afterwards. So the approach 
>>> is to
>>> call an optional hook, which can be implemented in the platform driver.
>>>
>>> This series has been tested on Raspberry Pi 3 B Plus.
>>>
>>> Stefan Wahren (2):
>>>    gpiolib: of: Introduce hook for missing gpio-ranges
>>>    pinctrl: bcm2835: implement hook for missing gpio-ranges
>>
>> Looks good to me, is this something I should apply to the pinctrl
>> tree or should I wait for a non-RFC version?
>
> I would be inclined to slap a couple of different Fixes tag to each 
> commit because breaking older DTBs should IMHO be considered a 
> regression. So for the first patch I would add:
>
> Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges")
>
> and for the second patch:
>
> Fixes: 266423e60ea1 ("pinctrl: bcm2835: Change init order for gpio hogs")
>
> WDYT?
so you consider backporting this "feature"?
Florian Fainelli March 17, 2022, 5:17 p.m. UTC | #4
On 3/17/22 4:48 AM, Stefan Wahren wrote:
> Hi,
> 
> Am 17.03.22 um 03:02 schrieb Florian Fainelli:
>>
>>
>> On 3/16/2022 6:15 PM, Linus Walleij wrote:
>>> On Wed, Mar 9, 2022 at 8:44 PM Stefan Wahren <stefan.wahren@i2se.com>
>>> wrote:
>>>
>>>> This patch series tries to provide backward compatibility for DTB which
>>>> lacks the gpio-ranges property.
>>>>
>>>> The commit ("pinctrl: msm: fix gpio-hog related boot issues") by
>>>> Christian
>>>> Lamparter already contains a fallback in case the gpio-ranges property
>>>> is missing. But this approach doesn't work on BCM2835 with a gpio-hog
>>>> defined for the SoC GPIOs.
>>>>
>>>> Based Christian's on explanation i conclude that the fallback must
>>>> happen
>>>> during the gpiochip_add() call and not afterwards. So the approach
>>>> is to
>>>> call an optional hook, which can be implemented in the platform driver.
>>>>
>>>> This series has been tested on Raspberry Pi 3 B Plus.
>>>>
>>>> Stefan Wahren (2):
>>>>    gpiolib: of: Introduce hook for missing gpio-ranges
>>>>    pinctrl: bcm2835: implement hook for missing gpio-ranges
>>>
>>> Looks good to me, is this something I should apply to the pinctrl
>>> tree or should I wait for a non-RFC version?
>>
>> I would be inclined to slap a couple of different Fixes tag to each
>> commit because breaking older DTBs should IMHO be considered a
>> regression. So for the first patch I would add:
>>
>> Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges")
>>
>> and for the second patch:
>>
>> Fixes: 266423e60ea1 ("pinctrl: bcm2835: Change init order for gpio hogs")
>>
>> WDYT?
> so you consider backporting this "feature"?

Yes, I would consider your changes fixes actually. If I am the only one
deeply concerned about backwards compatibility I suppose I could
backport those into our tree.
Stefan Wahren March 17, 2022, 7:23 p.m. UTC | #5
Hi,

Am 17.03.22 um 18:17 schrieb Florian Fainelli:
> On 3/17/22 4:48 AM, Stefan Wahren wrote:
>> Hi,
>>
>> Am 17.03.22 um 03:02 schrieb Florian Fainelli:
>>>
>>> On 3/16/2022 6:15 PM, Linus Walleij wrote:
>>>> On Wed, Mar 9, 2022 at 8:44 PM Stefan Wahren <stefan.wahren@i2se.com>
>>>> wrote:
>>>>
>>>>> This patch series tries to provide backward compatibility for DTB which
>>>>> lacks the gpio-ranges property.
>>>>>
>>>>> The commit ("pinctrl: msm: fix gpio-hog related boot issues") by
>>>>> Christian
>>>>> Lamparter already contains a fallback in case the gpio-ranges property
>>>>> is missing. But this approach doesn't work on BCM2835 with a gpio-hog
>>>>> defined for the SoC GPIOs.
>>>>>
>>>>> Based Christian's on explanation i conclude that the fallback must
>>>>> happen
>>>>> during the gpiochip_add() call and not afterwards. So the approach
>>>>> is to
>>>>> call an optional hook, which can be implemented in the platform driver.
>>>>>
>>>>> This series has been tested on Raspberry Pi 3 B Plus.
>>>>>
>>>>> Stefan Wahren (2):
>>>>>     gpiolib: of: Introduce hook for missing gpio-ranges
>>>>>     pinctrl: bcm2835: implement hook for missing gpio-ranges
>>>> Looks good to me, is this something I should apply to the pinctrl
>>>> tree or should I wait for a non-RFC version?
>>> I would be inclined to slap a couple of different Fixes tag to each
>>> commit because breaking older DTBs should IMHO be considered a
>>> regression. So for the first patch I would add:
>>>
>>> Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges")
>>>
>>> and for the second patch:
>>>
>>> Fixes: 266423e60ea1 ("pinctrl: bcm2835: Change init order for gpio hogs")
>>>
>>> WDYT?
>> so you consider backporting this "feature"?
> Yes, I would consider your changes fixes actually. If I am the only one
> deeply concerned about backwards compatibility I suppose I could
> backport those into our tree.
i'm fine with backporting, but i thought these must be single 
independent patches.
Florian Fainelli March 21, 2022, 6:21 p.m. UTC | #6
On 3/17/22 12:23, Stefan Wahren wrote:
> Hi,
> 
> Am 17.03.22 um 18:17 schrieb Florian Fainelli:
>> On 3/17/22 4:48 AM, Stefan Wahren wrote:
>>> Hi,
>>>
>>> Am 17.03.22 um 03:02 schrieb Florian Fainelli:
>>>>
>>>> On 3/16/2022 6:15 PM, Linus Walleij wrote:
>>>>> On Wed, Mar 9, 2022 at 8:44 PM Stefan Wahren <stefan.wahren@i2se.com>
>>>>> wrote:
>>>>>
>>>>>> This patch series tries to provide backward compatibility for DTB 
>>>>>> which
>>>>>> lacks the gpio-ranges property.
>>>>>>
>>>>>> The commit ("pinctrl: msm: fix gpio-hog related boot issues") by
>>>>>> Christian
>>>>>> Lamparter already contains a fallback in case the gpio-ranges 
>>>>>> property
>>>>>> is missing. But this approach doesn't work on BCM2835 with a gpio-hog
>>>>>> defined for the SoC GPIOs.
>>>>>>
>>>>>> Based Christian's on explanation i conclude that the fallback must
>>>>>> happen
>>>>>> during the gpiochip_add() call and not afterwards. So the approach
>>>>>> is to
>>>>>> call an optional hook, which can be implemented in the platform 
>>>>>> driver.
>>>>>>
>>>>>> This series has been tested on Raspberry Pi 3 B Plus.
>>>>>>
>>>>>> Stefan Wahren (2):
>>>>>>     gpiolib: of: Introduce hook for missing gpio-ranges
>>>>>>     pinctrl: bcm2835: implement hook for missing gpio-ranges
>>>>> Looks good to me, is this something I should apply to the pinctrl
>>>>> tree or should I wait for a non-RFC version?
>>>> I would be inclined to slap a couple of different Fixes tag to each
>>>> commit because breaking older DTBs should IMHO be considered a
>>>> regression. So for the first patch I would add:
>>>>
>>>> Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without 
>>>> pin-ranges")
>>>>
>>>> and for the second patch:
>>>>
>>>> Fixes: 266423e60ea1 ("pinctrl: bcm2835: Change init order for gpio 
>>>> hogs")
>>>>
>>>> WDYT?
>>> so you consider backporting this "feature"?
>> Yes, I would consider your changes fixes actually. If I am the only one
>> deeply concerned about backwards compatibility I suppose I could
>> backport those into our tree.
> i'm fine with backporting, but i thought these must be single 
> independent patches.

Linus, how are we proceeding with these changes? Any hope to include 
them for 5.18?
Florian Fainelli March 24, 2022, 7:23 p.m. UTC | #7
On 3/24/22 12:00, Linus Walleij wrote:
> On Mon, Mar 21, 2022 at 7:21 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> Linus, how are we proceeding with these changes? Any hope to include
>> them for 5.18?
> 
> Yes if they are finished? These two have RFC prefix but if you say they
> should be applied I'll apply them, I trust you!

I do not really see much room for improvement, unless we wanted to move 
away from the callback approach and make it somewhat mandatory to put 
that code within the core pinctrl code, with the chances of possibly 
regression.

So as far as I can tell, they work as intended and advertised, tested 
them, ship it!
--
Florian