[PATCH] dt-bindings: nvmem: add binding for QTI SPMI SDAM

[PATCH] dt-bindings: nvmem: add binding for QTI SPMI SDAM

[Shyam Kumar Thella]

QTI SDAM allows PMIC peripherals to access the shared memory that is
available on QTI PMICs. Add documentation for it.

Signed-off-by: Shyam Kumar Thella <sthella@codeaurora.org>
---
 .../devicetree/bindings/nvmem/qcom,spmi-sdam.yaml  | 79 ++++++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml

diff --git a/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
new file mode 100644
index 0000000..8961a99
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/qcom,spmi-sdam.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. SPMI SDAM DT bindings
+
+maintainers:
+  - Shyam Kumar Thella <sthella@codeaurora.org>
+
+description: |
+  The SDAM provides scratch register space for the PMIC clients. This
+  memory can be used by software to store information or communicate
+  to/from the PBUS.
+
+allOf:
+  - $ref: "nvmem.yaml#"
+
+properties:
+  compatible:
+    enum:
+      - qcom,spmi-sdam
+
+  reg:
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+
+patternProperties:
+  "^.*@[0-9a-f]+$":
+    type: object
+
+    properties:
+      reg:
+        maxItems: 1
+        description:
+          Offset and size in bytes within the storage device.
+
+      bits:
+        maxItems: 1
+        items:
+          items:
+            - minimum: 0
+              maximum: 7
+              description:
+                Offset in bit within the address range specified by reg.
+            - minimum: 1
+              description:
+                Size in bit within the address range specified by reg.
+
+    required:
+      - reg
+
+    additionalProperties: false
+
+examples:
+  - |
+      sdam_1: nvram@b000 {
+         #address-cells = <1>;
+         #size-cells = <1>;
+         compatible = "qcom,spmi-sdam";
+          reg = <0xb000 0x100>;
+
+          /* Data cells */
+          restart_reason: restart@50 {
+              reg = <0x50 0x1>;
+              bits = <7 2>;
+          };
+      };
+...
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 a Linux Foundation Collaborative Project

Re: [PATCH] dt-bindings: nvmem: add binding for QTI SPMI SDAM

[Bjorn Andersson]

On Mon 23 Dec 21:32 PST 2019, Shyam Kumar Thella wrote:

> QTI SDAM allows PMIC peripherals to access the shared memory that is
> available on QTI PMICs. Add documentation for it.
> 
> Signed-off-by: Shyam Kumar Thella <sthella@codeaurora.org>
> ---
>  .../devicetree/bindings/nvmem/qcom,spmi-sdam.yaml  | 79 ++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> new file mode 100644
> index 0000000..8961a99
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/qcom,spmi-sdam.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies, Inc. SPMI SDAM DT bindings
> +
> +maintainers:
> +  - Shyam Kumar Thella <sthella@codeaurora.org>
> +
> +description: |
> +  The SDAM provides scratch register space for the PMIC clients. This
> +  memory can be used by software to store information or communicate
> +  to/from the PBUS.
> +
> +allOf:
> +  - $ref: "nvmem.yaml#"
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,spmi-sdam
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +patternProperties:
> +  "^.*@[0-9a-f]+$":
> +    type: object
> +
> +    properties:
> +      reg:
> +        maxItems: 1
> +        description:
> +          Offset and size in bytes within the storage device.
> +
> +      bits:
> +        maxItems: 1
> +        items:
> +          items:
> +            - minimum: 0
> +              maximum: 7
> +              description:
> +                Offset in bit within the address range specified by reg.
> +            - minimum: 1
> +              description:
> +                Size in bit within the address range specified by reg.
> +
> +    required:
> +      - reg
> +
> +    additionalProperties: false
> +
> +examples:
> +  - |
> +      sdam_1: nvram@b000 {
> +         #address-cells = <1>;
> +         #size-cells = <1>;
> +         compatible = "qcom,spmi-sdam";
> +          reg = <0xb000 0x100>;
> +
> +          /* Data cells */
> +          restart_reason: restart@50 {

So this register has moved out of the PON register set? What component
in the system is going to reference this? Should it have a compatible,
in the same way as "syscon-reboot-mode" does?

Regards,
Bjorn

> +              reg = <0x50 0x1>;
> +              bits = <7 2>;
> +          };
> +      };
> +...
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>  a Linux Foundation Collaborative Project

Re: [PATCH] dt-bindings: nvmem: add binding for QTI SPMI SDAM

[sthella]

On 2019-12-29 08:31, Bjorn Andersson wrote:
> On Mon 23 Dec 21:32 PST 2019, Shyam Kumar Thella wrote:
> 
>> QTI SDAM allows PMIC peripherals to access the shared memory that is
>> available on QTI PMICs. Add documentation for it.
>> 
>> Signed-off-by: Shyam Kumar Thella <sthella@codeaurora.org>
>> ---
>>  .../devicetree/bindings/nvmem/qcom,spmi-sdam.yaml  | 79 
>> ++++++++++++++++++++++
>>  1 file changed, 79 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml 
>> b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> new file mode 100644
>> index 0000000..8961a99
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> @@ -0,0 +1,79 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/nvmem/qcom,spmi-sdam.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Technologies, Inc. SPMI SDAM DT bindings
>> +
>> +maintainers:
>> +  - Shyam Kumar Thella <sthella@codeaurora.org>
>> +
>> +description: |
>> +  The SDAM provides scratch register space for the PMIC clients. This
>> +  memory can be used by software to store information or communicate
>> +  to/from the PBUS.
>> +
>> +allOf:
>> +  - $ref: "nvmem.yaml#"
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,spmi-sdam
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +patternProperties:
>> +  "^.*@[0-9a-f]+$":
>> +    type: object
>> +
>> +    properties:
>> +      reg:
>> +        maxItems: 1
>> +        description:
>> +          Offset and size in bytes within the storage device.
>> +
>> +      bits:
>> +        maxItems: 1
>> +        items:
>> +          items:
>> +            - minimum: 0
>> +              maximum: 7
>> +              description:
>> +                Offset in bit within the address range specified by 
>> reg.
>> +            - minimum: 1
>> +              description:
>> +                Size in bit within the address range specified by 
>> reg.
>> +
>> +    required:
>> +      - reg
>> +
>> +    additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +      sdam_1: nvram@b000 {
>> +         #address-cells = <1>;
>> +         #size-cells = <1>;
>> +         compatible = "qcom,spmi-sdam";
>> +          reg = <0xb000 0x100>;
>> +
>> +          /* Data cells */
>> +          restart_reason: restart@50 {
> 
> So this register has moved out of the PON register set? What component
> in the system is going to reference this? Should it have a compatible,
> in the same way as "syscon-reboot-mode" does?
This is just an example for using data cells. It is not used in the 
system.
> 
> Regards,
> Bjorn
> 
>> +              reg = <0x50 0x1>;
>> +              bits = <7 2>;
>> +          };
>> +      };
>> +...
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>>  a Linux Foundation Collaborative Project

Re: [PATCH] dt-bindings: nvmem: add binding for QTI SPMI SDAM

[Rob Herring]

On Tue, Dec 24, 2019 at 11:02:12AM +0530, Shyam Kumar Thella wrote:
> QTI SDAM allows PMIC peripherals to access the shared memory that is
> available on QTI PMICs. Add documentation for it.
> 
> Signed-off-by: Shyam Kumar Thella <sthella@codeaurora.org>
> ---
>  .../devicetree/bindings/nvmem/qcom,spmi-sdam.yaml  | 79 ++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> new file mode 100644
> index 0000000..8961a99
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings:

(GPL-2.0-only OR BSD-2-Clause)

Please spread the word in QCom. 

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/qcom,spmi-sdam.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies, Inc. SPMI SDAM DT bindings
> +
> +maintainers:
> +  - Shyam Kumar Thella <sthella@codeaurora.org>
> +
> +description: |
> +  The SDAM provides scratch register space for the PMIC clients. This
> +  memory can be used by software to store information or communicate
> +  to/from the PBUS.
> +
> +allOf:
> +  - $ref: "nvmem.yaml#"
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,spmi-sdam
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1

ranges? The child addresses should be translateable I assume.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +patternProperties:
> +  "^.*@[0-9a-f]+$":
> +    type: object
> +
> +    properties:
> +      reg:
> +        maxItems: 1
> +        description:
> +          Offset and size in bytes within the storage device.
> +
> +      bits:

Needs a type reference. 

> +        maxItems: 1
> +        items:
> +          items:
> +            - minimum: 0
> +              maximum: 7
> +              description:
> +                Offset in bit within the address range specified by reg.
> +            - minimum: 1

