Re: [PATCH 2/2] firmware: tpm: Introduce tpm-qcom driver

[Kuldeep Singh]

On 01-09-2026 19:16, Jarkko Sakkinen wrote:
> Short summary has a different driver name that is implemented i.e.,
> it should has "tpm_qcom", not "tpm-qcom".

True, let me make more generic like "Introduce Qualcomm TPM driver"
instead of having driver filename in commit title for any confusion.

>>  MAINTAINERS                 |   7 +
>>  drivers/char/tpm/Kconfig    |   9 ++
>>  drivers/char/tpm/Makefile   |   1 +
>>  drivers/char/tpm/tpm_qcom.c | 371 ++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/char/tpm/tpm_qcom.h |  82 ++++++++++
> 
> Rename it as tpm_qcom_tee just to be aligned on how Microsoft's fTPM
> driver is named.

I'd say firstly microsoft ftpm driver should be renamed.
For example, MS ftpm is based on optee(session based context) whereas
Qcom tpm driver is based on qcomtee(part of same TEE subsystem but uses
completely different mink-ipc based sharing mechanism).
So, it's completely different driver compared to MS.

My suggestions:
 - Rename MS driver i.e tpm_ftpm_tee.c -> tpm_microsoft.c
 - Rename Qcom driver i.e tpm_qcom.c -> tpm_qcom.c

I see pattern like tpm_{loongson/atmel/ibm} etc. so better to follow
this convention?

If ever need optee/qtee to specify, more namings can be tpm_ftpm_optee.c
and tpm_qcom_qtee.c?
Personally, I don't prefer tpm_ftpm_tee.c as it's all generic name with
no optee specification.
 >
>>  5 files changed, 470 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 47b04968e79a..cfa5b66823c7 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -22636,6 +22636,13 @@ S:	Maintained
>>  F:	Documentation/tee/qtee.rst
>>  F:	drivers/tee/qcomtee/
>>  
>> +QUALCOMM TPM DRIVER
>> +M:	Kuldeep Singh <kuldeep.singh@xxxxxxxxxxxxxxxx>
>> +L:	linux-arm-msm@xxxxxxxxxxxxxxx
>> +S:	Maintained
>> +F:	drivers/char/tpm/tpm_qcom.c
>> +F:	drivers/char/tpm/tpm_qcom.h
>> +
>>  QUALCOMM TRUST ZONE MEMORY ALLOCATOR
>>  M:	Bartosz Golaszewski <brgl@xxxxxxxxxx>
>>  L:	linux-arm-msm@xxxxxxxxxxxxxxx
> 
> this change should be a separate patch.

Ok, let me make MAINTAINERS entry as separate patch.
Also, i noticed none of other driver are being even compiled as 'M' like
atmel, ibm etc.
Any obvious reason behind this?
Should i enable tpm_qcom as Module in this patch only.

>> +	print_hex_dump_debug("tpm_qcom req: ", DUMP_PREFIX_OFFSET,
>> +			     16, 1, req, req_len, true);
> 
> Please don't do this. It can put production systems at risk.

Ok. I want to dump some contents on failures which makes debug easier.
Instead of dumping entire req/rsp buffer, dump tag, reqlen, rsplen etc?
>> +static struct tee_client_driver tpm_qcom_driver = {
>> +	.id_table	= tpm_qcom_id_table,
>> +	.probe		= tpm_qcom_probe,
>> +	.remove		= tpm_qcom_remove,
>> +	.driver		= {
>> +		.name	= "tpm-qcom",

Whatever naming we'll decide, I'll update same here too.

>> +
>> +/* dTPM SPI transfer optimization:
> 
> Block comments AFAIK should be
> 
> /*
>  * <TEXT>
> 

Let me fix it.

-- 
Regards
Kuldeep