Re: [PATCH v2 2/6] mailbox: dt-bindings: Add shared [driver <=> device tree] defines

On Wed, Aug 12, 2015 at 2:23 PM, Lee Jones [off-list ref] wrote:

On Mon, 10 Aug 2015, Jassi Brar wrote:

quoted

On Mon, Aug 10, 2015 at 1:54 PM, Lee Jones [off-list ref] wrote:

quoted

On Mon, 10 Aug 2015, Jassi Brar wrote:

quoted

On Mon, Jul 27, 2015 at 3:14 PM, Lee Jones [off-list ref] wrote:

quoted

This header is currently only used for defines pertaining to data
direction i.e. Rx, Tx or Loopback.

Signed-off-by: Lee Jones <redacted>
---
 include/dt-bindings/mailbox/mailbox.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 include/dt-bindings/mailbox/mailbox.h

diff --git a/include/dt-bindings/mailbox/mailbox.h b/include/dt-bindings/mailbox/mailbox.h
new file mode 100644
index 0000000..82e929a
--- /dev/null
+++ b/include/dt-bindings/mailbox/mailbox.h

@@ -0,0 +1,14 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __MAILBOX_CONTROLLER_DT_BINDINGS_H
+#define __MAILBOX_CONTROLLER_DT_BINDINGS_H
+
+#define MBOX_TX                0x1
+#define MBOX_RX                0x2
+#define MBOX_LOOPBACK  (MBOX_RX | MBOX_TX)
+

Not sure I understand 'loopback'.  Does it mean h/w has some
'loopback' mode for testing purposes? Or it simply means the
controller can send as well as receive messages?

'loopback' allows firmware to conduct some early function tests.
However, channels are simplex, so we provide protection against
multiple allocation of single channel.  By allocating a LOOPBACK
channel we over-ride this protection and allow a single channel to be
allocated twice, once for Rx and the other for Tx.

So basically hardware is half-duplex, not simplex. I think maybe you
should simply allow for RX and TX always. It should work. Just
handover any received data before send_data (reflecting the h/w
limitation). That way you don't need any such special flag.

Unfortunately no, that's not correct.  Only Mailbox 0 is half-duplex.
The others are simplex (Rx only).

Assuming that is indeed the case (though code and comments suggest
otherwise), it is still not a matter of choice for clients to 'make' a
channel RX or TX or RXTX. That is the property/constraint of the
controller and the controller driver should simply check for channel
ID to be zero in send_data() and return error if its non-zero.

 Ideally I'd like to keep the
LOOPBACK flag, as it's easier to figure out if what someone is
attempting to do is actually valid.

I am not for such paranoia. Provider drivers is not the place to check
for valid user data.
The controller driver should simply reject send_data() request on any
mailbox > 0 while the consumer driver should scream for attention
because that's where the problem is.

Please note, what I suggest will only make the code simpler while not
breaking anything.

Thanks.