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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxxxx>
Signed-off-by: Nirmesh Kumar Singh <quic_nkumarsi@xxxxxxxxxxx>

---
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@xxxxxxxxxxx/

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@xxxxxxxxxxx/
---
   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@xxxxxx/T/#m726d477dbce48c9e345e245f93d60f0aaa6f0994

Thanks,

Nirmesh

+	};
+};
--
2.34.1





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux