| Message ID | 20250122101424.1810844-1-quic_nkumarsi@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] arm64: dts: qcom: Add industrial mezzanine support for qcs6490-rb3gen2 | expand |
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. > - 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. > + }; > +}; > -- > 2.34.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 >>
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. > > [1] https://lore.kernel.org/lkml/20220407111849.5676-1-LinoSanfilippo@gmx.de/T/#m726d477dbce48c9e345e245f93d60f0aaa6f0994 > > Thanks, > > Nirmesh > > > > > > + }; > > > +}; > > > -- > > > 2.34.1 > > >
On 22.01.2025 11:14 AM, 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> > > --- [...] > +&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>; Apart from Dmitry's comments, please keep pinctrl-n above pinctrl-names for style uniformity Konrad
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>; + }; +};