diff mbox series

[v3] arm64: dts: qcom: Add industrial mezzanine support for qcs6490-rb3gen2

Message ID 20250122101424.1810844-1-quic_nkumarsi@quicinc.com
State New
Headers show
Series [v3] arm64: dts: qcom: Add industrial mezzanine support for qcs6490-rb3gen2 | expand

Commit Message

Nirmesh Kumar Singh (Temp) Jan. 22, 2025, 10:14 a.m. UTC
Add DTS support for Qualcomm qcs6490-rb3gen2 industrial mezzanine board.

Signed-off-by: Sahil Chandna <quic_chandna@quicinc.com>
Signed-off-by: Nirmesh Kumar Singh <quic_nkumarsi@quicinc.com>

---
Changes in v3:
- Fixed tpm pinctrl node label.
- Addressed comments by Dmitry.
- Improved indentation/formatting.
- Link to V2: https://lore.kernel.org/all/20250102190155.2593453-1-quic_nkumarsi@quicinc.com/

Changes in V2:
- Addressed comment by Konrad.
- Validated dts bindings with dtb_checks suggested by Krzysztof.
- Improved indentation/formatting.
- Fixed bug encountered during testing.
- Added dtb entry in makefile.
- Link to V1: https://lore.kernel.org/all/20241206065156.2573-1-quic_chandna@quicinc.com/
---
 arch/arm64/boot/dts/qcom/Makefile             |  4 +++
 .../qcs6490-rb3gen2-industrial-mezzanine.dtso | 35 +++++++++++++++++++
 2 files changed, 39 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-industrial-mezzanine.dtso

Comments

