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

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

[Sudeep Holla]

Hi Lee, 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. Let me know your thoughts on that.

Regards,
Sudeep

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                     | 50 ++++++++++++----------
 2 files changed, 29 insertions(+), 23 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Lee Jones]

On Thu, 11 Feb 2016, Sudeep Holla wrote:

> 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>
> Cc: 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(-)

Acked-by: Lee Jones <lee.jones@linaro.org>

> 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,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Sudeep Holla]

Hi Jassi,

On 11/02/16 17:13, Sudeep Holla wrote:
> Hi Lee, 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. Let me know your thoughts on that.
>

Can you review this ? It would be good to get this in for v4.6

-- 
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Jassi Brar]

On Mon, Feb 15, 2016 at 5:47 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> Hi Jassi,
>
> On 11/02/16 17:13, Sudeep Holla wrote:
>>
>> Hi Lee, 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. Let me know your thoughts on that.
>>
>
> Can you review this ? It would be good to get this in for v4.6
>
Seems simple enough. So once you and Lee are in sync...

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

[Sudeep Holla]

On 15/02/16 12:31, Jassi Brar wrote:
> On Mon, Feb 15, 2016 at 5:47 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> Hi Jassi,
>>
>> On 11/02/16 17:13, Sudeep Holla wrote:
>>>
>>> Hi Lee, 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. Let me know your thoughts on that.
>>>
>>
>> Can you review this ? It would be good to get this in for v4.6
>>
> Seems simple enough. So once you and Lee are in sync...
>

That was quick :). Anyways IIUC Lee agrees with most of the changes.

The question I am asking is not part of this series, I didn't make the
change to remove hexdump yet, I am trying to understand the reasoning
for doing that and also explaining my requirement as why I want to
remove it.

-- 
Regards,
Sudeep

[PATCH 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>
Cc: 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

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

[Lee Jones]

On Thu, 11 Feb 2016, Sudeep Holla wrote:

> 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>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Lee Jones <lee.jones@linaro.org>

> ---
>  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" },
>  	{},
>  };
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Rob Herring]

On Thu, Feb 11, 2016 at 05:13:26PM +0000, Sudeep Holla wrote:
> Underscores are usually forbidden in the compatible strings. So lets
> remove it before the first users of this is seen.
> 
> Cc: Jassi Brar <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mailbox/sti-mailbox.txt | 2 +-
>  drivers/mailbox/mailbox-test.c                            | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 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

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

[Lee Jones]

On Thu, 11 Feb 2016, Sudeep Holla wrote:

> 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.

Printing out the data in this way is kinda the point of the driver.
But if you don't have a use for it, then I guess it's okay to
downgrade it to DEBUG, as turning debug on, or hacking the driver back
again are trivial actions.

> Cc: Jassi Brar <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> ---
>  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);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Sudeep Holla]

On 12/02/16 09:12, Lee Jones wrote:
> On Thu, 11 Feb 2016, Sudeep Holla wrote:
>
>> 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.
>
> Printing out the data in this way is kinda the point of the driver.
> But if you don't have a use for it, then I guess it's okay to
> downgrade it to DEBUG, as turning debug on, or hacking the driver back
> again are trivial actions.
>

Agreed, I wanted check the same by adding additional note but forgot.
Sorry for missing that.

One of the reasons I preferred this it's easy to switch it on or off via
debug level or dynamic debug. It's useful when the test application
using these sysfs also logs and ends up cluttered by kernel logs.

-- 
Regards,
Sudeep

[PATCH 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>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/mailbox-test.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
index f690f11969a1..767b9ec37a96 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,13 @@ 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;
+	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 +309,9 @@ static int mbox_test_probe(struct platform_device *pdev)
 	if (!tdev->tx_channel && !tdev->rx_channel)
 		return -EPROBE_DEFER;
 
+	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

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

[Lee Jones]

On Thu, 11 Feb 2016, Sudeep Holla wrote:

> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/mailbox/mailbox-test.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
> index f690f11969a1..767b9ec37a96 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,13 @@ 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;

Nit: I'd prefer to see a new line separator here.

> +	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 +309,9 @@ static int mbox_test_probe(struct platform_device *pdev)
>  	if (!tdev->tx_channel && !tdev->rx_channel)
>  		return -EPROBE_DEFER;
>  
> +	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);

