[v7,1/6] dt-bindings: mailbox: allow mbox-cells to be equal to 0

Commit Message

Oleksij Rempel July 26, 2018, 6:53 a.m. UTC

From: Dong Aisheng <[e-mail]>

Mailbox devices may have only one channel which means the mbox-cells
at least 1 does not make sense for this type devices. Let's remove
that limitation to allow the mbox-cells to be equal to 0.

Cc: Mark Rutland <[e-mail]>
Cc: Sudeep Holla <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Reviewed-by: Rob Herring <[e-mail]>
Signed-off-by: Dong Aisheng <[e-mail]>
---
 Documentation/devicetree/bindings/mailbox/mailbox.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Jassi Brar July 26, 2018, 9:41 a.m. UTC | #1

On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel
<[e-mail]> wrote:
> From: Dong Aisheng <[e-mail]>
>
> Mailbox devices may have only one channel which means the mbox-cells
> at least 1 does not make sense for this type devices. Let's remove
> that limitation to allow the mbox-cells to be equal to 0.
>
OK

But please realise that you'll have to write more code -- implement
your own of_xlate() callback.
Most developers would prefer to call the only channel as channel
number 0, and use the common of_xlate implementation.

-jassi

Aisheng Dong July 26, 2018, 11 a.m. UTC | #2

> -----Original Message-----
> From: Jassi Brar [mailto:[e-mail]]
> Sent: Thursday, July 26, 2018 5:42 PM
> To: Oleksij Rempel <[e-mail]>
> Cc: Shawn Guo <[e-mail]>; Fabio Estevam
> <[e-mail]>; Rob Herring <[e-mail]>; Mark
> Rutland <[e-mail]>; A.s. Dong <[e-mail]>;
> Vladimir Zapolskiy <[e-mail]>; Sudeep Holla
> <[e-mail]>; Devicetree List <[e-mail]>;
> Linux Kernel Mailing List <[e-mail]>; , Sascha Hauer
> <[e-mail]>; , [e-mail], linux-
> [e-mail], srv_heupstream <linux-arm-
> [e-mail]>; dl-linux-imx <[e-mail]>
> Subject: Re: [PATCH v7 1/6] dt-bindings: mailbox: allow mbox-cells to be
> equal to 0
> 
> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel <[e-mail]>
> wrote:
> > From: Dong Aisheng <[e-mail]>
> >
> > Mailbox devices may have only one channel which means the mbox-cells
> > at least 1 does not make sense for this type devices. Let's remove
> > that limitation to allow the mbox-cells to be equal to 0.
> >
> OK
> 
> But please realise that you'll have to write more code -- implement your own
> of_xlate() callback.
> Most developers would prefer to call the only channel as channel number 0,
> and use the common of_xlate implementation.
> 

Yes, that's true because the common of_xlate assuming the mbox-cells is 1.
And there's already such users:
arch/arm/boot/dts/bcm283x.dtsi

More comments will be on your reply of patch-3.

Regards
Dong Aisheng

> -jassi

Jassi Brar July 26, 2018, 11:36 a.m. UTC | #3

On Thu, Jul 26, 2018 at 4:30 PM, A.s. Dong <[e-mail]> wrote:
>> -----Original Message-----
>> From: Jassi Brar [mailto:[e-mail]]
>> Sent: Thursday, July 26, 2018 5:42 PM
>> To: Oleksij Rempel <[e-mail]>
>> Cc: Shawn Guo <[e-mail]>; Fabio Estevam
>> <[e-mail]>; Rob Herring <[e-mail]>; Mark
>> Rutland <[e-mail]>; A.s. Dong <[e-mail]>;
>> Vladimir Zapolskiy <[e-mail]>; Sudeep Holla
>> <[e-mail]>; Devicetree List <[e-mail]>;
>> Linux Kernel Mailing List <[e-mail]>; , Sascha Hauer
>> <[e-mail]>; , [e-mail], linux-
>> [e-mail], srv_heupstream <linux-arm-
>> [e-mail]>; dl-linux-imx <[e-mail]>
>> Subject: Re: [PATCH v7 1/6] dt-bindings: mailbox: allow mbox-cells to be
>> equal to 0
>>
>> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel <[e-mail]>
>> wrote:
>> > From: Dong Aisheng <[e-mail]>
>> >
>> > Mailbox devices may have only one channel which means the mbox-cells
>> > at least 1 does not make sense for this type devices. Let's remove
>> > that limitation to allow the mbox-cells to be equal to 0.
>> >
>> OK
>>
>> But please realise that you'll have to write more code -- implement your own
>> of_xlate() callback.
>> Most developers would prefer to call the only channel as channel number 0,
>> and use the common of_xlate implementation.
>>
>
> Yes, that's true because the common of_xlate assuming the mbox-cells is 1.
> And there's already such users:
> arch/arm/boot/dts/bcm283x.dtsi
>
You mean that is a good thing, right?

