[PATCH v4 2/3] mailbox: Add iProc mailbox controller driver

[Jonathan Richardson]

On 17-02-16 10:20 PM, Jassi Brar wrote:
> On Fri, Jan 27, 2017 at 2:08 AM, Jonathan Richardson
> <jonathan.richardson at broadcom.com> wrote:
>
>> +static int iproc_mbox_send_data_m0(struct mbox_chan *chan, void *data)
>> +{
>> +       struct iproc_mbox *mbox = dev_get_drvdata(chan->mbox->dev);
>> +       struct iproc_mbox_msg *msg = (struct iproc_mbox_msg *)data;
>> +               unsigned long flags;
>> +       int err = 0;
>> +       const int poll_period_us = 5;
>> +       const int max_retries = (MAX_M0_TIMEOUT_MS * 1000) / poll_period_us;
>> +
>> +       if (!msg)
>> +               return -EINVAL;
>> +
>> +       spin_lock_irqsave(&mbox->lock, flags);
>> +
>> +       dev_dbg(mbox->dev, "Send msg to M0: cmd=0x%x, param=0x%x, wait_ack=%d\n",
>> +               msg->cmd, msg->param, msg->wait_ack);
>> +
> prints should be outside the spinlocks.
Will fix.
>
>> +       writel(msg->cmd, mbox->base + IPROC_CRMU_MAILBOX0_OFFSET);
>> +       writel(msg->param, mbox->base + IPROC_CRMU_MAILBOX1_OFFSET);
>> +
>> +       if (msg->wait_ack) {
>> +               int retries;
>> +
> move poll_period_us and max_retries in here or just define' them
Will fix.
>
>> +               err = msg->reply_code = -ETIMEDOUT;
>> +               for (retries = 0; retries < max_retries; retries++) {
>> +                       u32 val = readl(
>> +                               mbox->base + IPROC_CRMU_MAILBOX0_OFFSET);
>> +                       if (val & M0_IPC_CMD_DONE_MASK) {
>> +                               /*
>> +                                * M0 replied - save reply code and
>> +                                * clear error.
>> +                                */
>> +                               msg->reply_code = (val &
>> +                                       M0_IPC_CMD_REPLY_MASK) >>
>> +                                       M0_IPC_CMD_REPLY_SHIFT;
>> +                               err = 0;
>> +                               break;
>> +                       }
>> +                       udelay(poll_period_us);
>>
> potentially 2ms inside spin_lock_irqsave. Alternative is to implement
> a simple 'peek_data' and call it for requests with 'wait_ack'
The M0 responds in very few microseconds. The 2ms is an arbitrary value that came from the clients tx_tout being specified in ms. I can reduce the timeout to something smaller. No need to peek for data.

Thanks,
Jon