Re: [PATCH v5 1/8] dt-bindings: reset: Add AOSS reset bindings for SDM845 SoCs

From: Bjorn Andersson
Date: Fri Jun 22 2018 - 20:46:43 EST


On Mon 21 May 10:27 PDT 2018, Sibi Sankar wrote:

> Add SDM845 AOSS (always on subsystem) reset controller binding
>

I think it would be better if you made the binding represent the entire
clock controller, rather than only the reset-related portion of it.

As I can't find anything in the downstream kernel that references the
clock part of the hardware block I think the driver can be kept as is
(with updated compatible and adjust the offsets of the registers)


This makes the DT better represents the hardware and it makes it
possible to control the clocks in the future, without breaking backwards
compatibility with existing DTBs.

> Signed-off-by: Sibi Sankar <sibis@xxxxxxxxxxxxxx>
> ---
> .../bindings/reset/qcom,aoss-reset.txt | 52 +++++++++++++++++++
> include/dt-bindings/reset/qcom,sdm845-aoss.h | 17 ++++++
> 2 files changed, 69 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt
> create mode 100644 include/dt-bindings/reset/qcom,sdm845-aoss.h
>
> diff --git a/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt b/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt
> new file mode 100644
> index 000000000000..cd5dcafb4ed7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt
> @@ -0,0 +1,52 @@
> +Qualcomm AOSS Reset Controller
> +======================================
> +
> +This binding describes a reset-controller found on AOSS (always on subsystem)
> +for Qualcomm SDM845 SoCs.
> +
> +Required properties:
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: must be:
> + "qcom,sdm845-aoss-reset"

qcom,sdm845-aoss-cc

> +
> +- reg:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: must specify the base address and size of the register
> + space.
> +
> +- #reset-cells:
> + Usage: required
> + Value type: <uint>
> + Definition: must be 1; cell entry represents the reset index.
> +
> +Example:
> +
> +aoss_reset: reset-controller@c2b0000 {
> + compatible = "qcom,sdm845-aoss-reset";
> + reg = <0xc2b0000 0x21000>;

reg = <0xc2a0000 0x31000>;

> + #reset-cells = <1>;
> +};
> +

Apart from this the binding looks good!

Regards,
Bjorn