Otherwise code looks good.  Nice extension.

Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Sudeep Holla]

On 12/02/16 09:15, Lee Jones wrote:
> On Thu, 11 Feb 2016, Sudeep Holla wrote:
>

[...]

>> @@ -294,9 +295,13 @@ 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;
>
> Nit: I'd prefer to see a new line separator here.
>

OK will add it

>> +	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 +309,9 @@ static int mbox_test_probe(struct platform_device *pdev)
>>   	if (!tdev->tx_channel && !tdev->rx_channel)
>>   		return -EPROBE_DEFER;
>>
>> +	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);
>
> Otherwise code looks good.  Nice extension.
>

Thanks for the review.

> Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>

-- 
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Lee Jones]

On Thu, 11 Feb 2016, Sudeep Holla wrote:

> Hi Lee, 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) 

Not sure quite what you mean.  Hexdump can handle any data?  If the
hex value read isn't an ASCII value, a '.' is printed on the right
hand side.  What data are you expecting that you can't analyse with
hexdump?

> and userspace can deal with the
> actual interpretation of the data. Let me know your thoughts on that.
> 
> Regards,
> Sudeep
> 
> 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                     | 50 ++++++++++++----------
>  2 files changed, 29 insertions(+), 23 deletions(-)
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Sudeep Holla]

On 12/02/16 09:17, Lee Jones wrote:
> On Thu, 11 Feb 2016, Sudeep Holla wrote:
>
>> Hi Lee, 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)
>
> Not sure quite what you mean.  Hexdump can handle any data?  If the
> hex value read isn't an ASCII value, a '.' is printed on the right
> hand side.  What data are you expecting that you can't analyse with
> hexdump?
>

Sorry for not being clear. Hexdump can handle any data. I do see that
it displays quite nicely at the driver level which is nice.

However my question was why is the the buffer copied to user space is
not the original raw data, rather it's ASCII converted which is good for
nice logging.

One of the use-case the testing guys wants is the check the protocol
specification using this. In their case they expect the test driver to
send the raw data as is from the driver rather than the converted ASCII
buffer. I tend to agree with them for 2 reasons:

1. userspace can do the conversion if required
2. the input buffer is not converted(i.e. in the write path), so it's
    bit of inconsistent there.

-- 
Regards,
Sudeep

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

[Sudeep Holla]

Hi Lee,

On 12/02/16 09:41, Sudeep Holla wrote:
>
>
> On 12/02/16 09:17, Lee Jones wrote:
>> On Thu, 11 Feb 2016, Sudeep Holla wrote:
>>
>>> Hi Lee, 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)
>>
>> Not sure quite what you mean.  Hexdump can handle any data?  If the
>> hex value read isn't an ASCII value, a '.' is printed on the right
>> hand side.  What data are you expecting that you can't analyse with
>> hexdump?
>>
>
> Sorry for not being clear. Hexdump can handle any data. I do see that
> it displays quite nicely at the driver level which is nice.
>
> However my question was why is the the buffer copied to user space is
> not the original raw data, rather it's ASCII converted which is good for
> nice logging.
>
> One of the use-case the testing guys wants is the check the protocol
> specification using this. In their case they expect the test driver to
> send the raw data as is from the driver rather than the converted ASCII
> buffer. I tend to agree with them for 2 reasons:
>
> 1. userspace can do the conversion if required
> 2. the input buffer is not converted(i.e. in the write path), so it's
>     bit of inconsistent there.
>

Thoughts on this or am I still not clear ?

-- 
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html