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

[Krzysztof Kozlowski]

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, then probably
this could be implied by compatible. Otherwise, if this depends on
actual M4 firmware which can totally vary between each board of the same
type (I can run my own FW on M4, right?), then it is not suitable DT
property. How it would even look like? You add here 500 ms for all known
firmwares and then someone comes with FW requiring delay of 600 ms.

> 
> Sure, it is a defect of the NXP provided SDK firmware, but that may not 
> be fixable in all cases.

Best regards,
Krzysztof