Enable L1SS handling on RPI4 pcib

[hpvb]

Thanks to @kevans91 for pointing me in the right direction. FreeBSD had the same bug as Linux (see
https://bugzilla.kernel.org/show_bug.cgi?id=217276) where the ultimate solution was to honor the brcm,enable-l1ss FDT property.

In current versions of the dtb files this property has been added by default.

Without this on many, many pcie addin cards the pcib will Serror when trying to assert the clreq# pin on the pcie bus. Many cards do not have these hooked up.

PR: 260131, 277638, 277605

[kevans91]

This should close bugzilla#260131 bugzilla#277638 bugzilla#277605.

This line in the commit message should be:

PR:<tab><tab>260131, 277638, 277605

[hpvb]

This should close bugzilla#260131 bugzilla#277638 bugzilla#277605.

This line in the commit message should be:

PR:<tab><tab>260131, 277638, 277605

Fixed! thanks

[klaus4]

I will need „a moment“ to set up the compile/test environment for nvme…If you want to post bootlogs in advance: e.g. https://dmesgd.nycbug.org/ is a good place for that.
boot -v gives a bit more and uart_2ndstage=1
in config.txt for eeprom stuff…
side note:if you`re interested in man pages/docs stuff:
not wrong to prepare something similar like this …

[hpvb]

I made this: https://reviews.freebsd.org/D44823 as requested. Should I close this PR?

[emaste]

Should I close this PR?

I think it's reasonable to keep this open (and updated). IMO pull requests are more convenient for actually landing the patch -- we can cherry-pick the commit and all of the metadata will be preserved appropriately.

In addition we could see if we can get Jim Quinlan (Broadcom engineer on the kernel.org bugzilla entry) to review/comment, and it is possible they'll have a GitHub account already (and are unlikely to have, or create, a FreeBSD Phabricator account).

[bsdimp]

I made this: https://reviews.freebsd.org/D44823 as requested. Should I close this PR?

Who asked you to do that? It's usuall a pull request or a phab. Not both. Leave the pr open.

[hpvb]

@bsdimp sorry, I must've misunderstood. I'll just use phab from now on, but leave this one here then.

[klaus4]

I'll just use phab from now on…

much confusion 😁.. better stay here than phab, as @emaste stated…
my phab link above was just a referring example to add something to the docs perhaps later…

[bsdimp]

@bsdimp sorry, I must've misunderstood. I'll just use phab from now on, but leave this one here then.

For small things, at low volumes, we generally land pull requests faster than phab, where things often get lost...

[hpvb]

I applied several suggestions from the phab revision here. Again my apologies for the duplicating the revision.

[klaus4]

👍
CM4108032 pxe-boot :
https://dmesgd.nycbug.org/index.cgi?do=view&id=7689

[emaste]

Thank you for this work! Can you please add a Signed-off-by: trailer in the commit, certifying agreement with https://developercertificate.org/.

[hpvb]

@emaste done! should I add these to each PR I make?

[bsdimp]

@emaste done! should I add these to each PR I make?

Ideally, yes. But having you name right in the commit is more important.

[klaus4]

just to defend this @hpvb -patch , since the dtb-patch is merged ..
(after I read all the steps the Broadcom engineer took in the linux driver until his current V5 of patch series)... :

the only caveat with this patch I could have imagined
was that it could have any side effect to other RPI-models like the Pi4b by changing to l1ss(what would not be desired) . I have tested that extensively by doing the exact same pxe boot on the Pi4b (into the same file system), inserting the same nvme via USB(which corresponds to this exact same pcie-driver via xhci).
Fortunately this patch #1179 only touches the CM4 as expected&desired, so this is really save :
if (ofw_bus_has_prop(dev, "brcm,enable-l1ss"))
(while I not yet decompiled the 4b-dtb: L1ss_ seems fortunately not to exist in it)
As seen on the nycbug-dmesg above , my CM4-eeprom was older than the patch series from Broadcom.
Reading all the stuff on Linux-discussions it turned out that it has nothing to do with the eeprom on the CM4 also Mr. Quinlan wouldnt have done the patch series if that were the case. And he did the exact same like @hpvb (while a bit more extended), derived from the Raspbian approach(while the issue is not OS-specific). He also stated in a discussions that it's not easy(or not possible) to scan the device while probing on boot , if it has l1ss or not. So theres no alternative to handle the default-panic (with or without connected pcie-device) on the CM4-models than this patch.
while devmatch_enable=NO was a workaround to boot the device,
it paniced immediately after inserting pcie-HW or just typing : devmatch in terminal.

sorry for this much text(but I think this patch is ready to be merged;-)

[klaus4]

@hpvb , this seems to be the current version :
https://lore.kernel.org/lkml/20240403213902.26391-5-james.quinlan@broadcom.com/
(.. the mode is specified by the DT property "brcm,clkreq-mode"...)
far away from being an pcie-expert I must ask :
would you see sense in changing the driver to "brcm,clkreq-mode" in place of DT property "brcm,enable-l1ss" for the CM4? afaik you stated that all other modes except l1ss failed.
Sorry in advance if I`m missing something.
Regards

[hpvb]

@hpvb , this seems to be the current version : https://lore.kernel.org/lkml/20240403213902.26391-5-james.quinlan@broadcom.com/ (.. the mode is specified by the DT property "brcm,clkreq-mode"...) far away from being an pcie-expert I must ask : would you see sense in changing the driver to "brcm,clkreq-mode" in place of DT property "brcm,enable-l1ss" for the CM4? afaik you stated that all other modes except l1ss failed. Sorry in advance if I`m missing something. Regards

I just went with what raspbian was doing today, mostly so that people's devicetree files will have predictable results across Linux and FreeBSD.

It would maybe make sense to support both the unofficial raspbian as well as the upstream linux directives?

[klaus4]

It would maybe make sense to support both the unofficial raspbian as well as the upstream linux directives?

If I look at the CC list in the link, pretty much everyone is there ( except FreeBSD ;-)
I suspect that Ed would like to take over J. Quinlan's Broadcom directive.Just a suspicion ;-)
SInce we have the current broadcom sources in front of us, should @hpvb or we try that?

[hpvb]

It would maybe make sense to support both the unofficial raspbian as well as the upstream linux directives?

If I look at the CC list in the link, pretty much everyone is there ( except FreeBSD ;-) I suspect that Ed would like to take over J. Quinlan's Broadcom directive.Just a suspicion ;-) SInce we have the current broadcom sources in front of us, should @hpvb or we try that?

We could do that, but then we also have to update our dtb files, as right now they include brcm,enable-l1ss but not the new one. But basically what were doing now is with brcm,enable-l1ss we are basically running what the new one calls "default", without it we run in "no-l1ss" mode, and there's no way to run in "safe" mode.

[emaste]

I merged this in 10e0c34, as #1182 went in already in 1bd4f76; if any adjustments are necessary we can follow up with another pull request.

[bsdimp]

Merged so closing. New pr if needed for followup. Thanks