Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Joseph Lo]

On 07/19/2016 07:13 AM, Stephen Warren wrote:

On 07/11/2016 10:08 AM, Stephen Warren wrote:

On 07/11/2016 08:14 AM, Rob Herring wrote:

On Thu, Jul 07, 2016 at 12:35:02PM -0600, Stephen Warren wrote:

On 07/07/2016 12:13 PM, Sivaram Nair wrote:

On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives
for
interprocessor communication. So the interprocessor communication
(IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.



diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h
b/include/dt-bindings/mailbox/tegra186-hsp.h



+#define HSP_MBOX_TYPE_DB 0x0
+#define HSP_MBOX_TYPE_SM 0x1
+#define HSP_MBOX_TYPE_SS 0x2
+#define HSP_MBOX_TYPE_AS 0x3
+
+#define HSP_DB_MASTER_CCPLEX 17
+#define HSP_DB_MASTER_BPMP 19
+
+#define HSP_MBOX_ID(type, ID) \
+(HSP_MBOX_TYPE_##type << 16 | ID)


It will be nicer if you avoid the macro glue magic '##' for 'type'. I
would also suggest to use braces around 'type' and 'ID'.


This technique been used without issue in quite a few other places
without
issue, and has the benefit of simplifying the text wherever the
macro is
used. What issue do you foresee?


I'm not a fan of using the macros to begin with and less so anything
more complex than a single constant value. I'd rather see 2 cells here
with the first being the id and the 2nd being the type.

An issue with token pasting is grepping for DB, SM, etc. in kernel tree
is probably noisy. Not such a big deal here, but a major PIA when you
have more complex sets of includes.


Is that a NAK or simply a suggestion? Having a single cell makes DT
parsing a bit simpler, since pretty much every SW stack provides a
default "one-cell" of_xlate implementation, whereas >1 cell means custom
code for of_xlate.


I didn't see a response to this. Joseph, let's just use two cells
instead. I'm rather desperately waiting for this binding to be complete
so I can finalize the U-Boot code that uses it, and it sounds like
changing to two cells will get an ack faster. Can you post an updated
version of this series today/ASAP to get things moving? Thanks.


Okay, will use two cells instead.

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Joseph Lo]

On 07/19/2016 07:13 AM, Stephen Warren wrote:

On 07/11/2016 10:08 AM, Stephen Warren wrote:

On 07/11/2016 08:14 AM, Rob Herring wrote:

On Thu, Jul 07, 2016 at 12:35:02PM -0600, Stephen Warren wrote:

On 07/07/2016 12:13 PM, Sivaram Nair wrote:

On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives
for
interprocessor communication. So the interprocessor communication
(IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.



diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h
b/include/dt-bindings/mailbox/tegra186-hsp.h



+#define HSP_MBOX_TYPE_DB 0x0
+#define HSP_MBOX_TYPE_SM 0x1
+#define HSP_MBOX_TYPE_SS 0x2
+#define HSP_MBOX_TYPE_AS 0x3
+
+#define HSP_DB_MASTER_CCPLEX 17
+#define HSP_DB_MASTER_BPMP 19
+
+#define HSP_MBOX_ID(type, ID) \
+(HSP_MBOX_TYPE_##type << 16 | ID)


It will be nicer if you avoid the macro glue magic '##' for 'type'. I
would also suggest to use braces around 'type' and 'ID'.


This technique been used without issue in quite a few other places
without
issue, and has the benefit of simplifying the text wherever the
macro is
used. What issue do you foresee?


I'm not a fan of using the macros to begin with and less so anything
more complex than a single constant value. I'd rather see 2 cells here
with the first being the id and the 2nd being the type.

An issue with token pasting is grepping for DB, SM, etc. in kernel tree
is probably noisy. Not such a big deal here, but a major PIA when you
have more complex sets of includes.


Is that a NAK or simply a suggestion? Having a single cell makes DT
parsing a bit simpler, since pretty much every SW stack provides a
default "one-cell" of_xlate implementation, whereas >1 cell means custom
code for of_xlate.


I didn't see a response to this. Joseph, let's just use two cells
instead. I'm rather desperately waiting for this binding to be complete
so I can finalize the U-Boot code that uses it, and it sounds like
changing to two cells will get an ack faster. Can you post an updated
version of this series today/ASAP to get things moving? Thanks.


Okay, will use two cells instead.

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Stephen Warren]

On 07/11/2016 10:08 AM, Stephen Warren wrote:

On 07/11/2016 08:14 AM, Rob Herring wrote:

On Thu, Jul 07, 2016 at 12:35:02PM -0600, Stephen Warren wrote:

On 07/07/2016 12:13 PM, Sivaram Nair wrote:

On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives for
interprocessor communication. So the interprocessor communication
(IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.



diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h
b/include/dt-bindings/mailbox/tegra186-hsp.h



+#define HSP_MBOX_TYPE_DB 0x0
+#define HSP_MBOX_TYPE_SM 0x1
+#define HSP_MBOX_TYPE_SS 0x2
+#define HSP_MBOX_TYPE_AS 0x3
+
+#define HSP_DB_MASTER_CCPLEX 17
+#define HSP_DB_MASTER_BPMP 19
+
+#define HSP_MBOX_ID(type, ID) \
+(HSP_MBOX_TYPE_##type << 16 | ID)


It will be nicer if you avoid the macro glue magic '##' for 'type'. I
would also suggest to use braces around 'type' and 'ID'.


This technique been used without issue in quite a few other places
without
issue, and has the benefit of simplifying the text wherever the macro is
used. What issue do you foresee?


I'm not a fan of using the macros to begin with and less so anything
more complex than a single constant value. I'd rather see 2 cells here
with the first being the id and the 2nd being the type.

An issue with token pasting is grepping for DB, SM, etc. in kernel tree
is probably noisy. Not such a big deal here, but a major PIA when you
have more complex sets of includes.


Is that a NAK or simply a suggestion? Having a single cell makes DT
parsing a bit simpler, since pretty much every SW stack provides a
default "one-cell" of_xlate implementation, whereas >1 cell means custom
code for of_xlate.


I didn't see a response to this. Joseph, let's just use two cells 
instead. I'm rather desperately waiting for this binding to be complete 
so I can finalize the U-Boot code that uses it, and it sounds like 
changing to two cells will get an ack faster. Can you post an updated 
version of this series today/ASAP to get things moving? Thanks.

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Stephen Warren]

On 07/11/2016 10:08 AM, Stephen Warren wrote:

On 07/11/2016 08:14 AM, Rob Herring wrote:

On Thu, Jul 07, 2016 at 12:35:02PM -0600, Stephen Warren wrote:

On 07/07/2016 12:13 PM, Sivaram Nair wrote:

On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives for
interprocessor communication. So the interprocessor communication
(IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.



diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h
b/include/dt-bindings/mailbox/tegra186-hsp.h



+#define HSP_MBOX_TYPE_DB 0x0
+#define HSP_MBOX_TYPE_SM 0x1
+#define HSP_MBOX_TYPE_SS 0x2
+#define HSP_MBOX_TYPE_AS 0x3
+
+#define HSP_DB_MASTER_CCPLEX 17
+#define HSP_DB_MASTER_BPMP 19
+
+#define HSP_MBOX_ID(type, ID) \
+(HSP_MBOX_TYPE_##type << 16 | ID)


It will be nicer if you avoid the macro glue magic '##' for 'type'. I
would also suggest to use braces around 'type' and 'ID'.


This technique been used without issue in quite a few other places
without
issue, and has the benefit of simplifying the text wherever the macro is
used. What issue do you foresee?


I'm not a fan of using the macros to begin with and less so anything
more complex than a single constant value. I'd rather see 2 cells here
with the first being the id and the 2nd being the type.

An issue with token pasting is grepping for DB, SM, etc. in kernel tree
is probably noisy. Not such a big deal here, but a major PIA when you
have more complex sets of includes.


Is that a NAK or simply a suggestion? Having a single cell makes DT
parsing a bit simpler, since pretty much every SW stack provides a
default "one-cell" of_xlate implementation, whereas >1 cell means custom
code for of_xlate.


I didn't see a response to this. Joseph, let's just use two cells 
instead. I'm rather desperately waiting for this binding to be complete 
so I can finalize the U-Boot code that uses it, and it sounds like 
changing to two cells will get an ack faster. Can you post an updated 
version of this series today/ASAP to get things moving? Thanks.

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Stephen Warren]

On 07/11/2016 08:14 AM, Rob Herring wrote:

On Thu, Jul 07, 2016 at 12:35:02PM -0600, Stephen Warren wrote:

On 07/07/2016 12:13 PM, Sivaram Nair wrote:

On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives for
interprocessor communication. So the interprocessor communication (IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.



diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h 
b/include/dt-bindings/mailbox/tegra186-hsp.h



+#define HSP_MBOX_TYPE_DB 0x0
+#define HSP_MBOX_TYPE_SM 0x1
+#define HSP_MBOX_TYPE_SS 0x2
+#define HSP_MBOX_TYPE_AS 0x3
+
+#define HSP_DB_MASTER_CCPLEX 17
+#define HSP_DB_MASTER_BPMP 19
+
+#define HSP_MBOX_ID(type, ID) \
+   (HSP_MBOX_TYPE_##type << 16 | ID)


It will be nicer if you avoid the macro glue magic '##' for 'type'. I
would also suggest to use braces around 'type' and 'ID'.


This technique been used without issue in quite a few other places without
issue, and has the benefit of simplifying the text wherever the macro is
used. What issue do you foresee?


I'm not a fan of using the macros to begin with and less so anything
more complex than a single constant value. I'd rather see 2 cells here
with the first being the id and the 2nd being the type.

An issue with token pasting is grepping for DB, SM, etc. in kernel tree
is probably noisy. Not such a big deal here, but a major PIA when you
have more complex sets of includes.


Is that a NAK or simply a suggestion? Having a single cell makes DT 
parsing a bit simpler, since pretty much every SW stack provides a 
default "one-cell" of_xlate implementation, whereas >1 cell means custom 
code for of_xlate.

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Stephen Warren]

On 07/11/2016 08:14 AM, Rob Herring wrote:

On Thu, Jul 07, 2016 at 12:35:02PM -0600, Stephen Warren wrote:

On 07/07/2016 12:13 PM, Sivaram Nair wrote:

On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives for
interprocessor communication. So the interprocessor communication (IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.



diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h 
b/include/dt-bindings/mailbox/tegra186-hsp.h



+#define HSP_MBOX_TYPE_DB 0x0
+#define HSP_MBOX_TYPE_SM 0x1
+#define HSP_MBOX_TYPE_SS 0x2
+#define HSP_MBOX_TYPE_AS 0x3
+
+#define HSP_DB_MASTER_CCPLEX 17
+#define HSP_DB_MASTER_BPMP 19
+
+#define HSP_MBOX_ID(type, ID) \
+   (HSP_MBOX_TYPE_##type << 16 | ID)


It will be nicer if you avoid the macro glue magic '##' for 'type'. I
would also suggest to use braces around 'type' and 'ID'.


This technique been used without issue in quite a few other places without
issue, and has the benefit of simplifying the text wherever the macro is
used. What issue do you foresee?


I'm not a fan of using the macros to begin with and less so anything
more complex than a single constant value. I'd rather see 2 cells here
with the first being the id and the 2nd being the type.

An issue with token pasting is grepping for DB, SM, etc. in kernel tree
is probably noisy. Not such a big deal here, but a major PIA when you
have more complex sets of includes.


Is that a NAK or simply a suggestion? Having a single cell makes DT 
parsing a bit simpler, since pretty much every SW stack provides a 
default "one-cell" of_xlate implementation, whereas >1 cell means custom 
code for of_xlate.

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Rob Herring]

On Thu, Jul 07, 2016 at 12:35:02PM -0600, Stephen Warren wrote:
> On 07/07/2016 12:13 PM, Sivaram Nair wrote:
> >On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:
> >>Add DT binding for the Hardware Synchronization Primitives (HSP). The
> >>HSP is designed for the processors to share resources and communicate
> >>together. It provides a set of hardware synchronization primitives for
> >>interprocessor communication. So the interprocessor communication (IPC)
> >>protocols can use hardware synchronization primitive, when operating
> >>between two processors not in an SMP relationship.
> 
> >>diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h 
> >>b/include/dt-bindings/mailbox/tegra186-hsp.h
> 
> >>+#define HSP_MBOX_TYPE_DB 0x0
> >>+#define HSP_MBOX_TYPE_SM 0x1
> >>+#define HSP_MBOX_TYPE_SS 0x2
> >>+#define HSP_MBOX_TYPE_AS 0x3
> >>+
> >>+#define HSP_DB_MASTER_CCPLEX 17
> >>+#define HSP_DB_MASTER_BPMP 19
> >>+
> >>+#define HSP_MBOX_ID(type, ID) \
> >>+   (HSP_MBOX_TYPE_##type << 16 | ID)
> >
> >It will be nicer if you avoid the macro glue magic '##' for 'type'. I
> >would also suggest to use braces around 'type' and 'ID'.
> 
> This technique been used without issue in quite a few other places without
> issue, and has the benefit of simplifying the text wherever the macro is
> used. What issue do you foresee?

I'm not a fan of using the macros to begin with and less so anything 
more complex than a single constant value. I'd rather see 2 cells here 
with the first being the id and the 2nd being the type. 

An issue with token pasting is grepping for DB, SM, etc. in kernel tree 
is probably noisy. Not such a big deal here, but a major PIA when you 
have more complex sets of includes.

Rob

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Rob Herring]

On Thu, Jul 07, 2016 at 12:35:02PM -0600, Stephen Warren wrote:
> On 07/07/2016 12:13 PM, Sivaram Nair wrote:
> >On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:
> >>Add DT binding for the Hardware Synchronization Primitives (HSP). The
> >>HSP is designed for the processors to share resources and communicate
> >>together. It provides a set of hardware synchronization primitives for
> >>interprocessor communication. So the interprocessor communication (IPC)
> >>protocols can use hardware synchronization primitive, when operating
> >>between two processors not in an SMP relationship.
> 
> >>diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h 
> >>b/include/dt-bindings/mailbox/tegra186-hsp.h
> 
> >>+#define HSP_MBOX_TYPE_DB 0x0
> >>+#define HSP_MBOX_TYPE_SM 0x1
> >>+#define HSP_MBOX_TYPE_SS 0x2
> >>+#define HSP_MBOX_TYPE_AS 0x3
> >>+
> >>+#define HSP_DB_MASTER_CCPLEX 17
> >>+#define HSP_DB_MASTER_BPMP 19
> >>+
> >>+#define HSP_MBOX_ID(type, ID) \
> >>+   (HSP_MBOX_TYPE_##type << 16 | ID)
> >
> >It will be nicer if you avoid the macro glue magic '##' for 'type'. I
> >would also suggest to use braces around 'type' and 'ID'.
> 
> This technique been used without issue in quite a few other places without
> issue, and has the benefit of simplifying the text wherever the macro is
> used. What issue do you foresee?

I'm not a fan of using the macros to begin with and less so anything 
more complex than a single constant value. I'd rather see 2 cells here 
with the first being the id and the 2nd being the type. 

An issue with token pasting is grepping for DB, SM, etc. in kernel tree 
is probably noisy. Not such a big deal here, but a major PIA when you 
have more complex sets of includes.

Rob

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Sivaram Nair]

On Thu, Jul 07, 2016 at 12:35:02PM -0600, Stephen Warren wrote:
> On 07/07/2016 12:13 PM, Sivaram Nair wrote:
> >On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:
> >>Add DT binding for the Hardware Synchronization Primitives (HSP). The
> >>HSP is designed for the processors to share resources and communicate
> >>together. It provides a set of hardware synchronization primitives for
> >>interprocessor communication. So the interprocessor communication (IPC)
> >>protocols can use hardware synchronization primitive, when operating
> >>between two processors not in an SMP relationship.
> 
> >>diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h 
> >>b/include/dt-bindings/mailbox/tegra186-hsp.h
> 
> >>+#define HSP_MBOX_TYPE_DB 0x0
> >>+#define HSP_MBOX_TYPE_SM 0x1
> >>+#define HSP_MBOX_TYPE_SS 0x2
> >>+#define HSP_MBOX_TYPE_AS 0x3
> >>+
> >>+#define HSP_DB_MASTER_CCPLEX 17
> >>+#define HSP_DB_MASTER_BPMP 19
> >>+
> >>+#define HSP_MBOX_ID(type, ID) \
> >>+   (HSP_MBOX_TYPE_##type << 16 | ID)
> >
> >It will be nicer if you avoid the macro glue magic '##' for 'type'. I
> >would also suggest to use braces around 'type' and 'ID'.
> 
> This technique been used without issue in quite a few other places
> without issue, and has the benefit of simplifying the text wherever
> the macro is used. What issue do you foresee?

It improves readability where HSP_MBOX_ID is used (in tegra186.dtsi file
in this case) - but consider this as a cosmetic comment.

> 
> BTW, if this patch does need reposting, I'd suggest s/ID/id/ since
> macro parameters are usually lower-case.

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Sivaram Nair]

On Thu, Jul 07, 2016 at 12:35:02PM -0600, Stephen Warren wrote:
> On 07/07/2016 12:13 PM, Sivaram Nair wrote:
> >On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:
> >>Add DT binding for the Hardware Synchronization Primitives (HSP). The
> >>HSP is designed for the processors to share resources and communicate
> >>together. It provides a set of hardware synchronization primitives for
> >>interprocessor communication. So the interprocessor communication (IPC)
> >>protocols can use hardware synchronization primitive, when operating
> >>between two processors not in an SMP relationship.
> 
> >>diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h 
> >>b/include/dt-bindings/mailbox/tegra186-hsp.h
> 
> >>+#define HSP_MBOX_TYPE_DB 0x0
> >>+#define HSP_MBOX_TYPE_SM 0x1
> >>+#define HSP_MBOX_TYPE_SS 0x2
> >>+#define HSP_MBOX_TYPE_AS 0x3
> >>+
> >>+#define HSP_DB_MASTER_CCPLEX 17
> >>+#define HSP_DB_MASTER_BPMP 19
> >>+
> >>+#define HSP_MBOX_ID(type, ID) \
> >>+   (HSP_MBOX_TYPE_##type << 16 | ID)
> >
> >It will be nicer if you avoid the macro glue magic '##' for 'type'. I
> >would also suggest to use braces around 'type' and 'ID'.
> 
> This technique been used without issue in quite a few other places
> without issue, and has the benefit of simplifying the text wherever
> the macro is used. What issue do you foresee?

It improves readability where HSP_MBOX_ID is used (in tegra186.dtsi file
in this case) - but consider this as a cosmetic comment.

> 
> BTW, if this patch does need reposting, I'd suggest s/ID/id/ since
> macro parameters are usually lower-case.

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Stephen Warren]

On 07/07/2016 12:13 PM, Sivaram Nair wrote:

On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives for
interprocessor communication. So the interprocessor communication (IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.



diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h 
b/include/dt-bindings/mailbox/tegra186-hsp.h



+#define HSP_MBOX_TYPE_DB 0x0
+#define HSP_MBOX_TYPE_SM 0x1
+#define HSP_MBOX_TYPE_SS 0x2
+#define HSP_MBOX_TYPE_AS 0x3
+
+#define HSP_DB_MASTER_CCPLEX 17
+#define HSP_DB_MASTER_BPMP 19
+
+#define HSP_MBOX_ID(type, ID) \
+   (HSP_MBOX_TYPE_##type << 16 | ID)


It will be nicer if you avoid the macro glue magic '##' for 'type'. I
would also suggest to use braces around 'type' and 'ID'.


This technique been used without issue in quite a few other places 
without issue, and has the benefit of simplifying the text wherever the 
macro is used. What issue do you foresee?


BTW, if this patch does need reposting, I'd suggest s/ID/id/ since macro 
parameters are usually lower-case.

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Stephen Warren]

On 07/07/2016 12:13 PM, Sivaram Nair wrote:

On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives for
interprocessor communication. So the interprocessor communication (IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.



diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h 
b/include/dt-bindings/mailbox/tegra186-hsp.h



+#define HSP_MBOX_TYPE_DB 0x0
+#define HSP_MBOX_TYPE_SM 0x1
+#define HSP_MBOX_TYPE_SS 0x2
+#define HSP_MBOX_TYPE_AS 0x3
+
+#define HSP_DB_MASTER_CCPLEX 17
+#define HSP_DB_MASTER_BPMP 19
+
+#define HSP_MBOX_ID(type, ID) \
+   (HSP_MBOX_TYPE_##type << 16 | ID)


It will be nicer if you avoid the macro glue magic '##' for 'type'. I
would also suggest to use braces around 'type' and 'ID'.


This technique been used without issue in quite a few other places 
without issue, and has the benefit of simplifying the text wherever the 
macro is used. What issue do you foresee?


BTW, if this patch does need reposting, I'd suggest s/ID/id/ since macro 
parameters are usually lower-case.

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Sivaram Nair]

On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:
> Add DT binding for the Hardware Synchronization Primitives (HSP). The
> HSP is designed for the processors to share resources and communicate
> together. It provides a set of hardware synchronization primitives for
> interprocessor communication. So the interprocessor communication (IPC)
> protocols can use hardware synchronization primitive, when operating
> between two processors not in an SMP relationship.
> 
> Signed-off-by: Joseph Lo 
> ---
> Changes in V2:
> - revise the compatible string, interrupt-names, interrupts, and #mbox-cells
>   properties
> - remove "nvidia,hsp-function" property
> - fix the header file name
> - the binding supports the concept of multiple HSP sub-modules on one HSP HW
>   block now.
> ---
>  .../bindings/mailbox/nvidia,tegra186-hsp.txt   | 51 
> ++
>  include/dt-bindings/mailbox/tegra186-hsp.h | 23 ++
>  2 files changed, 74 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
>  create mode 100644 include/dt-bindings/mailbox/tegra186-hsp.h
> 
> diff --git 
> a/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt 
> b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
> new file mode 100644
> index ..10e53edbe1c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
> @@ -0,0 +1,51 @@
> +NVIDIA Tegra Hardware Synchronization Primitives (HSP)
> +
> +The HSP modules are used for the processors to share resources and 
> communicate
> +together. It provides a set of hardware synchronization primitives for
> +interprocessor communication. So the interprocessor communication (IPC)
> +protocols can use hardware synchronization primitives, when operating between
> +two processors not in an SMP relationship.
> +
> +The features that HSP supported are shared mailboxes, shared semaphores,
> +arbitrated semaphores and doorbells.
> +
> +Required properties:
> +- name : Should be hsp
> +- compatible
> +Array of strings.
> +one of:
> +- "nvidia,tegra186-hsp"
> +- reg : Offset and length of the register set for the device.
> +- interrupt-names
> +Array of strings.
> +Contains a list of names for the interrupts described by the interrupt
> +property. May contain the following entries, in any order:
> +- "doorbell"
> +Users of this binding MUST look up entries in the interrupt property
> +by name, using this interrupt-names property to do so.
> +- interrupts
> +Array of interrupt specifiers.
> +Must contain one entry per entry in the interrupt-names property,
> +in a matching order.
> +- #mbox-cells : Should be 1.
> +
> +The mbox specifier of the "mboxes" property in the client node should use
> +the "HSP_MBOX_ID" macro which integrates the HSP type and master ID data.
> +Those information can be found in the following file.
> +
> +- .
> +
> +Example:
> +
> +hsp_top0: hsp@3c0 {
> + compatible = "nvidia,tegra186-hsp";
> + reg = <0x0 0x03c0 0x0 0xa>;
> + interrupts = ;
> + interrupt-names = "doorbell";
> + #mbox-cells = <1>;
> +};
> +
> +client {
> + ...
> + mboxes = <_top0 HSP_MBOX_ID(DB, HSP_DB_MASTER_XXX)>;
> +};
> diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h 
> b/include/dt-bindings/mailbox/tegra186-hsp.h
> new file mode 100644
> index ..365dbeb5d894
> --- /dev/null
> +++ b/include/dt-bindings/mailbox/tegra186-hsp.h
> @@ -0,0 +1,23 @@
> +/*
> + * This header provides constants for binding nvidia,tegra186-hsp.
> + *
> + * The number with HSP_DB_MASTER prefix indicates the bit that is
> + * associated with a master ID in the doorbell registers.
> + */
> +
> +
> +#ifndef _DT_BINDINGS_MAILBOX_TEGRA186_HSP_H
> +#define _DT_BINDINGS_MAILBOX_TEGRA186_HSP_H
> +
> +#define HSP_MBOX_TYPE_DB 0x0
> +#define HSP_MBOX_TYPE_SM 0x1
> +#define HSP_MBOX_TYPE_SS 0x2
> +#define HSP_MBOX_TYPE_AS 0x3
> +
> +#define HSP_DB_MASTER_CCPLEX 17
> +#define HSP_DB_MASTER_BPMP 19
> +
> +#define HSP_MBOX_ID(type, ID) \
> + (HSP_MBOX_TYPE_##type << 16 | ID)

It will be nicer if you avoid the macro glue magic '##' for 'type'. I
would also suggest to use braces around 'type' and 'ID'.

> +
> +#endif   /* _DT_BINDINGS_MAILBOX_TEGRA186_HSP_H */
> -- 
> 2.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Sivaram Nair]

On Tue, Jul 05, 2016 at 05:04:22PM +0800, Joseph Lo wrote:
> Add DT binding for the Hardware Synchronization Primitives (HSP). The
> HSP is designed for the processors to share resources and communicate
> together. It provides a set of hardware synchronization primitives for
> interprocessor communication. So the interprocessor communication (IPC)
> protocols can use hardware synchronization primitive, when operating
> between two processors not in an SMP relationship.
> 
> Signed-off-by: Joseph Lo 
> ---
> Changes in V2:
> - revise the compatible string, interrupt-names, interrupts, and #mbox-cells
>   properties
> - remove "nvidia,hsp-function" property
> - fix the header file name
> - the binding supports the concept of multiple HSP sub-modules on one HSP HW
>   block now.
> ---
>  .../bindings/mailbox/nvidia,tegra186-hsp.txt   | 51 
> ++
>  include/dt-bindings/mailbox/tegra186-hsp.h | 23 ++
>  2 files changed, 74 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
>  create mode 100644 include/dt-bindings/mailbox/tegra186-hsp.h
> 
> diff --git 
> a/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt 
> b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
> new file mode 100644
> index ..10e53edbe1c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
> @@ -0,0 +1,51 @@
> +NVIDIA Tegra Hardware Synchronization Primitives (HSP)
> +
> +The HSP modules are used for the processors to share resources and 
> communicate
> +together. It provides a set of hardware synchronization primitives for
> +interprocessor communication. So the interprocessor communication (IPC)
> +protocols can use hardware synchronization primitives, when operating between
> +two processors not in an SMP relationship.
> +
> +The features that HSP supported are shared mailboxes, shared semaphores,
> +arbitrated semaphores and doorbells.
> +
> +Required properties:
> +- name : Should be hsp
> +- compatible
> +Array of strings.
> +one of:
> +- "nvidia,tegra186-hsp"
> +- reg : Offset and length of the register set for the device.
> +- interrupt-names
> +Array of strings.
> +Contains a list of names for the interrupts described by the interrupt
> +property. May contain the following entries, in any order:
> +- "doorbell"
> +Users of this binding MUST look up entries in the interrupt property
> +by name, using this interrupt-names property to do so.
> +- interrupts
> +Array of interrupt specifiers.
> +Must contain one entry per entry in the interrupt-names property,
> +in a matching order.
> +- #mbox-cells : Should be 1.
> +
> +The mbox specifier of the "mboxes" property in the client node should use
> +the "HSP_MBOX_ID" macro which integrates the HSP type and master ID data.
> +Those information can be found in the following file.
> +
> +- .
> +
> +Example:
> +
> +hsp_top0: hsp@3c0 {
> + compatible = "nvidia,tegra186-hsp";
> + reg = <0x0 0x03c0 0x0 0xa>;
> + interrupts = ;
> + interrupt-names = "doorbell";
> + #mbox-cells = <1>;
> +};
> +
> +client {
> + ...
> + mboxes = <_top0 HSP_MBOX_ID(DB, HSP_DB_MASTER_XXX)>;
> +};
> diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h 
> b/include/dt-bindings/mailbox/tegra186-hsp.h
> new file mode 100644
> index ..365dbeb5d894
> --- /dev/null
> +++ b/include/dt-bindings/mailbox/tegra186-hsp.h
> @@ -0,0 +1,23 @@
> +/*
> + * This header provides constants for binding nvidia,tegra186-hsp.
> + *
> + * The number with HSP_DB_MASTER prefix indicates the bit that is
> + * associated with a master ID in the doorbell registers.
> + */
> +
> +
> +#ifndef _DT_BINDINGS_MAILBOX_TEGRA186_HSP_H
> +#define _DT_BINDINGS_MAILBOX_TEGRA186_HSP_H
> +
> +#define HSP_MBOX_TYPE_DB 0x0
> +#define HSP_MBOX_TYPE_SM 0x1
> +#define HSP_MBOX_TYPE_SS 0x2
> +#define HSP_MBOX_TYPE_AS 0x3
> +
> +#define HSP_DB_MASTER_CCPLEX 17
> +#define HSP_DB_MASTER_BPMP 19
> +
> +#define HSP_MBOX_ID(type, ID) \
> + (HSP_MBOX_TYPE_##type << 16 | ID)

It will be nicer if you avoid the macro glue magic '##' for 'type'. I
would also suggest to use braces around 'type' and 'ID'.

> +
> +#endif   /* _DT_BINDINGS_MAILBOX_TEGRA186_HSP_H */
> -- 
> 2.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Joseph Lo]

On 07/07/2016 01:02 AM, Stephen Warren wrote:

On 07/05/2016 03:04 AM, Joseph Lo wrote:

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives for
interprocessor communication. So the interprocessor communication (IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.


Acked-by: Stephen Warren 


Thanks,
-Joseph


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

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Joseph Lo]

On 07/07/2016 01:02 AM, Stephen Warren wrote:

On 07/05/2016 03:04 AM, Joseph Lo wrote:

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives for
interprocessor communication. So the interprocessor communication (IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.


Acked-by: Stephen Warren 


Thanks,
-Joseph


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

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Stephen Warren]

On 07/05/2016 03:04 AM, Joseph Lo wrote:

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives for
interprocessor communication. So the interprocessor communication (IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.


Acked-by: Stephen Warren 

Re: [PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Stephen Warren]

On 07/05/2016 03:04 AM, Joseph Lo wrote:

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives for
interprocessor communication. So the interprocessor communication (IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.


Acked-by: Stephen Warren 

[PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Joseph Lo]

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives for
interprocessor communication. So the interprocessor communication (IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.

Signed-off-by: Joseph Lo 
---
Changes in V2:
- revise the compatible string, interrupt-names, interrupts, and #mbox-cells
  properties
- remove "nvidia,hsp-function" property
- fix the header file name
- the binding supports the concept of multiple HSP sub-modules on one HSP HW
  block now.
---
 .../bindings/mailbox/nvidia,tegra186-hsp.txt   | 51 ++
 include/dt-bindings/mailbox/tegra186-hsp.h | 23 ++
 2 files changed, 74 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
 create mode 100644 include/dt-bindings/mailbox/tegra186-hsp.h

diff --git a/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt 
b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
new file mode 100644
index ..10e53edbe1c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
@@ -0,0 +1,51 @@
+NVIDIA Tegra Hardware Synchronization Primitives (HSP)
+
+The HSP modules are used for the processors to share resources and communicate
+together. It provides a set of hardware synchronization primitives for
+interprocessor communication. So the interprocessor communication (IPC)
+protocols can use hardware synchronization primitives, when operating between
+two processors not in an SMP relationship.
+
+The features that HSP supported are shared mailboxes, shared semaphores,
+arbitrated semaphores and doorbells.
+
+Required properties:
+- name : Should be hsp
+- compatible
+Array of strings.
+one of:
+- "nvidia,tegra186-hsp"
+- reg : Offset and length of the register set for the device.
+- interrupt-names
+Array of strings.
+Contains a list of names for the interrupts described by the interrupt
+property. May contain the following entries, in any order:
+- "doorbell"
+Users of this binding MUST look up entries in the interrupt property
+by name, using this interrupt-names property to do so.
+- interrupts
+Array of interrupt specifiers.
+Must contain one entry per entry in the interrupt-names property,
+in a matching order.
+- #mbox-cells : Should be 1.
+
+The mbox specifier of the "mboxes" property in the client node should use
+the "HSP_MBOX_ID" macro which integrates the HSP type and master ID data.
+Those information can be found in the following file.
+
+- .
+
+Example:
+
+hsp_top0: hsp@3c0 {
+   compatible = "nvidia,tegra186-hsp";
+   reg = <0x0 0x03c0 0x0 0xa>;
+   interrupts = ;
+   interrupt-names = "doorbell";
+   #mbox-cells = <1>;
+};
+
+client {
+   ...
+   mboxes = <_top0 HSP_MBOX_ID(DB, HSP_DB_MASTER_XXX)>;
+};
diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h 
b/include/dt-bindings/mailbox/tegra186-hsp.h
new file mode 100644
index ..365dbeb5d894
--- /dev/null
+++ b/include/dt-bindings/mailbox/tegra186-hsp.h
@@ -0,0 +1,23 @@
+/*
+ * This header provides constants for binding nvidia,tegra186-hsp.
+ *
+ * The number with HSP_DB_MASTER prefix indicates the bit that is
+ * associated with a master ID in the doorbell registers.
+ */
+
+
+#ifndef _DT_BINDINGS_MAILBOX_TEGRA186_HSP_H
+#define _DT_BINDINGS_MAILBOX_TEGRA186_HSP_H
+
+#define HSP_MBOX_TYPE_DB 0x0
+#define HSP_MBOX_TYPE_SM 0x1
+#define HSP_MBOX_TYPE_SS 0x2
+#define HSP_MBOX_TYPE_AS 0x3
+
+#define HSP_DB_MASTER_CCPLEX 17
+#define HSP_DB_MASTER_BPMP 19
+
+#define HSP_MBOX_ID(type, ID) \
+   (HSP_MBOX_TYPE_##type << 16 | ID)
+
+#endif /* _DT_BINDINGS_MAILBOX_TEGRA186_HSP_H */
-- 
2.9.0

[PATCH V2 01/10] Documentation: dt-bindings: mailbox: tegra: Add binding for HSP mailbox

[Joseph Lo]

Add DT binding for the Hardware Synchronization Primitives (HSP). The
HSP is designed for the processors to share resources and communicate
together. It provides a set of hardware synchronization primitives for
interprocessor communication. So the interprocessor communication (IPC)
protocols can use hardware synchronization primitive, when operating
between two processors not in an SMP relationship.

Signed-off-by: Joseph Lo 
---
Changes in V2:
- revise the compatible string, interrupt-names, interrupts, and #mbox-cells
  properties
- remove "nvidia,hsp-function" property
- fix the header file name
- the binding supports the concept of multiple HSP sub-modules on one HSP HW
  block now.
---
 .../bindings/mailbox/nvidia,tegra186-hsp.txt   | 51 ++
 include/dt-bindings/mailbox/tegra186-hsp.h | 23 ++
 2 files changed, 74 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
 create mode 100644 include/dt-bindings/mailbox/tegra186-hsp.h

diff --git a/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt 
b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
new file mode 100644
index ..10e53edbe1c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
@@ -0,0 +1,51 @@
+NVIDIA Tegra Hardware Synchronization Primitives (HSP)
+
+The HSP modules are used for the processors to share resources and communicate
+together. It provides a set of hardware synchronization primitives for
+interprocessor communication. So the interprocessor communication (IPC)
+protocols can use hardware synchronization primitives, when operating between
+two processors not in an SMP relationship.
+
+The features that HSP supported are shared mailboxes, shared semaphores,
+arbitrated semaphores and doorbells.
+
+Required properties:
+- name : Should be hsp
+- compatible
+Array of strings.
+one of:
+- "nvidia,tegra186-hsp"
+- reg : Offset and length of the register set for the device.
+- interrupt-names
+Array of strings.
+Contains a list of names for the interrupts described by the interrupt
+property. May contain the following entries, in any order:
+- "doorbell"
+Users of this binding MUST look up entries in the interrupt property
+by name, using this interrupt-names property to do so.
+- interrupts
+Array of interrupt specifiers.
+Must contain one entry per entry in the interrupt-names property,
+in a matching order.
+- #mbox-cells : Should be 1.
+
+The mbox specifier of the "mboxes" property in the client node should use
+the "HSP_MBOX_ID" macro which integrates the HSP type and master ID data.
+Those information can be found in the following file.
+
+- .
+
+Example:
+
+hsp_top0: hsp@3c0 {
+   compatible = "nvidia,tegra186-hsp";
+   reg = <0x0 0x03c0 0x0 0xa>;
+   interrupts = ;
+   interrupt-names = "doorbell";
+   #mbox-cells = <1>;
+};
+
+client {
+   ...
+   mboxes = <_top0 HSP_MBOX_ID(DB, HSP_DB_MASTER_XXX)>;
+};
diff --git a/include/dt-bindings/mailbox/tegra186-hsp.h 
b/include/dt-bindings/mailbox/tegra186-hsp.h
new file mode 100644
index ..365dbeb5d894
--- /dev/null
+++ b/include/dt-bindings/mailbox/tegra186-hsp.h
@@ -0,0 +1,23 @@
+/*
+ * This header provides constants for binding nvidia,tegra186-hsp.
+ *
+ * The number with HSP_DB_MASTER prefix indicates the bit that is
+ * associated with a master ID in the doorbell registers.
+ */
+
+
+#ifndef _DT_BINDINGS_MAILBOX_TEGRA186_HSP_H
+#define _DT_BINDINGS_MAILBOX_TEGRA186_HSP_H
+
+#define HSP_MBOX_TYPE_DB 0x0
+#define HSP_MBOX_TYPE_SM 0x1
+#define HSP_MBOX_TYPE_SS 0x2
+#define HSP_MBOX_TYPE_AS 0x3
+
+#define HSP_DB_MASTER_CCPLEX 17
+#define HSP_DB_MASTER_BPMP 19
+
+#define HSP_MBOX_ID(type, ID) \
+   (HSP_MBOX_TYPE_##type << 16 | ID)
+
+#endif /* _DT_BINDINGS_MAILBOX_TEGRA186_HSP_H */
-- 
2.9.0