Aisheng Dong July 26, 2018, 11:55 a.m. UTC | #4

> -----Original Message-----
> From: Jassi Brar [mailto:[e-mail]]
> Sent: Thursday, July 26, 2018 7:37 PM
> To: A.s. Dong <[e-mail]>
> Cc: Oleksij Rempel <[e-mail]>; Shawn Guo
> <[e-mail]>; Fabio Estevam <[e-mail]>; Rob
> Herring <[e-mail]>; Mark Rutland <[e-mail]>;
> Vladimir Zapolskiy <[e-mail]>; Sudeep Holla
> <[e-mail]>; Devicetree List <[e-mail]>;
> Linux Kernel Mailing List <[e-mail]>; , Sascha Hauer
> <[e-mail]>; , [e-mail], linux-
> [e-mail], srv_heupstream <linux-arm-
> [e-mail]>; dl-linux-imx <[e-mail]>
> Subject: Re: [PATCH v7 1/6] dt-bindings: mailbox: allow mbox-cells to be
> equal to 0
> 
> On Thu, Jul 26, 2018 at 4:30 PM, A.s. Dong <[e-mail]> wrote:
> >> -----Original Message-----
> >> From: Jassi Brar [mailto:[e-mail]]
> >> Sent: Thursday, July 26, 2018 5:42 PM
> >> To: Oleksij Rempel <[e-mail]>
> >> Cc: Shawn Guo <[e-mail]>; Fabio Estevam
> >> <[e-mail]>; Rob Herring <[e-mail]>; Mark
> >> Rutland <[e-mail]>; A.s. Dong <[e-mail]>;
> >> Vladimir Zapolskiy <[e-mail]>; Sudeep Holla
> >> <[e-mail]>; Devicetree List <[e-mail]>;
> >> Linux Kernel Mailing List <[e-mail]>; , Sascha
> >> Hauer <[e-mail]>; ,
> >> [e-mail], linux-
> >> [e-mail], srv_heupstream <linux-arm-
> >> [e-mail]>; dl-linux-imx <[e-mail]>
> >> Subject: Re: [PATCH v7 1/6] dt-bindings: mailbox: allow mbox-cells to
> >> be equal to 0
> >>
> >> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel
> >> <[e-mail]>
> >> wrote:
> >> > From: Dong Aisheng <[e-mail]>
> >> >
> >> > Mailbox devices may have only one channel which means the
> >> > mbox-cells at least 1 does not make sense for this type devices.
> >> > Let's remove that limitation to allow the mbox-cells to be equal to 0.
> >> >
> >> OK
> >>
> >> But please realise that you'll have to write more code -- implement
> >> your own
> >> of_xlate() callback.
> >> Most developers would prefer to call the only channel as channel
> >> number 0, and use the common of_xlate implementation.
> >>
> >
> > Yes, that's true because the common of_xlate assuming the mbox-cells is 1.
> > And there's already such users:
> > arch/arm/boot/dts/bcm283x.dtsi
> >
> You mean that is a good thing, right?

No, I mean there's similar HW in kernel already that only has one channel.
Are you suggesting even for one channel mailbox, we should still use
Mbox-cells 1 and use 'mboxes = <&mailbox 0>' in devicetree?

Regards
Dong Aisheng

Jassi Brar July 26, 2018, 12:04 p.m. UTC | #5

