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

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

[Lee Jones]

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

[PATCH v2 1/5] ARM: STi: stih407-family: Add nodes for Mailbox

[Lee Jones]

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 <lee.jones@linaro.org>
---
 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@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@1 {
+			compatible	= "st,stih407-mailbox";
+			reg		= <0x8f01000 0x1000>;
+			#mbox-cells	= <2>;
+			mbox-name	= "st231_gp_1";
+			status		= "okay";
+		};
+
+		mailbox2: mailbox@2 {
+			compatible	= "st,stih407-mailbox";
+			reg		= <0x8f02000 0x1000>;
+			#mbox-cells	= <2>;
+			mbox-name	= "st231_gp_0";
+			status		= "okay";
+		};
+
+		mailbox3: mailbox@3 {
+			compatible	= "st,stih407-mailbox";
+			reg		= <0x8f03000 0x1000>;
+			#mbox-cells	= <2>;
+			mbox-name	= "st231_audio_video";
+			status		= "okay";
+		};
 	};
 };
-- 
1.9.1

[PATCH v2 2/5] ARM: STi: DT: STiH407: Enable Mailbox testing facility

[Lee Jones]

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 <lee.jones@linaro.org>
---
 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@0  {
 			compatible	= "st,stih407-mailbox";
 			reg		= <0x8f00000 0x1000>;
-- 
1.9.1

[PATCH v2 3/5] mailbox: mailbox-test: Use more consistent format for calling copy_from_user()

[Lee Jones]

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

Suggested-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 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

[PATCH v2 4/5] mailbox: mailbox-test: Prevent memory leak

[Lee Jones]

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 <dan.carpenter@oracle.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 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

[PATCH v2 5/5] mailbox: Stop using ENOSYS for anything other than unimplemented syscalls

[Lee Jones]

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 <seraphin.bonnaffe@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 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

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

[Lee Jones]

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

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

[Jassi Brar]

On Tue, Apr 12, 2016 at 12:57 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 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.
>
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.

Re: [PATCH v2 2/5] ARM: STi: DT: STiH407: Enable Mailbox testing facility

[Sudeep Holla]

Hi Lee,

On 23/03/16 14:43, Lee Jones wrote:
> 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 <lee.jones@linaro.org>
> ---
>   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" ?

-- 
Regards,
Sudeep

Re: [PATCH v2 2/5] ARM: STi: DT: STiH407: Enable Mailbox testing facility

[Lee Jones]

On Tue, 12 Apr 2016, Sudeep Holla wrote:

> Hi Lee,
> 
> On 23/03/16 14:43, Lee Jones wrote:
> >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 <lee.jones@linaro.org>
> >---
> >  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

Re: [PATCH v2 2/5] ARM: STi: DT: STiH407: Enable Mailbox testing facility

[Olof Johansson]

Hi,



On Wed, Mar 23, 2016 at 7:43 AM, Lee Jones <lee.jones@linaro.org> wrote:
> 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 <lee.jones@linaro.org>
> ---
>  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";
> +               };

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

Re: [PATCH v2 1/5] ARM: STi: stih407-family: Add nodes for Mailbox

[Olof Johansson]

Hi,

On Wed, Mar 23, 2016 at 7:43 AM, Lee Jones <lee.jones@linaro.org> wrote:
> 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 <lee.jones@linaro.org>
> ---
>  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@0  {

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


-Olof