On 31-08-2026 16:45, Konrad Dybcio wrote:
> On 8/31/26 11:43 AM, Kuldeep Singh wrote:
>> Add a TPM chip driver for platforms where a TPM 2.0 instance is
>> implemented by a Trusted Application (TA) running in Qualcomm's Trusted
>> Execution Environment (QTEE), reachable over the QCOMTEE object-IPC
>> transport.
>
> [...]
>
>> +static int tpm_qcom_get_client_env_obj(struct tee_context *ctx,
>> + struct tee_param_objref *client_env_obj)
>> +{
>> + int ret;
>> + struct tee_ioctl_object_invoke_arg inv_arg;
>> + struct tee_param param[2];
>
> nit: Reverse-Christmas-tree would be preferred
Ok.
>
>> +
>> + memset(&inv_arg, 0, sizeof(inv_arg));
>> + memset(¶m, 0, sizeof(param));
>
> You can zero-initialize local struct variables like this:
>
> struct foo bar = { };
>
> [...]
>
>> +static int tpm_qcom_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
>> + size_t cmd_len)
>> +{
>> + struct tpm_qcom_private *pvt_data = dev_get_drvdata(chip->dev.parent);
>> + size_t rsp_len = PAGE_ALIGN(MAX_RESPONSE_SIZE);
>> + size_t copy_len;
>> + int ret;
>> +
>> + if (cmd_len > MAX_COMMAND_SIZE) {
>> + dev_err(&chip->dev,
>> + "%s: len=%zd exceeds MAX_COMMAND_SIZE\n",
>> + __func__, cmd_len);
>
> The name of the function isn't helpful here, this is the only time this
> message appears, so it's easy to grep
Sure.
>
> [...]
>
>
>> + err = tpm_chip_register(pvt_data->chip);
>> + if (err) {
>> + dev_err(dev, "%s: tpm_chip_register failed with rc=%d\n",
>> + __func__, err);
>
> Likewise
Since it's dev_err so dev name should be sufficient i think.
Let me drop function naming from log.
>
> [...]
>
>> +#define QCOMTEE_TPM_GET_TA_VERSION_ID 0x0001000
>> +#define QCOMTEE_TPM_TA_VERSION_GET_MAJOR(ver) ((u32)(ver) >> 16)
>> +#define QCOMTEE_TPM_TA_VERSION_GET_MINOR(ver) ((u32)(ver) &
>> 0x0000ffffU)
>
> That's FIELD_GET(mask, x)
Sounds good, Let me use FIELD_GET.
--
Regards
Kuldeep