max is 7?

> +              description:
> +                Size in bit within the address range specified by reg.
> +
> +    required:
> +      - reg
> +
> +    additionalProperties: false
> +
> +examples:
> +  - |
> +      sdam_1: nvram@b000 {
> +         #address-cells = <1>;
> +         #size-cells = <1>;
> +         compatible = "qcom,spmi-sdam";
> +          reg = <0xb000 0x100>;
> +
> +          /* Data cells */
> +          restart_reason: restart@50 {
> +              reg = <0x50 0x1>;
> +              bits = <7 2>;

How do you have bit 8 in a 1 byte register?

> +          };
> +      };
> +...
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>  a Linux Foundation Collaborative Project
> 

Re: [PATCH] dt-bindings: nvmem: add binding for QTI SPMI SDAM

[sthella]

On 2020-01-08 22:09, Rob Herring wrote:
> On Tue, Dec 24, 2019 at 11:02:12AM +0530, Shyam Kumar Thella wrote:
>> QTI SDAM allows PMIC peripherals to access the shared memory that is
>> available on QTI PMICs. Add documentation for it.
>> 
>> Signed-off-by: Shyam Kumar Thella <sthella@codeaurora.org>
>> ---
>>  .../devicetree/bindings/nvmem/qcom,spmi-sdam.yaml  | 79 
>> ++++++++++++++++++++++
>>  1 file changed, 79 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml 
>> b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> new file mode 100644
>> index 0000000..8961a99
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> @@ -0,0 +1,79 @@
>> +# SPDX-License-Identifier: GPL-2.0
> 
> Dual license new bindings:
> 
> (GPL-2.0-only OR BSD-2-Clause)
> 
> Please spread the word in QCom.
Sure. I will add Dual license in next patchset.
> 
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/nvmem/qcom,spmi-sdam.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Technologies, Inc. SPMI SDAM DT bindings
>> +
>> +maintainers:
>> +  - Shyam Kumar Thella <sthella@codeaurora.org>
>> +
>> +description: |
>> +  The SDAM provides scratch register space for the PMIC clients. This
>> +  memory can be used by software to store information or communicate
>> +  to/from the PBUS.
>> +
>> +allOf:
>> +  - $ref: "nvmem.yaml#"
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,spmi-sdam
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 1
> 
> ranges? The child addresses should be translateable I assume.
The addresses are not memory mapped on the CPU's address domain. They 
are the SPMI addresses which can be accessed over SPMI controller.
> 
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +patternProperties:
>> +  "^.*@[0-9a-f]+$":
>> +    type: object
>> +
>> +    properties:
>> +      reg:
>> +        maxItems: 1
>> +        description:
>> +          Offset and size in bytes within the storage device.
>> +
>> +      bits:
> 
> Needs a type reference.
Yes. I will add a reference in the next patch set.
> 
>> +        maxItems: 1
>> +        items:
>> +          items:
>> +            - minimum: 0
>> +              maximum: 7
>> +              description:
>> +                Offset in bit within the address range specified by 
>> reg.
>> +            - minimum: 1
> 
> max is 7?
I don't think it is limited to 7 as it is the size within the address 
range specified by reg. If the address range is more than a byte size 
can be more.
> 
>> +              description:
>> +                Size in bit within the address range specified by 
>> reg.
>> +
>> +    required:
>> +      - reg
>> +
>> +    additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +      sdam_1: nvram@b000 {
>> +         #address-cells = <1>;
>> +         #size-cells = <1>;
>> +         compatible = "qcom,spmi-sdam";
>> +          reg = <0xb000 0x100>;
>> +
>> +          /* Data cells */
>> +          restart_reason: restart@50 {
>> +              reg = <0x50 0x1>;
>> +              bits = <7 2>;
> 
> How do you have bit 8 in a 1 byte register?
You are right. Thanks for it. I will correct the example in next patch 
set.
> 
>> +          };
>> +      };
>> +...
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>>  a Linux Foundation Collaborative Project
>> 

Re: [PATCH] dt-bindings: nvmem: add binding for QTI SPMI SDAM

[Rob Herring]

