From nobody Sat Aug 19 11:55:54 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 4RScdn5bF4z4qcmT for ; Sat, 19 Aug 2023 11:56:13 +0000 (UTC) (envelope-from mike@karels.net) Received: from mail2.karels.net (mail2.karels.net [3.19.118.201]) (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 "freebsd", Issuer "freebsd" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4RScdn3NNYz3XTS for ; Sat, 19 Aug 2023 11:56:13 +0000 (UTC) (envelope-from mike@karels.net) Authentication-Results: mx1.freebsd.org; none Received: from mail2.karels.net (localhost [IPv6:0:0:0:0:0:0:0:1]) by mail2.karels.net (8.17.1/8.17.1) with ESMTP id 37JBttIj092446; Sat, 19 Aug 2023 06:55:55 -0500 (CDT) (envelope-from mike@karels.net) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=karels.net; s=mail2; t=1692446156; bh=2BcU1E4XFEL3XyzQ2/idPq1U/rDt0vYGEMtJjSDUtdc=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=BSncnSzm9Qhv4uGRXghwgCCVAXF3UIrVei0ZnzGqCrMb6Kg1gcDx646tErm5Gxtij QQP5bRbIGAaR0cKRdlQyFY5zRGcZLoehQ4c12xVIjU02KCIg0S1EmhqnRjMctNMu3N Nxx/uy6BL1Gfq9C28B1kNpVQGicHPovN8/V7caWtTJod5ESMSI2d4BmCS6Z8Au/TvI EQJqcKxogtm0fao3bYzRaPuq/SE0JnrmulILpQFzFlh9yfySsBumUM169PjXpHIbyF S0J9Ah+G5HYzxi5G3TA0iUWu7jZeGDKG4V9JvDmeItGnM0RjBWF0voSC0PP3AmYqZ+ sEtFy28f8B0pw== Received: from [10.0.2.130] ([73.62.165.147]) by mail2.karels.net with ESMTPSA id 2LIXEMut4GQcaQEAs/W3XQ (envelope-from ); Sat, 19 Aug 2023 06:55:55 -0500 From: Mike Karels To: titus manea Cc: Emmanuel Vadot , freebsd-arm Subject: Re: ALPHA1 on Raspberry Pi 3B+ [added: and RPi4B] Date: Sat, 19 Aug 2023 06:55:54 -0500 X-Mailer: MailMate (1.14r5964) Message-ID: In-Reply-To: References: <20230819112124.46e66629ca18071f11d9d361@bidouilliste.com> 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 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 4RScdn3NNYz3XTS 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:16509, ipnet:3.16.0.0/14, country:US] On 19 Aug 2023, at 5:58, titus manea wrote: > the overlay route is probably the best. there are other cases with upst= ream/vendor dts where fdt data is incomplete because some stuff is hardco= ded in the linux kernel like clock-output-names and stuff > >> On Aug 19, 2023, at 12:21, Emmanuel Vadot wrot= e: >> >> =EF=BB=BFOn Mon, 14 Aug 2023 21:44:13 +0300 >> titus wrote: >> >>> can?t we OF_setprop some bogus string in the compatible property afte= r 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 wrote: >>>> >>>>> On 14 Aug 2023, at 11:19, Emmanuel Vadot wrote: >>>> >>>>> On Mon, 14 Aug 2023 10:55:39 -0500 >>>>> Mike Karels wrote: >>>>> >>>>>> On 14 Aug 2023, at 10:44, titus wrote: >>>>>> >>>>>>> + 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 = 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 befor= e. >>>>>> >>>>>> 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 p= er >>>> 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 wrote: >>>>>>>> >>>>>>>> On Mon, 14 Aug 2023 09:51:27 -0500 >>>>>>>> Mike Karels wrote: >>>>>>>> >>>>>>>>> On 14 Aug 2023, at 0:30, Emmanuel Vadot wrote: >>>>>>>>> >>>>>>>>>> On Mon, 14 Aug 2023 07:03:19 +0200 >>>>>>>>>> Emmanuel Vadot wrote: >>>>>>>>>> >>>>>>>>>>> On Sun, 13 Aug 2023 12:35:31 -0500 >>>>>>>>>>> Mike Karels wrote: >>>>>>>>>>> >>>>>>>>>>>> On 13 Aug 2023, at 12:19, Emmanuel Vadot wrote: >>>>>>>>>>>> >>>>>>>>>>>>> On Sun, 13 Aug 2023 11:25:25 -0500 >>>>>>>>>>>>> Mike Karels wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> On 13 Aug 2023, at 11:10, Mark Millard wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Aug 13, 2023, at 08:17, Warner Losh w= rote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Manu just updated Linux DTS in the tree. Maybe see if yo= u revert that if the problem persists. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> git: 69f8cc60aa1e - main - ofw_firmware: Only match if th= ere 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 >>>>>>>>>>>>>>> Sponsored by: Beckhoff Automation GmbH & Co. KG >>>>>>>>>>>>>>> Fixes: fdfd3a90b6ce ("ofw: Add a ofw_firmware d= river") >>>>>>>>>>>>>>> END QUOTE >>>>>>>>>>>>>> >>>>>>>>>>>>>> Just for completeness: that change fixes the bcm2835_cpufr= eq0/powerd >>>>>>>>>>>>>> problem and the gpioled0 problem, but not the clk_fixed2 p= roblem >>>>>>>>>>>>>> (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 preven= t 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 t= o ignore. >>>>>>>>>>>> >>>>>>>>>>>> Mike >>>>>>>>>>> >>>>>>>>>>> Detecting the disabled status makes no sense, a fixed clock c= annot be >>>>>>>>>>> disable, it's always present and running. >>>>>>>>>>> But I think that if we check that clock-frenquency isn't pres= ent in >>>>>>>>>>> the probe function, print a message and bail we will not atte= mpt 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 revers= al >>>>>>>>> and a malloc while holding a non-sleepable lock, associated wit= h >>>>>>>>> the gpio fix: >>>>>>>> >>>>>>>> Are you sure that they are new ? >>>>>>>> >>>>>>>>> gpioled0: on ofwbus0 >>>>>>>>> lock order reversal: (sleepable after non-sleepable) >>>>>>>>> 1st 0xffff000000d0f6f8 LED mtx (LED mtx, sleep mutex) @ /usr/sr= c/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-sleep= able locks held: >>>>>>>>> exclusive sleep mutex LED mtx (LED mtx) r =3D 0 (0xffff000000d0= f6f8) 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-sleep= able locks held: >>>>>>>>> exclusive sleep mutex LED mtx (LED mtx) r =3D 0 (0xffff000000d0= f6f8) 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-slee= pable locks held: >>>>>>>>> exclusive sleep mutex LED mtx (LED mtx) r =3D 0 (0xffff000000d0= f6f8) 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 >>>>>>>> >>>>> >>>>> >>>>> -- = >>>>> Emmanuel Vadot >>> >> >> >> -- = >> Emmanuel Vadot >>