2024-01-26 16:52:59

by Francesco Dolcini

[permalink] [raw]
Subject: [PATCH v1] arm64: dts: ti: verdin-am62: mallow: add TPM device

From: Francesco Dolcini <[email protected]>

Add TPM device to Mallow device tree file, the device is connected to
the SoC with SPI1/CS1, the same SPI interface is also available on an
extension header together with an additional CS0 signal.

Signed-off-by: Francesco Dolcini <[email protected]>
---
arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi b/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi
index 17b93534f658..77b1beb638ad 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi
@@ -127,6 +127,16 @@ &main_spi1 {
<&pinctrl_qspi1_cs2_gpio>;
cs-gpios = <0>, <&main_gpio0 12 GPIO_ACTIVE_LOW>;
status = "okay";
+
+ tpm@1 {
+ compatible = "infineon,slb9670", "tcg,tpm_tis-spi";
+ reg = <1>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_qspi1_dqs_gpio>;
+ interrupt-parent = <&main_gpio1>;
+ interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
+ spi-max-frequency = <18500000>;
+ };
};

/* Verdin UART_3 */
--
2.39.2



2024-02-06 04:42:50

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH v1] arm64: dts: ti: verdin-am62: mallow: add TPM device

Hi Francesco Dolcini,

On Fri, 26 Jan 2024 17:51:36 +0100, Francesco Dolcini wrote:
> Add TPM device to Mallow device tree file, the device is connected to
> the SoC with SPI1/CS1, the same SPI interface is also available on an
> extension header together with an additional CS0 signal.
>
>

I have applied the following to branch ti-k3-dts-next on [1].
Thank you!

[1/1] arm64: dts: ti: verdin-am62: mallow: add TPM device
commit: e55b0bf4c2b31cc0b202b375796f96b2b9af2e6a

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent up the chain during
the next merge window (or sooner if it is a relevant bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git
--
Vignesh


2024-02-06 18:29:41

by Andrew Davis

[permalink] [raw]
Subject: Re: [PATCH v1] arm64: dts: ti: verdin-am62: mallow: add TPM device

On 1/26/24 10:51 AM, Francesco Dolcini wrote:
> From: Francesco Dolcini <[email protected]>
>
> Add TPM device to Mallow device tree file, the device is connected to
> the SoC with SPI1/CS1, the same SPI interface is also available on an
> extension header together with an additional CS0 signal.
>
> Signed-off-by: Francesco Dolcini <[email protected]>
> ---
> arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi b/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi
> index 17b93534f658..77b1beb638ad 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi
> @@ -127,6 +127,16 @@ &main_spi1 {
> <&pinctrl_qspi1_cs2_gpio>;
> cs-gpios = <0>, <&main_gpio0 12 GPIO_ACTIVE_LOW>;
> status = "okay";
> +
> + tpm@1 {
> + compatible = "infineon,slb9670", "tcg,tpm_tis-spi";
> + reg = <1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_qspi1_dqs_gpio>;
> + interrupt-parent = <&main_gpio1>;
> + interrupts = <18 IRQ_TYPE_EDGE_FALLING>;

Just a heads-up, the SLB9670 datasheet says this device uses
an active low interrupt (IRQ_TYPE_LEVEL_LOW). Using TYPE_EDGE
here can cause missed interrupts if the line stays low for
multiple interrupts.

Andrew

> + spi-max-frequency = <18500000>;
> + };
> };
>
> /* Verdin UART_3 */

2024-02-06 18:37:18

by Francesco Dolcini

[permalink] [raw]
Subject: Re: [PATCH v1] arm64: dts: ti: verdin-am62: mallow: add TPM device



Il 6 febbraio 2024 19:29:13 CET, Andrew Davis <[email protected]> ha scritto:
>On 1/26/24 10:51 AM, Francesco Dolcini wrote:
>> From: Francesco Dolcini <[email protected]>
>>
>> Add TPM device to Mallow device tree file, the device is connected to
>> the SoC with SPI1/CS1, the same SPI interface is also available on an
>> extension header together with an additional CS0 signal.
>>
>> Signed-off-by: Francesco Dolcini <[email protected]>
>> ---
>> arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi b/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi
>> index 17b93534f658..77b1beb638ad 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi
>> @@ -127,6 +127,16 @@ &main_spi1 {
>> <&pinctrl_qspi1_cs2_gpio>;
>> cs-gpios = <0>, <&main_gpio0 12 GPIO_ACTIVE_LOW>;
>> status = "okay";
>> +
>> + tpm@1 {
>> + compatible = "infineon,slb9670", "tcg,tpm_tis-spi";
>> + reg = <1>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_qspi1_dqs_gpio>;
>> + interrupt-parent = <&main_gpio1>;
>> + interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>
>Just a heads-up, the SLB9670 datasheet says this device uses
>an active low interrupt (IRQ_TYPE_LEVEL_LOW). Using TYPE_EDGE
>here can cause missed interrupts if the line stays low for
>multiple interrupts.


The driver interrupt handler would need to take care of it, if needed.

The SOC does not support level interrupt, so there is no other solution, am I wrong?

Francesco


2024-02-06 18:51:45

by Andrew Davis

[permalink] [raw]
Subject: Re: [PATCH v1] arm64: dts: ti: verdin-am62: mallow: add TPM device

On 2/6/24 12:36 PM, Francesco Dolcini wrote:
>
>
> Il 6 febbraio 2024 19:29:13 CET, Andrew Davis <[email protected]> ha scritto:
>> On 1/26/24 10:51 AM, Francesco Dolcini wrote:
>>> From: Francesco Dolcini <[email protected]>
>>>
>>> Add TPM device to Mallow device tree file, the device is connected to
>>> the SoC with SPI1/CS1, the same SPI interface is also available on an
>>> extension header together with an additional CS0 signal.
>>>
>>> Signed-off-by: Francesco Dolcini <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi b/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi
>>> index 17b93534f658..77b1beb638ad 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi
>>> +++ b/arch/arm64/boot/dts/ti/k3-am62-verdin-mallow.dtsi
>>> @@ -127,6 +127,16 @@ &main_spi1 {
>>> <&pinctrl_qspi1_cs2_gpio>;
>>> cs-gpios = <0>, <&main_gpio0 12 GPIO_ACTIVE_LOW>;
>>> status = "okay";
>>> +
>>> + tpm@1 {
>>> + compatible = "infineon,slb9670", "tcg,tpm_tis-spi";
>>> + reg = <1>;
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&pinctrl_qspi1_dqs_gpio>;
>>> + interrupt-parent = <&main_gpio1>;
>>> + interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>>
>> Just a heads-up, the SLB9670 datasheet says this device uses
>> an active low interrupt (IRQ_TYPE_LEVEL_LOW). Using TYPE_EDGE
>> here can cause missed interrupts if the line stays low for
>> multiple interrupts.
>
>
> The driver interrupt handler would need to take care of it, if needed.
>
> The SOC does not support level interrupt, so there is no other solution, am I wrong?

Correct, our K3 SoCs do not support level interrupts and so are not compatible
with most hardware that uses interrupts (unless you are okay with missing
some interrupts every now and then..).

As you say, our interrupt driver needs to be modified to handle that. Possibly
by re-checking the line level after the interrupt handlers have all run to see
if it is still high/low, then manually re-triggering the interrupt if so.

The heads-up is just that you should be aware of this in case you have issues.
When(if) our interrupt driver is ever fixed you will need to update the DT here
to take advantage of the level handler.

Andrew

>
> Francesco
>