From nobody Sat Aug 19 10:58:19 2023 X-Original-To: freebsd-arm@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4RSbMR5Qf1z4qYsW for ; Sat, 19 Aug 2023 10:58:43 +0000 (UTC) (envelope-from titus@edc.ro) Received: from eatlas.ro (eatlas.ro [86.126.82.18]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "eatlas.ro", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4RSbMR1XKJz3PZZ for ; Sat, 19 Aug 2023 10:58:42 +0000 (UTC) (envelope-from titus@edc.ro) Authentication-Results: mx1.freebsd.org; none Received: from mail.edc.ro ([10.1.4.58]) by eatlas.ro (8.16.1/8.16.1) with ESMTPS id 37JAwUBg057676 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Sat, 19 Aug 2023 13:58:30 +0300 (EEST) (envelope-from titus@edc.ro) Received: from smtpclient.apple (79-113-65-76.rdsnet.ro [79.113.65.76] (may be forged)) (authenticated bits=0) by mail.edc.ro (8.16.1/8.16.1) with ESMTPSA id 37JAwS8D051503 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Sat, 19 Aug 2023 13:58:28 +0300 (EEST) (envelope-from titus@edc.ro) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=edc.ro; s=mail; t=1692442709; bh=9o64mwBQdgl+DTJL1tScAWEaO8QDYnqFBN0HEq84Tr4=; h=From:Subject:Date:References:Cc:In-Reply-To:To; b=5Mn1GExDxvK6xy/Ne7FeJATPKqE3sQZMnSx84JWf/uQi7bLpGn4xVEalq5PKtJ6m3 x4t+JOfNEmtw6BO7J11hdX4iZ4zWjWPsF2NE+iwsYfPnzXU94LliyHmwEIZws6Rl4M mP4FfLsoy5bCn86MriARIutHx4Efja0mOj1ZrjOY= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable From: titus manea List-Id: Porting FreeBSD to ARM processors List-Archive: https://lists.freebsd.org/archives/freebsd-arm List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-arm@freebsd.org Mime-Version: 1.0 (1.0) Subject: Re: ALPHA1 on Raspberry Pi 3B+ [added: and RPi4B] Date: Sat, 19 Aug 2023 13:58:19 +0300 Message-Id: References: <20230819112124.46e66629ca18071f11d9d361@bidouilliste.com> Cc: freebsd-arm In-Reply-To: <20230819112124.46e66629ca18071f11d9d361@bidouilliste.com> To: Emmanuel Vadot X-Mailer: iPhone Mail (20G75) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on ns.edc.ro X-Rspamd-Queue-Id: 4RSbMR1XKJz3PZZ X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:8708, ipnet:86.120.0.0/13, country:RO] 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 t= he linux kernel like clock-output-names and stuff > On Aug 19, 2023, at 12:21, Emmanuel Vadot wrote: >=20 > =EF=BB=BFOn Mon, 14 Aug 2023 21:44:13 +0300 > titus wrote: >=20 >> can?t we OF_setprop some bogus string in the compatible property after th= e first failure so it won?t come back at every bus scan >=20 > karels@ sent me a patch that do this. > 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. >=20 > Cheers, >=20 >>>> On 14 Aug 2023, at 20:44, Mike Karels wrote: >>>=20 >>>> On 14 Aug 2023, at 11:19, Emmanuel Vadot wrote: >>>=20 >>>> On Mon, 14 Aug 2023 10:55:39 -0500 >>>> Mike Karels wrote: >>>>=20 >>>>> On 14 Aug 2023, at 10:44, titus wrote: >>>>>=20 >>>>>> + if (OF_hasprop(ofw_bus_get_node(dev), "clock-frequency") =3D=3D 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 devi= ce clock or not >>>>=20 >>>> Meh, that's why I should do code at 7am :) >>>>=20 >>>>> 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. >>>>>=20 >>>>> Mike >>>>=20 >>>> Is that more or less than the previous message ? If it's the same >>>> there is no point to commit this. >>>=20 >>> 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. >>>=20 >>> Mike >>>=20 >>>>>>> On Aug 14, 2023, at 6:07 PM, Emmanuel Vadot w= rote: >>>>>>>=20 >>>>>>> On Mon, 14 Aug 2023 09:51:27 -0500 >>>>>>> Mike Karels wrote: >>>>>>>=20 >>>>>>>> On 14 Aug 2023, at 0:30, Emmanuel Vadot wrote: >>>>>>>>=20 >>>>>>>>> On Mon, 14 Aug 2023 07:03:19 +0200 >>>>>>>>> Emmanuel Vadot wrote: >>>>>>>>>=20 >>>>>>>>>> On Sun, 13 Aug 2023 12:35:31 -0500 >>>>>>>>>> Mike Karels wrote: >>>>>>>>>>=20 >>>>>>>>>>> On 13 Aug 2023, at 12:19, Emmanuel Vadot wrote: >>>>>>>>>>>=20 >>>>>>>>>>>> On Sun, 13 Aug 2023 11:25:25 -0500 >>>>>>>>>>>> Mike Karels wrote: >>>>>>>>>>>>=20 >>>>>>>>>>>>> On 13 Aug 2023, at 11:10, Mark Millard wrote: >>>>>>>>>>>>>=20 >>>>>>>>>>>>>> On Aug 13, 2023, at 08:17, Warner Losh wrote= : >>>>>>>>>>>>>>=20 >>>>>>>>>>>>>>> Manu just updated Linux DTS in the tree. Maybe see if you re= vert that if the problem persists. >>>>>>>>>>>>>>=20 >>>>>>>>>>>>>> git: 69f8cc60aa1e - main - ofw_firmware: Only match if there i= s no compatible >>>>>>>>>>>>>>=20 >>>>>>>>>>>>>> is the fix that Manu has committed: >>>>>>>>>>>>>>=20 >>>>>>>>>>>>>> QUOTE >>>>>>>>>>>>>> ofw_firmware: Only match if there is no compatible >>>>>>>>>>>>>>=20 >>>>>>>>>>>>>> If there is a compatible string it likely means that the fir= mware needs >>>>>>>>>>>>>> a dedicated driver (like on RPI*). >>>>>>>>>>>>>>=20 >>>>>>>>>>>>>> PR: 273087 >>>>>>>>>>>>>> Tested-by: Mark Millard >>>>>>>>>>>>>> Sponsored by: Beckhoff Automation GmbH & Co. KG >>>>>>>>>>>>>> Fixes: fdfd3a90b6ce ("ofw: Add a ofw_firmware drive= r") >>>>>>>>>>>>>> END QUOTE >>>>>>>>>>>>>=20 >>>>>>>>>>>>> Just for completeness: that change fixes the bcm2835_cpufreq0/= powerd >>>>>>>>>>>>> problem and the gpioled0 problem, but not the clk_fixed2 probl= em >>>>>>>>>>>>> (clk_fixed4 on rpi4). Installing an msdos boot partition from= the >>>>>>>>>>>>> 3 Aug image makes that problem disappear. >>>>>>>>>>>>>=20 >>>>>>>>>>>>> Mike >>>>>>>>>>>>=20 >>>>>>>>>>>> There is two fixed-clock in the DTB without clock-frequency pro= perty >>>>>>>>>>>> and with a status set to "disabled", this isn't conforming to t= he >>>>>>>>>>>> bindings >>>>>>>>>>>> (https://cgit.freebsd.org/src/tree/sys/contrib/device-tree/Bind= ings/clock/fixed-clock.yaml) >>>>>>>>>>>> so we complain on this, this is normal. >>>>>>>>>>>=20 >>>>>>>>>>> Would it be possible to detect the disabled status to prevent th= e errors >>>>>>>>>>> (I'm guessing not) or to suppress the repeats? 150 lines of err= ors seems >>>>>>>>>>> like a lot for an out-of-spec DTB entry, and makes it hard to ig= nore. >>>>>>>>>>>=20 >>>>>>>>>>> Mike >>>>>>>>>>=20 >>>>>>>>>> Detecting the disabled status makes no sense, a fixed clock canno= t be >>>>>>>>>> disable, it's always present and running. >>>>>>>>>> But I think that if we check that clock-frenquency isn't present i= n >>>>>>>>>> the probe function, print a message and bail we will not attempt t= o >>>>>>>>>> attach the driver at each pass. >>>>>>>>>> That's the only clean solution that I can see without making dirt= y >>>>>>>>>> hacks for some non-conforming DTB. >>>>>>>>>=20 >>>>>>>>> Something like this : >>>>>>>>> https://people.freebsd.org/~manu/0001-clk-fixed-Bail-early-if-ther= e-is-no-clock-frequency-.patch >>>>>>>>>=20 >>>>>>>>> Please let me know if that works. >>>>>>>>=20 >>>>>>>> Not exactly. This results in: >>>>>>>>=20 >>>>>>>> clk_fixed4: clock-fixed have no clock-frequency >>>>>>>>=20 >>>>>>>> but this appears 1562 times. btw, "have" should be "has". >>>>>>>=20 >>>>>>> Then I don't know how to fix this properly. >>>>>>>=20 >>>>>>>> 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: >>>>>>>=20 >>>>>>> Are you sure that they are new ? >>>>>>>=20 >>>>>>>> gpioled0: on ofwbus0 >>>>>>>> lock order reversal: (sleepable after non-sleepable) >>>>>>>> 1st 0xffff000000d0f6f8 LED mtx (LED mtx, sleep mutex) @ /usr/src/sy= s/dev/led/led.c:298 >>>>>>>> 2nd 0xffffa00001857c10 Raspberry Pi firmware gpio (Raspberry Pi fir= mware 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 =3D 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 =3D 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-sleepabl= e locks held: >>>>>>>> exclusive sleep mutex LED mtx (LED mtx) r =3D 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 >>>>>>>>=20 >>>>>>>> Mike >>>>>>>>=20 >>>>>>>=20 >>>>>>>=20 >>>>>>> --=20 >>>>>>> Emmanuel Vadot >>>>>>>=20 >>>>=20 >>>>=20 >>>> --=20 >>>> Emmanuel Vadot >>=20 >=20 >=20 > --=20 > Emmanuel Vadot >=20