On Thu, Jan 9, 2020 at 4:57 AM <sthella@codeaurora.org> wrote:
>
> On 2020-01-08 22:09, Rob Herring wrote:
> > On Tue, Dec 24, 2019 at 11:02:12AM +0530, Shyam Kumar Thella wrote:
> >> QTI SDAM allows PMIC peripherals to access the shared memory that is
> >> available on QTI PMICs. Add documentation for it.
> >>
> >> Signed-off-by: Shyam Kumar Thella <sthella@codeaurora.org>
> >> ---
> >>  .../devicetree/bindings/nvmem/qcom,spmi-sdam.yaml  | 79
> >> ++++++++++++++++++++++
> >>  1 file changed, 79 insertions(+)
> >>  create mode 100644
> >> Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> >> b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> >> new file mode 100644
> >> index 0000000..8961a99
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> >> @@ -0,0 +1,79 @@
> >> +# SPDX-License-Identifier: GPL-2.0
> >
> > Dual license new bindings:
> >
> > (GPL-2.0-only OR BSD-2-Clause)
> >
> > Please spread the word in QCom.
> Sure. I will add Dual license in next patchset.
> >
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/nvmem/qcom,spmi-sdam.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Qualcomm Technologies, Inc. SPMI SDAM DT bindings
> >> +
> >> +maintainers:
> >> +  - Shyam Kumar Thella <sthella@codeaurora.org>
> >> +
> >> +description: |
> >> +  The SDAM provides scratch register space for the PMIC clients. This
> >> +  memory can be used by software to store information or communicate
> >> +  to/from the PBUS.
> >> +
> >> +allOf:
> >> +  - $ref: "nvmem.yaml#"
> >> +
> >> +properties:
> >> +  compatible:
> >> +    enum:
> >> +      - qcom,spmi-sdam
> >> +
> >> +  reg:
> >> +    maxItems: 1
> >> +
> >> +  "#address-cells":
> >> +    const: 1
> >> +
> >> +  "#size-cells":
> >> +    const: 1
> >
> > ranges? The child addresses should be translateable I assume.
> The addresses are not memory mapped on the CPU's address domain. They
> are the SPMI addresses which can be accessed over SPMI controller.

Doesn't have to be a CPU address. Are the child offsets within the
range defined in the parent 'reg'? If so, then it should have
'ranges'.

> >
> >> +
> >> +required:
> >> +  - compatible
> >> +  - reg
> >> +
> >> +patternProperties:
> >> +  "^.*@[0-9a-f]+$":
> >> +    type: object
> >> +
> >> +    properties:
> >> +      reg:
> >> +        maxItems: 1
> >> +        description:
> >> +          Offset and size in bytes within the storage device.
> >> +
> >> +      bits:
> >
> > Needs a type reference.
> Yes. I will add a reference in the next patch set.
> >
> >> +        maxItems: 1
> >> +        items:
> >> +          items:
> >> +            - minimum: 0
> >> +              maximum: 7
> >> +              description:
> >> +                Offset in bit within the address range specified by
> >> reg.
> >> +            - minimum: 1
> >
> > max is 7?
> I don't think it is limited to 7 as it is the size within the address
> range specified by reg. If the address range is more than a byte size
> can be more.

Then why is the maximum offset 7?

Rob

Re: [PATCH] dt-bindings: nvmem: add binding for QTI SPMI SDAM

[sthella]

