On Tue, May 9, 2017 at 12:41 AM, Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> wrote: > On Sun 07 May 23:47 PDT 2017, Jassi Brar wrote: > >> On Mon, May 8, 2017 at 11:24 AM, Bjorn Andersson >> <bjorn.andersson@xxxxxxxxxx> wrote: >> > On Fri 05 May 21:48 PDT 2017, Jassi Brar wrote: >> > >> > The APCS IPC register serves the basis for all inter-processor >> > communication in a Qualcomm platform, so it's not only the RPM driver >> > discussed earlier that uses this. It's also used for other non-FIFO >> > based communication channels, where the signalled information either >> > isn't acked at all or acked on a system-level. >> > >> Something has to indicate consumption of data or "requested action >> taken". Otherwise the protocol is design-wise broken. >> > > The SMD and GLINK protocols work by providing two independent one-way > pipes that higher levels can use to send and receive messages. When some > driver pushes a message into the transmit-pipe we check if there's > space, then write the message, signal the remote (APCS IPC) and then > return. > "we check if there's space" -> this is what mailbox api tries to do with last_tx_done before starting the next message. >> >> The client should call mbox_client_txdone() after >> >> mbox_send_message(). >> > >> > So every time we call mbox_send_message() from any of the client drivers >> > we also needs to call mbox_client_txdone()? >> > >> Yes. >> >> > This seems like an awkward side effect of using the mailbox framework - >> > which has to be spread out in at least 6 different client drivers :( >> > >> No. Mailbox or whatever you implement - you must (and do) tick the >> state machine to keep the messages moving. > > But the state you have in the other mailbox drivers is not a concern of > the APCS IPC. > No, as you say above you check for space before writing the next message, this is what I call ticking the state machine. >> Best designs have some interrupt occurring when the message has been >> consumed by the remote. Some designs have a flag set which needs to be >> polled to detect completion. Very few (like yours) that support >> neither irq nor polling, have to be driven by the upper protocol layer >> by some ack packet (or tracking read/write pointers like you do). >> These three cases are denoted by TXDONE_BY_IRQ, TXDONE_BY_POLL and >> TXDONE_BY_ACK respectively. >> > > You're confusing the APCS IPC with the larger communication mechanism, > Maybe. I am not versed with QCom technologies like RPM, SMD, GLINK, APCS etc. Controller driver is what physically transmits a signal to remote. Users above the mailbox api are client drivers. > > This is why I suggested that this is a doorbell, rather than a mailbox. > Your argumentation of how a mailbox should work makes perfect sense, but > it's not how the Qualcomm IPC works. > Mailbox framework is designed to support, what you call doorbell type of communication, just fine. There is no need to define another class. > > Setting TXDONE_BY_POLL and specifying a dummy last_tx_done() comes with > a crazy overhead. To set a single bit in a register we will take the > channel spinlock 4 times, start a timer, iterate over all registered > channels and the client must be marked as blocking so we will get at > least 2 additional context switches. > How often does the platform send messages for it to be a considerable load? BTW, this is an option only if your client driver doesn't want to explicitly tick the state machine by calling mbox_client_txdone()... which I think should be done in the first place. thanks -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
