[PATCH v2 0/4] mailbox: mailbox-test: support single channel with separate Tx and Rx buffer

[PATCH v2 0/4] mailbox: mailbox-test: support single channel with separate Tx and Rx buffer

[Sudeep Holla]

Hi Jassi,

Assuming mailbox-test was designed to be generic, I am trying to extend
it to support single channel with separate Tx and Rx buffer. With these
changes I am able to test arm_mhu driver.

However I couldn't understand the intention of converting buffer to ASCII
hex dump in read method. I have a local change to remove that so that it
can deal with any data in any format(e.g. some protocol format) and
userspace can deal with the actual interpretation of the data. I will
post that separately and discuss that in that thread. For now this
series is sufficient and userspace can do reverse conversion.

Please consider applying this for v4.6

Regards,
Sudeep

v1: https://lkml.org/lkml/2016/2/11/561
v1-v2:
	- Minor/Cosmetic changes as suggested by Lee
	- Added Acks from RobH and Lee

Sudeep Holla (4):
  mailbox: mailbox-test: rename driver as generic test driver
  mailbox: mailbox-test: fix the compatible string
  mailbox: mailbox-test: use print_hex_dump_bytes to allow dynamic
    printk
  mailbox: mailbox-test: add support for separate tx/rx buffer with
    single channel

 .../devicetree/bindings/mailbox/sti-mailbox.txt    |  2 +-
 drivers/mailbox/mailbox-test.c                     | 53 +++++++++++++---------
 2 files changed, 32 insertions(+), 23 deletions(-)

-- 
1.9.1

[PATCH v2 1/4] mailbox: mailbox-test: rename driver as generic test driver

[Sudeep Holla]

This mailbox-test driver was designed to be generic, so let's remove ST
tag on it and make it generic.

