Re: [PATCH V4 2/5] mailbox: Add support for QTI CPUCP mailbox controller

[Sibi Sankar <quic_sibis@quicinc.com>]

On 5/1/24 07:44, Jassi Brar wrote:
> On Mon, Apr 22, 2024 at 11:41 AM Sibi Sankar <quic_sibis@quicinc.com> wrote:
>>
>> Add support for CPUSS Control Processor (CPUCP) mailbox controller,
>> this driver enables communication between AP and CPUCP by acting as
>> a doorbell between them.
>>

Hey Jassi,

Thanks for taking time to review the series :).

>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
>> ---
> 
> Do you want to add an entry in the MAINTAINERS ?

Thanks will add in the next re-spin.

> 
>> diff --git a/drivers/mailbox/qcom-cpucp-mbox.c b/drivers/mailbox/qcom-cpucp-mbox.c
>   .....
>> +static irqreturn_t qcom_cpucp_mbox_irq_fn(int irq, void *data)
>> +{
>> +       struct qcom_cpucp_mbox *cpucp = data;
>> +       struct mbox_chan *chan;
>> +       unsigned long flags;
>> +       u64 status;
>> +       u32 val;
>> +       int i;
>> +
> The variables flags, val and chan are better inside the for loop below.

Ack.

-Sibi

> 
>> +       status = readq(cpucp->rx_base + APSS_CPUCP_RX_MBOX_STAT);
>> +
>> +       for_each_set_bit(i, (unsigned long *)&status, APSS_CPUCP_IPC_CHAN_SUPPORTED) {
>> +               val = readl(cpucp->rx_base + APSS_CPUCP_RX_MBOX_CMD(i) + APSS_CPUCP_MBOX_CMD_OFF);
>> +               chan = &cpucp->chans[i];
>> +               /* Provide mutual exclusion with changes to chan->cl */
>> +               spin_lock_irqsave(&chan->lock, flags);
>> +               if (chan->cl)
>> +                       mbox_chan_received_data(chan, &val);
>> +               writeq(BIT(i), cpucp->rx_base + APSS_CPUCP_RX_MBOX_CLEAR);
>> +               spin_unlock_irqrestore(&chan->lock, flags);
>> +       }
>> +
>> +       return IRQ_HANDLED;
>> +}
>> +
> 
> Thanks
> Jassi