* [PATCH 1/5] firmware: xilinx: Add ZynqMP efuse access API
2023-10-13 10:14 [PATCH 0/5] Add ZynqMP efuse access support Praveen Teja Kundanala
@ 2023-10-13 10:14 ` Praveen Teja Kundanala
2023-10-13 10:14 ` [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml Praveen Teja Kundanala
` (3 subsequent siblings)
4 siblings, 0 replies; 23+ messages in thread
From: Praveen Teja Kundanala @ 2023-10-13 10:14 UTC (permalink / raw)
To: srinivas.kandagatla, robh+dt, krzysztof.kozlowski+dt, conor+dt,
michal.simek, devicetree, linux-arm-kernel
Cc: linux-kernel
Add zynqmp_pm_efuse_access API in the ZynqMP
firmware for read/write access of efuse memory.
Signed-off-by: Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
---
drivers/firmware/xilinx/zynqmp.c | 25 +++++++++++++++++++++++++
include/linux/firmware/xlnx-zynqmp.h | 8 ++++++++
2 files changed, 33 insertions(+)
diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index f8c4eb2b43f8..b0f6272e0844 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -3,6 +3,7 @@
* Xilinx Zynq MPSoC Firmware layer
*
* Copyright (C) 2014-2022 Xilinx, Inc.
+ * Copyright (C), 2022 - 2023 Advanced Micro Devices, Inc.
*
* Michal Simek <michal.simek@amd.com>
* Davorin Mista <davorin.mista@aggios.com>
@@ -1390,6 +1391,30 @@ int zynqmp_pm_aes_engine(const u64 address, u32 *out)
}
EXPORT_SYMBOL_GPL(zynqmp_pm_aes_engine);
+/**
+ * zynqmp_pm_efuse_access - Provides access to efuse memory.
+ * @address: Address of the efuse params structure
+ * @out: Returned output value
+ *
+ * Return: Returns status, either success or error code.
+ */
+int zynqmp_pm_efuse_access(const u64 address, u32 *out)
+{
+ u32 ret_payload[PAYLOAD_ARG_CNT];
+ int ret;
+
+ if (!out)
+ return -EINVAL;
+
+ ret = zynqmp_pm_invoke_fn(PM_EFUSE_ACCESS, upper_32_bits(address),
+ lower_32_bits(address), 0, 0,
+ ret_payload);
+ *out = ret_payload[1];
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_efuse_access);
+
/**
* zynqmp_pm_sha_hash - Access the SHA engine to calculate the hash
* @address: Address of the data/ Address of output buffer where
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 9dda7d9898ff..721cebae3f14 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -3,6 +3,7 @@
* Xilinx Zynq MPSoC Firmware layer
*
* Copyright (C) 2014-2021 Xilinx
+ * Copyright (C), 2022 - 2023 Advanced Micro Devices, Inc.
*
* Michal Simek <michal.simek@amd.com>
* Davorin Mista <davorin.mista@aggios.com>
@@ -130,6 +131,7 @@ enum pm_api_id {
PM_CLOCK_GETPARENT = 44,
PM_FPGA_READ = 46,
PM_SECURE_AES = 47,
+ PM_EFUSE_ACCESS = 53,
PM_FEATURE_CHECK = 63,
};
@@ -521,6 +523,7 @@ int zynqmp_pm_set_requirement(const u32 node, const u32 capabilities,
const u32 qos,
const enum zynqmp_pm_request_ack ack);
int zynqmp_pm_aes_engine(const u64 address, u32 *out);
+int zynqmp_pm_efuse_access(const u64 address, u32 *out);
int zynqmp_pm_sha_hash(const u64 address, const u32 size, const u32 flags);
int zynqmp_pm_fpga_load(const u64 address, const u32 size, const u32 flags);
int zynqmp_pm_fpga_get_status(u32 *value);
@@ -714,6 +717,11 @@ static inline int zynqmp_pm_aes_engine(const u64 address, u32 *out)
return -ENODEV;
}
+static inline int zynqmp_pm_efuse_access(const u64 address, u32 *out)
+{
+ return -ENODEV;
+}
+
static inline int zynqmp_pm_sha_hash(const u64 address, const u32 size,
const u32 flags)
{
--
2.36.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 10:14 [PATCH 0/5] Add ZynqMP efuse access support Praveen Teja Kundanala
2023-10-13 10:14 ` [PATCH 1/5] firmware: xilinx: Add ZynqMP efuse access API Praveen Teja Kundanala
@ 2023-10-13 10:14 ` Praveen Teja Kundanala
2023-10-13 10:30 ` Krzysztof Kozlowski
` (2 more replies)
2023-10-13 10:14 ` [PATCH 3/5] dt-bindings: nvmem: Add nodes for ZynqMP efuses Praveen Teja Kundanala
` (2 subsequent siblings)
4 siblings, 3 replies; 23+ messages in thread
From: Praveen Teja Kundanala @ 2023-10-13 10:14 UTC (permalink / raw)
To: srinivas.kandagatla, robh+dt, krzysztof.kozlowski+dt, conor+dt,
michal.simek, devicetree, linux-arm-kernel
Cc: linux-kernel
Convert the xlnx,zynqmp-nvmem.txt to yaml.
Signed-off-by: Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
---
.../bindings/nvmem/xlnx,zynqmp-nvmem.txt | 46 ---------------
.../bindings/nvmem/xlnx,zynqmp-nvmem.yaml | 59 +++++++++++++++++++
2 files changed, 59 insertions(+), 46 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
create mode 100644 Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
diff --git a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
deleted file mode 100644
index 4881561b3a02..000000000000
--- a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
+++ /dev/null
@@ -1,46 +0,0 @@
---------------------------------------------------------------------------
-= Zynq UltraScale+ MPSoC nvmem firmware driver binding =
---------------------------------------------------------------------------
-The nvmem_firmware node provides access to the hardware related data
-like soc revision, IDCODE... etc, By using the firmware interface.
-
-Required properties:
-- compatible: should be "xlnx,zynqmp-nvmem-fw"
-
-= Data cells =
-Are child nodes of silicon id, bindings of which as described in
-bindings/nvmem/nvmem.txt
-
--------
- Example
--------
-firmware {
- zynqmp_firmware: zynqmp-firmware {
- compatible = "xlnx,zynqmp-firmware";
- method = "smc";
-
- nvmem_firmware {
- compatible = "xlnx,zynqmp-nvmem-fw";
- #address-cells = <1>;
- #size-cells = <1>;
-
- /* Data cells */
- soc_revision: soc_revision {
- reg = <0x0 0x4>;
- };
- };
- };
-};
-
-= Data consumers =
-Are device nodes which consume nvmem data cells.
-
-For example:
- pcap {
- ...
-
- nvmem-cells = <&soc_revision>;
- nvmem-cell-names = "soc_revision";
-
- ...
- };
diff --git a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
new file mode 100644
index 000000000000..e03ed8c32537
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/xlnx,zynqmp-nvmem.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Zynq UltraScale+ MPSoC Non Volatile Memory interface
+
+description: |
+ The ZynqMP MPSoC provides access to the hardware related data
+ like SOC revision, IDCODE.
+
+maintainers:
+ - Kalyani Akula <kalyani.akula@amd.com>
+ - Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
+
+allOf:
+ - $ref: "nvmem.yaml#"
+
+properties:
+ compatible:
+ const: xlnx,zynqmp-nvmem-fw
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 1
+
+required:
+ - compatible
+
+patternProperties:
+ "^soc_revision@0$":
+ type: object
+ description:
+ This node is used to read SOC version and IDCODE of ZynqMP. Read-only.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ nvmem_firmware {
+ compatible = "xlnx,zynqmp-nvmem-fw";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ /* Data cells */
+ soc_revision: soc_revision@0 {
+ reg = <0x0 0x4>;
+ };
+ };
--
2.36.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 10:14 ` [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml Praveen Teja Kundanala
@ 2023-10-13 10:30 ` Krzysztof Kozlowski
2023-10-13 11:22 ` Michal Simek
2023-10-13 11:09 ` Rob Herring
2023-10-13 16:23 ` Rob Herring
2 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-13 10:30 UTC (permalink / raw)
To: Praveen Teja Kundanala, srinivas.kandagatla, robh+dt,
krzysztof.kozlowski+dt, conor+dt, michal.simek, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 13/10/2023 12:14, Praveen Teja Kundanala wrote:
> Convert the xlnx,zynqmp-nvmem.txt to yaml.
>
> Signed-off-by: Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
> ---
> .../bindings/nvmem/xlnx,zynqmp-nvmem.txt | 46 ---------------
> .../bindings/nvmem/xlnx,zynqmp-nvmem.yaml | 59 +++++++++++++++++++
> 2 files changed, 59 insertions(+), 46 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
> create mode 100644 Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
>
> diff --git a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
> deleted file mode 100644
> index 4881561b3a02..000000000000
> --- a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
> +++ /dev/null
> @@ -1,46 +0,0 @@
> ---------------------------------------------------------------------------
> -= Zynq UltraScale+ MPSoC nvmem firmware driver binding =
> ---------------------------------------------------------------------------
> -The nvmem_firmware node provides access to the hardware related data
> -like soc revision, IDCODE... etc, By using the firmware interface.
> -
> -Required properties:
> -- compatible: should be "xlnx,zynqmp-nvmem-fw"
> -
> -= Data cells =
> -Are child nodes of silicon id, bindings of which as described in
> -bindings/nvmem/nvmem.txt
> -
> --------
> - Example
> --------
> -firmware {
> - zynqmp_firmware: zynqmp-firmware {
> - compatible = "xlnx,zynqmp-firmware";
> - method = "smc";
> -
> - nvmem_firmware {
> - compatible = "xlnx,zynqmp-nvmem-fw";
> - #address-cells = <1>;
> - #size-cells = <1>;
> -
> - /* Data cells */
> - soc_revision: soc_revision {
> - reg = <0x0 0x4>;
> - };
> - };
> - };
> -};
> -
> -= Data consumers =
> -Are device nodes which consume nvmem data cells.
> -
> -For example:
> - pcap {
> - ...
> -
> - nvmem-cells = <&soc_revision>;
> - nvmem-cell-names = "soc_revision";
> -
> - ...
> - };
> diff --git a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
> new file mode 100644
> index 000000000000..e03ed8c32537
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/xlnx,zynqmp-nvmem.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Zynq UltraScale+ MPSoC Non Volatile Memory interface
> +
> +description: |
> + The ZynqMP MPSoC provides access to the hardware related data
> + like SOC revision, IDCODE.
> +
> +maintainers:
> + - Kalyani Akula <kalyani.akula@amd.com>
> + - Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
> +
> +allOf:
> + - $ref: "nvmem.yaml#"
Drop quotes.
> +
> +properties:
> + compatible:
> + const: xlnx,zynqmp-nvmem-fw
> +
> + '#address-cells':
> + const: 1
Drop
> +
> + '#size-cells':
> + const: 1
Drop
> +
> +required:
> + - compatible
required: block goes after patternProperties: block
> +
> +patternProperties:
> + "^soc_revision@0$":
Why do you define individual memory cells? Is this part of a binding?
IOW, OS/Linux requires this?
> + type: object
> + description:
> + This node is used to read SOC version and IDCODE of ZynqMP. Read-only.
> +
> + properties:
> + reg:
> + maxItems: 1
> +
> + required:
> + - reg
> +
> +additionalProperties: false
Instead: unevaluatedProperties: false
> +
> +examples:
> + - |
> + nvmem_firmware {
Underscores are not allowed in node names.
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
Look at other files for examples.
> + compatible = "xlnx,zynqmp-nvmem-fw";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + /* Data cells */
> + soc_revision: soc_revision@0 {
Drop unused label. No underscores in node names.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 10:30 ` Krzysztof Kozlowski
@ 2023-10-13 11:22 ` Michal Simek
2023-10-13 11:46 ` Krzysztof Kozlowski
0 siblings, 1 reply; 23+ messages in thread
From: Michal Simek @ 2023-10-13 11:22 UTC (permalink / raw)
To: Krzysztof Kozlowski, Praveen Teja Kundanala, srinivas.kandagatla,
robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 10/13/23 12:30, Krzysztof Kozlowski wrote:
> On 13/10/2023 12:14, Praveen Teja Kundanala wrote:
>> Convert the xlnx,zynqmp-nvmem.txt to yaml.
>>
>> Signed-off-by: Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
>> ---
>> .../bindings/nvmem/xlnx,zynqmp-nvmem.txt | 46 ---------------
>> .../bindings/nvmem/xlnx,zynqmp-nvmem.yaml | 59 +++++++++++++++++++
>> 2 files changed, 59 insertions(+), 46 deletions(-)
>> delete mode 100644 Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
>> create mode 100644 Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
>> deleted file mode 100644
>> index 4881561b3a02..000000000000
>> --- a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
>> +++ /dev/null
>> @@ -1,46 +0,0 @@
>> ---------------------------------------------------------------------------
>> -= Zynq UltraScale+ MPSoC nvmem firmware driver binding =
>> ---------------------------------------------------------------------------
>> -The nvmem_firmware node provides access to the hardware related data
>> -like soc revision, IDCODE... etc, By using the firmware interface.
>> -
>> -Required properties:
>> -- compatible: should be "xlnx,zynqmp-nvmem-fw"
>> -
>> -= Data cells =
>> -Are child nodes of silicon id, bindings of which as described in
>> -bindings/nvmem/nvmem.txt
>> -
>> --------
>> - Example
>> --------
>> -firmware {
>> - zynqmp_firmware: zynqmp-firmware {
>> - compatible = "xlnx,zynqmp-firmware";
>> - method = "smc";
>> -
>> - nvmem_firmware {
>> - compatible = "xlnx,zynqmp-nvmem-fw";
>> - #address-cells = <1>;
>> - #size-cells = <1>;
>> -
>> - /* Data cells */
>> - soc_revision: soc_revision {
>> - reg = <0x0 0x4>;
>> - };
>> - };
>> - };
>> -};
>> -
>> -= Data consumers =
>> -Are device nodes which consume nvmem data cells.
>> -
>> -For example:
>> - pcap {
>> - ...
>> -
>> - nvmem-cells = <&soc_revision>;
>> - nvmem-cell-names = "soc_revision";
>> -
>> - ...
>> - };
>> diff --git a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
>> new file mode 100644
>> index 000000000000..e03ed8c32537
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
>> @@ -0,0 +1,59 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/nvmem/xlnx,zynqmp-nvmem.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Zynq UltraScale+ MPSoC Non Volatile Memory interface
>> +
>> +description: |
>> + The ZynqMP MPSoC provides access to the hardware related data
>> + like SOC revision, IDCODE.
>> +
>> +maintainers:
>> + - Kalyani Akula <kalyani.akula@amd.com>
>> + - Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
>> +
>> +allOf:
>> + - $ref: "nvmem.yaml#"
>
> Drop quotes.
>
>> +
>> +properties:
>> + compatible:
>> + const: xlnx,zynqmp-nvmem-fw
>> +
>> + '#address-cells':
>> + const: 1
>
> Drop
>
>> +
>> + '#size-cells':
>> + const: 1
>
> Drop
>
>> +
>> +required:
>> + - compatible
>
> required: block goes after patternProperties: block
>
>> +
>> +patternProperties:
>> + "^soc_revision@0$":
>
> Why do you define individual memory cells? Is this part of a binding?
> IOW, OS/Linux requires this?
nvmem has in kernel interface where you can reference to nodes. nvmem_cell_get()
calls. It means you should be able to describe internal layout that's why names
are used. And address in name is there because of reg property is used to
describe base offset and size.
Thanks,
Michal
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 11:22 ` Michal Simek
@ 2023-10-13 11:46 ` Krzysztof Kozlowski
2023-10-13 11:51 ` Michal Simek
0 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-13 11:46 UTC (permalink / raw)
To: Michal Simek, Praveen Teja Kundanala, srinivas.kandagatla,
robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 13/10/2023 13:22, Michal Simek wrote:
>>
>>> +
>>> +required:
>>> + - compatible
>>
>> required: block goes after patternProperties: block
>>
>>> +
>>> +patternProperties:
>>> + "^soc_revision@0$":
>>
>> Why do you define individual memory cells? Is this part of a binding?
>> IOW, OS/Linux requires this?
>
> nvmem has in kernel interface where you can reference to nodes. nvmem_cell_get()
> calls. It means you should be able to describe internal layout that's why names
> are used. And address in name is there because of reg property is used to
> describe base offset and size.
That's not really what I am asking. Why internal layout of memory must
be part of the bindings?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 11:46 ` Krzysztof Kozlowski
@ 2023-10-13 11:51 ` Michal Simek
2023-10-13 11:58 ` Krzysztof Kozlowski
0 siblings, 1 reply; 23+ messages in thread
From: Michal Simek @ 2023-10-13 11:51 UTC (permalink / raw)
To: Krzysztof Kozlowski, Praveen Teja Kundanala, srinivas.kandagatla,
robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 10/13/23 13:46, Krzysztof Kozlowski wrote:
> On 13/10/2023 13:22, Michal Simek wrote:
>>>
>>>> +
>>>> +required:
>>>> + - compatible
>>>
>>> required: block goes after patternProperties: block
>>>
>>>> +
>>>> +patternProperties:
>>>> + "^soc_revision@0$":
>>>
>>> Why do you define individual memory cells? Is this part of a binding?
>>> IOW, OS/Linux requires this?
>>
>> nvmem has in kernel interface where you can reference to nodes. nvmem_cell_get()
>> calls. It means you should be able to describe internal layout that's why names
>> are used. And address in name is there because of reg property is used to
>> describe base offset and size.
>
> That's not really what I am asking. Why internal layout of memory must
> be part of the bindings?
It doesn't need to be but offsets are hardcoded inside the driver itself and
they can't be different. On different nvmem locations like MAC location in
eeprom this can vary across boards but in this case location has to be only like
this.
I am fine if they don't need to be actually check but there is no any other way
how they can be composed. And also others are not valid that's why not to
describe only valid one.
Thanks,
Michal
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 11:51 ` Michal Simek
@ 2023-10-13 11:58 ` Krzysztof Kozlowski
2023-10-13 12:08 ` Michal Simek
0 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-13 11:58 UTC (permalink / raw)
To: Michal Simek, Praveen Teja Kundanala, srinivas.kandagatla,
robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 13/10/2023 13:51, Michal Simek wrote:
>
>
> On 10/13/23 13:46, Krzysztof Kozlowski wrote:
>> On 13/10/2023 13:22, Michal Simek wrote:
>>>>
>>>>> +
>>>>> +required:
>>>>> + - compatible
>>>>
>>>> required: block goes after patternProperties: block
>>>>
>>>>> +
>>>>> +patternProperties:
>>>>> + "^soc_revision@0$":
>>>>
>>>> Why do you define individual memory cells? Is this part of a binding?
>>>> IOW, OS/Linux requires this?
>>>
>>> nvmem has in kernel interface where you can reference to nodes. nvmem_cell_get()
>>> calls. It means you should be able to describe internal layout that's why names
>>> are used. And address in name is there because of reg property is used to
>>> describe base offset and size.
>>
>> That's not really what I am asking. Why internal layout of memory must
>> be part of the bindings?
>
> It doesn't need to be but offsets are hardcoded inside the driver itself and
> they can't be different.
Hm, where? I opened drivers/nvmem/zynqmp_nvmem.c and I do not see any
hard-coded offsets.
> On different nvmem locations like MAC location in
> eeprom this can vary across boards but in this case location has to be only like
> this.
> I am fine if they don't need to be actually check but there is no any other way
> how they can be composed. And also others are not valid that's why not to
> describe only valid one.
OK, that would be valid (if I find anywhere the offsets) and answers my
questions but I wish it was documented somewhere. Because now you are
making it a binding, so it cannot change (e.g. for different devices
with same hardware but different firmware or manufacturing process, for
future hardware sharing this binding).
In any case the binding should have only items which are really fixed
and OS depends on them. Neither this nor next commit answers this.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 11:58 ` Krzysztof Kozlowski
@ 2023-10-13 12:08 ` Michal Simek
2023-10-13 12:54 ` Krzysztof Kozlowski
0 siblings, 1 reply; 23+ messages in thread
From: Michal Simek @ 2023-10-13 12:08 UTC (permalink / raw)
To: Krzysztof Kozlowski, Praveen Teja Kundanala, srinivas.kandagatla,
robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 10/13/23 13:58, Krzysztof Kozlowski wrote:
> On 13/10/2023 13:51, Michal Simek wrote:
>>
>>
>> On 10/13/23 13:46, Krzysztof Kozlowski wrote:
>>> On 13/10/2023 13:22, Michal Simek wrote:
>>>>>
>>>>>> +
>>>>>> +required:
>>>>>> + - compatible
>>>>>
>>>>> required: block goes after patternProperties: block
>>>>>
>>>>>> +
>>>>>> +patternProperties:
>>>>>> + "^soc_revision@0$":
>>>>>
>>>>> Why do you define individual memory cells? Is this part of a binding?
>>>>> IOW, OS/Linux requires this?
>>>>
>>>> nvmem has in kernel interface where you can reference to nodes. nvmem_cell_get()
>>>> calls. It means you should be able to describe internal layout that's why names
>>>> are used. And address in name is there because of reg property is used to
>>>> describe base offset and size.
>>>
>>> That's not really what I am asking. Why internal layout of memory must
>>> be part of the bindings?
>>
>> It doesn't need to be but offsets are hardcoded inside the driver itself and
>> they can't be different.
>
> Hm, where? I opened drivers/nvmem/zynqmp_nvmem.c and I do not see any
> hard-coded offsets.
Current driver supports only soc revision from offset 0.
But if you look at 5/5 you need to define offsets where information is present.
+#define SOC_VERSION_OFFSET 0x0
+#define EFUSE_START_OFFSET 0xC
+#define EFUSE_END_OFFSET 0xFC
+#define EFUSE_PUF_START_OFFSET 0x100
+#define EFUSE_PUF_MID_OFFSET 0x140
+#define EFUSE_PUF_END_OFFSET 0x17F
>
>> On different nvmem locations like MAC location in
>> eeprom this can vary across boards but in this case location has to be only like
>> this.
>> I am fine if they don't need to be actually check but there is no any other way
>> how they can be composed. And also others are not valid that's why not to
>> describe only valid one.
>
> OK, that would be valid (if I find anywhere the offsets) and answers my
> questions but I wish it was documented somewhere. Because now you are
> making it a binding, so it cannot change (e.g. for different devices
> with same hardware but different firmware or manufacturing process, for
> future hardware sharing this binding).
ZynqMP firmware with xlnx,zynqmp-nvmem-fw compatible string is using this map.
If there is different firmware likely xlnx,zynqmp-nvmem-fw compatible string
can't be used.
> In any case the binding should have only items which are really fixed
> and OS depends on them. Neither this nor next commit answers this.
Actually 5/5 has this in commit message
0 - SOC version(read only)
0xC - 0xFC -ZynqMP specific purpose efuses
0x100 - 0x17F - Physical Unclonable Function(PUF)
efuses repurposed as user efuses
Thanks,
Michal
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 12:08 ` Michal Simek
@ 2023-10-13 12:54 ` Krzysztof Kozlowski
2023-10-13 13:06 ` Michal Simek
0 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-13 12:54 UTC (permalink / raw)
To: Michal Simek, Praveen Teja Kundanala, srinivas.kandagatla,
robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 13/10/2023 14:08, Michal Simek wrote:
>
>
> On 10/13/23 13:58, Krzysztof Kozlowski wrote:
>> On 13/10/2023 13:51, Michal Simek wrote:
>>>
>>>
>>> On 10/13/23 13:46, Krzysztof Kozlowski wrote:
>>>> On 13/10/2023 13:22, Michal Simek wrote:
>>>>>>
>>>>>>> +
>>>>>>> +required:
>>>>>>> + - compatible
>>>>>>
>>>>>> required: block goes after patternProperties: block
>>>>>>
>>>>>>> +
>>>>>>> +patternProperties:
>>>>>>> + "^soc_revision@0$":
>>>>>>
>>>>>> Why do you define individual memory cells? Is this part of a binding?
>>>>>> IOW, OS/Linux requires this?
>>>>>
>>>>> nvmem has in kernel interface where you can reference to nodes. nvmem_cell_get()
>>>>> calls. It means you should be able to describe internal layout that's why names
>>>>> are used. And address in name is there because of reg property is used to
>>>>> describe base offset and size.
>>>>
>>>> That's not really what I am asking. Why internal layout of memory must
>>>> be part of the bindings?
>>>
>>> It doesn't need to be but offsets are hardcoded inside the driver itself and
>>> they can't be different.
>>
>> Hm, where? I opened drivers/nvmem/zynqmp_nvmem.c and I do not see any
>> hard-coded offsets.
>
> Current driver supports only soc revision from offset 0.
> But if you look at 5/5 you need to define offsets where information is present.
> +#define SOC_VERSION_OFFSET 0x0
> +#define EFUSE_START_OFFSET 0xC
> +#define EFUSE_END_OFFSET 0xFC
> +#define EFUSE_PUF_START_OFFSET 0x100
> +#define EFUSE_PUF_MID_OFFSET 0x140
> +#define EFUSE_PUF_END_OFFSET 0x17F
There is nothing like this in existing driver, so the argument that "I
am adding this to the binding during conversion because driver needs it"
is not true. Conversion is only a conversion.
Now, if you want to add something new to the binding because of new
driver changes, that's separate topic.
And since it is new change in the driver I can comment: please don't.
Your nvmem driver should not depend on it. nvmem is only the provider.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 12:54 ` Krzysztof Kozlowski
@ 2023-10-13 13:06 ` Michal Simek
2023-10-13 13:10 ` Krzysztof Kozlowski
0 siblings, 1 reply; 23+ messages in thread
From: Michal Simek @ 2023-10-13 13:06 UTC (permalink / raw)
To: Krzysztof Kozlowski, Praveen Teja Kundanala, srinivas.kandagatla,
robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 10/13/23 14:54, Krzysztof Kozlowski wrote:
> On 13/10/2023 14:08, Michal Simek wrote:
>>
>>
>> On 10/13/23 13:58, Krzysztof Kozlowski wrote:
>>> On 13/10/2023 13:51, Michal Simek wrote:
>>>>
>>>>
>>>> On 10/13/23 13:46, Krzysztof Kozlowski wrote:
>>>>> On 13/10/2023 13:22, Michal Simek wrote:
>>>>>>>
>>>>>>>> +
>>>>>>>> +required:
>>>>>>>> + - compatible
>>>>>>>
>>>>>>> required: block goes after patternProperties: block
>>>>>>>
>>>>>>>> +
>>>>>>>> +patternProperties:
>>>>>>>> + "^soc_revision@0$":
>>>>>>>
>>>>>>> Why do you define individual memory cells? Is this part of a binding?
>>>>>>> IOW, OS/Linux requires this?
>>>>>>
>>>>>> nvmem has in kernel interface where you can reference to nodes. nvmem_cell_get()
>>>>>> calls. It means you should be able to describe internal layout that's why names
>>>>>> are used. And address in name is there because of reg property is used to
>>>>>> describe base offset and size.
>>>>>
>>>>> That's not really what I am asking. Why internal layout of memory must
>>>>> be part of the bindings?
>>>>
>>>> It doesn't need to be but offsets are hardcoded inside the driver itself and
>>>> they can't be different.
>>>
>>> Hm, where? I opened drivers/nvmem/zynqmp_nvmem.c and I do not see any
>>> hard-coded offsets.
>>
>> Current driver supports only soc revision from offset 0.
>> But if you look at 5/5 you need to define offsets where information is present.
>> +#define SOC_VERSION_OFFSET 0x0
>> +#define EFUSE_START_OFFSET 0xC
>> +#define EFUSE_END_OFFSET 0xFC
>> +#define EFUSE_PUF_START_OFFSET 0x100
>> +#define EFUSE_PUF_MID_OFFSET 0x140
>> +#define EFUSE_PUF_END_OFFSET 0x17F
>
> There is nothing like this in existing driver, so the argument that "I
> am adding this to the binding during conversion because driver needs it"
> is not true. Conversion is only a conversion.
Conversion in 2/5 is adding only soc revision which is already there. It is
starting from 0 and world size is 1. And 0 is not listed because that's start
all the time.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/nvmem/zynqmp_nvmem.c?h=v6.6-rc5#n39
And soc revision was also listed in origin binding example.
> Now, if you want to add something new to the binding because of new
> driver changes, that's separate topic.
Functionality in firmware is there for quite a long time but as I said I am fine
if map is not going to be inside dt binding spec.
> And since it is new change in the driver I can comment: please don't.
> Your nvmem driver should not depend on it. nvmem is only the provider.
Let's see what Srinivas says about implementation. If driver should be just
provider then pretty much current driver should be completely rewritten to
different style. I mean to have just transport via SMCs with offset/size and
then providing functionality in firmware.
Thanks,
Michal
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 13:06 ` Michal Simek
@ 2023-10-13 13:10 ` Krzysztof Kozlowski
2023-10-13 13:12 ` Michal Simek
2023-10-16 8:01 ` Michal Simek
0 siblings, 2 replies; 23+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-13 13:10 UTC (permalink / raw)
To: Michal Simek, Praveen Teja Kundanala, srinivas.kandagatla,
robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 13/10/2023 15:06, Michal Simek wrote:
>
>
> On 10/13/23 14:54, Krzysztof Kozlowski wrote:
>> On 13/10/2023 14:08, Michal Simek wrote:
>>>
>>>
>>> On 10/13/23 13:58, Krzysztof Kozlowski wrote:
>>>> On 13/10/2023 13:51, Michal Simek wrote:
>>>>>
>>>>>
>>>>> On 10/13/23 13:46, Krzysztof Kozlowski wrote:
>>>>>> On 13/10/2023 13:22, Michal Simek wrote:
>>>>>>>>
>>>>>>>>> +
>>>>>>>>> +required:
>>>>>>>>> + - compatible
>>>>>>>>
>>>>>>>> required: block goes after patternProperties: block
>>>>>>>>
>>>>>>>>> +
>>>>>>>>> +patternProperties:
>>>>>>>>> + "^soc_revision@0$":
>>>>>>>>
>>>>>>>> Why do you define individual memory cells? Is this part of a binding?
>>>>>>>> IOW, OS/Linux requires this?
>>>>>>>
>>>>>>> nvmem has in kernel interface where you can reference to nodes. nvmem_cell_get()
>>>>>>> calls. It means you should be able to describe internal layout that's why names
>>>>>>> are used. And address in name is there because of reg property is used to
>>>>>>> describe base offset and size.
>>>>>>
>>>>>> That's not really what I am asking. Why internal layout of memory must
>>>>>> be part of the bindings?
>>>>>
>>>>> It doesn't need to be but offsets are hardcoded inside the driver itself and
>>>>> they can't be different.
>>>>
>>>> Hm, where? I opened drivers/nvmem/zynqmp_nvmem.c and I do not see any
>>>> hard-coded offsets.
>>>
>>> Current driver supports only soc revision from offset 0.
>>> But if you look at 5/5 you need to define offsets where information is present.
>>> +#define SOC_VERSION_OFFSET 0x0
>>> +#define EFUSE_START_OFFSET 0xC
>>> +#define EFUSE_END_OFFSET 0xFC
>>> +#define EFUSE_PUF_START_OFFSET 0x100
>>> +#define EFUSE_PUF_MID_OFFSET 0x140
>>> +#define EFUSE_PUF_END_OFFSET 0x17F
>>
>> There is nothing like this in existing driver, so the argument that "I
>> am adding this to the binding during conversion because driver needs it"
>> is not true. Conversion is only a conversion.
>
> Conversion in 2/5 is adding only soc revision which is already there. It is
> starting from 0 and world size is 1. And 0 is not listed because that's start
> all the time.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/nvmem/zynqmp_nvmem.c?h=v6.6-rc5#n39
This defines the nvmem config, not what should be where.
>
> And soc revision was also listed in origin binding example.
Example is not a binding. Please drop enforcement of some specific nodes
from the binding.
>
>> Now, if you want to add something new to the binding because of new
>> driver changes, that's separate topic.
>
> Functionality in firmware is there for quite a long time but as I said I am fine
> if map is not going to be inside dt binding spec.
>
>> And since it is new change in the driver I can comment: please don't.
>> Your nvmem driver should not depend on it. nvmem is only the provider.
>
> Let's see what Srinivas says about implementation. If driver should be just
> provider then pretty much current driver should be completely rewritten to
> different style. I mean to have just transport via SMCs with offset/size and
> then providing functionality in firmware.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 13:10 ` Krzysztof Kozlowski
@ 2023-10-13 13:12 ` Michal Simek
2023-10-16 8:01 ` Michal Simek
1 sibling, 0 replies; 23+ messages in thread
From: Michal Simek @ 2023-10-13 13:12 UTC (permalink / raw)
To: Krzysztof Kozlowski, Praveen Teja Kundanala, srinivas.kandagatla,
robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 10/13/23 15:10, Krzysztof Kozlowski wrote:
> On 13/10/2023 15:06, Michal Simek wrote:
>>
>>
>> On 10/13/23 14:54, Krzysztof Kozlowski wrote:
>>> On 13/10/2023 14:08, Michal Simek wrote:
>>>>
>>>>
>>>> On 10/13/23 13:58, Krzysztof Kozlowski wrote:
>>>>> On 13/10/2023 13:51, Michal Simek wrote:
>>>>>>
>>>>>>
>>>>>> On 10/13/23 13:46, Krzysztof Kozlowski wrote:
>>>>>>> On 13/10/2023 13:22, Michal Simek wrote:
>>>>>>>>>
>>>>>>>>>> +
>>>>>>>>>> +required:
>>>>>>>>>> + - compatible
>>>>>>>>>
>>>>>>>>> required: block goes after patternProperties: block
>>>>>>>>>
>>>>>>>>>> +
>>>>>>>>>> +patternProperties:
>>>>>>>>>> + "^soc_revision@0$":
>>>>>>>>>
>>>>>>>>> Why do you define individual memory cells? Is this part of a binding?
>>>>>>>>> IOW, OS/Linux requires this?
>>>>>>>>
>>>>>>>> nvmem has in kernel interface where you can reference to nodes. nvmem_cell_get()
>>>>>>>> calls. It means you should be able to describe internal layout that's why names
>>>>>>>> are used. And address in name is there because of reg property is used to
>>>>>>>> describe base offset and size.
>>>>>>>
>>>>>>> That's not really what I am asking. Why internal layout of memory must
>>>>>>> be part of the bindings?
>>>>>>
>>>>>> It doesn't need to be but offsets are hardcoded inside the driver itself and
>>>>>> they can't be different.
>>>>>
>>>>> Hm, where? I opened drivers/nvmem/zynqmp_nvmem.c and I do not see any
>>>>> hard-coded offsets.
>>>>
>>>> Current driver supports only soc revision from offset 0.
>>>> But if you look at 5/5 you need to define offsets where information is present.
>>>> +#define SOC_VERSION_OFFSET 0x0
>>>> +#define EFUSE_START_OFFSET 0xC
>>>> +#define EFUSE_END_OFFSET 0xFC
>>>> +#define EFUSE_PUF_START_OFFSET 0x100
>>>> +#define EFUSE_PUF_MID_OFFSET 0x140
>>>> +#define EFUSE_PUF_END_OFFSET 0x17F
>>>
>>> There is nothing like this in existing driver, so the argument that "I
>>> am adding this to the binding during conversion because driver needs it"
>>> is not true. Conversion is only a conversion.
>>
>> Conversion in 2/5 is adding only soc revision which is already there. It is
>> starting from 0 and world size is 1. And 0 is not listed because that's start
>> all the time.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/nvmem/zynqmp_nvmem.c?h=v6.6-rc5#n39
>
> This defines the nvmem config, not what should be where.
If you have only one entry you are also saying where it is.
Thanks,
Michal
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 13:10 ` Krzysztof Kozlowski
2023-10-13 13:12 ` Michal Simek
@ 2023-10-16 8:01 ` Michal Simek
2023-10-16 10:06 ` Kundanala, Praveen Teja
1 sibling, 1 reply; 23+ messages in thread
From: Michal Simek @ 2023-10-16 8:01 UTC (permalink / raw)
To: Krzysztof Kozlowski, Praveen Teja Kundanala, srinivas.kandagatla,
robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 10/13/23 15:10, Krzysztof Kozlowski wrote:
> On 13/10/2023 15:06, Michal Simek wrote:
>>
>>
>> On 10/13/23 14:54, Krzysztof Kozlowski wrote:
>>> On 13/10/2023 14:08, Michal Simek wrote:
>>>>
>>>>
>>>> On 10/13/23 13:58, Krzysztof Kozlowski wrote:
>>>>> On 13/10/2023 13:51, Michal Simek wrote:
>>>>>>
>>>>>>
>>>>>> On 10/13/23 13:46, Krzysztof Kozlowski wrote:
>>>>>>> On 13/10/2023 13:22, Michal Simek wrote:
>>>>>>>>>
>>>>>>>>>> +
>>>>>>>>>> +required:
>>>>>>>>>> + - compatible
>>>>>>>>>
>>>>>>>>> required: block goes after patternProperties: block
>>>>>>>>>
>>>>>>>>>> +
>>>>>>>>>> +patternProperties:
>>>>>>>>>> + "^soc_revision@0$":
>>>>>>>>>
>>>>>>>>> Why do you define individual memory cells? Is this part of a binding?
>>>>>>>>> IOW, OS/Linux requires this?
>>>>>>>>
>>>>>>>> nvmem has in kernel interface where you can reference to nodes. nvmem_cell_get()
>>>>>>>> calls. It means you should be able to describe internal layout that's why names
>>>>>>>> are used. And address in name is there because of reg property is used to
>>>>>>>> describe base offset and size.
>>>>>>>
>>>>>>> That's not really what I am asking. Why internal layout of memory must
>>>>>>> be part of the bindings?
>>>>>>
>>>>>> It doesn't need to be but offsets are hardcoded inside the driver itself and
>>>>>> they can't be different.
>>>>>
>>>>> Hm, where? I opened drivers/nvmem/zynqmp_nvmem.c and I do not see any
>>>>> hard-coded offsets.
>>>>
>>>> Current driver supports only soc revision from offset 0.
>>>> But if you look at 5/5 you need to define offsets where information is present.
>>>> +#define SOC_VERSION_OFFSET 0x0
>>>> +#define EFUSE_START_OFFSET 0xC
>>>> +#define EFUSE_END_OFFSET 0xFC
>>>> +#define EFUSE_PUF_START_OFFSET 0x100
>>>> +#define EFUSE_PUF_MID_OFFSET 0x140
>>>> +#define EFUSE_PUF_END_OFFSET 0x17F
>>>
>>> There is nothing like this in existing driver, so the argument that "I
>>> am adding this to the binding during conversion because driver needs it"
>>> is not true. Conversion is only a conversion.
>>
>> Conversion in 2/5 is adding only soc revision which is already there. It is
>> starting from 0 and world size is 1. And 0 is not listed because that's start
>> all the time.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/nvmem/zynqmp_nvmem.c?h=v6.6-rc5#n39
>
> This defines the nvmem config, not what should be where.
>
>>
>> And soc revision was also listed in origin binding example.
>
> Example is not a binding. Please drop enforcement of some specific nodes
> from the binding.
ok. Fine.
Praveen: Please drop that descriptions about sub nodes.
Thanks,
Michal
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-16 8:01 ` Michal Simek
@ 2023-10-16 10:06 ` Kundanala, Praveen Teja
0 siblings, 0 replies; 23+ messages in thread
From: Kundanala, Praveen Teja @ 2023-10-16 10:06 UTC (permalink / raw)
To: Simek, Michal, Krzysztof Kozlowski,
srinivas.kandagatla@linaro.org, robh+dt@kernel.org,
krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
[AMD Official Use Only - General]
> -----Original Message-----
> From: Simek, Michal <michal.simek@amd.com>
> Sent: Monday, October 16, 2023 1:32 PM
> To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>; Kundanala, Praveen
> Teja <praveen.teja.kundanala@amd.com>; srinivas.kandagatla@linaro.org;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt
> to yaml
>
>
>
> On 10/13/23 15:10, Krzysztof Kozlowski wrote:
> > On 13/10/2023 15:06, Michal Simek wrote:
> >>
> >>
> >> On 10/13/23 14:54, Krzysztof Kozlowski wrote:
> >>> On 13/10/2023 14:08, Michal Simek wrote:
> >>>>
> >>>>
> >>>> On 10/13/23 13:58, Krzysztof Kozlowski wrote:
> >>>>> On 13/10/2023 13:51, Michal Simek wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 10/13/23 13:46, Krzysztof Kozlowski wrote:
> >>>>>>> On 13/10/2023 13:22, Michal Simek wrote:
> >>>>>>>>>
> >>>>>>>>>> +
> >>>>>>>>>> +required:
> >>>>>>>>>> + - compatible
> >>>>>>>>>
> >>>>>>>>> required: block goes after patternProperties: block
> >>>>>>>>>
> >>>>>>>>>> +
> >>>>>>>>>> +patternProperties:
> >>>>>>>>>> + "^soc_revision@0$":
> >>>>>>>>>
> >>>>>>>>> Why do you define individual memory cells? Is this part of a
> binding?
> >>>>>>>>> IOW, OS/Linux requires this?
> >>>>>>>>
> >>>>>>>> nvmem has in kernel interface where you can reference to nodes.
> >>>>>>>> nvmem_cell_get() calls. It means you should be able to describe
> >>>>>>>> internal layout that's why names are used. And address in name
> >>>>>>>> is there because of reg property is used to describe base offset and
> size.
> >>>>>>>
> >>>>>>> That's not really what I am asking. Why internal layout of
> >>>>>>> memory must be part of the bindings?
> >>>>>>
> >>>>>> It doesn't need to be but offsets are hardcoded inside the driver
> >>>>>> itself and they can't be different.
> >>>>>
> >>>>> Hm, where? I opened drivers/nvmem/zynqmp_nvmem.c and I do not
> see
> >>>>> any hard-coded offsets.
> >>>>
> >>>> Current driver supports only soc revision from offset 0.
> >>>> But if you look at 5/5 you need to define offsets where information is
> present.
> >>>> +#define SOC_VERSION_OFFSET 0x0
> >>>> +#define EFUSE_START_OFFSET 0xC
> >>>> +#define EFUSE_END_OFFSET 0xFC
> >>>> +#define EFUSE_PUF_START_OFFSET 0x100
> >>>> +#define EFUSE_PUF_MID_OFFSET 0x140
> >>>> +#define EFUSE_PUF_END_OFFSET 0x17F
> >>>
> >>> There is nothing like this in existing driver, so the argument that
> >>> "I am adding this to the binding during conversion because driver needs it"
> >>> is not true. Conversion is only a conversion.
> >>
> >> Conversion in 2/5 is adding only soc revision which is already there.
> >> It is starting from 0 and world size is 1. And 0 is not listed
> >> because that's start all the time.
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tr
> >> ee/drivers/nvmem/zynqmp_nvmem.c?h=v6.6-rc5#n39
> >
> > This defines the nvmem config, not what should be where.
> >
> >>
> >> And soc revision was also listed in origin binding example.
> >
> > Example is not a binding. Please drop enforcement of some specific
> > nodes from the binding.
>
> ok. Fine.
> Praveen: Please drop that descriptions about sub nodes.
>[Kundanala, Praveen Teja] Okay.
>
> Thanks,
> Michal
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 10:14 ` [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml Praveen Teja Kundanala
2023-10-13 10:30 ` Krzysztof Kozlowski
@ 2023-10-13 11:09 ` Rob Herring
2023-10-13 16:23 ` Rob Herring
2 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2023-10-13 11:09 UTC (permalink / raw)
To: Praveen Teja Kundanala
Cc: devicetree, conor+dt, krzysztof.kozlowski+dt, linux-kernel,
robh+dt, michal.simek, linux-arm-kernel, srinivas.kandagatla
On Fri, 13 Oct 2023 15:44:47 +0530, Praveen Teja Kundanala wrote:
> Convert the xlnx,zynqmp-nvmem.txt to yaml.
>
> Signed-off-by: Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
> ---
> .../bindings/nvmem/xlnx,zynqmp-nvmem.txt | 46 ---------------
> .../bindings/nvmem/xlnx,zynqmp-nvmem.yaml | 59 +++++++++++++++++++
> 2 files changed, 59 insertions(+), 46 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
> create mode 100644 Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml:18:11: [error] string value is redundantly quoted with any quotes (quoted-strings)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231013101450.573-3-praveen.teja.kundanala@amd.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
2023-10-13 10:14 ` [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml Praveen Teja Kundanala
2023-10-13 10:30 ` Krzysztof Kozlowski
2023-10-13 11:09 ` Rob Herring
@ 2023-10-13 16:23 ` Rob Herring
2 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2023-10-13 16:23 UTC (permalink / raw)
To: Praveen Teja Kundanala
Cc: srinivas.kandagatla, krzysztof.kozlowski+dt, conor+dt,
michal.simek, devicetree, linux-arm-kernel, linux-kernel
On Fri, Oct 13, 2023 at 03:44:47PM +0530, Praveen Teja Kundanala wrote:
> Convert the xlnx,zynqmp-nvmem.txt to yaml.
>
> Signed-off-by: Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
> ---
> .../bindings/nvmem/xlnx,zynqmp-nvmem.txt | 46 ---------------
> .../bindings/nvmem/xlnx,zynqmp-nvmem.yaml | 59 +++++++++++++++++++
> 2 files changed, 59 insertions(+), 46 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
> create mode 100644 Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
>
> diff --git a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
> deleted file mode 100644
> index 4881561b3a02..000000000000
> --- a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.txt
> +++ /dev/null
> @@ -1,46 +0,0 @@
> ---------------------------------------------------------------------------
> -= Zynq UltraScale+ MPSoC nvmem firmware driver binding =
> ---------------------------------------------------------------------------
> -The nvmem_firmware node provides access to the hardware related data
> -like soc revision, IDCODE... etc, By using the firmware interface.
> -
> -Required properties:
> -- compatible: should be "xlnx,zynqmp-nvmem-fw"
> -
> -= Data cells =
> -Are child nodes of silicon id, bindings of which as described in
> -bindings/nvmem/nvmem.txt
> -
> --------
> - Example
> --------
> -firmware {
> - zynqmp_firmware: zynqmp-firmware {
> - compatible = "xlnx,zynqmp-firmware";
> - method = "smc";
> -
> - nvmem_firmware {
> - compatible = "xlnx,zynqmp-nvmem-fw";
> - #address-cells = <1>;
> - #size-cells = <1>;
> -
> - /* Data cells */
> - soc_revision: soc_revision {
> - reg = <0x0 0x4>;
> - };
> - };
> - };
> -};
> -
> -= Data consumers =
> -Are device nodes which consume nvmem data cells.
> -
> -For example:
> - pcap {
> - ...
> -
> - nvmem-cells = <&soc_revision>;
> - nvmem-cell-names = "soc_revision";
> -
> - ...
> - };
> diff --git a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
> new file mode 100644
> index 000000000000..e03ed8c32537
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/xlnx,zynqmp-nvmem.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Zynq UltraScale+ MPSoC Non Volatile Memory interface
> +
> +description: |
> + The ZynqMP MPSoC provides access to the hardware related data
> + like SOC revision, IDCODE.
> +
> +maintainers:
> + - Kalyani Akula <kalyani.akula@amd.com>
> + - Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
> +
> +allOf:
> + - $ref: "nvmem.yaml#"
> +
> +properties:
> + compatible:
> + const: xlnx,zynqmp-nvmem-fw
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 1
> +
> +required:
> + - compatible
> +
> +patternProperties:
> + "^soc_revision@0$":
Fixed string, not a pattern. dtschema will now flag this. Thanks for the
test case. ;)
> + type: object
Needs 'additionalProperties' or...
> + description:
> + This node is used to read SOC version and IDCODE of ZynqMP. Read-only.
> +
> + properties:
> + reg:
> + maxItems: 1
> +
> + required:
> + - reg
fixed-cell.yaml (via nvmem.yaml) already says this, so this can all be
dropped. Except that this[1] just landed, so you will need to adapt to
it.
Though there is an issue that fixed-cell.yaml doesn't restrict the
allowed properties, so that needs to happen somewhere... Not sure what
the fix is. Hopefully fixed-cell.yaml can just be changed to
'additionalProperties: false', but need to see if other properties are
in use.
Rob
[1] https://lore.kernel.org/all/169667333484.74178.7121029453685069845.b4-ty@linaro.org/
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/5] dt-bindings: nvmem: Add nodes for ZynqMP efuses
2023-10-13 10:14 [PATCH 0/5] Add ZynqMP efuse access support Praveen Teja Kundanala
2023-10-13 10:14 ` [PATCH 1/5] firmware: xilinx: Add ZynqMP efuse access API Praveen Teja Kundanala
2023-10-13 10:14 ` [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml Praveen Teja Kundanala
@ 2023-10-13 10:14 ` Praveen Teja Kundanala
2023-10-13 10:31 ` Krzysztof Kozlowski
2023-10-13 10:14 ` [PATCH 4/5] arm64: zynqmp: Add ZynqnMP nvmem nodes Praveen Teja Kundanala
2023-10-13 10:14 ` [PATCH 5/5] nvmem: zynqmp_nvmem: Add support to access efuse Praveen Teja Kundanala
4 siblings, 1 reply; 23+ messages in thread
From: Praveen Teja Kundanala @ 2023-10-13 10:14 UTC (permalink / raw)
To: srinivas.kandagatla, robh+dt, krzysztof.kozlowski+dt, conor+dt,
michal.simek, devicetree, linux-arm-kernel
Cc: linux-kernel
Added nodes for ZynqMP specific purpose and PUF user efuses
Signed-off-by: Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
---
.../bindings/nvmem/xlnx,zynqmp-nvmem.yaml | 213 +++++++++++++++++-
1 file changed, 212 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
index e03ed8c32537..d2a036a80cda 100644
--- a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
@@ -8,7 +8,7 @@ title: Zynq UltraScale+ MPSoC Non Volatile Memory interface
description: |
The ZynqMP MPSoC provides access to the hardware related data
- like SOC revision, IDCODE.
+ like SOC revision, IDCODE and specific purpose efuses.
maintainers:
- Kalyani Akula <kalyani.akula@amd.com>
@@ -43,6 +43,140 @@ patternProperties:
required:
- reg
+ "^efuse_dna@c$":
+ type: object
+ description:
+ This node is used to read DNA of ZynqMP SOC. Read-only.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+ "^efuse_usr(0@20|1@24|2@28|3@2c|4@30|5@34|6@38|7@3c)$":
+ type: object
+ description:
+ Eight 32-bit user efuses. Read and Write is supported.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+ "^efuse_miscusr@40$":
+ type: object
+ description:
+ 32-bit MISC user efuse space. Read and Write is supported.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+ "^efuse_chash@50$":
+ type: object
+ description:
+ 32-bit PUF chash space. Read and Write is supported.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+ "^efuse_pufmisc@54$":
+ type: object
+ description:
+ 32-bit PUF MISC control space. Read and Write is supported.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+ "^efuse_sec@58$":
+ type: object
+ description:
+ 32-bit secure control space. Read and Write is supported.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+ "^efuse_spkid@5c$":
+ type: object
+ description:
+ 32-bit SPK ID. Read and Write is supported.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+ "^efuse_aeskey@60$":
+ type: object
+ description:
+ 256-bit aes key. Only Write is supported.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+ "^efuse_ppk0hash@a0$":
+ type: object
+ description:
+ 384-bit PPK0 hash. Read and Write is supported.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+ "^efuse_ppk1hash@d0$":
+ type: object
+ description:
+ 384-bit PPK1 hash. Read and Write is supported.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+ "^efuse_pufuser@100$":
+ type: object
+ description:
+ This node represents the 127(0x7F) 32-bit PUF(Physical Unclonable Function)
+ helper data efuses which are repurposed as user fuses.
+ Read and Write is supported.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
additionalProperties: false
examples:
@@ -56,4 +190,81 @@ examples:
soc_revision: soc_revision@0 {
reg = <0x0 0x4>;
};
+ /*
+ * efuse memory access:
+ * all the efuse fields need to be read
+ * with the exact size specified in the node
+ */
+ /* DNA */
+ efuse_dna: efuse_dna@c {
+ reg = <0xc 0xc>;
+ };
+ /* User 0 */
+ efuse_usr0: efuse_usr0@20 {
+ reg = <0x20 0x4>;
+ };
+ /* User 1 */
+ efuse_usr1: efuse_usr1@24 {
+ reg = <0x24 0x4>;
+ };
+ /* User 2 */
+ efuse_usr2: efuse_usr2@28 {
+ reg = <0x28 0x4>;
+ };
+ /* User 3 */
+ efuse_usr3: efuse_usr3@2c {
+ reg = <0x2c 0x4>;
+ };
+ /* User 4 */
+ efuse_usr4: efuse_usr4@30 {
+ reg = <0x30 0x4>;
+ };
+ /* User 5 */
+ efuse_usr5: efuse_usr5@34 {
+ reg = <0x34 0x4>;
+ };
+ /* User 6 */
+ efuse_usr6: efuse_usr6@38 {
+ reg = <0x38 0x4>;
+ };
+ /* User 7 */
+ efuse_usr7: efuse_usr7@3c {
+ reg = <0x3c 0x4>;
+ };
+ /* Misc user control bits */
+ efuse_miscusr: efuse_miscusr@40 {
+ reg = <0x40 0x4>;
+ };
+ /* PUF chash */
+ efuse_chash: efuse_chash@50 {
+ reg = <0x50 0x4>;
+ };
+ /* PUF misc */
+ efuse_pufmisc: efuse_pufmisc@54 {
+ reg = <0x54 0x4>;
+ };
+ /* SEC_CTRL */
+ efuse_sec: efuse_sec@58 {
+ reg = <0x58 0x4>;
+ };
+ /* SPK ID */
+ efuse_spkid: efuse_spkid@5c {
+ reg = <0x5c 0x4>;
+ };
+ /* AES Key */
+ efuse_aeskey: efuse_aeskey@60 {
+ reg = <0x60 0x20>;
+ };
+ /* PPK0 hash */
+ efuse_ppk0hash: efuse_ppk0hash@a0 {
+ reg = <0xa0 0x30>;
+ };
+ /* PPK1 hash */
+ efuse_ppk1hash: efuse_ppk1hash@d0 {
+ reg = <0xd0 0x30>;
+ };
+ /* PUF user fuses */
+ efuse_pufuser: efuse_pufuser@100 {
+ reg = <0x100 0x7F>;
+ };
};
--
2.36.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 3/5] dt-bindings: nvmem: Add nodes for ZynqMP efuses
2023-10-13 10:14 ` [PATCH 3/5] dt-bindings: nvmem: Add nodes for ZynqMP efuses Praveen Teja Kundanala
@ 2023-10-13 10:31 ` Krzysztof Kozlowski
0 siblings, 0 replies; 23+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-13 10:31 UTC (permalink / raw)
To: Praveen Teja Kundanala, srinivas.kandagatla, robh+dt,
krzysztof.kozlowski+dt, conor+dt, michal.simek, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 13/10/2023 12:14, Praveen Teja Kundanala wrote:
> Added nodes for ZynqMP specific purpose and PUF user efuses
Why?
>
> Signed-off-by: Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
> ---
> .../bindings/nvmem/xlnx,zynqmp-nvmem.yaml | 213 +++++++++++++++++-
> 1 file changed, 212 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
> index e03ed8c32537..d2a036a80cda 100644
> --- a/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
> @@ -8,7 +8,7 @@ title: Zynq UltraScale+ MPSoC Non Volatile Memory interface
>
> description: |
> The ZynqMP MPSoC provides access to the hardware related data
> - like SOC revision, IDCODE.
> + like SOC revision, IDCODE and specific purpose efuses.
>
> maintainers:
> - Kalyani Akula <kalyani.akula@amd.com>
> @@ -43,6 +43,140 @@ patternProperties:
> required:
> - reg
>
> + "^efuse_dna@c$":
No, no underscores in node names. From where did you get this pattern?
Which upstream code has it?
All this and further parts are questionable. I don't understand why do
you do it.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/5] arm64: zynqmp: Add ZynqnMP nvmem nodes
2023-10-13 10:14 [PATCH 0/5] Add ZynqMP efuse access support Praveen Teja Kundanala
` (2 preceding siblings ...)
2023-10-13 10:14 ` [PATCH 3/5] dt-bindings: nvmem: Add nodes for ZynqMP efuses Praveen Teja Kundanala
@ 2023-10-13 10:14 ` Praveen Teja Kundanala
2023-10-13 10:32 ` Krzysztof Kozlowski
2023-10-13 10:14 ` [PATCH 5/5] nvmem: zynqmp_nvmem: Add support to access efuse Praveen Teja Kundanala
4 siblings, 1 reply; 23+ messages in thread
From: Praveen Teja Kundanala @ 2023-10-13 10:14 UTC (permalink / raw)
To: srinivas.kandagatla, robh+dt, krzysztof.kozlowski+dt, conor+dt,
michal.simek, devicetree, linux-arm-kernel
Cc: linux-kernel
Add nvmem DT nodes for ZynqMP SOC
Signed-off-by: Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
---
arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 55 ++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index 02cfcc716936..b8807dcce442 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -190,6 +190,61 @@ nvmem_firmware {
soc_revision: soc_revision@0 {
reg = <0x0 0x4>;
};
+ /* efuse access */
+ efuse_dna: efuse_dna@c {
+ reg = <0xc 0xc>;
+ };
+ efuse_usr0: efuse_usr0@20 {
+ reg = <0x20 0x4>;
+ };
+ efuse_usr1: efuse_usr1@24 {
+ reg = <0x24 0x4>;
+ };
+ efuse_usr2: efuse_usr2@28 {
+ reg = <0x28 0x4>;
+ };
+ efuse_usr3: efuse_usr3@2c {
+ reg = <0x2c 0x4>;
+ };
+ efuse_usr4: efuse_usr4@30 {
+ reg = <0x30 0x4>;
+ };
+ efuse_usr5: efuse_usr5@34 {
+ reg = <0x34 0x4>;
+ };
+ efuse_usr6: efuse_usr6@38 {
+ reg = <0x38 0x4>;
+ };
+ efuse_usr7: efuse_usr7@3c {
+ reg = <0x3c 0x4>;
+ };
+ efuse_miscusr: efuse_miscusr@40 {
+ reg = <0x40 0x4>;
+ };
+ efuse_chash: efuse_chash@50 {
+ reg = <0x50 0x4>;
+ };
+ efuse_pufmisc: efuse_pufmisc@54 {
+ reg = <0x54 0x4>;
+ };
+ efuse_sec: efuse_sec@58 {
+ reg = <0x58 0x4>;
+ };
+ efuse_spkid: efuse_spkid@5c {
+ reg = <0x5c 0x4>;
+ };
+ efuse_aeskey: efuse_aeskey@60 {
+ reg = <0x60 0x20>;
+ };
+ efuse_ppk0hash: efuse_ppk0hash@a0 {
+ reg = <0xa0 0x30>;
+ };
+ efuse_ppk1hash: efuse_ppk1hash@d0 {
+ reg = <0xd0 0x30>;
+ };
+ efuse_pufuser: efuse_pufuser@100 {
+ reg = <0x100 0x7F>;
+ };
};
zynqmp_pcap: pcap {
--
2.36.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 4/5] arm64: zynqmp: Add ZynqnMP nvmem nodes
2023-10-13 10:14 ` [PATCH 4/5] arm64: zynqmp: Add ZynqnMP nvmem nodes Praveen Teja Kundanala
@ 2023-10-13 10:32 ` Krzysztof Kozlowski
2023-10-13 11:18 ` Michal Simek
0 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-13 10:32 UTC (permalink / raw)
To: Praveen Teja Kundanala, srinivas.kandagatla, robh+dt,
krzysztof.kozlowski+dt, conor+dt, michal.simek, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 13/10/2023 12:14, Praveen Teja Kundanala wrote:
> Add nvmem DT nodes for ZynqMP SOC
>
> Signed-off-by: Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
> ---
> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 55 ++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> index 02cfcc716936..b8807dcce442 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> @@ -190,6 +190,61 @@ nvmem_firmware {
> soc_revision: soc_revision@0 {
> reg = <0x0 0x4>;
> };
> + /* efuse access */
> + efuse_dna: efuse_dna@c {
No underscores in node names. I see now from where did you get the
initial pattern. Would be great if you fixed Xilinx DTS :/
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 4/5] arm64: zynqmp: Add ZynqnMP nvmem nodes
2023-10-13 10:32 ` Krzysztof Kozlowski
@ 2023-10-13 11:18 ` Michal Simek
0 siblings, 0 replies; 23+ messages in thread
From: Michal Simek @ 2023-10-13 11:18 UTC (permalink / raw)
To: Krzysztof Kozlowski, Praveen Teja Kundanala, srinivas.kandagatla,
robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-arm-kernel
Cc: linux-kernel
On 10/13/23 12:32, Krzysztof Kozlowski wrote:
> On 13/10/2023 12:14, Praveen Teja Kundanala wrote:
>> Add nvmem DT nodes for ZynqMP SOC
>>
>> Signed-off-by: Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
>> ---
>> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 55 ++++++++++++++++++++++++++
>> 1 file changed, 55 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> index 02cfcc716936..b8807dcce442 100644
>> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> @@ -190,6 +190,61 @@ nvmem_firmware {
>> soc_revision: soc_revision@0 {
>> reg = <0x0 0x4>;
>> };
>> + /* efuse access */
>> + efuse_dna: efuse_dna@c {
>
> No underscores in node names. I see now from where did you get the
> initial pattern. Would be great if you fixed Xilinx DTS :/
I actually fixed soc-revision here.
https://lore.kernel.org/all/5137958580c85a35cf6aadd1c33a2f6bcf81a9e5.1695040866.git.michal.simek@amd.com/
Thanks,
Michal
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 5/5] nvmem: zynqmp_nvmem: Add support to access efuse
2023-10-13 10:14 [PATCH 0/5] Add ZynqMP efuse access support Praveen Teja Kundanala
` (3 preceding siblings ...)
2023-10-13 10:14 ` [PATCH 4/5] arm64: zynqmp: Add ZynqnMP nvmem nodes Praveen Teja Kundanala
@ 2023-10-13 10:14 ` Praveen Teja Kundanala
4 siblings, 0 replies; 23+ messages in thread
From: Praveen Teja Kundanala @ 2023-10-13 10:14 UTC (permalink / raw)
To: srinivas.kandagatla, robh+dt, krzysztof.kozlowski+dt, conor+dt,
michal.simek, devicetree, linux-arm-kernel
Cc: linux-kernel
Add support to read/write efuse memory map of ZynqMP
Below are the offsets of ZynqMP efuse memory map
0 - SOC version(read only)
0xC - 0xFC -ZynqMP specific purpose efuses
0x100 - 0x17F - Physical Unclonable Function(PUF)
efuses repurposed as user efuses
Signed-off-by: Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>
---
drivers/nvmem/zynqmp_nvmem.c | 216 ++++++++++++++++++++++++++++++-----
1 file changed, 185 insertions(+), 31 deletions(-)
diff --git a/drivers/nvmem/zynqmp_nvmem.c b/drivers/nvmem/zynqmp_nvmem.c
index f49bb9a26d05..e6123a32268a 100644
--- a/drivers/nvmem/zynqmp_nvmem.c
+++ b/drivers/nvmem/zynqmp_nvmem.c
@@ -1,8 +1,10 @@
// SPDX-License-Identifier: GPL-2.0+
/*
* Copyright (C) 2019 Xilinx, Inc.
+ * Copyright (C), 2022 - 2023 Advanced Micro Devices, Inc.
*/
+#include <linux/dma-mapping.h>
#include <linux/module.h>
#include <linux/nvmem-provider.h>
#include <linux/of.h>
@@ -10,36 +12,190 @@
#include <linux/firmware/xlnx-zynqmp.h>
#define SILICON_REVISION_MASK 0xF
+#define P_USER_0_64_UPPER_MASK GENMASK(31, 16)
+#define P_USER_127_LOWER_4_BIT_MASK GENMASK(3, 0)
+#define WORD_INBYTES 4
+#define SOC_VER_SIZE 0x4
+#define EFUSE_MEMORY_SIZE 0x177
+#define UNUSED_SPACE 0x8
+#define ZYNQMP_NVMEM_SIZE (SOC_VER_SIZE + UNUSED_SPACE + \
+ EFUSE_MEMORY_SIZE)
+#define SOC_VERSION_OFFSET 0x0
+#define EFUSE_START_OFFSET 0xC
+#define EFUSE_END_OFFSET 0xFC
+#define EFUSE_PUF_START_OFFSET 0x100
+#define EFUSE_PUF_MID_OFFSET 0x140
+#define EFUSE_PUF_END_OFFSET 0x17F
+#define EFUSE_NOT_ENABLED 29
-struct zynqmp_nvmem_data {
- struct device *dev;
- struct nvmem_device *nvmem;
+/*
+ * efuse access type
+ */
+enum efuse_access {
+ EFUSE_READ = 0,
+ EFUSE_WRITE
+};
+
+/**
+ * struct xilinx_efuse - the basic structure
+ * @src: address of the buffer to store the data to be write/read
+ * @size: read/write word count
+ * @offset: read/write offset
+ * @flag: 0 - represents efuse read and 1- represents efuse write
+ * @pufuserfuse:0 - represents non-puf efuses, offset is used for read/write
+ * 1 - represents puf user fuse row number.
+ *
+ * this structure stores all the required details to
+ * read/write efuse memory.
+ */
+struct xilinx_efuse {
+ u64 src;
+ u32 size;
+ u32 offset;
+ enum efuse_access flag;
+ u32 pufuserfuse;
};
-static int zynqmp_nvmem_read(void *context, unsigned int offset,
- void *val, size_t bytes)
+static int zynqmp_efuse_access(void *context, unsigned int offset,
+ void *val, size_t bytes, enum efuse_access flag,
+ unsigned int pufflag)
{
+ struct device *dev = context;
+ struct xilinx_efuse *efuse;
+ dma_addr_t dma_addr;
+ dma_addr_t dma_buf;
+ size_t words = bytes / WORD_INBYTES;
int ret;
- int idcode, version;
- struct zynqmp_nvmem_data *priv = context;
-
- ret = zynqmp_pm_get_chipid(&idcode, &version);
- if (ret < 0)
- return ret;
+ int value;
+ char *data;
+
+ if (bytes % WORD_INBYTES != 0) {
+ dev_err(dev, "Bytes requested should be word aligned\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (pufflag == 0 && offset % WORD_INBYTES) {
+ dev_err(dev, "Offset requested should be word aligned\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (pufflag == 1 && flag == EFUSE_WRITE) {
+ memcpy(&value, val, bytes);
+ if ((offset == EFUSE_PUF_START_OFFSET ||
+ offset == EFUSE_PUF_MID_OFFSET) &&
+ value & P_USER_0_64_UPPER_MASK) {
+ dev_err(dev, "Only lower 4 bytes are allowed to be programmed in P_USER_0 & P_USER_64\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (offset == EFUSE_PUF_END_OFFSET &&
+ (value & P_USER_127_LOWER_4_BIT_MASK)) {
+ dev_err(dev, "Only MSB 28 bits are allowed to be programmed for P_USER_127\n");
+ return -EOPNOTSUPP;
+ }
+ }
+
+ efuse = dma_alloc_coherent(dev, sizeof(struct xilinx_efuse),
+ &dma_addr, GFP_KERNEL);
+ if (!efuse)
+ return -ENOMEM;
- dev_dbg(priv->dev, "Read chipid val %x %x\n", idcode, version);
- *(int *)val = version & SILICON_REVISION_MASK;
+ data = dma_alloc_coherent(dev, sizeof(bytes),
+ &dma_buf, GFP_KERNEL);
+ if (!data) {
+ ret = -ENOMEM;
+ goto efuse_data_fail;
+ }
+
+ if (flag == EFUSE_WRITE) {
+ memcpy(data, val, bytes);
+ efuse->flag = EFUSE_WRITE;
+ } else {
+ efuse->flag = EFUSE_READ;
+ }
+
+ efuse->src = dma_buf;
+ efuse->size = words;
+ efuse->offset = offset;
+ efuse->pufuserfuse = pufflag;
+
+ zynqmp_pm_efuse_access(dma_addr, (u32 *)&ret);
+ if (ret != 0) {
+ if (ret == EFUSE_NOT_ENABLED) {
+ dev_err(dev, "efuse access is not enabled\n");
+ ret = -EOPNOTSUPP;
+ } else {
+ dev_err(dev, "Error in efuse read %x\n", ret);
+ ret = -EPERM;
+ }
+ goto efuse_access_err;
+ }
+
+ if (flag == EFUSE_READ)
+ memcpy(val, data, bytes);
+efuse_access_err:
+ dma_free_coherent(dev, sizeof(bytes),
+ data, dma_buf);
+efuse_data_fail:
+ dma_free_coherent(dev, sizeof(struct xilinx_efuse),
+ efuse, dma_addr);
+
+ return ret;
+}
- return 0;
+static int zynqmp_nvmem_read(void *context, unsigned int offset, void *val, size_t bytes)
+{
+ struct device *dev = context;
+ int ret;
+ int pufflag = 0;
+ int idcode;
+ int version;
+
+ if (offset >= EFUSE_PUF_START_OFFSET && offset <= EFUSE_PUF_END_OFFSET)
+ pufflag = 1;
+
+ switch (offset) {
+ /* Soc version offset is zero */
+ case SOC_VERSION_OFFSET:
+ if (bytes != SOC_VER_SIZE)
+ return -EOPNOTSUPP;
+
+ ret = zynqmp_pm_get_chipid((u32 *)&idcode, (u32 *)&version);
+ if (ret < 0)
+ return ret;
+
+ dev_dbg(dev, "Read chipid val %x %x\n", idcode, version);
+ *(int *)val = version & SILICON_REVISION_MASK;
+ break;
+ /* Efuse offset starts from 0xc */
+ case EFUSE_START_OFFSET ... EFUSE_END_OFFSET:
+ case EFUSE_PUF_START_OFFSET ... EFUSE_PUF_END_OFFSET:
+ ret = zynqmp_efuse_access(context, offset, val,
+ bytes, EFUSE_READ, pufflag);
+ break;
+ default:
+ *(u32 *)val = 0xDEADBEEF;
+ ret = 0;
+ break;
+ }
+
+ return ret;
}
-static struct nvmem_config econfig = {
- .name = "zynqmp-nvmem",
- .owner = THIS_MODULE,
- .word_size = 1,
- .size = 1,
- .read_only = true,
-};
+static int zynqmp_nvmem_write(void *context,
+ unsigned int offset, void *val, size_t bytes)
+{
+ int pufflag = 0;
+
+ if (offset < EFUSE_START_OFFSET || offset > EFUSE_PUF_END_OFFSET)
+ return -EOPNOTSUPP;
+
+ if (offset >= EFUSE_PUF_START_OFFSET && offset <= EFUSE_PUF_END_OFFSET)
+ pufflag = 1;
+
+ return zynqmp_efuse_access(context, offset,
+ val, bytes, EFUSE_WRITE, pufflag);
+}
static const struct of_device_id zynqmp_nvmem_match[] = {
{ .compatible = "xlnx,zynqmp-nvmem-fw", },
@@ -50,20 +206,18 @@ MODULE_DEVICE_TABLE(of, zynqmp_nvmem_match);
static int zynqmp_nvmem_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct zynqmp_nvmem_data *priv;
-
- priv = devm_kzalloc(dev, sizeof(struct zynqmp_nvmem_data), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
+ struct nvmem_config econfig = {};
- priv->dev = dev;
+ econfig.name = "zynqmp-nvmem";
+ econfig.owner = THIS_MODULE;
+ econfig.word_size = 1;
+ econfig.size = ZYNQMP_NVMEM_SIZE;
econfig.dev = dev;
+ econfig.priv = dev;
econfig.reg_read = zynqmp_nvmem_read;
- econfig.priv = priv;
-
- priv->nvmem = devm_nvmem_register(dev, &econfig);
+ econfig.reg_write = zynqmp_nvmem_write;
- return PTR_ERR_OR_ZERO(priv->nvmem);
+ return PTR_ERR_OR_ZERO(devm_nvmem_register(dev, &econfig));
}
static struct platform_driver zynqmp_nvmem_driver = {
--
2.36.1
^ permalink raw reply related [flat|nested] 23+ messages in thread