| Message ID | 20180812133149.7710-2-erosca@de.adit-jv.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [v2,1/7] dt-bindings: arm: Document Renesas R-Car M3-N-based ULCB board | expand |
On Sun, Aug 12, 2018 at 03:31:44PM +0200, Eugeniu Rosca wrote: > Document the support for rcar_can on R8A77965 SoC devices. > Add R8A77965 to the list of SoCs which require the "assigned-clocks" and > "assigned-clock-rates" properties (thanks, Sergei). Rewrap text. > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > --- > Changes in v2: > - [Kieran Bingham] Simplified commit description. Rewrapped text. > - [Sergei Shtylyov] Replaced footnotes with inline text. > - Pushed all dt-bindings patches to the beginning of the series. > --- > Documentation/devicetree/bindings/net/can/rcar_can.txt | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Simon Horman <horms+renesas@verge.net.au> As this is a CAN patch I believe it should be: * Targeted at the net-next tree. * Have net-next in the subject prefix e.g.: "[PATCH v3 net-next] dt-bindings: can: rcar_can: document r8a77965 support" * Sent to: Wolfgang Grandegger <wg@grandegger.com> Marc Kleine-Budde <mkl@pengutronix.de> "David S. Miller" <davem@davemloft.net> linux-can@vger.kernel.org netdev@vger.kernel.org As it is a DT binding patch I believe it should also be sent to: Rob Herring <robh+dt@kernel.org> Mark Rutland <mark.rutland@arm.com> devicetree@vger.kernel.org And as it is related to Renesas SoCs it should also be sent to: Simon Horman <horms@verge.net.au> Magnus Damm <damm+renesas@opensource.se> linux-renesas-soc@vger.kernel.org Sending it to more people is ok too :)
Hi Simon, On Fri, Aug 17, 2018 at 11:04:03AM +0200, Simon Horman wrote: > On Sun, Aug 12, 2018 at 03:31:44PM +0200, Eugeniu Rosca wrote: > > Document the support for rcar_can on R8A77965 SoC devices. > > Add R8A77965 to the list of SoCs which require the "assigned-clocks" and > > "assigned-clock-rates" properties (thanks, Sergei). Rewrap text. > > > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > --- > > Changes in v2: > > - [Kieran Bingham] Simplified commit description. Rewrapped text. > > - [Sergei Shtylyov] Replaced footnotes with inline text. > > - Pushed all dt-bindings patches to the beginning of the series. > > --- > > Documentation/devicetree/bindings/net/can/rcar_can.txt | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > Reviewed-by: Simon Horman <horms+renesas@verge.net.au> > > As this is a CAN patch I believe it should be: > > * Targeted at the net-next tree. > > * Have net-next in the subject prefix > > e.g.: > > "[PATCH v3 net-next] dt-bindings: can: rcar_can: document r8a77965 support" I will re-spin this particular patch as v3 with your Reviewed-by and all the suggested names included in the "Sent to" field. > > * Sent to: > Wolfgang Grandegger <wg@grandegger.com> > Marc Kleine-Budde <mkl@pengutronix.de> > "David S. Miller" <davem@davemloft.net> > linux-can@vger.kernel.org > netdev@vger.kernel.org > > As it is a DT binding patch I believe it should also be sent to: > Rob Herring <robh+dt@kernel.org> > Mark Rutland <mark.rutland@arm.com> > devicetree@vger.kernel.org > > And as it is related to Renesas SoCs it should also be sent to: > Simon Horman <horms@verge.net.au> > Magnus Damm <damm+renesas@opensource.se> > linux-renesas-soc@vger.kernel.org > > Sending it to more people is ok too :) Best regards, Eugeniu.
Hi Eugeniu Thank you for the patch. On 12/08/18 14:31, Eugeniu Rosca wrote: > Document the support for rcar_can on R8A77965 SoC devices. > Add R8A77965 to the list of SoCs which require the "assigned-clocks" and > "assigned-clock-rates" properties (thanks, Sergei). Rewrap text. I don't think you needed to say you rewrapped the text in the commit log - but it's fine :) > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > Changes in v2: > - [Kieran Bingham] Simplified commit description. Rewrapped text. > - [Sergei Shtylyov] Replaced footnotes with inline text. > - Pushed all dt-bindings patches to the beginning of the series. > --- > Documentation/devicetree/bindings/net/can/rcar_can.txt | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt b/Documentation/devicetree/bindings/net/can/rcar_can.txt > index 94a7f33ac5e9..60daa878c9a2 100644 > --- a/Documentation/devicetree/bindings/net/can/rcar_can.txt > +++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt > @@ -13,6 +13,7 @@ Required properties: > "renesas,can-r8a7794" if CAN controller is a part of R8A7794 SoC. > "renesas,can-r8a7795" if CAN controller is a part of R8A7795 SoC. > "renesas,can-r8a7796" if CAN controller is a part of R8A7796 SoC. > + "renesas,can-r8a77965" if CAN controller is a part of R8A77965 SoC. > "renesas,rcar-gen1-can" for a generic R-Car Gen1 compatible device. > "renesas,rcar-gen2-can" for a generic R-Car Gen2 or RZ/G1 > compatible device. > @@ -28,11 +29,10 @@ Required properties: > - pinctrl-0: pin control group to be used for this controller. > - pinctrl-names: must be "default". > > -Required properties for "renesas,can-r8a7795" and "renesas,can-r8a7796" > -compatible: > -In R8A7795 and R8A7796 SoCs, "clkp2" can be CANFD clock. This is a div6 clock > -and can be used by both CAN and CAN FD controller at the same time. It needs to > -be scaled to maximum frequency if any of these controllers use it. This is done > +Required properties for R8A7795, R8A7796 and R8A77965: > +For the denoted SoCs, "clkp2" can be CANFD clock. This is a div6 clock and can > +be used by both CAN and CAN FD controller at the same time. It needs to be > +scaled to maximum frequency if any of these controllers use it. This is done > using the below properties: > > - assigned-clocks: phandle of clkp2(CANFD) clock. >
Hello Kieran, On Fri, Aug 17, 2018 at 02:44:25PM +0100, Kieran Bingham wrote: > Hi Eugeniu > > Thank you for the patch. > > On 12/08/18 14:31, Eugeniu Rosca wrote: > > Document the support for rcar_can on R8A77965 SoC devices. > > Add R8A77965 to the list of SoCs which require the "assigned-clocks" and > > "assigned-clock-rates" properties (thanks, Sergei). Rewrap text. > > I don't think you needed to say you rewrapped the text in the commit log > - but it's fine :) IMHO "Rewrap text" is pretty much from the same category as "no functional change was intended". As a reviewer, I would take these details in the commit description any day (and sometimes I would NAK a patch which lacks these details), since they precisely express the goals set by the author and make reviewer's life easier. But, of course, preferences vary and therefore I won't elaborate on that too much. > > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > Best regards, Eugeniu.
Hi Eugeniu, On 17/08/18 16:56, Eugeniu Rosca wrote: > Hello Kieran, > > On Fri, Aug 17, 2018 at 02:44:25PM +0100, Kieran Bingham wrote: >> Hi Eugeniu >> >> Thank you for the patch. >> >> On 12/08/18 14:31, Eugeniu Rosca wrote: >>> Document the support for rcar_can on R8A77965 SoC devices. >>> Add R8A77965 to the list of SoCs which require the "assigned-clocks" and >>> "assigned-clock-rates" properties (thanks, Sergei). Rewrap text. >> >> I don't think you needed to say you rewrapped the text in the commit log >> - but it's fine :) > > IMHO "Rewrap text" is pretty much from the same category as "no > functional change was intended". Indeed, but in this instance - there was a functional change. You modified the paragraph. In fact, mentioning that you have rewrapped the text, thus implying that you have made no functional change might cause a reviewer not to look deeper at the actual differences? > As a reviewer, I would take these > details in the commit description any day (and sometimes I would NAK a > patch which lacks these details), since they precisely express the goals > set by the author and make reviewer's life easier. > > But, of course, preferences vary and therefore I won't elaborate on that > too much. If this was a separate hunk, which you had re-wrapped without making a change to - I would absolutely agree with you here. The 'rewrapping' should be mentioned in the commit message, but this in relation to a paragraph which you had modified. IMO - if you modify a paragraph of text, rewrapping to make sure it fits the constraints is part of that modification ... but ... yes we are debating minor details and preferences here ;) - I have no objection to you mentioning it. Regards Kieran > >> >>> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> >> >> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> > > Best regards, > Eugeniu. >
Hi Kieran, On Fri, Aug 17, 2018 at 05:10:06PM +0100, Kieran Bingham wrote: > Hi Eugeniu, > > On 17/08/18 16:56, Eugeniu Rosca wrote: > > Hello Kieran, > > > > On Fri, Aug 17, 2018 at 02:44:25PM +0100, Kieran Bingham wrote: > >> Hi Eugeniu > >> > >> Thank you for the patch. > >> > >> On 12/08/18 14:31, Eugeniu Rosca wrote: > >>> Document the support for rcar_can on R8A77965 SoC devices. > >>> Add R8A77965 to the list of SoCs which require the "assigned-clocks" and > >>> "assigned-clock-rates" properties (thanks, Sergei). Rewrap text. > >> > >> I don't think you needed to say you rewrapped the text in the commit log > >> - but it's fine :) > > > > IMHO "Rewrap text" is pretty much from the same category as "no > > functional change was intended". > > Indeed, but in this instance - there was a functional change. You > modified the paragraph. In fact, mentioning that you have rewrapped the > text, thus implying that you have made no functional change might cause > a reviewer not to look deeper at the actual differences? > > > > As a reviewer, I would take these > > details in the commit description any day (and sometimes I would NAK a > > patch which lacks these details), since they precisely express the goals > > set by the author and make reviewer's life easier. > > > > But, of course, preferences vary and therefore I won't elaborate on that > > too much. > > If this was a separate hunk, which you had re-wrapped without making a > change to - I would absolutely agree with you here. The 'rewrapping' > should be mentioned in the commit message, but this in relation to a > paragraph which you had modified. > > IMO - if you modify a paragraph of text, rewrapping to make sure it fits > the constraints is part of that modification ... but ... yes we are > debating minor details and preferences here ;) - I have no objection to > you mentioning it. Ok, I get your point. I have to tune my auto-pilot to avoid writing down obvious things in the commit descriptions. > > Regards > > Kieran Thanks, Eugeniu.
diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt b/Documentation/devicetree/bindings/net/can/rcar_can.txt index 94a7f33ac5e9..60daa878c9a2 100644 --- a/Documentation/devicetree/bindings/net/can/rcar_can.txt +++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt @@ -13,6 +13,7 @@ Required properties: "renesas,can-r8a7794" if CAN controller is a part of R8A7794 SoC. "renesas,can-r8a7795" if CAN controller is a part of R8A7795 SoC. "renesas,can-r8a7796" if CAN controller is a part of R8A7796 SoC. + "renesas,can-r8a77965" if CAN controller is a part of R8A77965 SoC. "renesas,rcar-gen1-can" for a generic R-Car Gen1 compatible device. "renesas,rcar-gen2-can" for a generic R-Car Gen2 or RZ/G1 compatible device. @@ -28,11 +29,10 @@ Required properties: - pinctrl-0: pin control group to be used for this controller. - pinctrl-names: must be "default". -Required properties for "renesas,can-r8a7795" and "renesas,can-r8a7796" -compatible: -In R8A7795 and R8A7796 SoCs, "clkp2" can be CANFD clock. This is a div6 clock -and can be used by both CAN and CAN FD controller at the same time. It needs to -be scaled to maximum frequency if any of these controllers use it. This is done +Required properties for R8A7795, R8A7796 and R8A77965: +For the denoted SoCs, "clkp2" can be CANFD clock. This is a div6 clock and can +be used by both CAN and CAN FD controller at the same time. It needs to be +scaled to maximum frequency if any of these controllers use it. This is done using the below properties: - assigned-clocks: phandle of clkp2(CANFD) clock.
Document the support for rcar_can on R8A77965 SoC devices. Add R8A77965 to the list of SoCs which require the "assigned-clocks" and "assigned-clock-rates" properties (thanks, Sergei). Rewrap text. Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> --- Changes in v2: - [Kieran Bingham] Simplified commit description. Rewrapped text. - [Sergei Shtylyov] Replaced footnotes with inline text. - Pushed all dt-bindings patches to the beginning of the series. --- Documentation/devicetree/bindings/net/can/rcar_can.txt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)