On 2020-01-09 21:01, Rob Herring wrote:
> On Thu, Jan 9, 2020 at 4:57 AM <sthella@codeaurora.org> wrote:
>> 
>> On 2020-01-08 22:09, Rob Herring wrote:
>> > On Tue, Dec 24, 2019 at 11:02:12AM +0530, Shyam Kumar Thella wrote:
>> >> QTI SDAM allows PMIC peripherals to access the shared memory that is
>> >> available on QTI PMICs. Add documentation for it.
>> >>
>> >> Signed-off-by: Shyam Kumar Thella <sthella@codeaurora.org>
>> >> ---
>> >>  .../devicetree/bindings/nvmem/qcom,spmi-sdam.yaml  | 79
>> >> ++++++++++++++++++++++
>> >>  1 file changed, 79 insertions(+)
>> >>  create mode 100644
>> >> Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> >>
>> >> diff --git
>> >> a/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> >> b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> >> new file mode 100644
>> >> index 0000000..8961a99
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> >> @@ -0,0 +1,79 @@
>> >> +# SPDX-License-Identifier: GPL-2.0
>> >
>> > Dual license new bindings:
>> >
>> > (GPL-2.0-only OR BSD-2-Clause)
>> >
>> > Please spread the word in QCom.
>> Sure. I will add Dual license in next patchset.
>> >
>> >> +%YAML 1.2
>> >> +---
>> >> +$id: http://devicetree.org/schemas/nvmem/qcom,spmi-sdam.yaml#
>> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> >> +
>> >> +title: Qualcomm Technologies, Inc. SPMI SDAM DT bindings
>> >> +
>> >> +maintainers:
>> >> +  - Shyam Kumar Thella <sthella@codeaurora.org>
>> >> +
>> >> +description: |
>> >> +  The SDAM provides scratch register space for the PMIC clients. This
>> >> +  memory can be used by software to store information or communicate
>> >> +  to/from the PBUS.
>> >> +
>> >> +allOf:
>> >> +  - $ref: "nvmem.yaml#"
>> >> +
>> >> +properties:
>> >> +  compatible:
>> >> +    enum:
>> >> +      - qcom,spmi-sdam
>> >> +
>> >> +  reg:
>> >> +    maxItems: 1
>> >> +
>> >> +  "#address-cells":
>> >> +    const: 1
>> >> +
>> >> +  "#size-cells":
>> >> +    const: 1
>> >
>> > ranges? The child addresses should be translateable I assume.
>> The addresses are not memory mapped on the CPU's address domain. They
>> are the SPMI addresses which can be accessed over SPMI controller.
> 
> Doesn't have to be a CPU address. Are the child offsets within the
> range defined in the parent 'reg'? If so, then it should have
> 'ranges'.
Yes the child offsets fall within parent reg's address space.
I will add ranges in the next patch set.
> 
>> >
>> >> +
>> >> +required:
>> >> +  - compatible
>> >> +  - reg
>> >> +
>> >> +patternProperties:
>> >> +  "^.*@[0-9a-f]+$":
>> >> +    type: object
>> >> +
>> >> +    properties:
>> >> +      reg:
>> >> +        maxItems: 1
>> >> +        description:
>> >> +          Offset and size in bytes within the storage device.
>> >> +
>> >> +      bits:
>> >
>> > Needs a type reference.
>> Yes. I will add a reference in the next patch set.
>> >
>> >> +        maxItems: 1
>> >> +        items:
>> >> +          items:
>> >> +            - minimum: 0
>> >> +              maximum: 7
>> >> +              description:
>> >> +                Offset in bit within the address range specified by
>> >> reg.
>> >> +            - minimum: 1
>> >
>> > max is 7?
>> I don't think it is limited to 7 as it is the size within the address
>> range specified by reg. If the address range is more than a byte size
>> can be more.
> 
> Then why is the maximum offset 7?
I see. Offset can be more than 7 within the address range specified in 
case
of data cells with more than a byte. I will remove maximum in the next
patch set.
> 
> Rob

Re: [PATCH] dt-bindings: nvmem: add binding for QTI SPMI SDAM

[Rob Herring]

