Re: [PATCH v4 2/2] mailbox: sprd: Add Spreadtrum mailbox driver

From: Baolin Wang
Date: Thu May 21 2020 - 08:24:54 EST


Hi Jassi,

On Wed, May 13, 2020 at 2:32 PM Baolin Wang <baolin.wang7@xxxxxxxxx> wrote:
>
> On Wed, May 13, 2020 at 2:05 PM Jassi Brar <jassisinghbrar@xxxxxxxxx> wrote:
> >
> > On Tue, May 12, 2020 at 11:14 PM Baolin Wang <baolin.wang7@xxxxxxxxx> wrote:
> > >
> > > Hi Jassi,
> > >
> > > On Thu, May 7, 2020 at 11:23 AM Baolin Wang <baolin.wang7@xxxxxxxxx> wrote:
> > > >
> > > > Hi Jassi,
> > > >
> > > > On Thu, May 7, 2020 at 7:25 AM Jassi Brar <jassisinghbrar@xxxxxxxxx> wrote:
> > > > >
> > > > > On Wed, May 6, 2020 at 8:29 AM Baolin Wang <baolin.wang7@xxxxxxxxx> wrote:
> > > > > >
> > > > > > Hi Jassi,
> > > > > >
> > > > > > On Tue, Apr 28, 2020 at 11:10 AM Baolin Wang <baolin.wang7@xxxxxxxxx> wrote:
> > > > > > >
> > > > > > > From: Baolin Wang <baolin.wang@xxxxxxxxxx>
> > > > > > >
> > > > > > > The Spreadtrum mailbox controller supports 8 channels to communicate
> > > > > > > with MCUs, and it contains 2 different parts: inbox and outbox, which
> > > > > > > are used to send and receive messages by IRQ mode.
> > > > > > >
> > > > > > > Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx>
> > > > > > > Signed-off-by: Baolin Wang <baolin.wang7@xxxxxxxxx>
> > > > > > > ---
> > > > > > > Changes from v3:
> > > > > > > - Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'
> > > > > > >
> > > > > > > Changes from v2:
> > > > > > > - None.
> > > > > > >
> > > > > > > Changes from v1:
> > > > > > > - None
> > > > > >
> > > > > > Gentle ping, do you have any other comments? Thanks.
> > > > > >
> > > > > Yea, I am still not sure about the error returned in send_data(). It
> > > > > will either never hit or there will be no easy recovery from it. The
> > > > > api expects the driver to tell it the last-tx was done only when it
> > > > > can send the next message. (There may be case like sending depend on
> > > > > remote, which can't be ensured before hand).
> > > >
> > > > Actually this is an unusual case, suppose the remote target did not
> > > > fetch the message as soon as possile, which will cause the FIFO
> > > > overflow, so in this case we can not send messages to the remote
> > > > target any more, otherwise messages will be lost. Thus we can return
> > > > errors to users to indicate that something wrong with the remote
> > > > target need to be checked.
> > > >
> > > > So this validation in send_data() is mostly for debugging for this
> > > > abnormal case and we will not trigger this issue if the remote target
> > > > works well. So I think it is useful to keep this validation in
> > > > send_data(). Thanks.
> > >
> > > Any comments? Thanks.
> > >
> > Same as my last post.
>
> I think I've explained the reason why we need add this validation in
> my previous email, I am not sure how do you think? You still want to
> remove this validation?

Gentle ping.

As I explained in previous email, this validation is for an unusual
case, suppose the remote target did not fetch the message as soon as
possile, which will cause the FIFO overflow, so in this case we can
not send messages to the remote
target any more, otherwise messages will be lost. Thus we can return
errors to users to indicate that something wrong with the remote
target need to be checked.

So this validation in send_data() is mostly for debugging for this
abnormal case and we will not trigger this issue if the remote target
works well. So I think it is useful to keep this validation in
send_data(). What do you think? Thanks.

--
Baolin Wang