Thread (10 messages) 10 messages, 4 authors, 2021-10-16

Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes

flat view

From: Jassi Brar <jassisinghbrar@gmail.com>
Date: 2021-10-16 19:04:49
Also in: linux-devicetree, lkml

On Thu, Sep 16, 2021 at 10:49 AM Sven Peter [off-list ref] wrote:

....
quoted hunk ↗ jump to hunk
+static const struct apple_mbox_hw apple_mbox_asc_hw = {
+       .control_full = APPLE_ASC_MBOX_CONTROL_FULL,
+       .control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
+
+       .a2i_control = APPLE_ASC_MBOX_A2I_CONTROL,
+       .a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0,
+       .a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1,
+
+       .i2a_control = APPLE_ASC_MBOX_I2A_CONTROL,
+       .i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0,
+       .i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1,
+
+       .has_irq_controls = false,
+};
+
+static const struct apple_mbox_hw apple_mbox_m3_hw = {
+       .control_full = APPLE_M3_MBOX_CONTROL_FULL,
+       .control_empty = APPLE_M3_MBOX_CONTROL_EMPTY,
+
+       .a2i_control = APPLE_M3_MBOX_A2I_CONTROL,
+       .a2i_send0 = APPLE_M3_MBOX_A2I_SEND0,
+       .a2i_send1 = APPLE_M3_MBOX_A2I_SEND1,
+
+       .i2a_control = APPLE_M3_MBOX_I2A_CONTROL,
+       .i2a_recv0 = APPLE_M3_MBOX_I2A_RECV0,
+       .i2a_recv1 = APPLE_M3_MBOX_I2A_RECV1,
+
+       .has_irq_controls = true,
+       .irq_enable = APPLE_M3_MBOX_IRQ_ENABLE,
+       .irq_ack = APPLE_M3_MBOX_IRQ_ACK,
+       .irq_bit_recv_not_empty = APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY,
+       .irq_bit_send_empty = APPLE_M3_MBOX_IRQ_A2I_EMPTY,
+};
+
+static const struct of_device_id apple_mbox_of_match[] = {
+       { .compatible = "apple,t8103-asc-mailbox", .data = &apple_mbox_asc_hw },
+       { .compatible = "apple,t8103-m3-mailbox", .data = &apple_mbox_m3_hw },
+       {}
+};
+MODULE_DEVICE_TABLE(of, apple_mbox_of_match);
+
Traditionally, these go at the end of file.

....
quoted hunk ↗ jump to hunk
+
+static int apple_mbox_hw_send(struct apple_mbox *apple_mbox,
+                             struct apple_mbox_msg *msg)
+{
+       if (!apple_mbox_hw_can_send(apple_mbox))
+               return -EBUSY;
+
+       dev_dbg(apple_mbox->dev, "> TX %016llx %08x\n", msg->msg0, msg->msg1);
+
+       /*
+        * This message may be related to a shared memory buffer and we must
+        * ensure all previous writes to normal memory are visible before
+        * submitting it.
+        */
+       dma_wmb();
+
This may cause unnecessary overhead.
mbox_client.tx_prepare() is where the SHMEM data is copied, which
should also call dma_wmb() just before return.
......
quoted hunk ↗ jump to hunk
+
+static int apple_mbox_hw_recv(struct apple_mbox *apple_mbox,
+                             struct apple_mbox_msg *msg)
+{
+       if (!apple_mbox_hw_can_recv(apple_mbox))
+               return -ENOMSG;
+
+       msg->msg0 = readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv0);
+       msg->msg1 = FIELD_GET(
+               APPLE_MBOX_MSG1_MSG,
+               readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv1));
+
+       dev_dbg(apple_mbox->dev, "< RX %016llx %08x\n", msg->msg0, msg->msg1);
+
+       /*
+        * This message may be related to a shared memory buffer and we must
+        * ensure any following reads from normal memory only happen after
+        * having read this message.
+        */
+       dma_rmb();
+
.... similarly should be done by the client as the first thing in recv callback.

quoted hunk ↗ jump to hunk
+static struct mbox_chan *apple_mbox_of_xlate(struct mbox_controller *mbox,
+                                            const struct of_phandle_args *spec)
+{
+       struct apple_mbox *apple_mbox = dev_get_drvdata(mbox->dev);
+
+       if (spec->args_count != 0)
+               return ERR_PTR(-EINVAL);
+       if (apple_mbox->chan.con_priv)
+               return ERR_PTR(-EBUSY);
+
+       apple_mbox->chan.con_priv = apple_mbox;
+       return &apple_mbox->chan;
+}
+
we could do without of_xlate callback, by assigning  chan.con_priv
already in the .probe()

quoted hunk ↗ jump to hunk
diff --git a/include/linux/apple-mailbox.h b/include/linux/apple-mailbox.h
+
+struct apple_mbox_msg {
+       u64 msg0;
+       u32 msg1;
+};
+
Aren't msg0/1 the Tx and Rx channels? If so you may want to separate
them out as such. But of course, I don't know the h/w details so I may
be wrong.

Cheers!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help