Re: ALPHA1 on Raspberry Pi 3B+ [added: and RPi4B]

In reply to: titus manea : "Re: ALPHA1 on Raspberry Pi 3B+ [added: and RPi4B]"
Go to: [ bottom of page ] [ top of archives ] [ this month ]
From: Mike Karels <mike_at_karels.net>
Date: Sat, 19 Aug 2023 11:55:54 UTC
On 19 Aug 2023, at 5:58, titus manea wrote:

> the overlay route is probably the best. there are other cases with upstream/vendor dts where fdt data is incomplete because some stuff is hardcoded in the linux kernel like clock-output-names and stuff
>
>> On Aug 19, 2023, at 12:21, Emmanuel Vadot <manu@bidouilliste.com> wrote:
>>
>> On Mon, 14 Aug 2023 21:44:13 +0300
>> titus <titus@edc.ro> wrote:
>>
>>> can?t we OF_setprop some bogus string in the compatible property after the first failure so it won?t come back at every bus scan
>>
>> karels@ sent me a patch that do this.

To be clear, my patch had two parts.  One part reduced the number
of bogus messages by 2/3 by testing for this specific problem.
The other attempted an OF_setprop to zap the node, but this failed
(it made other things stop working).  If anyone else wants to try
getting this to work, I can send what I have.

		Mike

>> TBH I'm totally against patching a live dtb because of some
>> non-conforming node.
>> There is two things that should be done to "fix" this :
>> 1/ Open an issue on the rpi-firmware github project saying that
>> they're using some non-standard node and that they should fix this.
>> 2/ Crafting an overlay for us that either remove the node or fix the
>> props.
>>
>> Cheers,
>>
>>>>> On 14 Aug 2023, at 20:44, Mike Karels <mike@karels.net> wrote:
>>>>
>>>>> On 14 Aug 2023, at 11:19, Emmanuel Vadot wrote:
>>>>
>>>>> On Mon, 14 Aug 2023 10:55:39 -0500
>>>>> Mike Karels <mike@karels.net> wrote:
>>>>>
>>>>>> On 14 Aug 2023, at 10:44, titus wrote:
>>>>>>
>>>>>>> +    if (OF_hasprop(ofw_bus_get_node(dev), "clock-frequency") == 0) {
>>>>>>> +        device_printf(dev, "clock-fixed have no clock-frequency\n");
>>>>>>> +        return (ENXIO);
>>>>>>> +    }
>>>>>>> this runs before compat_data is checked so will print it for any device clock or not
>>>>>
>>>>> Meh, that's why I should do code at 7am :)
>>>>>
>>>>>> Good point!  Shuffling a bit to check both, I see 58 occurrences
>>>>>> of the message.  That's better than the 50+ 3-line sequences before.
>>>>>>
>>>>>>        Mike
>>>>>
>>>>> Is that more or less than the previous message ? If it's the same
>>>>> there is no point to commit this.
>>>>
>>>> It's the same number of occurrences, but before there were 3 lines per
>>>> occurrence.  Now it's 1, and it's more clear what it means.  I'd say
>>>> it's worth it.
>>>>
>>>>        Mike
>>>>
>>>>>>>> On Aug 14, 2023, at 6:07 PM, Emmanuel Vadot <manu@bidouilliste.com> wrote:
>>>>>>>>
>>>>>>>> On Mon, 14 Aug 2023 09:51:27 -0500
>>>>>>>> Mike Karels <mike@karels.net> wrote:
>>>>>>>>
>>>>>>>>> On 14 Aug 2023, at 0:30, Emmanuel Vadot wrote:
>>>>>>>>>
>>>>>>>>>> On Mon, 14 Aug 2023 07:03:19 +0200
>>>>>>>>>> Emmanuel Vadot <manu@bidouilliste.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> On Sun, 13 Aug 2023 12:35:31 -0500
>>>>>>>>>>> Mike Karels <mike@karels.net> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> On 13 Aug 2023, at 12:19, Emmanuel Vadot wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> On Sun, 13 Aug 2023 11:25:25 -0500
>>>>>>>>>>>>> Mike Karels <mike@karels.net> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 13 Aug 2023, at 11:10, Mark Millard wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Aug 13, 2023, at 08:17, Warner Losh <imp@bsdimp.com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Manu just updated Linux DTS in the tree. Maybe see if you revert that if the problem persists.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> git: 69f8cc60aa1e - main - ofw_firmware: Only match if there is no compatible
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> is the fix that Manu has committed:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> QUOTE
>>>>>>>>>>>>>>>  ofw_firmware: Only match if there is no compatible
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>  If there is a compatible string it likely means that the firmware needs
>>>>>>>>>>>>>>>  a dedicated driver (like on RPI*).
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>  PR:     273087
>>>>>>>>>>>>>>>  Tested-by:      Mark Millard <marklmi26-fbsd@yahoo.com>
>>>>>>>>>>>>>>>  Sponsored by:   Beckhoff Automation GmbH & Co. KG
>>>>>>>>>>>>>>>  Fixes:          fdfd3a90b6ce ("ofw: Add a ofw_firmware driver")
>>>>>>>>>>>>>>> END QUOTE
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Just for completeness: that change fixes the bcm2835_cpufreq0/powerd
>>>>>>>>>>>>>> problem and the gpioled0 problem, but not the clk_fixed2 problem
>>>>>>>>>>>>>> (clk_fixed4 on rpi4).  Installing an msdos boot partition from the
>>>>>>>>>>>>>> 3 Aug image makes that problem disappear.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>        Mike
>>>>>>>>>>>>>
>>>>>>>>>>>>> There is two fixed-clock in the DTB without clock-frequency property
>>>>>>>>>>>>> and with a status set to "disabled", this isn't conforming to the
>>>>>>>>>>>>> bindings
>>>>>>>>>>>>> (https://cgit.freebsd.org/src/tree/sys/contrib/device-tree/Bindings/clock/fixed-clock.yaml)
>>>>>>>>>>>>> so we complain on this, this is normal.
>>>>>>>>>>>>
>>>>>>>>>>>> Would it be possible to detect the disabled status to prevent the errors
>>>>>>>>>>>> (I'm guessing not) or to suppress the repeats?  150 lines of errors seems
>>>>>>>>>>>> like a lot for an out-of-spec DTB entry, and makes it hard to ignore.
>>>>>>>>>>>>
>>>>>>>>>>>>        Mike
>>>>>>>>>>>
>>>>>>>>>>> Detecting the disabled status makes no sense, a fixed clock cannot be
>>>>>>>>>>> disable, it's always present and running.
>>>>>>>>>>> But I think that if we check that clock-frenquency isn't present in
>>>>>>>>>>> the probe function, print a message and bail we will not attempt to
>>>>>>>>>>> attach the driver at each pass.
>>>>>>>>>>> That's the only clean solution that I can see without making dirty
>>>>>>>>>>> hacks for some non-conforming DTB.
>>>>>>>>>>
>>>>>>>>>> Something like this :
>>>>>>>>>> https://people.freebsd.org/~manu/0001-clk-fixed-Bail-early-if-there-is-no-clock-frequency-.patch
>>>>>>>>>>
>>>>>>>>>> Please let me know if that works.
>>>>>>>>>
>>>>>>>>> Not exactly.  This results in:
>>>>>>>>>
>>>>>>>>> clk_fixed4: clock-fixed have no clock-frequency
>>>>>>>>>
>>>>>>>>> but this appears 1562 times.  btw, "have" should be "has".
>>>>>>>>
>>>>>>>> Then I don't know how to fix this properly.
>>>>>>>>
>>>>>>>>> I hadn't noticed them before, but there are a lock order reversal
>>>>>>>>> and a malloc while holding a non-sleepable lock, associated with
>>>>>>>>> the gpio fix:
>>>>>>>>
>>>>>>>> Are you sure that they are new ?
>>>>>>>>
>>>>>>>>> gpioled0: <GPIO LEDs> on ofwbus0
>>>>>>>>> lock order reversal: (sleepable after non-sleepable)
>>>>>>>>> 1st 0xffff000000d0f6f8 LED mtx (LED mtx, sleep mutex) @ /usr/src/sys/dev/led/led.c:298
>>>>>>>>> 2nd 0xffffa00001857c10 Raspberry Pi firmware gpio (Raspberry Pi firmware gpio, sx) @ /usr/src/sys/arm/broadcom/bcm2835/raspberrypi_gpio.c:252
>>>>>>>>> lock order LED mtx -> Raspberry Pi firmware gpio attempted at:
>>>>>>>>> #0 0xffff0000004d3984 at witness_checkorder+0xa98
>>>>>>>>> #1 0xffff00000046ecb4 at _sx_xlock+0x7c
>>>>>>>>> #2 0xffff0000008b1700 at rpi_fw_gpio_pin_set+0xe0
>>>>>>>>> #3 0xffff0000001c7400 at led_create_state+0x158
>>>>>>>>> #4 0xffff0000001910d4 at gpioled_attach+0x290
>>>>>>>>> #5 0xffff00000049efa4 at device_attach+0x3f8
>>>>>>>>> #6 0xffff00000049eb14 at device_probe_and_attach+0x7c
>>>>>>>>> #7 0xffff0000004a0e54 at bus_generic_new_pass+0xfc
>>>>>>>>> #8 0xffff0000004a0e04 at bus_generic_new_pass+0xac
>>>>>>>>> #9 0xffff0000004a0e04 at bus_generic_new_pass+0xac
>>>>>>>>> #10 0xffff00000049bd30 at bus_set_pass+0x4c
>>>>>>>>> #11 0xffff0000003ea3bc at mi_startup+0x1fc
>>>>>>>>> #12 0xffff0000000008ac at virtdone+0x70
>>>>>>>>> uma_zalloc_debug: zone "malloc-64" with the following non-sleepable locks held:
>>>>>>>>> exclusive sleep mutex LED mtx (LED mtx) r = 0 (0xffff000000d0f6f8) locked @ /usr/src/sys/dev/led/led.c:298
>>>>>>>>> stack backtrace:
>>>>>>>>> #0 0xffff0000004d3dc8 at witness_debugger+0x5c
>>>>>>>>> #1 0xffff0000004d4fcc at witness_warn+0x400
>>>>>>>>> #2 0xffff0000007830a0 at uma_zalloc_debug+0x30
>>>>>>>>> #3 0xffff000000782c20 at uma_zalloc_arg+0x2c
>>>>>>>>> #4 0xffff000000437abc at malloc+0x8c
>>>>>>>>> #5 0xffff0000008a69b4 at bcm2835_firmware_property+0x44
>>>>>>>>> #6 0xffff0000008b1718 at rpi_fw_gpio_pin_set+0xf8
>>>>>>>>> #7 0xffff0000001c7400 at led_create_state+0x158
>>>>>>>>> #8 0xffff0000001910d4 at gpioled_attach+0x290
>>>>>>>>> #9 0xffff00000049efa4 at device_attach+0x3f8
>>>>>>>>> #10 0xffff00000049eb14 at device_probe_and_attach+0x7c
>>>>>>>>> #11 0xffff0000004a0e54 at bus_generic_new_pass+0xfc
>>>>>>>>> #12 0xffff0000004a0e04 at bus_generic_new_pass+0xac
>>>>>>>>> #13 0xffff0000004a0e04 at bus_generic_new_pass+0xac
>>>>>>>>> #14 0xffff00000049bd30 at bus_set_pass+0x4c
>>>>>>>>> #15 0xffff0000003ea3bc at mi_startup+0x1fc
>>>>>>>>> #16 0xffff0000000008ac at virtdone+0x70
>>>>>>>>> uma_zalloc_debug: zone "malloc-16" with the following non-sleepable locks held:
>>>>>>>>> exclusive sleep mutex LED mtx (LED mtx) r = 0 (0xffff000000d0f6f8) locked @ /usr/src/sys/dev/led/led.c:298
>>>>>>>>> stack backtrace:
>>>>>>>>> #0 0xffff0000004d3dc8 at witness_debugger+0x5c
>>>>>>>>> #1 0xffff0000004d4fcc at witness_warn+0x400
>>>>>>>>> #2 0xffff0000007830a0 at uma_zalloc_debug+0x30
>>>>>>>>> #3 0xffff000000782c20 at uma_zalloc_arg+0x2c
>>>>>>>>> #4 0xffff000000437abc at malloc+0x8c
>>>>>>>>> #5 0xffff0000007ce15c at bounce_bus_dmamem_alloc+0x50
>>>>>>>>> #6 0xffff0000008a9404 at bcm2835_mbox_property+0xdc
>>>>>>>>> #7 0xffff0000008a69e8 at bcm2835_firmware_property+0x78
>>>>>>>>> #8 0xffff0000008b1718 at rpi_fw_gpio_pin_set+0xf8
>>>>>>>>> #9 0xffff0000001c7400 at led_create_state+0x158
>>>>>>>>> #10 0xffff0000001910d4 at gpioled_attach+0x290
>>>>>>>>> #11 0xffff00000049efa4 at device_attach+0x3f8
>>>>>>>>> #12 0xffff00000049eb14 at device_probe_and_attach+0x7c
>>>>>>>>> #13 0xffff0000004a0e54 at bus_generic_new_pass+0xfc
>>>>>>>>> #14 0xffff0000004a0e04 at bus_generic_new_pass+0xac
>>>>>>>>> #15 0xffff0000004a0e04 at bus_generic_new_pass+0xac
>>>>>>>>> #16 0xffff00000049bd30 at bus_set_pass+0x4c
>>>>>>>>> #17 0xffff0000003ea3bc at mi_startup+0x1fc
>>>>>>>>> uma_zalloc_debug: zone "malloc-128" with the following non-sleepable locks held:
>>>>>>>>> exclusive sleep mutex LED mtx (LED mtx) r = 0 (0xffff000000d0f6f8) locked @ /usr/src/sys/dev/led/led.c:298
>>>>>>>>> stack backtrace:
>>>>>>>>> #0 0xffff0000004d3dc8 at witness_debugger+0x5c
>>>>>>>>> #1 0xffff0000004d4fcc at witness_warn+0x400
>>>>>>>>> #2 0xffff0000007830a0 at uma_zalloc_debug+0x30
>>>>>>>>> #3 0xffff000000782c20 at uma_zalloc_arg+0x2c
>>>>>>>>> #4 0xffff000000437abc at malloc+0x8c
>>>>>>>>> #5 0xffff0000007ce1ac at bounce_bus_dmamem_alloc+0xa0
>>>>>>>>> #6 0xffff0000008a9404 at bcm2835_mbox_property+0xdc
>>>>>>>>> #7 0xffff0000008a69e8 at bcm2835_firmware_property+0x78
>>>>>>>>> #8 0xffff0000008b1718 at rpi_fw_gpio_pin_set+0xf8
>>>>>>>>> #9 0xffff0000001c7400 at led_create_state+0x158
>>>>>>>>> #10 0xffff0000001910d4 at gpioled_attach+0x290
>>>>>>>>> #11 0xffff00000049efa4 at device_attach+0x3f8
>>>>>>>>> #12 0xffff00000049eb14 at device_probe_and_attach+0x7c
>>>>>>>>> #13 0xffff0000004a0e54 at bus_generic_new_pass+0xfc
>>>>>>>>> #14 0xffff0000004a0e04 at bus_generic_new_pass+0xac
>>>>>>>>> #15 0xffff0000004a0e04 at bus_generic_new_pass+0xac
>>>>>>>>> #16 0xffff00000049bd30 at bus_set_pass+0x4c
>>>>>>>>> #17 0xffff0000003ea3bc at mi_startup+0x1fc
>>>>>>>>>
>>>>>>>>>        Mike
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>
>>>>>>>>
>>>>>
>>>>>
>>>>> -- 
>>>>> Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>
>>>
>>
>>
>> -- 
>> Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>
>>