Re: [PATCH 3/6] crypto: ccp: Move some PSP mailbox bit definitions into common header

On 2/14/23 16:05, Limonciello, Mario wrote:

[Public]

quoted

-----Original Message-----
From: Jan Dąbroś <redacted>
Sent: Tuesday, February 14, 2023 03:04
To: Limonciello, Mario <Mario.Limonciello@amd.com>
Cc: Grzegorz Bernacki <redacted>; Thomas, Rijo-john <Rijo-
john.Thomas@amd.com>; Lendacky, Thomas
[off-list ref]; herbert@gondor.apana.org.au; Allen, John
[off-list ref]; Singh, Brijesh [off-list ref]; Jarkko
Nikula [off-list ref]; Andy Shevchenko
[off-list ref]; Mika Westerberg
[off-list ref]; linux-i2c@vger.kernel.org; linux-
crypto@vger.kernel.org; linux-kernel@vger.kernel.org; David S. Miller
[off-list ref]
Subject: Re: [PATCH 3/6] crypto: ccp: Move some PSP mailbox bit definitions
into common header

(...)

quoted

@@ -99,7 +93,7 @@ static int psp_check_mbox_recovery(struct psp_mbox

__iomem *mbox)

quoted

         tmp = readl(&mbox->cmd_fields);

-       return FIELD_GET(PSP_MBOX_FIELDS_RECOVERY, tmp);
+       return FIELD_GET(PSP_CMDRESP_RECOVERY, tmp);
  }

  static int psp_wait_cmd(struct psp_mbox __iomem *mbox)

@@ -107,7 +101,7 @@ static int psp_wait_cmd(struct psp_mbox __iomem

*mbox)

quoted

         u32 tmp, expected;

         /* Expect mbox_cmd to be cleared and ready bit to be set by PSP */
-       expected = FIELD_PREP(PSP_MBOX_FIELDS_READY, 1);
+       expected = FIELD_PREP(PSP_CMDRESP_RESP, 1);

What's the meaning of "PSP_CMDRESP_RESP"? I see that this new macro
name is currently used by other drivers, but in my opinion "READY" is
more descriptive. (It is also aligned to the comment above this line.)

It should indicate that the PSP has responded.  I think both terms work
to describe what's going on.

Tom - What's your preference?
I'll either adjust all the drivers to use READY or fix the comment for v2.

I think the comment should be changed.

Thanks,
Tom