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 <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
While we're at it, ensure copy-to location is NULL'ed in the error path.
Suggested-by: Dan Carpenter <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
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 <[email protected]> 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.
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> >---
> > 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 <[email protected]> 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 <[email protected]>
> ---
> 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
Hi,
On Wed, Mar 23, 2016 at 7:43 AM, Lee Jones <[email protected]> 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 <[email protected]>
> ---
> 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