On Fri, Jan 10, 2020 at 2:54 AM <sthella@codeaurora.org> wrote:
>
> On 2020-01-09 21:01, Rob Herring wrote:
> > On Thu, Jan 9, 2020 at 4:57 AM <sthella@codeaurora.org> wrote:
> >>
> >> On 2020-01-08 22:09, Rob Herring wrote:
> >> > On Tue, Dec 24, 2019 at 11:02:12AM +0530, Shyam Kumar Thella wrote:
> >> >> QTI SDAM allows PMIC peripherals to access the shared memory that is
> >> >> available on QTI PMICs. Add documentation for it.
> >> >>
> >> >> Signed-off-by: Shyam Kumar Thella <sthella@codeaurora.org>
> >> >> ---
> >> >>  .../devicetree/bindings/nvmem/qcom,spmi-sdam.yaml  | 79
> >> >> ++++++++++++++++++++++
> >> >>  1 file changed, 79 insertions(+)
> >> >>  create mode 100644
> >> >> Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> >> >>
> >> >> diff --git
> >> >> a/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> >> >> b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> >> >> new file mode 100644
> >> >> index 0000000..8961a99
> >> >> --- /dev/null
> >> >> +++ b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
> >> >> @@ -0,0 +1,79 @@
> >> >> +# SPDX-License-Identifier: GPL-2.0
> >> >
> >> > Dual license new bindings:
> >> >
> >> > (GPL-2.0-only OR BSD-2-Clause)
> >> >
> >> > Please spread the word in QCom.
> >> Sure. I will add Dual license in next patchset.
> >> >
> >> >> +%YAML 1.2
> >> >> +---
> >> >> +$id: http://devicetree.org/schemas/nvmem/qcom,spmi-sdam.yaml#
> >> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> >> +
> >> >> +title: Qualcomm Technologies, Inc. SPMI SDAM DT bindings
> >> >> +
> >> >> +maintainers:
> >> >> +  - Shyam Kumar Thella <sthella@codeaurora.org>
> >> >> +
> >> >> +description: |
> >> >> +  The SDAM provides scratch register space for the PMIC clients. This
> >> >> +  memory can be used by software to store information or communicate
> >> >> +  to/from the PBUS.
> >> >> +
> >> >> +allOf:
> >> >> +  - $ref: "nvmem.yaml#"
> >> >> +
> >> >> +properties:
> >> >> +  compatible:
> >> >> +    enum:
> >> >> +      - qcom,spmi-sdam
> >> >> +
> >> >> +  reg:
> >> >> +    maxItems: 1
> >> >> +
> >> >> +  "#address-cells":
> >> >> +    const: 1
> >> >> +
> >> >> +  "#size-cells":
> >> >> +    const: 1
> >> >
> >> > ranges? The child addresses should be translateable I assume.
> >> The addresses are not memory mapped on the CPU's address domain. They
> >> are the SPMI addresses which can be accessed over SPMI controller.
> >
> > Doesn't have to be a CPU address. Are the child offsets within the
> > range defined in the parent 'reg'? If so, then it should have
> > 'ranges'.
> Yes the child offsets fall within parent reg's address space.
> I will add ranges in the next patch set.
> >
> >> >
> >> >> +
> >> >> +required:
> >> >> +  - compatible
> >> >> +  - reg
> >> >> +
> >> >> +patternProperties:
> >> >> +  "^.*@[0-9a-f]+$":
> >> >> +    type: object
> >> >> +
> >> >> +    properties:
> >> >> +      reg:
> >> >> +        maxItems: 1
> >> >> +        description:
> >> >> +          Offset and size in bytes within the storage device.
> >> >> +
> >> >> +      bits:
> >> >
> >> > Needs a type reference.
> >> Yes. I will add a reference in the next patch set.
> >> >
> >> >> +        maxItems: 1
> >> >> +        items:
> >> >> +          items:
> >> >> +            - minimum: 0
> >> >> +              maximum: 7
> >> >> +              description:
> >> >> +                Offset in bit within the address range specified by
> >> >> reg.
> >> >> +            - minimum: 1
> >> >
> >> > max is 7?
> >> I don't think it is limited to 7 as it is the size within the address
> >> range specified by reg. If the address range is more than a byte size
> >> can be more.
> >
> > Then why is the maximum offset 7?
> I see. Offset can be more than 7 within the address range specified in
> case
> of data cells with more than a byte. I will remove maximum in the next
> patch set.

That's the wrong thing to do though. If the offset is more than 7, you
should just increase 'reg' value. IOW, 'bits' should only be used to
express bit position up to the minimum alignment of 'reg'. I guess you
could have an unaligned multi-byte field, so I guess this is fine
as-is.

Rob

Re: [PATCH] dt-bindings: nvmem: add binding for QTI SPMI SDAM

[sthella]

