[1/5] dt-bindings: clock: Add YAML schemas for LPASSCC and reset on SC8280XP

The LPASS(Low Power Audio Subsystem) clock provider provides reset
controller support when is driven by the Q6DSP.
This patch adds support for those resets and adds IDs for clients
to request the reset.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../bindings/clock/qcom,sc8280xp-lpasscc.yaml | 57 +++++++++++++++++++
 .../dt-bindings/clock/qcom,lpasscc-sc8280xp.h | 12 ++++
 2 files changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
 create mode 100644 include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h

On Thu, May 18, 2023 at 12:37:56PM +0100, Srinivas Kandagatla wrote:
> The LPASS(Low Power Audio Subsystem) clock provider provides reset

Missing space after LPASS acronym.

s/provider/controller/?

> controller support when is driven by the Q6DSP.

s/controller//?

"when is driven by": sounds like there are some words missing here.

> This patch adds support for those resets and adds IDs for clients

There is never any need to say "this patch" in a commit message. Just say

	Add support for...

> to request the reset.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  .../bindings/clock/qcom,sc8280xp-lpasscc.yaml | 57 +++++++++++++++++++
>  .../dt-bindings/clock/qcom,lpasscc-sc8280xp.h | 12 ++++
>  2 files changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
>  create mode 100644 include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
> new file mode 100644
> index 000000000000..7c30614a0af9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,sc8280xp-lpasscc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm LPASS Core & Audio Clock Controller on SC8280XP
> +
> +maintainers:
> +  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> +
> +description: |
> +  Qualcomm LPASS core and audio clock control module provides the clocks,
> +  reset and power domains on SC8280XP.

"power domains"? copy-paste error?

> +
> +  See also::
> +    include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
> +
> +properties:
> +  reg: true
> +
> +  compatible:
> +    enum:
> +      - qcom,sc8280xp-lpasscc
> +
> +  '#reset-cells':
> +    const: 1
> +
> +  '#clock-cells':
> +    const: 1

Move above #reset-cells for some sorting of related attributes. Same
below (in two places).

> +
> +  qcom,adsp-pil-mode:
> +    description:
> +      Indicates if the LPASS would be brought out of reset using
> +      peripheral loader.
> +    type: boolean

Move above the provider cells properties?

> +
> +required:
> +  - compatible
> +  - reg
> +  - qcom,adsp-pil-mode

If this boolean property is always needed, shouldn't that simply be
handled by the driver based on the compatible?

> +  - '#reset-cells'
> +  - '#clock-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
> +    lpasscc: clock-controller@3900000 {
> +      compatible = "qcom,sc8280xp-lpasscc";

binding examples use 4-space indentation.

> +      reg = <0x033e0000 0x12000>;

Does not match the node unit address.

> +      #reset-cells = <1>;
> +      #clock-cells = <1>;
> +      qcom,adsp-pil-mode;
> +    };
> +...
> diff --git a/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
> new file mode 100644
> index 000000000000..df800ea2741c
> --- /dev/null
> +++ b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) 2023, Linaro Ltd.
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
> +#define _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
> +
> +/* LPASS TCSR */
> +#define LPASS_AUDIO_SWR_TX_CGCR				0
> +
> +#endif

thanks Johan for review,

On 22/05/2023 09:21, Johan Hovold wrote:
> On Thu, May 18, 2023 at 12:37:56PM +0100, Srinivas Kandagatla wrote:
>> The LPASS(Low Power Audio Subsystem) clock provider provides reset
> 
> Missing space after LPASS acronym.
> 
> s/provider/controller/?
> 
>> controller support when is driven by the Q6DSP.
> 
> s/controller//?
> 
> "when is driven by": sounds like there are some words missing here.
> 
>> This patch adds support for those resets and adds IDs for clients
> 
> There is never any need to say "this patch" in a commit message. Just say
> 
> 	Add support for...
> 
>> to request the reset.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   .../bindings/clock/qcom,sc8280xp-lpasscc.yaml | 57 +++++++++++++++++++
>>   .../dt-bindings/clock/qcom,lpasscc-sc8280xp.h | 12 ++++
>>   2 files changed, 69 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
>>   create mode 100644 include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
>> new file mode 100644
>> index 000000000000..7c30614a0af9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
>> @@ -0,0 +1,57 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/qcom,sc8280xp-lpasscc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm LPASS Core & Audio Clock Controller on SC8280XP
>> +
>> +maintainers:
>> +  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> +
>> +description: |
>> +  Qualcomm LPASS core and audio clock control module provides the clocks,
>> +  reset and power domains on SC8280XP.
> 
> "power domains"? copy-paste error?
> 
>> +
>> +  See also::
>> +    include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
>> +
>> +properties:
>> +  reg: true
>> +
>> +  compatible:
>> +    enum:
>> +      - qcom,sc8280xp-lpasscc
>> +
>> +  '#reset-cells':
>> +    const: 1
>> +
>> +  '#clock-cells':
>> +    const: 1
> 
> Move above #reset-cells for some sorting of related attributes. Same
> below (in two places).
> 
>> +
>> +  qcom,adsp-pil-mode:
>> +    description:
>> +      Indicates if the LPASS would be brought out of reset using
>> +      peripheral loader.
>> +    type: boolean
> 
> Move above the provider cells properties?
> 
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - qcom,adsp-pil-mode
> 
> If this boolean property is always needed, shouldn't that simply be
> handled by the driver based on the compatible?

Traditionally in Qcom SoCs LPASS is under the control of ADSP, there 
have been some other variants specially chrome platforms that have moved 
this control to APPs processor.

Having this property at Device tree level provides more flexibility, 
given that both the cases use same compatible strings.


Am okay with reset of the comments, Will fix them in v2.
thanks,
Srini

> 
>> +  - '#reset-cells'
>> +  - '#clock-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
>> +    lpasscc: clock-controller@3900000 {
>> +      compatible = "qcom,sc8280xp-lpasscc";
> 
> binding examples use 4-space indentation.
> 
>> +      reg = <0x033e0000 0x12000>;
> 
> Does not match the node unit address.
> 
>> +      #reset-cells = <1>;
>> +      #clock-cells = <1>;
>> +      qcom,adsp-pil-mode;
>> +    };
>> +...
>> diff --git a/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
>> new file mode 100644
>> index 000000000000..df800ea2741c
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
>> @@ -0,0 +1,12 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>> +/*
>> + * Copyright (c) 2023, Linaro Ltd.
>> + */
>> +
>> +#ifndef _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
>> +#define _DT_BINDINGS_CLK_QCOM_LPASSCC_SC8280XP_H
>> +
>> +/* LPASS TCSR */
>> +#define LPASS_AUDIO_SWR_TX_CGCR				0
>> +
>> +#endif