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

[Jarkko Sakkinen]

On Wed, Sep 02, 2026 at 02:52:17PM +0530, Kuldeep Singh wrote:
> 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

This goes for argument using tpm_qcom.c but we don't rename ko's in
normal circumstances (unfortunately so).

> 
> 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
> 

BR, Jarkko