LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH V1] dt-bindings: remoteproc: k3-dsp: Update example to remove board specific
@ 2021-08-18  7:40 Sinthu Raja
  2021-08-18 13:05 ` Nishanth Menon
  2021-08-18 17:03 ` Rob Herring
  0 siblings, 2 replies; 5+ messages in thread
From: Sinthu Raja @ 2021-08-18  7:40 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Rob Herring, Suman Anna, Mathieu Poirier
  Cc: linux-remoteproc, devicetree, linux-kernel, Nishanth Menon,
	Lokesh Vutla, Sinthu Raja

The example includes a board-specific compatible property, but developers
need to add the board name each time when a new board is added to the K3
J721E SoC list. This grows the compatible string-list. So, drop the
board-specific compatible string and add cbass_main as a parent node to
avoid parent node and child node address-cells mismatch error.

Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
---
Changes in V1:
Fixed alignment issue which caused the yaml parse error.

 .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml     | 6 ++----
 1 file changed, 2 insertions(+), 4 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..e44a9397b8db 100644
--- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
@@ -132,10 +132,8 @@ required:
 unevaluatedProperties: false
 
 examples:
-  - |
-    / {
-        model = "Texas Instruments K3 J721E SoC";
-        compatible = "ti,j721e";
+  - |+
+    cbass_main {
         #address-cells = <2>;
         #size-cells = <2>;
 
-- 
2.31.1


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

* Re: [PATCH V1] dt-bindings: remoteproc: k3-dsp: Update example to remove board specific
  2021-08-18  7:40 [PATCH V1] dt-bindings: remoteproc: k3-dsp: Update example to remove board specific Sinthu Raja
@ 2021-08-18 13:05 ` Nishanth Menon
  2021-08-20  6:26   ` Sinthu Raja M
  2021-08-18 17:03 ` Rob Herring
  1 sibling, 1 reply; 5+ messages in thread
From: Nishanth Menon @ 2021-08-18 13:05 UTC (permalink / raw)
  To: Sinthu Raja, Nagalla, Hari
  Cc: Ohad Ben-Cohen, Rob Herring, Suman Anna, Mathieu Poirier,
	linux-remoteproc, devicetree, linux-kernel, Lokesh Vutla,
	Sinthu Raja

On 13:10-20210818, Sinthu Raja wrote:
> The example includes a board-specific compatible property, but developers
> need to add the board name each time when a new board is added to the K3
> J721E SoC list. This grows the compatible string-list. So, drop the
> board-specific compatible string and add cbass_main as a parent node to

What is cbass_main node?

> avoid parent node and child node address-cells mismatch error.
> 

I think you mean that since the existing example uses address cells and
size for 64bit addresses and sizes, you are introducing a bus segment
indicative of the same capability to reduce the churn in the binding.
Correct? if so, rephrase accordingly.

> Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>

Your From: and Signed-off-by email IDs do not match. You might want to
re-read the contribution guidelines documentation in linux kernel.

This should be also tagged with Fixes: since it is fixing a pre-existing
binding that slipped through our review.

NOTE: at least my test.. (I think rob's system will still complain)
base: next-20210818
b4 am -o  ~/tmp -3 -g -t -l -c -s --no-cover 20210818074030.1877-1-sinthu.raja@ti.com
	https://pastebin.ubuntu.com/p/VxzzvzpY9N/

I mean, both these can be caught with checkpatch and standard checks, so
did you see that in your basic vett prior to posting?

> ---
> Changes in V1:
> Fixed alignment issue which caused the yaml parse error.

Some 101 comments:

A) when you post a new revision, post a url like previous versions in
   diffstat - :
   https://lore.kernel.org/linux-devicetree/20210817152005.21575-1-sinthu.raja@ti.com/
B) When you are sending the very first patch, it is already V1 and
   does'nt need to be explicitly stated. this update to your original
   post is a V2, so, when you update this patch to address the review
   comments, the next revision will be V3.

> 
>  .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml     | 6 ++----
>  1 file changed, 2 insertions(+), 4 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..e44a9397b8db 100644
> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> @@ -132,10 +132,8 @@ required:
>  unevaluatedProperties: false
>  
>  examples:
> -  - |
> -    / {
> -        model = "Texas Instruments K3 J721E SoC";
> -        compatible = "ti,j721e";
> +  - |+

minor detail: you are also doing one additional change -> you are now using
the standard example template and adding the example node instead of a
complete example node as well here. Personally, I do prefer this
approach rather than the previous example.

> +    cbass_main {
>          #address-cells = <2>;
>          #size-cells = <2>;



Usually, when one sees problems like these, they tend to be
symptomatic, and we need to look if there is a similar pattern of
error else where in the codebase.

Sigh, in this case, I see the same problem in:
a) Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
b) Documentation/devicetree/bindings/hwlock/ti,omap-hwspinlock.yaml

Hari, Sinthu,
Can we fix these in a series that belongs to each maintainer?

>  
> -- 
> 2.31.1
> 

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

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

* Re: [PATCH V1] dt-bindings: remoteproc: k3-dsp: Update example to remove board specific
  2021-08-18  7:40 [PATCH V1] dt-bindings: remoteproc: k3-dsp: Update example to remove board specific Sinthu Raja
  2021-08-18 13:05 ` Nishanth Menon
@ 2021-08-18 17:03 ` Rob Herring
  1 sibling, 0 replies; 5+ messages in thread
From: Rob Herring @ 2021-08-18 17:03 UTC (permalink / raw)
  To: Sinthu Raja
  Cc: Ohad Ben-Cohen, Suman Anna, Mathieu Poirier, linux-remoteproc,
	devicetree, linux-kernel, Nishanth Menon, Lokesh Vutla,
	Sinthu Raja

On Wed, Aug 18, 2021 at 01:10:30PM +0530, Sinthu Raja wrote:
> The example includes a board-specific compatible property, but developers
> need to add the board name each time when a new board is added to the K3
> J721E SoC list. This grows the compatible string-list. So, drop the
> board-specific compatible string and add cbass_main as a parent node to
> avoid parent node and child node address-cells mismatch error.
> 
> Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>

The author and S-o-b emails don't match.

> ---
> Changes in V1:
> Fixed alignment issue which caused the yaml parse error.
> 
>  .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml     | 6 ++----
>  1 file changed, 2 insertions(+), 4 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..e44a9397b8db 100644
> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> @@ -132,10 +132,8 @@ required:
>  unevaluatedProperties: false
>  
>  examples:
> -  - |
> -    / {
> -        model = "Texas Instruments K3 J721E SoC";
> -        compatible = "ti,j721e";
> +  - |+
> +    cbass_main {
>          #address-cells = <2>;
>          #size-cells = <2>;
>  
> -- 
> 2.31.1
> 
> 

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

* Re: [PATCH V1] dt-bindings: remoteproc: k3-dsp: Update example to remove board specific
  2021-08-18 13:05 ` Nishanth Menon
@ 2021-08-20  6:26   ` Sinthu Raja M
  2021-08-20 17:34     ` Nishanth Menon
  0 siblings, 1 reply; 5+ messages in thread
From: Sinthu Raja M @ 2021-08-20  6:26 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: Nagalla, Hari, Ohad Ben-Cohen, Rob Herring, Suman Anna,
	Mathieu Poirier, linux-remoteproc, Device Tree Mailing List,
	linux-kernel, Lokesh Vutla, Sinthu Raja

With Regards

Sinthu Raja


On Wed, Aug 18, 2021 at 6:35 PM Nishanth Menon <nm@ti.com> wrote:
>
> On 13:10-20210818, Sinthu Raja wrote:
> > The example includes a board-specific compatible property, but developers
> > need to add the board name each time when a new board is added to the K3
> > J721E SoC list. This grows the compatible string-list. So, drop the
> > board-specific compatible string and add cbass_main as a parent node to
>
> What is cbass_main node?
>
> > avoid parent node and child node address-cells mismatch error.
> >
>
> I think you mean that since the existing example uses address cells and
> size for 64bit addresses and sizes, you are introducing a bus segment
> indicative of the same capability to reduce the churn in the binding.
> Correct? if so, rephrase accordingly.
>
> > Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
>
> Your From: and Signed-off-by email IDs do not match. You might want to
> re-read the contribution guidelines documentation in linux kernel.
>
> This should be also tagged with Fixes: since it is fixing a pre-existing
> binding that slipped through our review.

Hi Nishanth,
May I know to which commit I have to tag the Fixes. If you are
referring to the below review, then the patch is not merged and how
shall we add the Fixes tag for this patch.
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210607093314.23909-2-sinthu.raja@ti.com/


>
> NOTE: at least my test.. (I think rob's system will still complain)
> base: next-20210818
> b4 am -o  ~/tmp -3 -g -t -l -c -s --no-cover 20210818074030.1877-1-sinthu.raja@ti.com
>         https://pastebin.ubuntu.com/p/VxzzvzpY9N/
>
> I mean, both these can be caught with checkpatch and standard checks, so
> did you see that in your basic vett prior to posting?

It didn't catch in my basic patch verification. But the generated
patch does have the From header, but sometimes the From header is
getting truncated when submitting for review. Still working on that to
fix it. (using Gmail client to submitting the patch)

With Regards
Sinthu Raja
>
> > ---
> > Changes in V1:
> > Fixed alignment issue which caused the yaml parse error.
>
> Some 101 comments:
>
> A) when you post a new revision, post a url like previous versions in
>    diffstat - :
>    https://lore.kernel.org/linux-devicetree/20210817152005.21575-1-sinthu.raja@ti.com/
> B) When you are sending the very first patch, it is already V1 and
>    does'nt need to be explicitly stated. this update to your original
>    post is a V2, so, when you update this patch to address the review
>    comments, the next revision will be V3.
>
> >
> >  .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml     | 6 ++----
> >  1 file changed, 2 insertions(+), 4 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..e44a9397b8db 100644
> > --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> > +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> > @@ -132,10 +132,8 @@ required:
> >  unevaluatedProperties: false
> >
> >  examples:
> > -  - |
> > -    / {
> > -        model = "Texas Instruments K3 J721E SoC";
> > -        compatible = "ti,j721e";
> > +  - |+
>
> minor detail: you are also doing one additional change -> you are now using
> the standard example template and adding the example node instead of a
> complete example node as well here. Personally, I do prefer this
> approach rather than the previous example.
>
> > +    cbass_main {
> >          #address-cells = <2>;
> >          #size-cells = <2>;
>
>
>
> Usually, when one sees problems like these, they tend to be
> symptomatic, and we need to look if there is a similar pattern of
> error else where in the codebase.
>
> Sigh, in this case, I see the same problem in:
> a) Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
> b) Documentation/devicetree/bindings/hwlock/ti,omap-hwspinlock.yaml
>
> Hari, Sinthu,
> Can we fix these in a series that belongs to each maintainer?
>
> >
> > --
> > 2.31.1
> >
>
> --
> Regards,
> Nishanth Menon
> Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

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

* Re: [PATCH V1] dt-bindings: remoteproc: k3-dsp: Update example to remove board specific
  2021-08-20  6:26   ` Sinthu Raja M
@ 2021-08-20 17:34     ` Nishanth Menon
  0 siblings, 0 replies; 5+ messages in thread
From: Nishanth Menon @ 2021-08-20 17:34 UTC (permalink / raw)
  To: Sinthu Raja M
  Cc: Nagalla, Hari, Ohad Ben-Cohen, Rob Herring, Suman Anna,
	Mathieu Poirier, linux-remoteproc, Device Tree Mailing List,
	linux-kernel, Lokesh Vutla, Sinthu Raja

On 11:56-20210820, Sinthu Raja M wrote:
[...]

> May I know to which commit I have to tag the Fixes. If you are

git blame Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml ?

Look at the commit that introduced the original code that you are
fixing. In this case, 2a2180206ab62 perhaps?

[...]
> It didn't catch in my basic patch verification. But the generated
> patch does have the From header, but sometimes the From header is
> getting truncated when submitting for review. Still working on that to
> fix it. (using Gmail client to submitting the patch)


https://www.kernel.org/doc/html/v4.11/process/email-clients.html

"Gmail (Web GUI)

Does not work for sending patches.
"


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

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

end of thread, other threads:[~2021-08-20 17:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-18  7:40 [PATCH V1] dt-bindings: remoteproc: k3-dsp: Update example to remove board specific Sinthu Raja
2021-08-18 13:05 ` Nishanth Menon
2021-08-20  6:26   ` Sinthu Raja M
2021-08-20 17:34     ` Nishanth Menon
2021-08-18 17:03 ` Rob Herring

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).