linux-remoteproc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3 0/2] dt-bindings: remoteproc: k3-r5f|dsp: Remove
@ 2021-09-17  9:54 Sinthu Raja
  2021-09-17  9:54 ` [PATCH V3 1/2] dt-bindings: remoteproc: k3-r5f: Remove board-specific compatible from DT example Sinthu Raja
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Sinthu Raja @ 2021-09-17  9:54 UTC (permalink / raw)
  To: Suman Anna, Rob Herring, Mathieu Poirier, Bjorn Andersson,
	Ohad Ben-Cohen
  Cc: linux-kernel, devicetree, linux-arm-kernel, linux-remoteproc,
	Nishanth Menon, Sinthu Raja

From: Sinthu Raja <sinthu.raja@ti.com>

Bjorn,

The series of patches are meant to help make the rproc bindings for K3
r5f and dsp support independent of board/platform involved. The current
examples get in the way of the device tree cleanups and new platform
introductions [1].

When applying this series, it would greatly help us if you could provide
us with a immutable tag for Nishanth to merge in and help introduce the
newer platforms that are blocked by this cleanup. See [2] for further
context of why we are requesting for immutable tag.

Changes in V3:
- Added r5f cleanup as well
- Commit message, $subject and simplification of the fixup.

V2: https://lore.kernel.org/all/20210818074030.1877-1-sinthu.raja@ti.com/
V1: https://lore.kernel.org/all/20210817152005.21575-1-sinthu.raja@ti.com/


Sinthu Raja (2):
  dt-bindings: remoteproc: k3-r5f: Remove board-specific compatible from
    DT example
  dt-bindings: remoteproc: k3-dsp: Remove board-specific compatible from
    DT example

 .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml       | 4 +---
 .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml       | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

[1] https://lore.kernel.org/all/1631794913.472895.1119414.nullmailer@robh.at.kernel.org/ 
[2] https://lore.kernel.org/linux-arm-kernel/20210125141642.4yybjnklk3qsqjdy@steersman/
-- 
2.32.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH V3 1/2] dt-bindings: remoteproc: k3-r5f: Remove board-specific compatible from DT example
  2021-09-17  9:54 [PATCH V3 0/2] dt-bindings: remoteproc: k3-r5f|dsp: Remove Sinthu Raja
@ 2021-09-17  9:54 ` Sinthu Raja
  2021-09-22 20:00   ` Rob Herring
  2021-09-17  9:54 ` [PATCH V3 2/2] dt-bindings: remoteproc: k3-dsp: " Sinthu Raja
  2021-09-17 14:47 ` [PATCH V3 0/2] dt-bindings: remoteproc: k3-r5f|dsp: Remove Nishanth Menon
  2 siblings, 1 reply; 11+ messages in thread
From: Sinthu Raja @ 2021-09-17  9:54 UTC (permalink / raw)
  To: Suman Anna, Rob Herring, Mathieu Poirier, Bjorn Andersson,
	Ohad Ben-Cohen
  Cc: linux-kernel, devicetree, linux-arm-kernel, linux-remoteproc,
	Nishanth Menon, Sinthu Raja

From: Sinthu Raja <sinthu.raja@ti.com>

The example includes a board-specific compatible property, this is
wrong as the example should be board agnostic. Replace the same with a
generic soc node.

Fixes: 5ee79c2ed5bd ("dt-bindings: remoteproc: Add bindings for R5F subsystem on TI K3 SoCs")
Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
---

Changes in V3: new patch in the series.

 .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml       | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
index 130fbaacc4b1..eeef255c4045 100644
--- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
@@ -230,9 +230,7 @@ additionalProperties: false
 
 examples:
   - |
-    / {
-        model = "Texas Instruments K3 AM654 SoC";
-        compatible = "ti,am654-evm", "ti,am654";
+    soc {
         #address-cells = <2>;
         #size-cells = <2>;
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V3 2/2] dt-bindings: remoteproc: k3-dsp: Remove board-specific compatible from DT example
  2021-09-17  9:54 [PATCH V3 0/2] dt-bindings: remoteproc: k3-r5f|dsp: Remove Sinthu Raja
  2021-09-17  9:54 ` [PATCH V3 1/2] dt-bindings: remoteproc: k3-r5f: Remove board-specific compatible from DT example Sinthu Raja
