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. > > [1] https://lore.kernel.org/lkml/20220407111849.5676-1-LinoSanfilippo@xxxxxx/T/#m726d477dbce48c9e345e245f93d60f0aaa6f0994 > > Thanks, > > Nirmesh > > > > > > + }; > > > +}; > > > -- > > > 2.34.1 > > > -- With best wishes Dmitry
