RE: [PATCH v8 4/4] mailbox: Add support for i.MX7D messaging unit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Jassi Brar [mailto:jassisinghbrar@xxxxxxxxx]
> Sent: Wednesday, August 1, 2018 5:45 PM

[...]

> >> > +
> >> > +static void imx_mu_txdb_work(struct work_struct *work) {
> >> > +       struct imx_mu_con_priv *cp =
> >> > +                       container_of(work, struct imx_mu_con_priv, work);
> >> > +       mbox_chan_txdone(cp->chan, 0); }
> >> >
> >> The api assumes a controller have same type of channels. So we are
> >> having to do this here.
> >> I am not sure, but would declaring two mailbox controllers (one with
> >> doorbells and the other data channels) work?
> >> If not, at least we could use a tasklet instead of a work queue?
> >>
> >
> > I'm also a bit curious what the meaning of calling mbox_chan_txdone
> > for a doorbell mailbox?
> >
> > Is there any recommended way to use it?
> >
> The framework submits a message (a signal with or without data) for
> controller to transmit. Obviously, it has to know when the transfer completes
> (so that it can submit the next message).
> 
> Depending on the controller h/w design, there can be three ways.
> 
> a) Controller has some irq that fires when the signal is consumed by the
> other end. Like MU does. So upon TX-IRQ, the controller driver calls
> mbox_chan_txdone() to tell the framework that the last submitted transfer
> has completed. This is indicated by setting only  txdone_irq = true
> 
> b) Controller does not have any tx-irq, but it can read some register to know
> the status. That is it has to be polled. The framework calls
> last_tx_done() periodically to check the status. This is indicated by setting
> only  txdone_poll = true.
> 
> c) Controller has neither irq, nor any status register. This is indicated by
> setting only  txdone_irq = false and  txdone_poll =
> false.   In this case, the protocol/client has to tell the framework
> (using some protocol level indicator like some ack packet reception) when
> the last submitted message was transferred, by calling
> mbox_client_txdone()
> 

Thanks for a detailed and clear explanation.

> A 'doorbell' is  (c) type channel -- just an irq raised at other end without any
> ack irq or status bit to check. So, the client would simply do
>                 mbox_send_message(chan, msg);
>                 mbox_client_txdone(chan, 0);

Just a bit wondering whether we need client to call mbox_client_txdone()
here as it actually does not know whether it's done. Not sure if it's better to
handle them in mailbox controller driver or framework.
(Oleksij seems do it in the controller driver in this patch)

And see mbox_client_txdone() definition, it seems it's only used by TXDONE_BY_ACK
case. Is the doorbell mailbox without IRQ or ACK support still suitable to call it to
notify the framework the transfer is done?

void mbox_client_txdone(struct mbox_chan *chan, int r)                                                                                                                                                              
{
        if (unlikely(!(chan->txdone_method & TXDONE_BY_ACK))) {
                dev_err(chan->mbox->dev, "Client can't run the TX ticker\n");                                                                                                                                       
                return;
        }

        tx_tick(chan, r);
}

For i.MX MU case, the controller is defined as TXDONE_BY_IRQ for all channels
Including TX doorbell. That's probably why we use mbox_chan_txdone to complete
It in controller driver. Slightly a bit strange.
Not sure if better idea to handle it in controller driver or framework.

Regards
Dong Aisheng
��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux