From nobody Sat Aug 19 09:21:24 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 4RSYCR0R10z4qT1X for ; Sat, 19 Aug 2023 09:21:39 +0000 (UTC) (envelope-from manu@bidouilliste.com) Received: from mx.blih.net (mx.blih.net [212.83.155.74]) (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 "mx.blih.net", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4RSYCP54gRz3DRR for ; Sat, 19 Aug 2023 09:21:37 +0000 (UTC) (envelope-from manu@bidouilliste.com) Authentication-Results: mx1.freebsd.org; dkim=pass header.d=bidouilliste.com header.s=mx header.b=roRFkc2Y; spf=pass (mx1.freebsd.org: domain of manu@bidouilliste.com designates 212.83.155.74 as permitted sender) smtp.mailfrom=manu@bidouilliste.com; dmarc=pass (policy=none) header.from=bidouilliste.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bidouilliste.com; s=mx; t=1692436888; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=g38BYGZYkH1DT0HC95kl3yLOs8FM5GgA1zSysqFalEo=; b=roRFkc2YUvg88mwzLCcFx9OUnW+STLlaBb2Jo0lyiZE723g7+l7BZ4yodJ1RySqKde1icw MIgSmcrzr819gzY7XQ6l2c2QhYlt8hKEMN4zbwLJpzwPIvmdShJLV82AWdwXXiG81ggHW8 lv2vNjNWnBrWxnRfXP0e0jra1Q47C+0= Received: from skull.home.blih.net (lfbn-lyo-1-2174-135.w90-66.abo.wanadoo.fr [90.66.97.135]) by mx.blih.net (OpenSMTPD) with ESMTPSA id a4ab7d69 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Sat, 19 Aug 2023 09:21:28 +0000 (UTC) Date: Sat, 19 Aug 2023 11:21:24 +0200 From: Emmanuel Vadot To: titus Cc: freebsd-arm Subject: Re: ALPHA1 on Raspberry Pi 3B+ [added: and RPi4B] Message-Id: <20230819112124.46e66629ca18071f11d9d361@bidouilliste.com> In-Reply-To: <94A67CE4-8530-4080-AC54-DFBB6B8613EF@edc.ro> References: <1C94FEAF-C616-498F-8562-2E99CF12417D@edc.ro> <4F7960AE-F607-4FEF-8A02-2013862A37E3@yahoo.com> <20230813191924.1b9b7927f0d98ce9937a571f@bidouilliste.com> <20230814070319.1d04005ae92b8c0bc0ccf025@bidouilliste.com> <20230814073025.c06a3d2fe2c766b179ab6d0c@bidouilliste.com> <6877B734-A9F2-40FF-8176-6A0E5DC2BD2E@karels.net> <20230814170741.dab652fc3e9475620eae29ea@bidouilliste.com> <2DF7EF89-DC3F-44D4-8708-51CB4B6C8EC4@edc.ro> <0C830898-AAAE-48BC-9844-1B9D426C0767@karels.net> <20230814181921.70596c64343261a24351837a@bidouilliste.com> <94A67CE4-8530-4080-AC54-DFBB6B8613EF@edc.ro> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.33; amd64-portbld-freebsd14.0) 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=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-Spamd-Result: default: False [-3.40 / 15.00]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_SHORT(-1.00)[-0.999]; MV_CASE(0.50)[]; DMARC_POLICY_ALLOW(-0.50)[bidouilliste.com,none]; R_DKIM_ALLOW(-0.20)[bidouilliste.com:s=mx]; R_SPF_ALLOW(-0.20)[+ip4:212.83.155.74/32]; MIME_GOOD(-0.10)[text/plain]; ONCE_RECEIVED(0.10)[]; RCVD_TLS_ALL(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_ONE(0.00)[1]; FROM_EQ_ENVFROM(0.00)[]; MLMMJ_DEST(0.00)[freebsd-arm@freebsd.org]; ASN(0.00)[asn:12876, ipnet:212.83.128.0/19, country:FR]; DKIM_TRACE(0.00)[bidouilliste.com:+]; MID_RHS_MATCH_FROM(0.00)[]; FREEFALL_USER(0.00)[manu]; ARC_NA(0.00)[]; RCPT_COUNT_TWO(0.00)[2]; FROM_HAS_DN(0.00)[]; TO_DN_ALL(0.00)[]; TO_MATCH_ENVRCPT_SOME(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[] X-Spamd-Bar: --- X-Rspamd-Queue-Id: 4RSYCP54gRz3DRR On Mon, 14 Aug 2023 21:44:13 +0300 titus wrote: > 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 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. Cheers, > > 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 dev= ice 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 = wrote: > >>>>>=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 wrot= e: > >>>>>>>>>>>>=20 > >>>>>>>>>>>>> Manu just updated Linux DTS in the tree. Maybe see if you r= evert that if the problem persists. > >>>>>>>>>>>>=20 > >>>>>>>>>>>> git: 69f8cc60aa1e - main - ofw_firmware: Only match if there= is 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 f= irmware 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 dri= ver") > >>>>>>>>>>>> END QUOTE > >>>>>>>>>>>=20 > >>>>>>>>>>> Just for completeness: that change fixes the bcm2835_cpufreq0= /powerd > >>>>>>>>>>> problem and the gpioled0 problem, but not the clk_fixed2 prob= lem > >>>>>>>>>>> (clk_fixed4 on rpi4). Installing an msdos boot partition fro= m the > >>>>>>>>>>> 3 Aug image makes that problem disappear. > >>>>>>>>>>>=20 > >>>>>>>>>>> Mike > >>>>>>>>>>=20 > >>>>>>>>>> There is two fixed-clock in the DTB without clock-frequency pr= operty > >>>>>>>>>> and with a status set to "disabled", this isn't conforming to = the > >>>>>>>>>> bindings > >>>>>>>>>> (https://cgit.freebsd.org/src/tree/sys/contrib/device-tree/Bin= dings/clock/fixed-clock.yaml) > >>>>>>>>>> so we complain on this, this is normal. > >>>>>>>>>=20 > >>>>>>>>> Would it be possible to detect the disabled status to prevent t= he errors > >>>>>>>>> (I'm guessing not) or to suppress the repeats? 150 lines of er= rors seems > >>>>>>>>> like a lot for an out-of-spec DTB entry, and makes it hard to i= gnore. > >>>>>>>>>=20 > >>>>>>>>> Mike > >>>>>>>>=20 > >>>>>>>> Detecting the disabled status makes no sense, a fixed clock cann= ot 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 dir= ty > >>>>>>>> hacks for some non-conforming DTB. > >>>>>>>=20 > >>>>>>> Something like this : > >>>>>>> https://people.freebsd.org/~manu/0001-clk-fixed-Bail-early-if-the= re-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/s= ys/dev/led/led.c:298 > >>>>>> 2nd 0xffffa00001857c10 Raspberry Pi firmware gpio (Raspberry Pi fi= rmware 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-sleepabl= e locks held: > >>>>>> exclusive sleep mutex LED mtx (LED mtx) r =3D 0 (0xffff000000d0f6f= 8) 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-sleepabl= e locks held: > >>>>>> exclusive sleep mutex LED mtx (LED mtx) r =3D 0 (0xffff000000d0f6f= 8) 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-sleepab= le locks held: > >>>>>> exclusive sleep mutex LED mtx (LED mtx) r =3D 0 (0xffff000000d0f6f= 8) 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 Emmanuel Vadot