On Thu, Jul 26, 2018 at 5:25 PM, A.s. Dong <[e-mail]> wrote:
>> -----Original Message-----
>> From: Jassi Brar [mailto:[e-mail]]
>> Sent: Thursday, July 26, 2018 7:37 PM
>> To: A.s. Dong <[e-mail]>
>> Cc: Oleksij Rempel <[e-mail]>; Shawn Guo
>> <[e-mail]>; Fabio Estevam <[e-mail]>; Rob
>> Herring <[e-mail]>; Mark Rutland <[e-mail]>;
>> Vladimir Zapolskiy <[e-mail]>; Sudeep Holla
>> <[e-mail]>; Devicetree List <[e-mail]>;
>> Linux Kernel Mailing List <[e-mail]>; , Sascha Hauer
>> <[e-mail]>; , [e-mail], linux-
>> [e-mail], srv_heupstream <linux-arm-
>> [e-mail]>; dl-linux-imx <[e-mail]>
>> Subject: Re: [PATCH v7 1/6] dt-bindings: mailbox: allow mbox-cells to be
>> equal to 0
>>
>> On Thu, Jul 26, 2018 at 4:30 PM, A.s. Dong <[e-mail]> wrote:
>> >> -----Original Message-----
>> >> From: Jassi Brar [mailto:[e-mail]]
>> >> Sent: Thursday, July 26, 2018 5:42 PM
>> >> To: Oleksij Rempel <[e-mail]>
>> >> Cc: Shawn Guo <[e-mail]>; Fabio Estevam
>> >> <[e-mail]>; Rob Herring <[e-mail]>; Mark
>> >> Rutland <[e-mail]>; A.s. Dong <[e-mail]>;
>> >> Vladimir Zapolskiy <[e-mail]>; Sudeep Holla
>> >> <[e-mail]>; Devicetree List <[e-mail]>;
>> >> Linux Kernel Mailing List <[e-mail]>; , Sascha
>> >> Hauer <[e-mail]>; ,
>> >> [e-mail], linux-
>> >> [e-mail], srv_heupstream <linux-arm-
>> >> [e-mail]>; dl-linux-imx <[e-mail]>
>> >> Subject: Re: [PATCH v7 1/6] dt-bindings: mailbox: allow mbox-cells to
>> >> be equal to 0
>> >>
>> >> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel
>> >> <[e-mail]>
>> >> wrote:
>> >> > From: Dong Aisheng <[e-mail]>
>> >> >
>> >> > Mailbox devices may have only one channel which means the
>> >> > mbox-cells at least 1 does not make sense for this type devices.
>> >> > Let's remove that limitation to allow the mbox-cells to be equal to 0.
>> >> >
>> >> OK
>> >>
>> >> But please realise that you'll have to write more code -- implement
>> >> your own
>> >> of_xlate() callback.
>> >> Most developers would prefer to call the only channel as channel
>> >> number 0, and use the common of_xlate implementation.
>> >>
>> >
>> > Yes, that's true because the common of_xlate assuming the mbox-cells is 1.
>> > And there's already such users:
>> > arch/arm/boot/dts/bcm283x.dtsi
>> >
>> You mean that is a good thing, right?
>
> No, I mean there's similar HW in kernel already that only has one channel.
> Are you suggesting even for one channel mailbox, we should still use
> Mbox-cells 1 and use 'mboxes = <&mailbox 0>' in devicetree?
>
As i said, I am ok with the mbox-cells to be 0.

But if you have two instances of the controller, one with 4 channels
and the other with 1 channel. Then you can/should not have different
bindings.

And, if the controller can have only 1 channel, then please feel free
to have mbox-cells=0 and implement your own of_xlate().

Patch

diff --git a/Documentation/devicetree/bindings/mailbox/mailbox.txt b/Documentation/devicetree/bindings/mailbox/mailbox.txt
index af8ecee2ac68..c2fcd054141a 100644
--- a/Documentation/devicetree/bindings/mailbox/mailbox.txt
+++ b/Documentation/devicetree/bindings/mailbox/mailbox.txt
@@ -6,8 +6,7 @@  assign appropriate mailbox channel to client drivers.
 * Mailbox Controller
 
 Required property:
-- #mbox-cells: Must be at least 1. Number of cells in a mailbox
-		specifier.
+- #mbox-cells: Number of cells in a mailbox specifier.
 
 Example:
 	mailbox: mailbox {