Cc: Jassi Brar <jassisinghbrar@gmail.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/mailbox-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
index 684ae17dcf39..3813f6d9eba9 100644
--- a/drivers/mailbox/mailbox-test.c
+++ b/drivers/mailbox/mailbox-test.c
@@ -348,7 +348,7 @@ static const struct of_device_id mbox_test_match[] = {
 
 static struct platform_driver mbox_test_driver = {
 	.driver = {
-		.name = "mailbox_sti_test",
+		.name = "mailbox_test",
 		.of_match_table = mbox_test_match,
 	},
 	.probe  = mbox_test_probe,
-- 
1.9.1

[PATCH v2 2/4] mailbox: mailbox-test: fix the compatible string

[Sudeep Holla]

Underscores are usually forbidden in the compatible strings. So lets
remove it before the first users of this is seen.

Cc: Jassi Brar <jassisinghbrar@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 Documentation/devicetree/bindings/mailbox/sti-mailbox.txt | 2 +-
 drivers/mailbox/mailbox-test.c                            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt b/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
index b61eec920359..351f612673fc 100644
--- a/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
+++ b/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
@@ -44,7 +44,7 @@ Optional properties
 Example:
 
 mailbox_test {
-	compatible	= "mailbox_test";
+	compatible	= "mailbox-test";
 	reg		= <0x[shared_memory_address], [shared_memory_size]>;
 	mboxes		= <&mailbox2 0 1>, <&mailbox0 2 1>;
 	mbox-names	= "tx",	"rx";
diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
index 3813f6d9eba9..036a852b5fa1 100644
--- a/drivers/mailbox/mailbox-test.c
+++ b/drivers/mailbox/mailbox-test.c
@@ -342,7 +342,7 @@ static int mbox_test_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id mbox_test_match[] = {
-	{ .compatible = "mailbox_test" },
+	{ .compatible = "mailbox-test" },
 	{},
 };
 
-- 
1.9.1

[PATCH v2 3/4] mailbox: mailbox-test: use print_hex_dump_bytes to allow dynamic printk

[Sudeep Holla]

Reduce the logging from info to debug. Also use print_hex_dump_bytes
instead as it has support for dynamic printk providing options to
conditionally enable/disable these logs.

Cc: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/mailbox-test.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
index 036a852b5fa1..f690f11969a1 100644
--- a/drivers/mailbox/mailbox-test.c
+++ b/drivers/mailbox/mailbox-test.c
@@ -113,15 +113,15 @@ static ssize_t mbox_test_message_write(struct file *filp,
 	 * MMIO to subsequently pass the message through
 	 */
 	if (tdev->mmio && tdev->signal) {
-		print_hex_dump(KERN_INFO, "Client: Sending: Signal: ", DUMP_PREFIX_ADDRESS,
-			       MBOX_BYTES_PER_LINE, 1, tdev->signal, MBOX_MAX_SIG_LEN, true);
+		print_hex_dump_bytes("Client: Sending: Signal: ", DUMP_PREFIX_ADDRESS,
+				     tdev->signal, MBOX_MAX_SIG_LEN);
 
 		data = tdev->signal;
 	} else
 		data = tdev->message;
 
-	print_hex_dump(KERN_INFO, "Client: Sending: Message: ", DUMP_PREFIX_ADDRESS,
-		       MBOX_BYTES_PER_LINE, 1, tdev->message, MBOX_MAX_MSG_LEN, true);
+	print_hex_dump_bytes("Client: Sending: Message: ", DUMP_PREFIX_ADDRESS,
+			     tdev->message, MBOX_MAX_MSG_LEN);
 
 	ret = mbox_send_message(tdev->tx_channel, data);
 	if (ret < 0)
@@ -222,13 +222,11 @@ static void mbox_test_receive_message(struct mbox_client *client, void *message)
 	spin_lock_irqsave(&tdev->lock, flags);
 	if (tdev->mmio) {
 		memcpy_fromio(tdev->rx_buffer, tdev->mmio, MBOX_MAX_MSG_LEN);
-		print_hex_dump(KERN_INFO, "Client: Received [MMIO]: ",
-			       DUMP_PREFIX_ADDRESS, MBOX_BYTES_PER_LINE, 1,
-			       tdev->rx_buffer, MBOX_MAX_MSG_LEN, true);
+		print_hex_dump_bytes("Client: Received [MMIO]: ", DUMP_PREFIX_ADDRESS,
+				     tdev->rx_buffer, MBOX_MAX_MSG_LEN);
 	} else if (message) {
-		print_hex_dump(KERN_INFO, "Client: Received [API]: ",
-			       DUMP_PREFIX_ADDRESS, MBOX_BYTES_PER_LINE, 1,
-			       message, MBOX_MAX_MSG_LEN, true);
+		print_hex_dump_bytes("Client: Received [API]: ", DUMP_PREFIX_ADDRESS,
+				     message, MBOX_MAX_MSG_LEN);
 		memcpy(tdev->rx_buffer, message, MBOX_MAX_MSG_LEN);
 	}
 	spin_unlock_irqrestore(&tdev->lock, flags);
-- 
1.9.1

[PATCH v2 4/4] mailbox: mailbox-test: add support for separate tx/rx buffer with single channel

[Sudeep Holla]

This patch adds support for different MMIO region for Tx and Rx paths.
If only one region is specified, it's assumed to be shared between Rx
and Tx, thereby retaining backward compatibility.

Also in order to support single channel dealing with both Tx and Rx with
dedicated MMIO regions, Tx channel itself is assigned to Rx if MMIO
regions are different and Rx is not specified.

Cc: Jassi Brar <jassisinghbrar@gmail.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/mailbox-test.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
index f690f11969a1..dc11bbf27274 100644
--- a/drivers/mailbox/mailbox-test.c
+++ b/drivers/mailbox/mailbox-test.c
@@ -31,7 +31,8 @@ static struct dentry *root_debugfs_dir;
 
 struct mbox_test_device {
 	struct device		*dev;
-	void __iomem		*mmio;
+	void __iomem		*tx_mmio;
+	void __iomem		*rx_mmio;
 	struct mbox_chan	*tx_channel;
 	struct mbox_chan	*rx_channel;
 	char			*rx_buffer;
@@ -112,7 +113,7 @@ static ssize_t mbox_test_message_write(struct file *filp,
 	 * A separate signal is only of use if there is
 	 * MMIO to subsequently pass the message through
 	 */
-	if (tdev->mmio && tdev->signal) {
+	if (tdev->tx_mmio && tdev->signal) {
 		print_hex_dump_bytes("Client: Sending: Signal: ", DUMP_PREFIX_ADDRESS,
 				     tdev->signal, MBOX_MAX_SIG_LEN);
 
@@ -220,8 +221,8 @@ static void mbox_test_receive_message(struct mbox_client *client, void *message)
 	unsigned long flags;
 
 	spin_lock_irqsave(&tdev->lock, flags);
-	if (tdev->mmio) {
-		memcpy_fromio(tdev->rx_buffer, tdev->mmio, MBOX_MAX_MSG_LEN);
+	if (tdev->rx_mmio) {
+		memcpy_fromio(tdev->rx_buffer, tdev->rx_mmio, MBOX_MAX_MSG_LEN);
 		print_hex_dump_bytes("Client: Received [MMIO]: ", DUMP_PREFIX_ADDRESS,
 				     tdev->rx_buffer, MBOX_MAX_MSG_LEN);
 	} else if (message) {
@@ -236,11 +237,11 @@ static void mbox_test_prepare_message(struct mbox_client *client, void *message)
 {
 	struct mbox_test_device *tdev = dev_get_drvdata(client->dev);
 
-	if (tdev->mmio) {
+	if (tdev->tx_mmio) {
 		if (tdev->signal)
-			memcpy_toio(tdev->mmio, tdev->message, MBOX_MAX_MSG_LEN);
+			memcpy_toio(tdev->tx_mmio, tdev->message, MBOX_MAX_MSG_LEN);
 		else
-			memcpy_toio(tdev->mmio, message, MBOX_MAX_MSG_LEN);
+			memcpy_toio(tdev->tx_mmio, message, MBOX_MAX_MSG_LEN);
 	}
 }
 
@@ -294,9 +295,15 @@ static int mbox_test_probe(struct platform_device *pdev)
 
 	/* It's okay for MMIO to be NULL */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	tdev->mmio = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(tdev->mmio))
-		tdev->mmio = NULL;
+	tdev->tx_mmio = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(tdev->tx_mmio))
+		tdev->tx_mmio = NULL;
+
+	/* If specified, second reg entry is Rx MMIO */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	tdev->rx_mmio = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(tdev->rx_mmio))
+		tdev->rx_mmio = tdev->tx_mmio;
 
 	tdev->tx_channel = mbox_test_request_channel(pdev, "tx");
 	tdev->rx_channel = mbox_test_request_channel(pdev, "rx");
@@ -304,6 +311,10 @@ static int mbox_test_probe(struct platform_device *pdev)
 	if (!tdev->tx_channel && !tdev->rx_channel)
 		return -EPROBE_DEFER;
 
+	/* If Rx is not specified but has Rx MMIO, then Rx = Tx */
+	if (!tdev->rx_channel && (tdev->rx_mmio != tdev->tx_mmio))
+		tdev->rx_channel = tdev->tx_channel;
+
 	tdev->dev = &pdev->dev;
 	platform_set_drvdata(pdev, tdev);
 
-- 
1.9.1