Re: [PATCH V4 2/2] firmware: arm_scmi: add smc/hvc transport

[Florian Fainelli <f.fainelli@gmail.com>]

On 3/6/20 6:23 AM, Sudeep Holla wrote:
> On Fri, Mar 06, 2020 at 08:07:19AM +0000, Peng Fan wrote:
>>> Subject: Re: [PATCH V4 2/2] firmware: arm_scmi: add smc/hvc transport
>>>
>>> On 3/5/20 8:06 AM, Sudeep Holla wrote:
>>>> On Thu, Mar 05, 2020 at 11:25:35AM +0000, Peng Fan wrote:
>>>>
>>>> [...]
>>>>
>>>>>>
>>>>>> Yes, this may fix the issue. However I would like to know if we need
>>>>>> to support multiple channels/shared memory simultaneously. It is
>>>>>> fair requirement and may need some work which should be fine.
>>>>>
>>>>> Do you have any suggestions? Currently I have not worked out an good
>>>>> solution.
>>>>>
>>>>
>>>> TBH, I haven't given it a much thought. I would like to know if people
>>>> are happy with just one SMC channel for SCMI or do they need more ?
>>>> If they need it, we can try to solve it. Otherwise, what you have will
>>>> suffice IMO.
>>>
>>> On our platforms we have one channel/shared memory area/mailbox
>>> instance for all standard SCMI protocols, and we have a separate
>>> channel/shared memory area/mailbox driver instance for a proprietary one.
>>> They happen to have difference throughput requirements, hence the split.
>>>
> 
> OK, when you refer proprietary protocol, do you mean outside the scope of
> SCMI ? The reason I ask is SCMI allows vendor specific protocols and if
> you are using other channel for that, it still make sense to add
> multi-channel support here.

Sorry this was not clear, I meant a protocol ID which is in the 0x80 -
0xFF range. We are using one pair of channels (rx and tx) plus shared
memory area for the standard SCMI protocol numbers, and we have another
pair of rx/tx channels and shared memory area for this vendor specific
protocol.

Maybe providing the Device Tree entries would be clearer, so this is
what it looks like (this is the output from the bootloader generated
Device Tree):


/       brcm_scmi_mailbox@0 {
                #mbox-cells = <0x1>;
                compatible = "brcm,brcmstb-mbox";
                status = "disabled";
                linux,phandle = <0xe>;
                phandle = <0xe>;
        };

        brcm_scmi@0 {
                compatible = "arm,scmi";
                mboxes = <0xe 0x0 0xe 0x1>;
                mbox-names = "tx", "rx";
                shmem = <0xf>;
                status = "disabled";
                #address-cells = <0x1>;
                #size-cells = <0x0>;

                protocol@13 {
                        reg = <0x13>;
                };

                protocol@14 {
                        reg = <0x14>;
                        #clock-cells = <0x1>;
                        linux,phandle = <0x3>;
                        phandle = <0x3>;
                };

                protocol@15 {
                        reg = <0x15>;
                        #sensor-cells = <0x1>;
                        #thermal-sensor-cells = <0x1>;
                        linux,phandle = <0x12>;
                        phandle = <0x12>;
                };

                protocol@80 {
                        reg = <0x80>;
                };
        };

        brcm_scmi_mailbox@1 {
                #mbox-cells = <0x1>;
                compatible = "brcm,brcmstb-mbox";
                status = "disabled";
                linux,phandle = <0x10>;
                phandle = <0x10>;
        };

        brcm_scmi@1 {
                compatible = "arm,scmi";
                mboxes = <0x10 0x0 0x10 0x1>;
                mbox-names = "tx", "rx";
                shmem = <0x11>;
                status = "disabled";
                #address-cells = <0x1>;
                #size-cells = <0x0>;

                protocol@82 {
                        reg = <0x82>;
                };
        };


> 
>>> If I read Peng's submission correctly, it seems to me that the usage model
>>> described before is still fine.
>>
>> Thanks.
>>
>> Sudeep,
>>
>> Then should I repost with the global mutex added?
>>
> 
> Sure, you can send the updated. I will think about adding support for more
> than one channel and send a patch on top of it if I get around it.
> 
> Note that I sent PR for v5.7 last earlier this week, so this will be for v5.8
> 
> --
> Regards,
> Sudeep
> 


-- 
Florian