[PATCH v2 0/5] mailbox: A few fixes due for the v4.6 -rcs

Hi Jassi,

Resending these with patches 1 and 2 merged, as requested.

Kind regards,
Lee

v1 => v2:
  - Patch 2 merged into patch 
  - No functional changes

Lee Jones (5):
  ARM: STi: stih407-family: Add nodes for Mailbox
  ARM: STi: DT: STiH407: Enable Mailbox testing facility
  mailbox: mailbox-test: Use more consistent format for calling
    copy_from_user()
  mailbox: mailbox-test: Prevent memory leak
  mailbox: Stop using ENOSYS for anything other than unimplemented
    syscalls

 arch/arm/boot/dts/stih407-family.dtsi | 39 +++++++++++++++++++++++++++++++++++
 drivers/mailbox/mailbox-test.c        | 16 +++++++-------
 drivers/mailbox/mailbox.c             |  4 ++--
 3 files changed, 50 insertions(+), 9 deletions(-)

-- 
1.9.1

If we set the Signal twice or more, without using it as part of a message,
memory will be re-allocated and the pointer over-written.  Prevent this
potential leak by only allocating memory when there isn't any already.

Reported-by: Dan Carpenter <redacted>
Signed-off-by: Lee Jones <redacted>
---
 drivers/mailbox/mailbox-test.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
index 5f4b439..58d0472 100644
--- a/drivers/mailbox/mailbox-test.c
+++ b/drivers/mailbox/mailbox-test.c

@@ -59,9 +59,12 @@ static ssize_t mbox_test_signal_write(struct file *filp,
 		return -EINVAL;
 	}
 
