Re: [PATCH v6 02/13] dt-bindings: remoteproc: qcom: pas: Add QMP property

Sibi Sankar
2021-09-16T03:13:23Z

Hey Rob,

Thanks for taking time to review
the series.

On 2021-09-08 19:21, Rob Herring wrote:
> On Mon, Sep 06, 2021 at 01:53:46PM +0530, Sibi Sankar wrote:
>> The load state power-domain, used by the co-processors to notify the
>> Always on Subsystem (AOSS) that a particular co-processor is up/down,
>> suffers from the side-effect of changing states during suspend/resume.
>> However the co-processors enter low-power modes independent to that of
>> the application processor and their states are expected to remain
>> unaltered across system suspend/resume cycles. To achieve this
>> behavior
>> let's drop the load state power-domain and replace them with the qmp
>> property for all SoCs supporting low power mode signalling.
>>
>> Due to the current broken load state implementation, we can afford the
>> binding breakage that ensues and the remoteproc functionality will
>> remain
>> the same when using newer kernels with older dtbs.
>>
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
>> ---
>>
>> v6:
>> * Updated commit message to explain binding breakage. [Stephen]
>>
>> .../devicetree/bindings/remoteproc/qcom,adsp.yaml | 61
>> +++++++++++-----------
>> 1 file changed, 31 insertions(+), 30 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>> b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>> index 0c112f3264a9..0d2b5bd4907a 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>> @@ -93,6 +93,10 @@ properties:
>> maxItems: 1
>> description: Reference to the reserved-memory for the Hexagon
>> core
>>
>> + qcom,qmp:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description: Reference to the AOSS side-channel message RAM.
>> +
>
> [...]
>
>> @@ -511,6 +486,32 @@ allOf:
>> - const: mss_restart
>> - const: pdc_reset
>>
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - qcom,sc7180-mpss-pas
>> + - qcom,sc8180x-adsp-pas
>> + - qcom,sc8180x-cdsp-pas
>> + - qcom,sc8180x-mpss-pas
>> + - qcom,sm8150-adsp-pas
>> + - qcom,sm8150-cdsp-pas
>> + - qcom,sm8150-mpss-pas
>> + - qcom,sm8150-slpi-pas
>> + - qcom,sm8250-adsp-pas
>> + - qcom,sm8250-cdsp-pas
>> + - qcom,sm8250-slpi-pas
>> + - qcom,sm8350-adsp-pas
>> + - qcom,sm8350-cdsp-pas
>> + - qcom,sm8350-mpss-pas
>> + - qcom,sm8350-slpi-pas
>> + then:
>> + properties:
>> + qcom,qmp:
>> + items:
>> + - description: Reference to the AOSS side-channel message
>> RAM.
>
> This doesn't do anything. The property is already allowed for all
> compatibles. Perhaps you want to negate the if and put 'qcom,qmp:
> false'
> here.

sure will fix it in the next re-spin.

>
> Rob

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.