[PATCH 1/2] dt-bindings: remoteproc: imx_rproc: Document fsl,startup-delay-ms

[Marek Vasut]

On 7/11/23 00:01, Mathieu Poirier wrote:
> On Mon, 10 Jul 2023 at 15:53, Marek Vasut <marex at denx.de> wrote:
>>
>> On 7/10/23 22:00, Krzysztof Kozlowski wrote:
>>> On 10/07/2023 15:46, Marek Vasut wrote:
>>>> On 7/10/23 14:52, Krzysztof Kozlowski wrote:
>>>>> On 10/07/2023 11:18, Marek Vasut wrote:
>>>>>> On 7/10/23 10:12, Krzysztof Kozlowski wrote:
>>>>>>> On 08/07/2023 01:24, Marek Vasut wrote:
>>>>>>>> Document fsl,startup-delay-ms property which indicates how long
>>>>>>>> the system software should wait until attempting to communicate
>>>>>>>> with the CM firmware. This gives the CM firmware a bit of time
>>>>>>>> to boot and get ready for communication.
>>>>>>>>
>>>>>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>>>>>> ---
>>>>>>>> Cc: Bjorn Andersson <andersson at kernel.org>
>>>>>>>> Cc: Conor Dooley <conor+dt at kernel.org>
>>>>>>>> Cc: Fabio Estevam <festevam at gmail.com>
>>>>>>>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt at linaro.org>
>>>>>>>> Cc: Mathieu Poirier <mathieu.poirier at linaro.org>
>>>>>>>> Cc: NXP Linux Team <linux-imx at nxp.com>
>>>>>>>> Cc: Peng Fan <peng.fan at nxp.com>
>>>>>>>> Cc: Pengutronix Kernel Team <kernel at pengutronix.de>
>>>>>>>> Cc: Rob Herring <robh+dt at kernel.org>
>>>>>>>> Cc: Sascha Hauer <s.hauer at pengutronix.de>
>>>>>>>> Cc: Shawn Guo <shawnguo at kernel.org>
>>>>>>>> Cc: devicetree at vger.kernel.org
>>>>>>>> Cc: linux-arm-kernel at lists.infradead.org
>>>>>>>> Cc: linux-remoteproc at vger.kernel.org
>>>>>>>> ---
>>>>>>>>      .../devicetree/bindings/remoteproc/fsl,imx-rproc.yaml        | 5 +++++
>>>>>>>>      1 file changed, 5 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
>>>>>>>> index 0c3910f152d1d..c940199ce89df 100644
>>>>>>>> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
>>>>>>>> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
>>>>>>>> @@ -76,6 +76,11 @@ properties:
>>>>>>>>            This property is to specify the resource id of the remote processor in SoC
>>>>>>>>            which supports SCFW
>>>>>>>>
>>>>>>>> +  fsl,startup-delay-ms:
>>>>>>>> +    default: 0
>>>>>>>> +    description:
>>>>>>>> +      CM firmware start up delay.
>>>>>>>
>>>>>>> I don't see particular improvements from v2 and no responses addressing
>>>>>>> my comment:
>>>>>>> https://lore.kernel.org/all/20221102112451.128110-2-peng.fan@oss.nxp.com/
>>>>>>
>>>>>> I wasn't aware of this being submitted before, esp. since I wrote the
>>>>>> binding document from scratch. Which comment is not addressed, the type
>>>>>> ref is not present and the sentence starts with caps, so what is missing ?
>>>>>
>>>>>
>>>>> That the property looks like a hacky solution to some SW problem. Why
>>>>> this delay should be different on different boards?
>>>>
>>>> It probably depends more on the CM4 firmware that is being launched. The
>>>> ones I tested were fine with 50..500ms delay, but the delay was always
>>>> needed.
>>>
>>> If this is for some official remoteproc FW running on M4
>>
>> It is not, it is some SDK which can be downloaded from NXP website,
>> which can then be used to compile the firmware blob. The license is
>> BSD-3 however, so it is conductive to producing binaries without
>> matching sources ...
>>
> 
> Why can't the SDK be upgraded to provide some kind of hand-shake
> mechanism, as suggested when I first reviewed this patchset?

I'd argue because of legacy firmware that is already deployed.
New firmware builds can, old ones probably cannot be fixed.

Do you have a suggestion how such a mechanism should look like?
As far as I can tell, the MX8M SDK stuff looks very similar to the STM32 
Cube stuff, so maybe the mechanism is already there ?