DTB backward/forward compatibility with "pinctrl: bcm2835: Change init order for gpio hogs"

[Florian Fainelli]

On 1/25/2022 11:58 AM, Florian Fainelli wrote:
> 
> 
> On 1/25/2022 11:48 AM, Phil Elwell wrote:
>> Florian,
>>
>> On 25/01/2022 19:39, Florian Fainelli wrote:
>>> Hi,
>>>
>>> I am a bit frustrated by this commit, we picked it up via the stable 
>>> 5.10 and 5.15 trees into our downstream tree, and in the absence of a 
>>> suitable 'gpio-ranges' property for the GPIO controller, the SPI 
>>> controller keeps getting -EPROBE_DEFER for its chip select. If the 
>>> property is present, then all is well.
>>>
>>> Now the problem in my case is that the boot loader is responsible for 
>>> providing the DTB to the kernel, and until recently, we did not 
>>> update it to contain a suitable 'gpio-ranges' property. Now that it 
>>> has been updated however, older kernels which *do not* have said 
>>> change in the subject are also getting -EPROBE_DEFER for the SPI chip 
>>> select.
>>>
>>> So this is just breaking backward/forward compatibility with the DTB 
>>> unless both are updated in lock steps which is *extremely* inconvenient.
>>>
>>> This is death by a thousand cuts.
>>>
>>> So how do we remedy this?
>>
>> It's unfortunate that both the DTS and driver were incorrect, and that 
>> the fixes were inter-dependent. At Raspberry Pi we make a point of 
>> treating the DTBs as part of the kernel, updating them at the same 
>> time. Is that not possible in your situation? You don't specify which 
>> bootloader you are using (or even which platform), but U-boot can be 
>> configured to use the DTB loaded and adjusted by the firmware.
> 
> We use a boot loader called BOLT which has an offline DTS generation 
> part an a runtime patching part. At no point in time has it been 
> necessary, desirable or even reliably possible to look at the kernel 
> version and mangle the Device Tree such that "things" work. I do not 
> even want to fathom what the code doing that would look like other than 
> a big pile of mess. This is utterly error prone and completely breaks 
> the boot loader/kernel abstraction.
> 
> We strive to be able to update the boot loader and the kernel seemingly 
> independently from one another even though obviously there are times 
> where locksteps are necessary. What I like to do is:
> 
> - put the changes in the DTB that are necessary for a *future* kernel 
> well in advance such that by the time that newer kernel gets used these 
> properties are already there
> 
> - adding new properties does not break older kernels, they simply ignore 
> them
> 
> And this allows us to define a combination that always works while 
> having a sliding window of backward/forward compatibility if we have 
> locksteps in between.
> 
> I can make changes in the downstream kernel such that we prune DT 
> properties, but once we open that door, the list goes on and on and it 
> is just a damage control strategy anyway.
> 
> Why cannot the pinctrl framework infer that we have a 1:1 mapping in the 
> absence of a 'gpio-ranges' property, but instead does the following:
> 
> # cat /sys/kernel/debug/pinctrl/47e200000.gpio-pinctrl-bcm2711/gpio-ranges
> GPIO ranges handled:
> 0: pinctrl-bcm2711 GPIOS [4294967295 - 56] PINS [0 - 57]
> #
> 
> clearly it determined the end GPIO but it is not able to determine the 
> start GPIO?
> 
> And now, on a 5.10 kernel which does contain both the 'gpio-ranges' 
> property and the said change to pinctrl-bcm2835.c I have this:
> 
> # cat /sys/kernel/debug/pinctrl/47e200000.gpio-pinctrl-bcm2711/gpio-ranges
> GPIO ranges handled:
> 0: pinctrl-bcm2711 GPIOS [4294967295 - 56] PINS [0 - 57]
> 0: pinctrl-bcm2711 GPIOS [454 - 511] PINS [0 - 57]
> #
> 
> What the heck?

And just to be clear, at this point I understand that the old kernel, 
new DTB/DTS is not something we can obviously fix, however new kernel 
old DTB/DTS *without* 'gpio-ranges' is something that should still work 
to ease the pain.

I will dig more into why we have this duplicated debugfs output (most 
likely we are not unwinding an error path when we get -EPROBE_DEFER) and 
also why pinctrl is not able to figure out the gpio start properly 
without 'gpio-ranges'.
-- 
Florian