@ 2021-09-17  9:54 ` Sinthu Raja
  2021-09-22 20:00   ` Rob Herring
  2021-09-24 16:10   ` Suman Anna
  2021-09-17 14:47 ` [PATCH V3 0/2] dt-bindings: remoteproc: k3-r5f|dsp: Remove Nishanth Menon
  2 siblings, 2 replies; 11+ messages in thread
From: Sinthu Raja @ 2021-09-17  9:54 UTC (permalink / raw)
  To: Suman Anna, Rob Herring, Mathieu Poirier, Bjorn Andersson,
	Ohad Ben-Cohen
  Cc: linux-kernel, devicetree, linux-arm-kernel, linux-remoteproc,
	Nishanth Menon, Sinthu Raja

From: Sinthu Raja <sinthu.raja@ti.com>

The example includes a board-specific compatible property, this is
wrong as the example should be board agnostic and gets in the way of
additions for newer platforms. Replace the same with a generic soc
node.

Fixes: 2a2180206ab6 ("dt-bindings: remoteproc: Add bindings for C66x DSPs on TI K3 SoCs")
Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
---

Changes since V2:
* review comment updates, including simplifying the changes, commit
  message and $subject updates.

V2: https://lore.kernel.org/all/20210818074030.1877-1-sinthu.raja@ti.com/
V1: https://lore.kernel.org/all/20210817152005.21575-1-sinthu.raja@ti.com/

 .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml       | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
index 6070456a7b67..5ec6505ac408 100644
--- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
@@ -133,9 +133,7 @@ unevaluatedProperties: false
 
 examples:
   - |