Nirmesh Kumar Singh (Temp) Jan. 23, 2025, 11:05 a.m. UTC | #1
On 1/23/2025 12:16 AM, Dmitry Baryshkov wrote:
> On Wed, Jan 22, 2025 at 03:44:24PM +0530, Nirmesh Kumar Singh wrote:
>> Add DTS support for Qualcomm qcs6490-rb3gen2 industrial mezzanine board.
>>
>> Signed-off-by: Sahil Chandna <quic_chandna@quicinc.com>
>> Signed-off-by: Nirmesh Kumar Singh <quic_nkumarsi@quicinc.com>
>>
>> ---
>> Changes in v3:
>> - Fixed tpm pinctrl node label.
>> - Addressed comments by Dmitry.
> Which ones? Pleas be more specific in changelogs.
Ack
>
>> - Improved indentation/formatting.
>> - Link to V2: https://lore.kernel.org/all/20250102190155.2593453-1-quic_nkumarsi@quicinc.com/
>>
>> Changes in V2:
>> - Addressed comment by Konrad.
>> - Validated dts bindings with dtb_checks suggested by Krzysztof.
>> - Improved indentation/formatting.
>> - Fixed bug encountered during testing.
>> - Added dtb entry in makefile.
>> - Link to V1: https://lore.kernel.org/all/20241206065156.2573-1-quic_chandna@quicinc.com/
>> ---
>>   arch/arm64/boot/dts/qcom/Makefile             |  4 +++
>>   .../qcs6490-rb3gen2-industrial-mezzanine.dtso | 35 +++++++++++++++++++
>>   2 files changed, 39 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-industrial-mezzanine.dtso
>>
>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>> index 6ca8db4b8afe..16ac008c58d2 100644
>> --- a/arch/arm64/boot/dts/qcom/Makefile
>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>> @@ -111,6 +111,10 @@ dtb-$(CONFIG_ARCH_QCOM)	+= qcm6490-shift-otter.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-1000.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-4000.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= qcs6490-rb3gen2.dtb
>> +
>> +qcs6490-rb3gen2-industrial-mezzanine-dtbs	:= qcs6490-rb3gen2.dtb qcs6490-rb3gen2-industrial-mezzanine.dtbo
>> +
>> +dtb-$(CONFIG_ARCH_QCOM)	+= qcs6490-rb3gen2-industrial-mezzanine.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= qcs8550-aim300-aiot.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3.dtb
>> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-industrial-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-industrial-mezzanine.dtso
>> new file mode 100644
>> index 000000000000..1498f32bd069
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-industrial-mezzanine.dtso
>> @@ -0,0 +1,35 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +/*
>> + * Copyright (c) 2025, Qualcomm Innovation Center, Inc. All rights reserved.
>> +*/
>> +
>> +/dts-v1/;
>> +/plugin/;
>> +#include <dt-bindings/clock/qcom,gcc-sc7280.h>
>> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
>> +
>> +&pm7250b_gpios {
>> +	tpm_spi_reset: tpm-spi-reset-state {
>> +		pins = "gpio5";
>> +		function = "normal";
>> +		power-source = <1>;
>> +		output-high;
>> +		input-disable;
>> +		bias-pull-up;
>> +		qcom,drive-strength = <3>;
>> +	};
>> +};
>> +
>> +&spi11 {
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	status = "okay";
>> +
>> +	st33htpm0: tpm@0 {
>> +		compatible = "st,st33htpm-spi", "tcg,tpm_tis-spi";
>> +		reg = <0>;
>> +		spi-max-frequency = <20000000>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&tpm_spi_reset>;
> Missing reset-gpios property. Otherwise there is no point in specifying
> the pinctrl.
The community previously rejected the GPIO reset function in the TPM 
driver (tpm_tis_core.c). You can refer to the discussion [1].

 From what I understand from the discussion in the patch, this decision 
was made to prevent software from executing an incorrect reset sequence, 
which could potentially reset the PCR banks of TPM chip.

However, a pinctrl node is necessary to ensure the PMIC GPIO is in the 
correct state as required by the TPM chip.

[1] 
https://lore.kernel.org/lkml/20220407111849.5676-1-LinoSanfilippo@gmx.de/T/#m726d477dbce48c9e345e245f93d60f0aaa6f0994

Thanks,

Nirmesh

>
>> +	};
>> +};
>> -- 
>> 2.34.1
>>
Nirmesh Kumar Singh (Temp) March 12, 2025, 11:08 a.m. UTC | #2
On 1/23/2025 4:48 PM, Dmitry Baryshkov wrote:
> On Thu, Jan 23, 2025 at 04:35:34PM +0530, Nirmesh Kumar Singh (Temp) wrote:
>> On 1/23/2025 12:16 AM, Dmitry Baryshkov wrote:
>>> On Wed, Jan 22, 2025 at 03:44:24PM +0530, Nirmesh Kumar Singh wrote:
>>>> Add DTS support for Qualcomm qcs6490-rb3gen2 industrial mezzanine board.
>>>>
>>>> Signed-off-by: Sahil Chandna <quic_chandna@quicinc.com>
>>>> Signed-off-by: Nirmesh Kumar Singh <quic_nkumarsi@quicinc.com>
>>>>
>>>> ---
>>>> Changes in v3:
>>>> - Fixed tpm pinctrl node label.
>>>> - Addressed comments by Dmitry.
>>> Which ones? Pleas be more specific in changelogs.
>> Ack
>>>> - Improved indentation/formatting.
>>>> - Link to V2: https://lore.kernel.org/all/20250102190155.2593453-1-quic_nkumarsi@quicinc.com/
>>>>
>>>> Changes in V2:
>>>> - Addressed comment by Konrad.
>>>> - Validated dts bindings with dtb_checks suggested by Krzysztof.
>>>> - Improved indentation/formatting.
>>>> - Fixed bug encountered during testing.
>>>> - Added dtb entry in makefile.
>>>> - Link to V1: https://lore.kernel.org/all/20241206065156.2573-1-quic_chandna@quicinc.com/
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/Makefile             |  4 +++
>>>>    .../qcs6490-rb3gen2-industrial-mezzanine.dtso | 35 +++++++++++++++++++
>>>>    2 files changed, 39 insertions(+)
>>>>    create mode 100644 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-industrial-mezzanine.dtso
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>>>> index 6ca8db4b8afe..16ac008c58d2 100644
>>>> --- a/arch/arm64/boot/dts/qcom/Makefile
>>>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>>>> @@ -111,6 +111,10 @@ dtb-$(CONFIG_ARCH_QCOM)	+= qcm6490-shift-otter.dtb
>>>>    dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-1000.dtb
>>>>    dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-4000.dtb
>>>>    dtb-$(CONFIG_ARCH_QCOM)	+= qcs6490-rb3gen2.dtb
>>>> +
>>>> +qcs6490-rb3gen2-industrial-mezzanine-dtbs	:= qcs6490-rb3gen2.dtb qcs6490-rb3gen2-industrial-mezzanine.dtbo
>>>> +
>>>> +dtb-$(CONFIG_ARCH_QCOM)	+= qcs6490-rb3gen2-industrial-mezzanine.dtb
>>>>    dtb-$(CONFIG_ARCH_QCOM)	+= qcs8550-aim300-aiot.dtb
>>>>    dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride.dtb
>>>>    dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3.dtb
>>>> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-industrial-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-industrial-mezzanine.dtso
>>>> new file mode 100644
>>>> index 000000000000..1498f32bd069
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-industrial-mezzanine.dtso
>>>> @@ -0,0 +1,35 @@
>>>> +// SPDX-License-Identifier: BSD-3-Clause
>>>> +/*
>>>> + * Copyright (c) 2025, Qualcomm Innovation Center, Inc. All rights reserved.
>>>> +*/
>>>> +
>>>> +/dts-v1/;
>>>> +/plugin/;
>>>> +#include <dt-bindings/clock/qcom,gcc-sc7280.h>
>>>> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
>>>> +
>>>> +&pm7250b_gpios {
>>>> +	tpm_spi_reset: tpm-spi-reset-state {
>>>> +		pins = "gpio5";
>>>> +		function = "normal";
>>>> +		power-source = <1>;
>>>> +		output-high;
>>>> +		input-disable;
>>>> +		bias-pull-up;
>>>> +		qcom,drive-strength = <3>;
>>>> +	};
>>>> +};
>>>> +
>>>> +&spi11 {
>>>> +	#address-cells = <1>;
>>>> +	#size-cells = <0>;
>>>> +	status = "okay";
>>>> +
>>>> +	st33htpm0: tpm@0 {
>>>> +		compatible = "st,st33htpm-spi", "tcg,tpm_tis-spi";
>>>> +		reg = <0>;
>>>> +		spi-max-frequency = <20000000>;
>>>> +		pinctrl-names = "default";
>>>> +		pinctrl-0 = <&tpm_spi_reset>;
>>> Missing reset-gpios property. Otherwise there is no point in specifying
>>> the pinctrl.
>> The community previously rejected the GPIO reset function in the TPM driver
>> (tpm_tis_core.c). You can refer to the discussion [1].
>>
>>  From what I understand from the discussion in the patch, this decision was
>> made to prevent software from executing an incorrect reset sequence, which
>> could potentially reset the PCR banks of TPM chip.
>>
>> However, a pinctrl node is necessary to ensure the PMIC GPIO is in the
>> correct state as required by the TPM chip.
> No, pinctrl is not a replacement for GPIO calls. Please don't force GPIO
> levels using pinctrl.
>
> Also, at least tpm-common.yaml defines reset-gpios. So declaring a GPIO
> using that property is a proper course of actions.
>
> The discussion clearly stated: if the GPIO is under software control,
> then it should be clear that PCRs do not work in such a system. Please
> consider implementing that suggestion.

Thanks for the inputs! We re-worked our H/W to make the default GPIO 
state to high

which is required for this chip to work properly. I will re-send patch 
removing the pinctrl

nodes, as it is not required with the re-worked H/W.

>> [1] https://lore.kernel.org/lkml/20220407111849.5676-1-LinoSanfilippo@gmx.de/T/#m726d477dbce48c9e345e245f93d60f0aaa6f0994
>>
>> Thanks,
>>
>> Nirmesh
>>
>>>> +	};
>>>> +};
>>>> -- 
>>>> 2.34.1
>>>>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 6ca8db4b8afe..16ac008c58d2 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -111,6 +111,10 @@  dtb-$(CONFIG_ARCH_QCOM)	+= qcm6490-shift-otter.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-1000.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-4000.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs6490-rb3gen2.dtb
+
+qcs6490-rb3gen2-industrial-mezzanine-dtbs	:= qcs6490-rb3gen2.dtb qcs6490-rb3gen2-industrial-mezzanine.dtbo
+
+dtb-$(CONFIG_ARCH_QCOM)	+= qcs6490-rb3gen2-industrial-mezzanine.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs8550-aim300-aiot.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3.dtb
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-industrial-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-industrial-mezzanine.dtso
new file mode 100644
index 000000000000..1498f32bd069
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-industrial-mezzanine.dtso
@@ -0,0 +1,35 @@ 
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2025, Qualcomm Innovation Center, Inc. All rights reserved.
+*/
+
+/dts-v1/;
+/plugin/;
+#include <dt-bindings/clock/qcom,gcc-sc7280.h>
+#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
+
+&pm7250b_gpios {
+	tpm_spi_reset: tpm-spi-reset-state {
+		pins = "gpio5";
+		function = "normal";
+		power-source = <1>;
+		output-high;
+		input-disable;
+		bias-pull-up;
+		qcom,drive-strength = <3>;
+	};
+};
+
+&spi11 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	status = "okay";
+
+	st33htpm0: tpm@0 {
+		compatible = "st,st33htpm-spi", "tcg,tpm_tis-spi";
+		reg = <0>;
+		spi-max-frequency = <20000000>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&tpm_spi_reset>;
+	};
+};