On Tue, 3 Jun 2025, Ilpo Järvinen wrote: > On Tue, 3 Jun 2025, Tudor Ambarus wrote: > > On 6/3/25 3:13 PM, Ilpo Järvinen wrote: > > > On Tue, 3 Jun 2025, Tudor Ambarus wrote: > > >> On 6/3/25 9:13 AM, Ilpo Järvinen wrote: > > >>> So please test if this patch solves your problem: > > >> > > >> It fails in a different way, the bridge window resource never gets > > >> assigned with the proposed patch. > > > > > > Is that a failure? I was expecting that to occur. It didn't assign > > > any resources into that bridge window. > > > > It leads to a watchdog interrupt on my pixel6. Last print I see on my > > console is related to the modem booting status. My wild guess is that > > that modem accesses something from the unassigned bridge window. > > The bridge window is not for the bridge device itself. It's as the name > says, a window into where subordinate busses can assign their resources. > The bridge knows it must forward that window address range to the > subordinate bus. > > > In the working case I see the bridge window printed: > > [ 15.457310][ T1083] pcieport 0000:00:00.0: [s51xx_pcie_probe] BAR 14: > > tmp rsc : [mem 0x40000000-0x401fffff] > > > > [ 15.457683][ T1083] cpif: s51xx_pcie_probe: Set Doorbell register > > address. > > > > In the failing case I see: > > [ 15.623270][ T1113] pcieport 0000:00:00.0: [s51xx_pcie_probe] BAR 14: > > tmp rsc : [??? 0x00000000 flags 0x0] > > > > [ 15.623638][ T1113] cpif: s51xx_pcie_probe: Set Doorbell register > > address. > > Oh, is it this one? > > https://github.com/oberdfr/google-modules_radio_samsung_s5300/blob/11a10f955a267a45a1997f65671d7054adf1a33a/s51xx_pcie.c#L366 > > There are number of crazy things going on there... Probe shouldn't be > messing resources like that. If it wants to change resources, a quirk > would be more appropriate place I guess but I'm very unsure what that > even tries to achieve with all that craziness ("Disable BAR resources" by > assigning them :-/). Or maybe DT, I'm not very familiar with DT things. -- i. > But yes, it seems to take the bridge window's address and assumes > something is there (which isn't there as we know). So this driver code is > plain wrong. > > Perhaps it would want to use the address of some endpoint device resource > instead of the bridge window address (e.g., that device with class 0?). > > > > If there's nothing to be assigned into the bridge window, the bridge > > > window itself is not created, that is the expected behavior (working as > > > designed). So you're comparing to the bridge window that was made too > > > large due to the disparity (and left unused, AFAICT). > > > > > > It would be possible to put the condition inside the block which adds > > > the resource to the realloc_head, I initially put it there but then > > > decided to remove the disparity completely because why keep it if no > > > resource is going to be placed into the bridge window. > > > > > Thanks for the educative answers. > > > > > What's that class 0 device anyway? Why it has class 0? > > > > > I don't know yet, it's the first time I'm dealing with a PCI driver. Any > > idea where is the class typically assigned? > > https://pcisig.com/sites/default/files/files/PCI_Code-ID_r_1_12__v9_Jan_2020.pdf > > Perhaps try a quirk which changes the class of the device underneath the > bridge to something else than 0, it should make the resource fitting and > allocation to assign its resources. > > But honestly, that s51xx_pcie_probe() has more than one thing wrong. > > > >> With the patch applied: https://termbin.com/h3w0 > > >> With the blamed commit reverted: https://termbin.com/3rh6 > > > > > > >
