在 2023/3/3 19:07, Sudeep Holla 写道:
On Fri, Mar 03, 2023 at 09:50:00AM +0800, lihuisong (C) wrote:
在 2023/3/2 21:52, Sudeep Holla 写道:
On Thu, Mar 02, 2023 at 09:57:35AM +0800, lihuisong (C) wrote:
在 2023/3/1 21:24, Sudeep Holla 写道:
[...]
+static bool pcc_mbox_cmd_complete_check(struct pcc_chan_info *pchan)
+{
+ u64 val;
+ int ret;
+
+ ret = pcc_chan_reg_read(&pchan->cmd_complete, &val);
+ if (ret)
+ return false;
+
we indeed already check if cmd_complete register is exist.
IMO, it can simply the code logic and reduce the risk of problems if we
return true here for the type without this register.
what do you think?
IIUC, your concern is about returning true for type 4 when the register
doesn't exist, right ?
Return true in advance for the type without the cmd_complete register.
If support the register, we judge if the channel should respond the
interrupt based on the value of cmd_complete, like bellow.
Right, sorry for missing that.
-->8
+static bool pcc_mbox_cmd_complete_check(struct pcc_chan_info *pchan)
+{
+ u64 val;
+ int ret;
+
+ ret = pcc_chan_reg_read(&pchan->cmd_complete, &val);
+ if (ret)
+ return false;
+
+ if (!pchan->cmd_complete.gas)
+ return true;
+
Yes we need the above check.
+ /*
+ * Judge if the channel respond the interrupt based on the value of
+ * command complete.
+ */
+ val &= pchan->cmd_complete.status_mask;
+ /*
+ * If this is PCC slave subspace channel, then the command complete
+ * bit 0 indicates that Platform is sending a notification and OSPM
+ * needs to respond this interrupt to process this command.
+ */
+ if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
+ return !val;
+ else
+ return !!val;
+}
I am saying it won't happen as we bail out if there is no GAS register
from pcc_chan_reg_init(). Or am I missing something here ?
Yes, what you say is also ok. Just wondering if it is better to simply the
logic.
Understood now.
+ val &= pchan->cmd_complete.status_mask;
+
+ /*
+ * If this is PCC slave subspace channel, then the command complete
+ * bit 0 indicates that Platform is sending a notification and OSPM
+ * needs to respond this interrupt to process this command.
+ */
+ if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
+ return !val;
+ else
+ return !!val;
This else branch is not applicable to type 3. type 3 will cannot respond
interrupt.
Sorry I don't understand what you mean by that.
Sorry for my mistake.
I meant that the type2 channel always return false in this function and
never respond the interrupt if no check for the GAS register.
Because the 'val' for the type without the register is zero.
Agreed as mentioned above, we need to bail out with true return if no GAS is
found.
Ok, I will fix it as mentioned above.
.
