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

[Florian Fainelli]

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?
-- 
Florian