-    / {
-        model = "Texas Instruments K3 J721E SoC";
-        compatible = "ti,j721e";
+    soc {
         #address-cells = <2>;
         #size-cells = <2>;
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 0/2] dt-bindings: remoteproc: k3-r5f|dsp: Remove
  2021-09-17  9:54 [PATCH V3 0/2] dt-bindings: remoteproc: k3-r5f|dsp: Remove Sinthu Raja
  2021-09-17  9:54 ` [PATCH V3 1/2] dt-bindings: remoteproc: k3-r5f: Remove board-specific compatible from DT example Sinthu Raja
  2021-09-17  9:54 ` [PATCH V3 2/2] dt-bindings: remoteproc: k3-dsp: " Sinthu Raja
@ 2021-09-17 14:47 ` Nishanth Menon
  2 siblings, 0 replies; 11+ messages in thread
From: Nishanth Menon @ 2021-09-17 14:47 UTC (permalink / raw)
  To: Sinthu Raja
  Cc: Suman Anna, Rob Herring, Mathieu Poirier, Bjorn Andersson,
	Ohad Ben-Cohen, linux-kernel, devicetree, linux-arm-kernel,
	linux-remoteproc, Sinthu Raja

oops.. cover-letter $subject is probably improved with "board-specific
compatible"?, but anyways, does'nt matter for the rest f the actual
patches..

On 15:24-20210917, Sinthu Raja wrote:
> From: Sinthu Raja <sinthu.raja@ti.com>
> 
> Bjorn,
> 
> The series of patches are meant to help make the rproc bindings for K3
> r5f and dsp support independent of board/platform involved. The current
> examples get in the way of the device tree cleanups and new platform
> introductions [1].
> 
> When applying this series, it would greatly help us if you could provide
> us with a immutable tag for Nishanth to merge in and help introduce the
> newer platforms that are blocked by this cleanup. See [2] for further
> context of why we are requesting for immutable tag.

Yes again please. once Rob reviews the binding patches, This series is
blocking me and thanks for respinning them.

For the series:

Reviewed-by: Nishanth Menon <nm@ti.com>

> 
> Changes in V3:
> - Added r5f cleanup as well
> - Commit message, $subject and simplification of the fixup.
> 
> V2: https://lore.kernel.org/all/20210818074030.1877-1-sinthu.raja@ti.com/
> V1: https://lore.kernel.org/all/20210817152005.21575-1-sinthu.raja@ti.com/
> 
> 
> Sinthu Raja (2):
>   dt-bindings: remoteproc: k3-r5f: Remove board-specific compatible from
>     DT example
>   dt-bindings: remoteproc: k3-dsp: Remove board-specific compatible from
>     DT example
> 
>  .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml       | 4 +---
>  .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml       | 4 +---
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> [1] https://lore.kernel.org/all/1631794913.472895.1119414.nullmailer@robh.at.kernel.org/ 
> [2] https://lore.kernel.org/linux-arm-kernel/20210125141642.4yybjnklk3qsqjdy@steersman/
> -- 
> 2.32.0
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 1/2] dt-bindings: remoteproc: k3-r5f: Remove board-specific compatible from DT example
  2021-09-17  9:54 ` [PATCH V3 1/2] dt-bindings: remoteproc: k3-r5f: Remove board-specific compatible from DT example Sinthu Raja
@ 2021-09-22 20:00   ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2021-09-22 20:00 UTC (permalink / raw)
  To: Sinthu Raja
  Cc: Suman Anna, linux-kernel, Sinthu Raja, Rob Herring,
	Bjorn Andersson, linux-remoteproc, devicetree, Ohad Ben-Cohen,
	Mathieu Poirier, linux-arm-kernel, Nishanth Menon

On Fri, 17 Sep 2021 15:24:25 +0530, Sinthu Raja wrote:
> From: Sinthu Raja <sinthu.raja@ti.com>
> 
> The example includes a board-specific compatible property, this is
> wrong as the example should be board agnostic. Replace the same with a
> generic soc node.
> 
> Fixes: 5ee79c2ed5bd ("dt-bindings: remoteproc: Add bindings for R5F subsystem on TI K3 SoCs")
> Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
> ---
> 
> Changes in V3: new patch in the series.
> 
>  .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml       | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 2/2] dt-bindings: remoteproc: k3-dsp: Remove board-specific compatible from DT example
  2021-09-17  9:54 ` [PATCH V3 2/2] dt-bindings: remoteproc: k3-dsp: " Sinthu Raja
@ 2021-09-22 20:00   ` Rob Herring
  2021-09-24 16:10   ` Suman Anna
  1 sibling, 0 replies; 11+ messages in thread
From: Rob Herring @ 2021-09-22 20:00 UTC (permalink / raw)
  To: Sinthu Raja
  Cc: linux-arm-kernel, Mathieu Poirier, devicetree, Suman Anna,
	Sinthu Raja, Ohad Ben-Cohen, linux-remoteproc, Bjorn Andersson,
	linux-kernel, Rob Herring, Nishanth Menon

On Fri, 17 Sep 2021 15:24:26 +0530, Sinthu Raja wrote:
> From: Sinthu Raja <sinthu.raja@ti.com>
> 
> The example includes a board-specific compatible property, this is
> wrong as the example should be board agnostic and gets in the way of
> additions for newer platforms. Replace the same with a generic soc
> node.
> 
> Fixes: 2a2180206ab6 ("dt-bindings: remoteproc: Add bindings for C66x DSPs on TI K3 SoCs")
> Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
> ---
> 
> Changes since V2:
> * review comment updates, including simplifying the changes, commit
>   message and $subject updates.
> 
> V2: https://lore.kernel.org/all/20210818074030.1877-1-sinthu.raja@ti.com/
> V1: https://lore.kernel.org/all/20210817152005.21575-1-sinthu.raja@ti.com/
> 
>  .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml       | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 2/2] dt-bindings: remoteproc: k3-dsp: Remove board-specific compatible from DT example
  2021-09-17  9:54 ` [PATCH V3 2/2] dt-bindings: remoteproc: k3-dsp: " Sinthu Raja
  2021-09-22 20:00   ` Rob Herring
@ 2021-09-24 16:10   ` Suman Anna
  2021-09-24 16:29     ` Nishanth Menon
  1 sibling, 1 reply; 11+ messages in thread
From: Suman Anna @ 2021-09-24 16:10 UTC (permalink / raw)
  To: Sinthu Raja, Rob Herring, Mathieu Poirier, Bjorn Andersson,
	Ohad Ben-Cohen
  Cc: linux-kernel, devicetree, linux-arm-kernel, linux-remoteproc,
	Nishanth Menon, Sinthu Raja, Nagalla, Hari

Hi Sinthu,

On 9/17/21 4:54 AM, Sinthu Raja wrote:
> From: Sinthu Raja <sinthu.raja@ti.com>
> 
> The example includes a board-specific compatible property, this is
> wrong as the example should be board agnostic and gets in the way of
> additions for newer platforms. Replace the same with a generic soc
> node.

What board specific property? This description looks wrong.

> 
> Fixes: 2a2180206ab6 ("dt-bindings: remoteproc: Add bindings for C66x DSPs on TI K3 SoCs")

What error are you trying to fix exactly? The example used below is actually how
it exactly appears in the J721E dts files, and there are no errors with
dt_binding_check.

This is more a cleanup than a fix.  You can look through the original binding
submission patches to see why it is done like this.

If this is triggered by the changes you are making to k3.yaml file as part of
the J721E EAIK changes, then you probably may want to look at how you are doing
that again. Looks like the k3.yaml file is being modified now to enforce
"board-compatible", "soc-compatible" which may have triggered an error in this file.

Please evaluate if you need to modify it to support just the "soc-compatible" as
one of the items.


> Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
> ---
> 
> Changes since V2:
> * review comment updates, including simplifying the changes, commit
>   message and $subject updates.
> 
> V2: https://lore.kernel.org/all/20210818074030.1877-1-sinthu.raja@ti.com/
> V1: https://lore.kernel.org/all/20210817152005.21575-1-sinthu.raja@ti.com/
> 
>  .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml       | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> index 6070456a7b67..5ec6505ac408 100644
> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> @@ -133,9 +133,7 @@ unevaluatedProperties: false
>  
>  examples:
>    - |
> -    / {
> -        model = "Texas Instruments K3 J721E SoC";
> -        compatible = "ti,j721e";
> +    soc {

While this may be resolving the dt_bindings_check you might be seeing with the
modified k3.yaml, note that "soc" property is not used on K3 dts files, you
might be creating confusion for people who look at this example and the actual
usage.

regards
Suman

>          #address-cells = <2>;
>          #size-cells = <2>;
>  
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 2/2] dt-bindings: remoteproc: k3-dsp: Remove board-specific compatible from DT example
  2021-09-24 16:10   ` Suman Anna