On 2020-01-13 21:12, Rob Herring wrote:
> On Fri, Jan 10, 2020 at 2:54 AM <sthella@codeaurora.org> wrote:
>> 
>> On 2020-01-09 21:01, Rob Herring wrote:
>> > On Thu, Jan 9, 2020 at 4:57 AM <sthella@codeaurora.org> wrote:
>> >>
>> >> On 2020-01-08 22:09, Rob Herring wrote:
>> >> > On Tue, Dec 24, 2019 at 11:02:12AM +0530, Shyam Kumar Thella wrote:
>> >> >> QTI SDAM allows PMIC peripherals to access the shared memory that is
>> >> >> available on QTI PMICs. Add documentation for it.
>> >> >>
>> >> >> Signed-off-by: Shyam Kumar Thella <sthella@codeaurora.org>
>> >> >> ---
>> >> >>  .../devicetree/bindings/nvmem/qcom,spmi-sdam.yaml  | 79
>> >> >> ++++++++++++++++++++++
>> >> >>  1 file changed, 79 insertions(+)
>> >> >>  create mode 100644
>> >> >> Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> >> >>
>> >> >> diff --git
>> >> >> a/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> >> >> b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> >> >> new file mode 100644
>> >> >> index 0000000..8961a99
>> >> >> --- /dev/null
>> >> >> +++ b/Documentation/devicetree/bindings/nvmem/qcom,spmi-sdam.yaml
>> >> >> @@ -0,0 +1,79 @@
>> >> >> +# SPDX-License-Identifier: GPL-2.0
>> >> >
>> >> > Dual license new bindings:
>> >> >
>> >> > (GPL-2.0-only OR BSD-2-Clause)
>> >> >
>> >> > Please spread the word in QCom.
>> >> Sure. I will add Dual license in next patchset.
>> >> >
>> >> >> +%YAML 1.2
>> >> >> +---
>> >> >> +$id: http://devicetree.org/schemas/nvmem/qcom,spmi-sdam.yaml#
>> >> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> >> >> +
>> >> >> +title: Qualcomm Technologies, Inc. SPMI SDAM DT bindings
>> >> >> +
>> >> >> +maintainers:
>> >> >> +  - Shyam Kumar Thella <sthella@codeaurora.org>
>> >> >> +
>> >> >> +description: |
>> >> >> +  The SDAM provides scratch register space for the PMIC clients. This
>> >> >> +  memory can be used by software to store information or communicate
>> >> >> +  to/from the PBUS.
>> >> >> +
>> >> >> +allOf:
>> >> >> +  - $ref: "nvmem.yaml#"
>> >> >> +
>> >> >> +properties:
>> >> >> +  compatible:
>> >> >> +    enum:
>> >> >> +      - qcom,spmi-sdam
>> >> >> +
>> >> >> +  reg:
>> >> >> +    maxItems: 1
>> >> >> +
>> >> >> +  "#address-cells":
>> >> >> +    const: 1
>> >> >> +
>> >> >> +  "#size-cells":
>> >> >> +    const: 1
>> >> >
>> >> > ranges? The child addresses should be translateable I assume.
>> >> The addresses are not memory mapped on the CPU's address domain. They
>> >> are the SPMI addresses which can be accessed over SPMI controller.
>> >
>> > Doesn't have to be a CPU address. Are the child offsets within the
>> > range defined in the parent 'reg'? If so, then it should have
>> > 'ranges'.
>> Yes the child offsets fall within parent reg's address space.
>> I will add ranges in the next patch set.
>> >
>> >> >
>> >> >> +
>> >> >> +required:
>> >> >> +  - compatible
>> >> >> +  - reg
>> >> >> +
>> >> >> +patternProperties:
>> >> >> +  "^.*@[0-9a-f]+$":
>> >> >> +    type: object
>> >> >> +
>> >> >> +    properties:
>> >> >> +      reg:
>> >> >> +        maxItems: 1
>> >> >> +        description:
>> >> >> +          Offset and size in bytes within the storage device.
>> >> >> +
>> >> >> +      bits:
>> >> >
>> >> > Needs a type reference.
>> >> Yes. I will add a reference in the next patch set.
>> >> >
>> >> >> +        maxItems: 1
>> >> >> +        items:
>> >> >> +          items:
>> >> >> +            - minimum: 0
>> >> >> +              maximum: 7
>> >> >> +              description:
>> >> >> +                Offset in bit within the address range specified by
>> >> >> reg.
>> >> >> +            - minimum: 1
>> >> >
>> >> > max is 7?
>> >> I don't think it is limited to 7 as it is the size within the address
>> >> range specified by reg. If the address range is more than a byte size
>> >> can be more.
>> >
>> > Then why is the maximum offset 7?
>> I see. Offset can be more than 7 within the address range specified in
>> case
>> of data cells with more than a byte. I will remove maximum in the next
>> patch set.
> 
> That's the wrong thing to do though. If the offset is more than 7, you
> should just increase 'reg' value. IOW, 'bits' should only be used to
> express bit position up to the minimum alignment of 'reg'. I guess you
> could have an unaligned multi-byte field, so I guess this is fine
> as-is.
> 
> Rob
Okay.

Regards,
Shyam