-	tdev->signal = kzalloc(MBOX_MAX_SIG_LEN, GFP_KERNEL);
-	if (!tdev->signal)
-		return -ENOMEM;
+	/* Only allocate memory if we need to */
+	if (!tdev->signal) {
+		tdev->signal = kzalloc(MBOX_MAX_SIG_LEN, GFP_KERNEL);
+		if (!tdev->signal)
+			return -ENOMEM;
+	}
 
 	if (copy_from_user(tdev->signal, userbuf, count)) {
 		kfree(tdev->signal);

-- 
1.9.1

While we're at it, ensure copy-to location is NULL'ed in the error path.

Suggested-by: Dan Carpenter <redacted>
Signed-off-by: Lee Jones <redacted>
---
 drivers/mailbox/mailbox-test.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
index dc11bbf..5f4b439 100644
--- a/drivers/mailbox/mailbox-test.c
+++ b/drivers/mailbox/mailbox-test.c

@@ -46,7 +46,6 @@ static ssize_t mbox_test_signal_write(struct file *filp,
 				       size_t count, loff_t *ppos)
 {
 	struct mbox_test_device *tdev = filp->private_data;
-	int ret;
 
 	if (!tdev->tx_channel) {
 		dev_err(tdev->dev, "Channel cannot do Tx\n");

@@ -64,13 +63,13 @@ static ssize_t mbox_test_signal_write(struct file *filp,
 	if (!tdev->signal)
 		return -ENOMEM;
 
-	ret = copy_from_user(tdev->signal, userbuf, count);
-	if (ret) {
+	if (copy_from_user(tdev->signal, userbuf, count)) {
 		kfree(tdev->signal);
+		tdev->signal = NULL;
 		return -EFAULT;
 	}
 
-	return ret < 0 ? ret : count;
+	return count;
 }
 
 static const struct file_operations mbox_test_signal_ops = {

-- 
1.9.1

This patch supplies a Client node to enable the Mailbox testing
facility.  It will be used to send and receive messages from any
given co-processor in order to test the STi Mailbox Controller
driver.

Signed-off-by: Lee Jones <redacted>
---
 arch/arm/boot/dts/stih407-family.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index e6e34b4..9376c38 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi

@@ -695,6 +695,12 @@
 			status		= "okay";
 		};
 
+		mailbox_test {
+			compatible = "mailbox_test";
+			mboxes = <&mailbox2 1 31>, <&mailbox0 1 31>;
+			mbox-names = "tx", "rx";
+		};
+
 		mailbox0: mailbox at 0  {
 			compatible	= "st,stih407-mailbox";
 			reg		= <0x8f00000 0x1000>;

-- 
1.9.1

This patch supplies the Mailbox Controller nodes.  In order to
request channels, these nodes will be referenced by Mailbox
Client nodes.

Signed-off-by: Lee Jones <redacted>
---
 arch/arm/boot/dts/stih407-family.dtsi | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index 81f8121..e6e34b4 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi

@@ -694,5 +694,38 @@
 			clocks          = <&clk_sysin>;
 			status		= "okay";
 		};
+
+		mailbox0: mailbox at 0  {
+			compatible	= "st,stih407-mailbox";
+			reg		= <0x8f00000 0x1000>;
+			interrupts	= <GIC_SPI 1 IRQ_TYPE_NONE>;
+			#mbox-cells	= <2>;
+			mbox-name	= "a9";
+			status		= "okay";
+		};
+
+		mailbox1: mailbox at 1 {
+			compatible	= "st,stih407-mailbox";
+			reg		= <0x8f01000 0x1000>;
+			#mbox-cells	= <2>;
+			mbox-name	= "st231_gp_1";
+			status		= "okay";
+		};
+
+		mailbox2: mailbox at 2 {
+			compatible	= "st,stih407-mailbox";
+			reg		= <0x8f02000 0x1000>;
+			#mbox-cells	= <2>;
+			mbox-name	= "st231_gp_0";
+			status		= "okay";
+		};
+
+		mailbox3: mailbox at 3 {
+			compatible	= "st,stih407-mailbox";
+			reg		= <0x8f03000 0x1000>;
+			#mbox-cells	= <2>;
+			mbox-name	= "st231_audio_video";
+			status		= "okay";
+		};
 	};
 };

-- 
1.9.1

In accordance with e15f431fe2d5 ("errno.h: Improve ENOSYS's comment") and
91c9afaf97ee ("checkpatch.pl: new instances of ENOSYS are errors") we're
converting from the old meaning of: ENOSYS "Function not implemented" to
a more standard EINVAL.

Reported-by: Seraphin Bonnaffe <redacted>
Signed-off-by: Lee Jones <redacted>
---
 drivers/mailbox/mailbox.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 6a4811f..4a36632 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c

@@ -375,13 +375,13 @@ struct mbox_chan *mbox_request_channel_byname(struct mbox_client *cl,
 
 	if (!np) {
 		dev_err(cl->dev, "%s() currently only supports DT\n", __func__);
-		return ERR_PTR(-ENOSYS);
+		return ERR_PTR(-EINVAL);
 	}
 
 	if (!of_get_property(np, "mbox-names", NULL)) {
 		dev_err(cl->dev,
 			"%s() requires an \"mbox-names\" property\n", __func__);
-		return ERR_PTR(-ENOSYS);
+		return ERR_PTR(-EINVAL);
 	}
 
 	of_property_for_each_string(np, "mbox-names", prop, mbox_name) {

-- 
1.9.1

On Wed, 23 Mar 2016, Lee Jones wrote:

Hi Jassi,

Resending these with patches 1 and 2 merged, as requested.

Jassi,

Not heard anything from you in quite some time now.

Kind regards,
Lee

v1 => v2:
  - Patch 2 merged into patch 
  - No functional changes

Lee Jones (5):
  ARM: STi: stih407-family: Add nodes for Mailbox
  ARM: STi: DT: STiH407: Enable Mailbox testing facility
  mailbox: mailbox-test: Use more consistent format for calling
    copy_from_user()
  mailbox: mailbox-test: Prevent memory leak
  mailbox: Stop using ENOSYS for anything other than unimplemented
    syscalls

 arch/arm/boot/dts/stih407-family.dtsi | 39 +++++++++++++++++++++++++++++++++++
 drivers/mailbox/mailbox-test.c        | 16 +++++++-------
 drivers/mailbox/mailbox.c             |  4 ++--
 3 files changed, 50 insertions(+), 9 deletions(-)

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

On Tue, Apr 12, 2016 at 12:57 PM, Lee Jones [off-list ref] wrote:

On Wed, 23 Mar 2016, Lee Jones wrote:

quoted

Hi Jassi,

Resending these with patches 1 and 2 merged, as requested.

Jassi,

Not heard anything from you in quite some time now.

Please feel free to call as soon as it goes beyond 2weeks.

Patch-1 & 2 should not go via mailbox tree, as you know.
3,4&5 have been picked.

Thanks.

Hi Lee,

On 23/03/16 14:43, Lee Jones wrote:

IIRC, we changed this to "mailbox-text" ?

-- 
Regards,
Sudeep

On Tue, 12 Apr 2016, Sudeep Holla wrote:

Hi Lee,

On 23/03/16 14:43, Lee Jones wrote:

quoted

This patch supplies a Client node to enable the Mailbox testing
facility.  It will be used to send and receive messages from any
given co-processor in order to test the STi Mailbox Controller
driver.

Signed-off-by: Lee Jones <redacted>
---
 arch/arm/boot/dts/stih407-family.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index e6e34b4..9376c38 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi

@@ -695,6 +695,12 @@
 			status		= "okay";
 		};

+		mailbox_test {
+			compatible = "mailbox_test";

IIRC, we changed this to "mailbox-text" ?

Great spot, thanks.  I will fix before sending to Maxime.

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

Hi,



On Wed, Mar 23, 2016 at 7:43 AM, Lee Jones [off-list ref] wrote:

As far as I understand, this doesn't describe something in your
system, and just configures the test driver to run a test. Is that
correct?



-Olof

Hi,

On Wed, Mar 23, 2016 at 7:43 AM, Lee Jones [off-list ref] wrote:

This isn't the right unit-id here. It should be @8f000000. Same for
the other two.


-Olof