@ 2021-09-24 16:29     ` Nishanth Menon
  2021-09-24 17:25       ` Suman Anna
  0 siblings, 1 reply; 11+ messages in thread
From: Nishanth Menon @ 2021-09-24 16:29 UTC (permalink / raw)
  To: Suman Anna
  Cc: Sinthu Raja, Rob Herring, Mathieu Poirier, Bjorn Andersson,
	Ohad Ben-Cohen, linux-kernel, devicetree, linux-arm-kernel,
	linux-remoteproc, Sinthu Raja, Nagalla, Hari

On 11:10-20210924, Suman Anna wrote:
> Hi Sinthu,
> 
> On 9/17/21 4:54 AM, Sinthu Raja wrote:
> > From: Sinthu Raja <sinthu.raja@ti.com>
> > 
> > The example includes a board-specific compatible property, this is
> > wrong as the example should be board agnostic and gets in the way of
> > additions for newer platforms. Replace the same with a generic soc
> > node.
> 
> What board specific property? This description looks wrong.

See https://lore.kernel.org/all/1631794913.472895.1119414.nullmailer@robh.at.kernel.org/

> 
> > 
> > Fixes: 2a2180206ab6 ("dt-bindings: remoteproc: Add bindings for C66x DSPs on TI K3 SoCs")
> 
> What error are you trying to fix exactly? The example used below is actually how
> it exactly appears in the J721E dts files, and there are no errors with
> dt_binding_check.

The rproc binding should have nothing to do with j721e SoC node
description. it should describe the rproc node that is described in
binding.

> 
> This is more a cleanup than a fix.  You can look through the original binding
> submission patches to see why it is done like this.

This is blocking any updates we would want to do in k3.yaml.
> 
> If this is triggered by the changes you are making to k3.yaml file as part of
> the J721E EAIK changes, then you probably may want to look at how you are doing

> that again. Looks like the k3.yaml file is being modified now to enforce
> "board-compatible", "soc-compatible" which may have triggered an error in this file.
> 
> Please evaluate if you need to modify it to support just the "soc-compatible" as
> one of the items.

See above link. This is not to do with eaik / sk. I am trying to
standardize the board definitions in yaml for k3 and this binding
specifically is getting in the way.


I still don't understand what your contention is here. Are you arguing
that the binding example is correct and should be tied to a platform?


Yes, I know I can introduce oneOf and a little more intricate solution,
	but besides that, i disagree that a rproc binding should even
	have SoC specific top level node description in it.
a) rproc.yaml does'nt even describe the SoC. soc.yaml does.
b) The node property examples are supposed to be examples not tied to a
   specific SoC.

> > Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
> > ---
> > 
> > Changes since V2:
> > * review comment updates, including simplifying the changes, commit
> >   message and $subject updates.
> > 
> > V2: https://lore.kernel.org/all/20210818074030.1877-1-sinthu.raja@ti.com/
> > V1: https://lore.kernel.org/all/20210817152005.21575-1-sinthu.raja@ti.com/
> > 
> >  .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml       | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> > index 6070456a7b67..5ec6505ac408 100644
> > --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> > +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> > @@ -133,9 +133,7 @@ unevaluatedProperties: false
> >  
> >  examples:
> >    - |
> > -    / {
> > -        model = "Texas Instruments K3 J721E SoC";
> > -        compatible = "ti,j721e";
> > +    soc {
> 
> While this may be resolving the dt_bindings_check you might be seeing with the
> modified k3.yaml, note that "soc" property is not used on K3 dts files, you
> might be creating confusion for people who look at this example and the actual
> usage.


It is a common usage model. NOTE: these are example nodes and NOT meant
as SoC representation. I dont see the confusion.
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 2/2] dt-bindings: remoteproc: k3-dsp: Remove board-specific compatible from DT example
  2021-09-24 16:29     ` Nishanth Menon
@ 2021-09-24 17:25       ` Suman Anna
  2021-09-24 17:54         ` Suman Anna
  0 siblings, 1 reply; 11+ messages in thread
From: Suman Anna @ 2021-09-24 17:25 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: Sinthu Raja, Rob Herring, Mathieu Poirier, Bjorn Andersson,
	Ohad Ben-Cohen, linux-kernel, devicetree, linux-arm-kernel,
	linux-remoteproc, Sinthu Raja, Nagalla, Hari

On 9/24/21 11:29 AM, Nishanth Menon wrote:
> On 11:10-20210924, Suman Anna wrote:
>> Hi Sinthu,
>>
>> On 9/17/21 4:54 AM, Sinthu Raja wrote:
>>> From: Sinthu Raja <sinthu.raja@ti.com>
>>>
>>> The example includes a board-specific compatible property, this is
>>> wrong as the example should be board agnostic and gets in the way of
>>> additions for newer platforms. Replace the same with a generic soc
>>> node.
>>
>> What board specific property? This description looks wrong.
> 
> See https://lore.kernel.org/all/1631794913.472895.1119414.nullmailer@robh.at.kernel.org/
> 

Yes, I understand you are now trying to add/scale for a board compatible and
your patch is what triggered the warnings.

I see "ti,j721e" as an SoC compatible not board-specific.

>>
>>>
>>> Fixes: 2a2180206ab6 ("dt-bindings: remoteproc: Add bindings for C66x DSPs on TI K3 SoCs")
>>
>> What error are you trying to fix exactly? The example used below is actually how
>> it exactly appears in the J721E dts files, and there are no errors with
>> dt_binding_check.
> 
> The rproc binding should have nothing to do with j721e SoC node
> description. it should describe the rproc node that is described in
> binding.

You can go back and look at my original dt-binding submissions and the reasons
for me to add a root-cell. They are to suppress the warnings seen with using two
address-cells in the DSP example nodes which use the actual node definitions
from the J721E SoC.

v1:
https://patchwork.kernel.org/project/linux-remoteproc/patch/20200325201839.15896-2-s-anna@ti.com/
v2:
https://patchwork.kernel.org/project/linux-remoteproc/patch/20200521001006.2725-3-s-anna@ti.com/
v3:
https://patchwork.kernel.org/project/linux-remoteproc/patch/20200612224914.7634-5-s-anna@ti.com/

> 
>>
>> This is more a cleanup than a fix.  You can look through the original binding
>> submission patches to see why it is done like this.
> 
> This is blocking any updates we would want to do in k3.yaml.

One other way would have been to just add the new enforced compatible (since you
are actually changing the k3.yaml binding and diverging from what was there
before) here along with your updates, if you didn't want to add it in the
previous compatible way.

FWIW, there are no dt_binding_check errors on this binding before your
modifications, that's why I am asking what is the "Fixes" with the original patch.

>>
>> If this is triggered by the changes you are making to k3.yaml file as part of
>> the J721E EAIK changes, then you probably may want to look at how you are doing
> 
>> that again. Looks like the k3.yaml file is being modified now to enforce
>> "board-compatible", "soc-compatible" which may have triggered an error in this file.
>>
>> Please evaluate if you need to modify it to support just the "soc-compatible" as
>> one of the items.
> 
> See above link. This is not to do with eaik / sk. I am trying to
> standardize the board definitions in yaml for k3 and this binding
> specifically is getting in the way.

Yeah finally. I remember I had asked you about why we are doing it differently
between AM65x and J721E/J7200. +1 for the direction.

> 
> 
> I still don't understand what your contention is here. Are you arguing
> that the binding example is correct and should be tied to a platform?

I am not saying it should be tied to a platform, but I have used the example as
it appears on J721E SoCs. I am commenting that it is not a "Fixes:" and the
patch description needs updates.

> 
> 
> Yes, I know I can introduce oneOf and a little more intricate solution,
> 	but besides that, i disagree that a rproc binding should even
> 	have SoC specific top level node description in it.

Please see the reasoning in the original submissions. I could not use 2
address-cells and size-cells without the top-level node additions, and I didn't
want to use bogus examples.

Yes, the intricate solution would not have triggered the warning in this
example, but your current change is also breaking your previous compatibility. I
understand that the reality is always actually a "board-compatible",
"soc-compatible", but as per your previous k3.yaml definition, all one needed
was just a "ti,j721e" compatible in their dts files. Changing it now and calling
the usage in this example "wrong" is not right either IMO.


> a) rproc.yaml does'nt even describe the SoC. soc.yaml does.
> b) The node property examples are supposed to be examples not tied to a
>    specific SoC.

I would rather not use a completely bogus example since it is not very useful
for customers trying to understand the binding. My philosophy has always been to
define an example as it appears on an actual SoC so that it is easier for
customers to comprehend the binding and example while comparing it to actual dts
nodes.

regards
Suman

> 
>>> Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
>>> ---
>>>
>>> Changes since V2:
>>> * review comment updates, including simplifying the changes, commit
>>>   message and $subject updates.
>>>
>>> V2: https://lore.kernel.org/all/20210818074030.1877-1-sinthu.raja@ti.com/
>>> V1: https://lore.kernel.org/all/20210817152005.21575-1-sinthu.raja@ti.com/
>>>
>>>  .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml       | 4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
>>> index 6070456a7b67..5ec6505ac408 100644
>>> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
>>> @@ -133,9 +133,7 @@ unevaluatedProperties: false
>>>  
>>>  examples:
>>>    - |
>>> -    / {
>>> -        model = "Texas Instruments K3 J721E SoC";
>>> -        compatible = "ti,j721e";
>>> +    soc {
>>
>> While this may be resolving the dt_bindings_check you might be seeing with the
>> modified k3.yaml, note that "soc" property is not used on K3 dts files, you
>> might be creating confusion for people who look at this example and the actual
>> usage.
> 
> 
> It is a common usage model. NOTE: these are example nodes and NOT meant
> as SoC representation. I dont see the confusion.
> 





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 2/2] dt-bindings: remoteproc: k3-dsp: Remove board-specific compatible from DT example
  2021-09-24 17:25       ` Suman Anna
@ 2021-09-24 17:54         ` Suman Anna
  2021-09-25 14:52           ` Nishanth Menon
  0 siblings, 1 reply; 11+ messages in thread
From: Suman Anna @ 2021-09-24 17:54 UTC (permalink / raw)
  To: Nishanth Menon, Sinthu Raja
  Cc: Rob Herring, Mathieu Poirier, Bjorn Andersson, Ohad Ben-Cohen,
	linux-kernel, devicetree, linux-arm-kernel, linux-remoteproc,
	Sinthu Raja, Nagalla, Hari

Hi Sinthu,

On 9/24/21 12:25 PM, Suman Anna wrote:
> On 9/24/21 11:29 AM, Nishanth Menon wrote:
>> On 11:10-20210924, Suman Anna wrote:
>>> Hi Sinthu,
>>>
>>> On 9/17/21 4:54 AM, Sinthu Raja wrote:
>>>> From: Sinthu Raja <sinthu.raja@ti.com>
>>>>
>>>> The example includes a board-specific compatible property, this is
>>>> wrong as the example should be board agnostic and gets in the way of
>>>> additions for newer platforms. Replace the same with a generic soc
>>>> node.
>>>
>>> What board specific property? This description looks wrong.

Can you please repost dropping the Fixes line, and modifying the patch
description as follows:

dt-bindings: remoteproc: k3-dsp: Cleanup SoC compatible from DT example

The K3 DSP binding example used the root-node with a SoC compatible
property originally to address the dt_binding_check warnings resulting
from using a value of 2 for #address-cells and #size-cells as per most
common usage on K3 SoCs. Clean this up and replace it with a generic soc
node to keep it agnostic of the SoC or board compatibles that are outside
the scope of this binding.

With that,
Acked-by: Suman Anna <s-anna@ti.com>

Please update the R5F binding patch as well similarly. You can retain the
already received Acks.

regards
Suman


>>
>> See https://lore.kernel.org/all/1631794913.472895.1119414.nullmailer@robh.at.kernel.org/
>>
> 
> Yes, I understand you are now trying to add/scale for a board compatible and
> your patch is what triggered the warnings.
> 
> I see "ti,j721e" as an SoC compatible not board-specific.
> 
>>>
>>>>
>>>> Fixes: 2a2180206ab6 ("dt-bindings: remoteproc: Add bindings for C66x DSPs on TI K3 SoCs")
>>>
>>> What error are you trying to fix exactly? The example used below is actually how
>>> it exactly appears in the J721E dts files, and there are no errors with
>>> dt_binding_check.
>>
>> The rproc binding should have nothing to do with j721e SoC node
>> description. it should describe the rproc node that is described in
>> binding.
> 
> You can go back and look at my original dt-binding submissions and the reasons
> for me to add a root-cell. They are to suppress the warnings seen with using two
> address-cells in the DSP example nodes which use the actual node definitions
> from the J721E SoC.
> 
> v1:
> https://patchwork.kernel.org/project/linux-remoteproc/patch/20200325201839.15896-2-s-anna@ti.com/
> v2:
> https://patchwork.kernel.org/project/linux-remoteproc/patch/20200521001006.2725-3-s-anna@ti.com/
> v3:
> https://patchwork.kernel.org/project/linux-remoteproc/patch/20200612224914.7634-5-s-anna@ti.com/
> 
>>
>>>
>>> This is more a cleanup than a fix.  You can look through the original binding
>>> submission patches to see why it is done like this.
>>
>> This is blocking any updates we would want to do in k3.yaml.
> 
> One other way would have been to just add the new enforced compatible (since you
> are actually changing the k3.yaml binding and diverging from what was there
> before) here along with your updates, if you didn't want to add it in the
> previous compatible way.
> 
> FWIW, there are no dt_binding_check errors on this binding before your
> modifications, that's why I am asking what is the "Fixes" with the original patch.
> 
>>>
>>> If this is triggered by the changes you are making to k3.yaml file as part of
>>> the J721E EAIK changes, then you probably may want to look at how you are doing
>>
>>> that again. Looks like the k3.yaml file is being modified now to enforce
>>> "board-compatible", "soc-compatible" which may have triggered an error in this file.
>>>
>>> Please evaluate if you need to modify it to support just the "soc-compatible" as
>>> one of the items.
>>
>> See above link. This is not to do with eaik / sk. I am trying to
>> standardize the board definitions in yaml for k3 and this binding
>> specifically is getting in the way.
> 
> Yeah finally. I remember I had asked you about why we are doing it differently
> between AM65x and J721E/J7200. +1 for the direction.
> 
>>
>>
>> I still don't understand what your contention is here. Are you arguing
>> that the binding example is correct and should be tied to a platform?
> 
> I am not saying it should be tied to a platform, but I have used the example as
> it appears on J721E SoCs. I am commenting that it is not a "Fixes:" and the
> patch description needs updates.
> 
>>
>>
>> Yes, I know I can introduce oneOf and a little more intricate solution,
>> 	but besides that, i disagree that a rproc binding should even
>> 	have SoC specific top level node description in it.
> 
> Please see the reasoning in the original submissions. I could not use 2
> address-cells and size-cells without the top-level node additions, and I didn't
> want to use bogus examples.
> 
> Yes, the intricate solution would not have triggered the warning in this
> example, but your current change is also breaking your previous compatibility. I
> understand that the reality is always actually a "board-compatible",
> "soc-compatible", but as per your previous k3.yaml definition, all one needed
> was just a "ti,j721e" compatible in their dts files. Changing it now and calling
> the usage in this example "wrong" is not right either IMO.
> 
> 
>> a) rproc.yaml does'nt even describe the SoC. soc.yaml does.
>> b) The node property examples are supposed to be examples not tied to a
>>    specific SoC.
> 
> I would rather not use a completely bogus example since it is not very useful
> for customers trying to understand the binding. My philosophy has always been to
> define an example as it appears on an actual SoC so that it is easier for
> customers to comprehend the binding and example while comparing it to actual dts
> nodes.
> 
> regards
> Suman
> 
>>
>>>> Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
>>>> ---
>>>>
>>>> Changes since V2:
>>>> * review comment updates, including simplifying the changes, commit
>>>>   message and $subject updates.
>>>>
>>>> V2: https://lore.kernel.org/all/20210818074030.1877-1-sinthu.raja@ti.com/
>>>> V1: https://lore.kernel.org/all/20210817152005.21575-1-sinthu.raja@ti.com/
>>>>
>>>>  .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml       | 4 +---
>>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
>>>> index 6070456a7b67..5ec6505ac408 100644
>>>> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
>>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
>>>> @@ -133,9 +133,7 @@ unevaluatedProperties: false
>>>>  
>>>>  examples:
>>>>    - |
>>>> -    / {
>>>> -        model = "Texas Instruments K3 J721E SoC";
>>>> -        compatible = "ti,j721e";
>>>> +    soc {
>>>
>>> While this may be resolving the dt_bindings_check you might be seeing with the
>>> modified k3.yaml, note that "soc" property is not used on K3 dts files, you
>>> might be creating confusion for people who look at this example and the actual
>>> usage.
>>
>>
>> It is a common usage model. NOTE: these are example nodes and NOT meant
>> as SoC representation. I dont see the confusion.
>>
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 2/2] dt-bindings: remoteproc: k3-dsp: Remove board-specific compatible from DT example
  2021-09-24 17:54         ` Suman Anna
@ 2021-09-25 14:52           ` Nishanth Menon
  0 siblings, 0 replies; 11+ messages in thread
From: Nishanth Menon @ 2021-09-25 14:52 UTC (permalink / raw)
  To: Suman Anna
  Cc: Sinthu Raja, Rob Herring, Mathieu Poirier, Bjorn Andersson,
	Ohad Ben-Cohen, linux-kernel, devicetree, linux-arm-kernel,
	linux-remoteproc, Sinthu Raja, Nagalla, Hari

On 12:54-20210924, Suman Anna wrote:
> Hi Sinthu,
> 
> On 9/24/21 12:25 PM, Suman Anna wrote:
> > On 9/24/21 11:29 AM, Nishanth Menon wrote:
> >> On 11:10-20210924, Suman Anna wrote:
> >>> Hi Sinthu,
> >>>
> >>> On 9/17/21 4:54 AM, Sinthu Raja wrote:
> >>>> From: Sinthu Raja <sinthu.raja@ti.com>
> >>>>
> >>>> The example includes a board-specific compatible property, this is
> >>>> wrong as the example should be board agnostic and gets in the way of
> >>>> additions for newer platforms. Replace the same with a generic soc
> >>>> node.
> >>>
> >>> What board specific property? This description looks wrong.
> 
> Can you please repost dropping the Fixes line, and modifying the patch
> description as follows:
> 
> dt-bindings: remoteproc: k3-dsp: Cleanup SoC compatible from DT example
> 
> The K3 DSP binding example used the root-node with a SoC compatible
> property originally to address the dt_binding_check warnings resulting
> from using a value of 2 for #address-cells and #size-cells as per most
> common usage on K3 SoCs. Clean this up and replace it with a generic soc
> node to keep it agnostic of the SoC or board compatibles that are outside
> the scope of this binding.


This looks good to me as well. Thanks.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D)/Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-09-25 14:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-17  9:54 [PATCH V3 0/2] dt-bindings: remoteproc: k3-r5f|dsp: Remove Sinthu Raja
2021-09-17  9:54 ` [PATCH V3 1/2] dt-bindings: remoteproc: k3-r5f: Remove board-specific compatible from DT example Sinthu Raja
2021-09-22 20:00   ` Rob Herring
2021-09-17  9:54 ` [PATCH V3 2/2] dt-bindings: remoteproc: k3-dsp: " Sinthu Raja
2021-09-22 20:00   ` Rob Herring
2021-09-24 16:10   ` Suman Anna
2021-09-24 16:29     ` Nishanth Menon
2021-09-24 17:25       ` Suman Anna
2021-09-24 17:54         ` Suman Anna
2021-09-25 14:52           ` Nishanth Menon
2021-09-17 14:47 ` [PATCH V3 0/2] dt-bindings: remoteproc: k3-r5f|dsp: Remove Nishanth Menon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).