Messages in this thread | | | From | Saravana Kannan <> | Date | Tue, 27 Apr 2021 09:28:16 -0700 | Subject | Re: [PATCH v1 3/3] Revert "Revert "driver core: Set fw_devlink=on by default"" |
| |
On Tue, Apr 27, 2021 at 9:25 AM Florian Fainelli <f.fainelli@gmail.com> wrote: > > > > On 4/27/2021 8:10 AM, Sudeep Holla wrote: > > On Tue, Apr 27, 2021 at 03:11:16PM +0100, Cristian Marussi wrote: > >> On Tue, Apr 27, 2021 at 09:33:31AM -0400, Jim Quinlan wrote: > > [...] > >>>> > >>> I believe that the brcmstb-mbox node is in our DT for backwards > >>> compatibility with our older Linux only. Note that we use the compatible > >>> string '"arm,scmi-smc", "arm,scmi"'; the former chooses SMC transport and > >>> ignores custom mailboxes such as brcmstb-mbox. > >>> > >> > >> Right..so it is even more wrong that it is waiting for the mailboxes...but > >> looking at the DT: > >> > >> brcm_scmi_mailbox@0 { > >> #mbox-cells = <0x01>; > >> compatible = "brcm,brcmstb-mbox"; > >> status = "disabled"; > >> linux,phandle = <0x04>; > >> phandle = <0x04>; > >> }; > >> > >> brcm_scmi@0 { > >> compatible = "arm,scmi-smc\0arm,scmi"; > >> mboxes = <0x04 0x00 0x04 0x01>; > >> mbox-names = "tx\0rx"; > >> shmem = <0x05>; > >> status = "disabled"; > >> arm,smc-id = <0x83000400>; > >> interrupt-names = "a2p"; > >> #address-cells = <0x01>; > >> #size-cells = <0x00>; > >> > >> it seems to me that even though you declare an SMC based transport (and in fact > >> you define the smc-id too) you also still define mboxes (as a fallback I suppose) > >> referring to the brcm_scmi_mailbox phandle, and while this is ignored by the SCMI > >> driver (because you have selected a compatible SMC transport) I imagine this dep > >> is picked up by fw_devlink which in fact says: > >> > >>> [ 0.300086] platform brcm_scmi@0: Linked as a consumer to brcm_scmi_mailbox@0 > >> > >> and stalls waiting for it. (but I'm not really familiar on how fw_devlink > >> internals works really...so I maybe off in these regards) > >> > > > > I was about to mention/ask the same when I saw Jim's reply. I see you have > > already asked that. Couple of my opinions based on my very limited knowledge > > on fw_devlink and how it works. > > > > 1. Since we have different compatible for SMC and mailbox, I am not sure > > if it correct to leave mailbox information in scmi node. Once we have > > proper yaml scheme, we must flag that error IMO. > > This is a self inflicted problem that we have in that the bootloader > provides a Device Tree to the kernel which is massaged in different ways > and intends to stay backwards compatible as much as possible. And indeed > after removing the 'mboxes' property gets us going with fw_devlink=on.
I'm sure you'll see my other email and reply to it, so I'll not respond to this part.
> > > > > 2. IIUC, the fw_devlink might use information from DT to establish the > > dependency and having mailbox information in this context may be > > considered wrong as there is no dependency if it is using SMC. > > Right, unfortunately, short of having some special casing for SCMI and > checking that if we have both an "arm,smc-id" and "mboxes" phandle we > should prefer the former, there is not probably much that can be done > here. Do we want to do that?
Definite no for special casing per device nodes in fw_devlink code :)
-Saravana
| |