2019-12-24 05:33:35

by Shyam Kumar Thella

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

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 <[email protected]>
---
.../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 <[email protected]>
+
+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


2019-12-29 03:02:35

by Bjorn Andersson

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

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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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

2019-12-31 06:45:50

by Shyam Kumar Thella

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

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 <[email protected]>
>> ---
>> .../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 <[email protected]>
>> +
>> +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

2020-01-08 17:57:24

by Rob Herring (Arm)

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

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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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
>

2020-01-09 13:58:27

by Shyam Kumar Thella

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

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 <[email protected]>
>> ---
>> .../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 <[email protected]>
>> +
>> +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
>>

2020-01-09 20:10:21

by Rob Herring (Arm)

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

On Thu, Jan 9, 2020 at 4:57 AM <[email protected]> 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 <[email protected]>
> >> ---
> >> .../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 <[email protected]>
> >> +
> >> +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

2020-01-10 08:56:15

by Shyam Kumar Thella

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

On 2020-01-09 21:01, Rob Herring wrote:
> On Thu, Jan 9, 2020 at 4:57 AM <[email protected]> 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 <[email protected]>
>> >> ---
>> >> .../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 <[email protected]>
>> >> +
>> >> +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

2020-01-13 15:43:42

by Rob Herring (Arm)

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

On Fri, Jan 10, 2020 at 2:54 AM <[email protected]> wrote:
>
> On 2020-01-09 21:01, Rob Herring wrote:
> > On Thu, Jan 9, 2020 at 4:57 AM <[email protected]> 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 <[email protected]>
> >> >> ---
> >> >> .../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 <[email protected]>
> >> >> +
> >> >> +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

2020-01-14 07:09:01

by Shyam Kumar Thella

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

On 2020-01-13 21:12, Rob Herring wrote:
> On Fri, Jan 10, 2020 at 2:54 AM <[email protected]> wrote:
>>
>> On 2020-01-09 21:01, Rob Herring wrote:
>> > On Thu, Jan 9, 2020 at 4:57 AM <[email protected]> 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 <[email protected]>
>> >> >> ---
>> >> >> .../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 <[email protected]>
>> >> >> +
>> >> >> +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