Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1640938 > unrolled thread

[PATCH v2 00/18] ARM64: meson: DT cleanups

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/18] ARM64: meson: DT cleanups Andreas Färber <afaerber@suse.de> - 2017-05-13 16:40 +0200
    [PATCH v2 08/18] arm64: dts: meson-gxl-s905x: Comment typo fix Andreas Färber <afaerber@suse.de> - 2017-05-13 16:40 +0200
      Re: [PATCH v2 08/18] arm64: dts: meson-gxl-s905x: Comment typo fix Neil Armstrong <narmstrong@baylibre.com> - 2017-05-15 10:20 +0200
    [PATCH v2 07/18] arm64: dts: meson-gxm-nexbox-a1: Drop UART comment Andreas Färber <afaerber@suse.de> - 2017-05-13 16:40 +0200
      Re: [PATCH v2 07/18] arm64: dts: meson-gxm-nexbox-a1: Drop UART  comment Neil Armstrong <narmstrong@baylibre.com> - 2017-05-15 10:20 +0200
    Re: [PATCH v2 00/18] ARM64: meson: DT cleanups Neil Armstrong <narmstrong@baylibre.com> - 2017-05-15 10:30 +0200
      Re: [PATCH v2 00/18] ARM64: meson: DT cleanups Andreas Färber <afaerber@suse.de> - 2017-05-15 21:20 +0200
        Re: [PATCH v2 00/18] ARM64: meson: DT cleanups Martin Blumenstingl <martin.blumenstingl@googlemail.com> - 2017-05-15 22:30 +0200
          Re: [PATCH v2 00/18] ARM64: meson: DT cleanups Andreas Färber <afaerber@suse.de> - 2017-05-20 16:00 +0200
        Re: [PATCH v2 00/18] ARM64: meson: DT cleanups Kevin Hilman <khilman@baylibre.com> - 2017-05-18 00:40 +0200
          Re: [PATCH v2 00/18] ARM64: meson: DT cleanups Andreas Färber <afaerber@suse.de> - 2017-05-20 15:20 +0200
    Re: [PATCH v2 00/18] ARM64: meson: DT cleanups Kevin Hilman <khilman@baylibre.com> - 2017-05-17 23:50 +0200
      Re: [PATCH v2 00/18] ARM64: meson: DT cleanups Andreas Färber <afaerber@suse.de> - 2017-05-20 17:50 +0200

#1640938 — [PATCH v2 00/18] ARM64: meson: DT cleanups

Hello Kevin,

This series fixes several cosmetic issues, on top of your for-next branch.

Patches 3-6 rename a node, the rest should all be non-functional changes.

PLEASE STOP merging random new nodes at the bottom of DT files!
Just like it's a convention to sort new nodes by unit address, it has been
a convention to sort by-label nodes by their label. As discussed here and
elsewhere, this helps avoid merge conflicts and makes nodes easy to find.
I don't care whether we order A0 before A or after, but adding new HDMI
or CVBS nodes at the very bottom is totally out of alphabetical order.
Since my v1 you really should've known that...

Similarly, Khadas Vim shouldn't have been merged with the "bcrmf" typo.

Which proves my point that we need to fix these issues now so that they
don't keep spreading (Broken Window Theory). New boards have not been
checked for sort order, only boards already touched in v1.

Board and Makefile order affect my pending R-Box Pro patches.
Node order affects Martin's pending Bluetooth patches among others.

Patches 7-9 (had and) have no dependency, please start applying.

Thanks,
Andreas

v1 -> v2:
* Rebased (new nodes/properties added)
* Chose a different name for the misnamed Wifi nodes (Rob)
* Added patch to fix another new misnamed Wifi node
* Dropped patch fixing a trailing white line error (resolved)
* Tweaked subjects

Cc: Kevin Hilman <khilman@baylibre.com>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Andreas Färber (18):
  dt-bindings: arm: amlogic: Reorder boards
  arm64: dts: amlogic: Sort Makefile
  arm64: dts: meson-gxbb-vega-s95: Fix Wifi node name
  arm64: dts: meson-gxbb-p20x: Fix Wifi node name
  arm64: dts: meson-gx-p23x-q20x: Fix Wifi node name
  arm64: dts: meson-gxl-s905x-khadas-vim: Fix Wifi node name
  arm64: dts: meson-gxm-nexbox-a1: Drop UART comment
  arm64: dts: meson-gxl-s905x: Comment typo fix
  arm64: dts: meson-gxbb-wetek-hub: Reference CVBS node by label
  arm64: dts: meson-gxbb: Fix node order
  arm64: dts: meson-gxbb-odroidc2: Fix node order
  arm64: dts: meson-gxbb-vega-s95: Fix node order
  arm64: dts: meson-gxbb-nexbox-a95x: Fix node order
  arm64: dts: meson-gxbb-p20x: Fix node order
  arm64: dts: meson-gxl: Fix node order
  arm64: dts: meson-gxl-s905x-nexbox-a95x: Fix node order
  arm64: dts: meson-gxm-nexbox-a1: Fix node order
  arm64: dts: meson-gx-p23x-q20x: Fix node order

 Documentation/devicetree/bindings/arm/amlogic.txt  |  20 ++-
 arch/arm64/boot/dts/amlogic/Makefile               |   6 +-
 .../arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi |  60 ++++----
 .../boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts    |  52 +++----
 .../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 108 ++++++-------
 arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi   |  66 ++++----
 .../boot/dts/amlogic/meson-gxbb-vega-s95.dtsi      |  56 +++----
 .../boot/dts/amlogic/meson-gxbb-wetek-hub.dts      |   6 +-
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        | 171 +++++++++++----------
 .../dts/amlogic/meson-gxl-s905x-khadas-vim.dts     |   2 +-
 .../dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts    |  52 +++----
 arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi   |   2 +-
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi         |  72 ++++-----
 .../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts |  87 ++++++-----
 14 files changed, 388 insertions(+), 372 deletions(-)

-- 
2.12.0

[toc] | [next] | [standalone]

#1640939 — [PATCH v2 08/18] arm64: dts: meson-gxl-s905x: Comment typo fix

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 v1 -> v2: Unchanged
 
 arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
index 0f78d836edaf..3314a0b3dad9 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
@@ -48,7 +48,7 @@
 	compatible = "amlogic,s905x", "amlogic,meson-gxl";
 };
 
-/* S905X Only has access to its internal PHY */
+/* S905X only has access to its internal PHY */
 &ethmac {
 	phy-mode = "rmii";
 	phy-handle = <&internal_phy>;
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1641369 — Re: [PATCH v2 08/18] arm64: dts: meson-gxl-s905x: Comment typo fix

On 05/13/2017 04:33 PM, Andreas Färber wrote:
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  v1 -> v2: Unchanged
>  
>  arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
> index 0f78d836edaf..3314a0b3dad9 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
> @@ -48,7 +48,7 @@
>  	compatible = "amlogic,s905x", "amlogic,meson-gxl";
>  };
>  
> -/* S905X Only has access to its internal PHY */
> +/* S905X only has access to its internal PHY */
>  &ethmac {
>  	phy-mode = "rmii";
>  	phy-handle = <&internal_phy>;
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

[toc] | [prev] | [next] | [standalone]

#1640940 — [PATCH v2 07/18] arm64: dts: meson-gxm-nexbox-a1: Drop UART comment

Product pictures show no DB9 connector, so this seems copy&paste.

Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 v1 -> v2: Unchanged
 
 arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
index 11b0bf46a95c..920fac10de93 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
@@ -113,7 +113,6 @@
 	};
 };
 
-/* This UART is brought out to the DB9 connector */
 &uart_AO {
 	status = "okay";
 	pinctrl-0 = <&uart_ao_a_pins>;
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1641365 — Re: [PATCH v2 07/18] arm64: dts: meson-gxm-nexbox-a1: Drop UART comment

On 05/13/2017 04:33 PM, Andreas Färber wrote:
> Product pictures show no DB9 connector, so this seems copy&paste.
> 
> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  v1 -> v2: Unchanged
>  
>  arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> index 11b0bf46a95c..920fac10de93 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> @@ -113,7 +113,6 @@
>  	};
>  };
>  
> -/* This UART is brought out to the DB9 connector */
>  &uart_AO {
>  	status = "okay";
>  	pinctrl-0 = <&uart_ao_a_pins>;
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

[toc] | [prev] | [next] | [standalone]

#1641382

Hi Andreas,

On 05/13/2017 04:33 PM, Andreas Färber wrote:
> Hello Kevin,
> 
> This series fixes several cosmetic issues, on top of your for-next branch.
> 
> Patches 3-6 rename a node, the rest should all be non-functional changes.

These are OK.

> 
> PLEASE STOP merging random new nodes at the bottom of DT files!
> Just like it's a convention to sort new nodes by unit address, it has been
> a convention to sort by-label nodes by their label. As discussed here and
> elsewhere, this helps avoid merge conflicts and makes nodes easy to find.
> I don't care whether we order A0 before A or after, but adding new HDMI
> or CVBS nodes at the very bottom is totally out of alphabetical order.
> Since my v1 you really should've known that...

It's not perfect, but now it's done, live with it, this has already been discussed.

Please try to refactor boards DTS with their parent reference design instead
like it was done with the P212 and what I did with the Wetek Hub and Play2.

> 
> Similarly, Khadas Vim shouldn't have been merged with the "bcrmf" typo.

Well, this is why we have 7 rc releases after the merge window...

> 
> Which proves my point that we need to fix these issues now so that they
> don't keep spreading (Broken Window Theory). New boards have not been
> checked for sort order, only boards already touched in v1.
> 
> Board and Makefile order affect my pending R-Box Pro patches.
> Node order affects Martin's pending Bluetooth patches among others.

Please order S905x after S905d, and we'll be OK.
> 
> Patches 7-9 (had and) have no dependency, please start applying.

I'll wait for Kevin's advice, but I'm against these since they are
only purely cosmetic and will break bisect and add unnecessary complexity
to handle further patches on these board.

> Thanks,
> Andreas

Thanks,
Neil
> 
> v1 -> v2:
> * Rebased (new nodes/properties added)
> * Chose a different name for the misnamed Wifi nodes (Rob)
> * Added patch to fix another new misnamed Wifi node
> * Dropped patch fixing a trailing white line error (resolved)
> * Tweaked subjects
> 
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> 
> Andreas Färber (18):
>   dt-bindings: arm: amlogic: Reorder boards
>   arm64: dts: amlogic: Sort Makefile
>   arm64: dts: meson-gxbb-vega-s95: Fix Wifi node name
>   arm64: dts: meson-gxbb-p20x: Fix Wifi node name
>   arm64: dts: meson-gx-p23x-q20x: Fix Wifi node name
>   arm64: dts: meson-gxl-s905x-khadas-vim: Fix Wifi node name
>   arm64: dts: meson-gxm-nexbox-a1: Drop UART comment
>   arm64: dts: meson-gxl-s905x: Comment typo fix
>   arm64: dts: meson-gxbb-wetek-hub: Reference CVBS node by label
>   arm64: dts: meson-gxbb: Fix node order
>   arm64: dts: meson-gxbb-odroidc2: Fix node order
>   arm64: dts: meson-gxbb-vega-s95: Fix node order
>   arm64: dts: meson-gxbb-nexbox-a95x: Fix node order
>   arm64: dts: meson-gxbb-p20x: Fix node order
>   arm64: dts: meson-gxl: Fix node order
>   arm64: dts: meson-gxl-s905x-nexbox-a95x: Fix node order
>   arm64: dts: meson-gxm-nexbox-a1: Fix node order
>   arm64: dts: meson-gx-p23x-q20x: Fix node order
> 
>  Documentation/devicetree/bindings/arm/amlogic.txt  |  20 ++-
>  arch/arm64/boot/dts/amlogic/Makefile               |   6 +-
>  .../arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi |  60 ++++----
>  .../boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts    |  52 +++----
>  .../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 108 ++++++-------
>  arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi   |  66 ++++----
>  .../boot/dts/amlogic/meson-gxbb-vega-s95.dtsi      |  56 +++----
>  .../boot/dts/amlogic/meson-gxbb-wetek-hub.dts      |   6 +-
>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        | 171 +++++++++++----------
>  .../dts/amlogic/meson-gxl-s905x-khadas-vim.dts     |   2 +-
>  .../dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts    |  52 +++----
>  arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi   |   2 +-
>  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi         |  72 ++++-----
>  .../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts |  87 ++++++-----
>  14 files changed, 388 insertions(+), 372 deletions(-)
> 

[toc] | [prev] | [next] | [standalone]

#1641993

Hi Neil,

Am 15.05.2017 um 10:16 schrieb Neil Armstrong:
> Hi Andreas,
> 
> On 05/13/2017 04:33 PM, Andreas Färber wrote:
>> Hello Kevin,
>>
>> This series fixes several cosmetic issues, on top of your for-next branch.
>>
>> Patches 3-6 rename a node, the rest should all be non-functional changes.
> 
> These are OK.
> 
>> PLEASE STOP merging random new nodes at the bottom of DT files!
>> Just like it's a convention to sort new nodes by unit address, it has been
>> a convention to sort by-label nodes by their label. As discussed here and
>> elsewhere, this helps avoid merge conflicts and makes nodes easy to find.
>> I don't care whether we order A0 before A or after, but adding new HDMI
>> or CVBS nodes at the very bottom is totally out of alphabetical order.
>> Since my v1 you really should've known that...
> 
> It's not perfect, but now it's done, live with it, this has already been discussed.

No.

Copy&pasting your comment N times does not make it any more valid. My
files, my rules - I insist on vega-s95, gxbb and gx, which you guys
refactored out from my gxbb, to be tidy.

> Please try to refactor boards DTS with their parent reference design instead
> like it was done with the P212 and what I did with the Wetek Hub and Play2.

Negative, that means any additions and changes for the reference boards
will slip through into boards that you do not test.

We've already seen how "well" that works with R-Box Pro having inherited
a broken U-Boot network configuration due to internal vs. external PHY.

>> Similarly, Khadas Vim shouldn't have been merged with the "bcrmf" typo.
> 
> Well, this is why we have 7 rc releases after the merge window...

If Kevin is the maintainer, then he needs to carefully review patches.
It is not my job to review all patches when BayLibre gets paid for it!

>> Which proves my point that we need to fix these issues now so that they
>> don't keep spreading (Broken Window Theory). New boards have not been
>> checked for sort order, only boards already touched in v1.
>>
>> Board and Makefile order affect my pending R-Box Pro patches.
>> Node order affects Martin's pending Bluetooth patches among others.
> 
> Please order S905x after S905d, and we'll be OK.

Isn't the historical SoC order S905X before S905D?

Otherwise your strict interpretation leads to meson-gxbb before meson8,
which seems unlogical to me.

>> Patches 7-9 (had and) have no dependency, please start applying.
> 
> I'll wait for Kevin's advice, but I'm against these since they are
> only purely cosmetic and will break bisect and add unnecessary complexity
> to handle further patches on these board.

See the gxbb patch for why that statement is wrong.

Also note that not cleaning up an existing mess and making it worse are
two things.

I will also remind that I was forced to clean up the node order in ALL
exynos5250 .dts files before I could get my new exynos5250-spring.dts
merged, so I have zero understanding about these "churn" and "live with
it" comments here. The same rules need to apply to all (that's égalité
for you!), BayLibre is not above everyone else.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

[toc] | [prev] | [next] | [standalone]

#1642020

On Mon, May 15, 2017 at 9:10 PM, Andreas Färber <afaerber@suse.de> wrote:
> Hi Neil,
>
> Am 15.05.2017 um 10:16 schrieb Neil Armstrong:
>> Hi Andreas,
>>
>> On 05/13/2017 04:33 PM, Andreas Färber wrote:
>>> Hello Kevin,
>>>
>>> This series fixes several cosmetic issues, on top of your for-next branch.
>>>
>>> Patches 3-6 rename a node, the rest should all be non-functional changes.
>>
>> These are OK.
>>
>>> PLEASE STOP merging random new nodes at the bottom of DT files!
>>> Just like it's a convention to sort new nodes by unit address, it has been
>>> a convention to sort by-label nodes by their label. As discussed here and
>>> elsewhere, this helps avoid merge conflicts and makes nodes easy to find.
>>> I don't care whether we order A0 before A or after, but adding new HDMI
>>> or CVBS nodes at the very bottom is totally out of alphabetical order.
>>> Since my v1 you really should've known that...
>>
>> It's not perfect, but now it's done, live with it, this has already been discussed.
>
> No.
>
> Copy&pasting your comment N times does not make it any more valid. My
> files, my rules - I insist on vega-s95, gxbb and gx, which you guys
> refactored out from my gxbb, to be tidy.
>
>> Please try to refactor boards DTS with their parent reference design instead
>> like it was done with the P212 and what I did with the Wetek Hub and Play2.
>
> Negative, that means any additions and changes for the reference boards
> will slip through into boards that you do not test.
>
> We've already seen how "well" that works with R-Box Pro having inherited
> a broken U-Boot network configuration due to internal vs. external PHY.
could you please share your vision how we can a) keep the amount of
duplicate .dts code low while b) avoiding "accidental" changes? maybe
there's a better way (compared to what we have now) which I've not
thought of yet

>>> Similarly, Khadas Vim shouldn't have been merged with the "bcrmf" typo.
>>
>> Well, this is why we have 7 rc releases after the merge window...
>
> If Kevin is the maintainer, then he needs to carefully review patches.
> It is not my job to review all patches when BayLibre gets paid for it!
please stop these accusations, I have just sent a fix which should
actually allow Kevin (and anybody else) to properly review the
Broadcom FullMAC wireless SDIO devices in the future: [0]

>>> Which proves my point that we need to fix these issues now so that they
>>> don't keep spreading (Broken Window Theory). New boards have not been
>>> checked for sort order, only boards already touched in v1.
>>>
>>> Board and Makefile order affect my pending R-Box Pro patches.
>>> Node order affects Martin's pending Bluetooth patches among others.
>>
>> Please order S905x after S905d, and we'll be OK.
>
> Isn't the historical SoC order S905X before S905D?
>
> Otherwise your strict interpretation leads to meson-gxbb before meson8,
> which seems unlogical to me.
>
>>> Patches 7-9 (had and) have no dependency, please start applying.
>>
>> I'll wait for Kevin's advice, but I'm against these since they are
>> only purely cosmetic and will break bisect and add unnecessary complexity
>> to handle further patches on these board.
>
> See the gxbb patch for why that statement is wrong.
>
> Also note that not cleaning up an existing mess and making it worse are
> two things.
>
> I will also remind that I was forced to clean up the node order in ALL
> exynos5250 .dts files before I could get my new exynos5250-spring.dts
> merged, so I have zero understanding about these "churn" and "live with
> it" comments here. The same rules need to apply to all (that's égalité
> for you!), BayLibre is not above everyone else.
>
> Regards,
> Andreas
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)

[0] https://marc.info/?l=linux-wireless&m=149487928516583&w=2

[toc] | [prev] | [next] | [standalone]

#1646132

Am 15.05.2017 um 22:24 schrieb Martin Blumenstingl:
> On Mon, May 15, 2017 at 9:10 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Hi Neil,
>>
>> Am 15.05.2017 um 10:16 schrieb Neil Armstrong:
>>> Hi Andreas,
>>>
>>> On 05/13/2017 04:33 PM, Andreas Färber wrote:
>>>> Hello Kevin,
>>>>
>>>> This series fixes several cosmetic issues, on top of your for-next branch.
>>>>
>>>> Patches 3-6 rename a node, the rest should all be non-functional changes.
>>>
>>> These are OK.
>>>
>>>> PLEASE STOP merging random new nodes at the bottom of DT files!
>>>> Just like it's a convention to sort new nodes by unit address, it has been
>>>> a convention to sort by-label nodes by their label. As discussed here and
>>>> elsewhere, this helps avoid merge conflicts and makes nodes easy to find.
>>>> I don't care whether we order A0 before A or after, but adding new HDMI
>>>> or CVBS nodes at the very bottom is totally out of alphabetical order.
>>>> Since my v1 you really should've known that...
>>>
>>> It's not perfect, but now it's done, live with it, this has already been discussed.
>>
>> No.
>>
>> Copy&pasting your comment N times does not make it any more valid. My
>> files, my rules - I insist on vega-s95, gxbb and gx, which you guys
>> refactored out from my gxbb, to be tidy.
>>
>>> Please try to refactor boards DTS with their parent reference design instead
>>> like it was done with the P212 and what I did with the Wetek Hub and Play2.
>>
>> Negative, that means any additions and changes for the reference boards
>> will slip through into boards that you do not test.
>>
>> We've already seen how "well" that works with R-Box Pro having inherited
>> a broken U-Boot network configuration due to internal vs. external PHY.
> could you please share your vision how we can a) keep the amount of
> duplicate .dts code low while b) avoiding "accidental" changes? maybe
> there's a better way (compared to what we have now) which I've not
> thought of yet

The reason for aligning all exynos5250 .dts files was to be able to use
diff -u to compare files for differences. Obviously the board compatible
will always show up, but by ordering alphabetically and by inheriting
some common order for the other nodes without particular convention only
the actual changes show up. That's why I had started the voluntary work
of also cleaning up boards that I do not have access to. You had already
adopted alphabetical order for Vim, thanks for that.

Basically we have two different concepts here:
One is physical reuse, e.g., the boards all use the same S905 chip from
Amlogic, so we can safely inherit meson-gxbb.dtsi.
The other is design reuse. Rather than reusing the same physical board,
they are different boards based on a common template but they may
contain variations. Thus there is no guarantee that everything in the
original template is in each derived board. The difficulty for us here
is that for most TV boxes no schematics are available to decide whether
there are any differences compared to Amlogic's P/Q reference boards. So
we would only find out when something breaks.

For example, continuing my work on the NanoPi K2 I have found several
minor variations of how power supplies are called and which GPIO is
being used, compared to Odroid-C2 or Vega.

Therefore the usual solution is that contributors of individual board
trees also need to maintain them long-term, as opposed to expecting
BayLibre to do that and magically getting everything for free.
Specifically, I added the vega-s95 and am adding the nanopi-k2, so once
you add serdev and Bluetooth nodes for your Vim, we will have to
manually adopt those for other boards. If we don't notice or don't do
our additions, the boards will continue to boot as before but just not
get any new cool features. On the other hand, if some driver change gets
performed, all in-tree files need to be updated, which provides some
more motivation for vendors and users to participate in mainline
development.

>>>> Similarly, Khadas Vim shouldn't have been merged with the "bcrmf" typo.
>>>
>>> Well, this is why we have 7 rc releases after the merge window...
>>
>> If Kevin is the maintainer, then he needs to carefully review patches.
>> It is not my job to review all patches when BayLibre gets paid for it!
> please stop these accusations,

Being the Amlogic maintainer is not an "accusation".

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts?id=e15d2774b8c096f116bf7192b37e8652da71369e

carries his Signed-off-by, that's for a fact.

Also FTR I found it a really weird assumption that ugly naming should be
noticed during RC testing. Therefore my point that this is about review,
not testing.

Fact is also that BayLibre people appear to be working on Amlogic stuff
during the week and with few exceptions unlike you and me are rather
silent the weekend, which is when I get to look into these platforms. So
whatever their formula of payment may be (I don't care), they seem to
have more time to spend on this as part of their job. Me, I am not a
kernel developer in my day job. So telling a hobbyist contributor to do
their job is really off.

> I have just sent a fix which should
> actually allow Kevin (and anybody else) to properly review the
> Broadcom FullMAC wireless SDIO devices in the future: [0]

Thanks for catching that. Note that you may have some more letter twists
in your cover letter.

Regards,
Andreas

> [0] https://marc.info/?l=linux-wireless&m=149487928516583&w=2

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

[toc] | [prev] | [next] | [standalone]

#1643680

Hi Andreas,

Andreas Färber <afaerber@suse.de> writes:

> Am 15.05.2017 um 10:16 schrieb Neil Armstrong:

[...]

>> 
>> It's not perfect, but now it's done, live with it, this has already been discussed.
>
> No.
>
> Copy&pasting your comment N times does not make it any more valid. My
> files, my rules - I insist on vega-s95, gxbb and gx, which you guys
> refactored out from my gxbb, to be tidy.

hmm, "my files, my rules, ... my gxbb"

Very interesting perspective, but sorry, these do not belong to you.
They belong to the kernel community.  You can insist if you like, but we
do not make decisions just because someone says "mine".  Again, the
confrontaional tone is not helpful to the dialogue.

[...]

>> 
>> Well, this is why we have 7 rc releases after the merge window...
>
> If Kevin is the maintainer, then he needs to carefully review patches.

Thank you for implying that I don't carefully review patches.  You're
winning me over. :(

Please see my reply to the cover letter as to why this kind of thing has
not been on my priority list of things I look for during review.

> It is not my job to review all patches when BayLibre gets paid for it!

You have no idea who is getting paid for what kind of work, so please
don't make decisions about what you review based on your assumptions.

[...]

> I will also remind that I was forced to clean up the node order in ALL
> exynos5250 .dts files before I could get my new exynos5250-spring.dts
> merged, so I have zero understanding about these "churn" and "live with
> it" comments here. The same rules need to apply to all

It's a nice ideal that the same rules would apply to all, and in some
areas of the kernel, it may be true.  However, in actual practice,
across the variety of kernel subsystems and platforms, there are in fact
a rather large variety of "rules" with a huge amount of discretion left
up to the maintainers.

While that is a point of endless frustration for some (many?) it's also
part of what makes the kernel community healthy, vibrant and still
alive.

Look, you convinced me based on sound technical arguments, good code,
well written changelogs and persistence, even in spite of your
accusatory tone and insinuations.

For future reference, I'd be much happier to review without the latter.

Thanks,

Kevin

[toc] | [prev] | [next] | [standalone]

#1646111

Hi Kevin,

Am 18.05.2017 um 00:34 schrieb Kevin Hilman:
> Andreas Färber <afaerber@suse.de> writes:
>> Am 15.05.2017 um 10:16 schrieb Neil Armstrong:
>>> It's not perfect, but now it's done, live with it, this has already been discussed.
>>
>> No.
>>
>> Copy&pasting your comment N times does not make it any more valid. My
>> files, my rules - I insist on vega-s95, gxbb and gx, which you guys
>> refactored out from my gxbb, to be tidy.
> 
> hmm, "my files, my rules, ... my gxbb"
> 
> Very interesting perspective, but sorry, these do not belong to you.
> They belong to the kernel community.  You can insist if you like, but we
> do not make decisions just because someone says "mine".  Again, the
> confrontaional tone is not helpful to the dialogue.

We can certainly argue who started which tone here. From my perspective
I kindly asked Jerome to fix his node order on IRC, he said okay, but
neither he nor any of you ever sent such patches. (This was back when we
had an actual mismerge.) Weeks later I grudgingly sent the v1 cleanup
series myself, but instead of thanking me for doing "your" work, I was
greeted with unfriendly opposition and - yes, frustratingly is a word
here - put my efforts elsewhere. Neil's actions - who is neither author
nor maintainer here - of copying an opinionated personal and partially
wrong statement into a dozen neutral patches was highly offensive to me
and further escalated things. Stating his opinion, which he is welcome
to do, once would has sufficed.

Also what this "my" is about - and that should give you thought - is
that you guys added nodes to the Vega .dtsi file without adding any
BayLibre or other copyright. To someone downloading a tarball without
git history, that makes it look as if _I_ misordered those nodes in the
file that carries my name. So I would kindly suggest that you add your
copyright when making non-trivial additions.

Thanks,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

[toc] | [prev] | [next] | [standalone]

#1643651

Hi Andreas,

Andreas Färber <afaerber@suse.de> writes:

> This series fixes several cosmetic issues, on top of your for-next branch.
>
> Patches 3-6 rename a node, the rest should all be non-functional changes.
>
> PLEASE STOP merging random new nodes at the bottom of DT files!
> Just like it's a convention to sort new nodes by unit address, it has been
> a convention to sort by-label nodes by their label. As discussed here and
> elsewhere, this helps avoid merge conflicts and makes nodes easy to find.
> I don't care whether we order A0 before A or after, but adding new HDMI
> or CVBS nodes at the very bottom is totally out of alphabetical order.
> Since my v1 you really should've known that...

Your tone is a bit tiresome and frankly makes me hesitate before
reviewing the patches.  If you expect cordial dialogue and producitve
collaboration, please dial the accusations back a notch.

[ ... deep breath ... ]

Yes, your point was clear in v1, but that doesn't mean I agreed with it,
or that I particularily pay attention to that in my reviews because
honestly, alphabetical sorting is not very high on the list of things I
care about.  Over many years of being a kernel maintainer, I've had to
deal with my share of headaches, but conflicts caused by poorly sorted
DT files hs never been on my list of pain points.

That being said, I understand your concerns and similar ones raised
maintainers on other platforms.  I'm not a big fan of the churn caused
by this kind of rework, *but* I'm also not opposed to keeping things
better organized.  So, since there is an active contributor (and
reviewer) that is willing to put in the time and effort to clean things
up, I don't see a good (enough) reason to say no.

So, I will apply the series as is to my v4.13/dt64 branch

Going forward, I will try to keep an eye on the organization of DTS
files, but honestly, it's low priority for me, so if this is something
you care about, I trust that you will continue to help review DTS files.

I know you're already doing many reviews along with your contributions,
so thanks for that, and please keep it up. :)

> Similarly, Khadas Vim shouldn't have been merged with the "bcrmf" typo.
>
> Which proves my point that we need to fix these issues now so that they
> don't keep spreading (Broken Window Theory). New boards have not been
> checked for sort order, only boards already touched in v1.
>
> Board and Makefile order affect my pending R-Box Pro patches.
> Node order affects Martin's pending Bluetooth patches among others.

Yes, this series will affect a few other series that are in flight, and
I apologize to those developers for the needed rebase, but I'd rather
apply a series like this that affects so many DT files early in the
cycle.

Thanks again for the fixes and cleanups,

Kevin

[toc] | [prev] | [next] | [standalone]

#1646141

Hi Kevin,

Am 17.05.2017 um 23:46 schrieb Kevin Hilman:
> Andreas Färber <afaerber@suse.de> writes:
>> This series fixes several cosmetic issues, on top of your for-next branch.
>>
>> Patches 3-6 rename a node, the rest should all be non-functional changes.
>>
>> PLEASE STOP merging random new nodes at the bottom of DT files!
>> Just like it's a convention to sort new nodes by unit address, it has been
>> a convention to sort by-label nodes by their label. As discussed here and
>> elsewhere, this helps avoid merge conflicts and makes nodes easy to find.
>> I don't care whether we order A0 before A or after, but adding new HDMI
>> or CVBS nodes at the very bottom is totally out of alphabetical order.
>> Since my v1 you really should've known that...
> 
> Your tone is a bit tiresome and frankly makes me hesitate before
> reviewing the patches.  If you expect cordial dialogue and producitve
> collaboration, please dial the accusations back a notch.

I have sketched the history in another reply, maybe you understand the
sentiments better now.

I was annoyed to see that contributions by several people made the
problem worse than it was originally. I.e., my v1 fix and now v2 grew
more invasive because the issue did not get contained, not just not
fixed. Rebasing this branch was no fun.

[snip]

I will continue to point things out in review or hindsight where I can,
but as explained to Martin, this is not something I can continuously do
in the pace that BayLibre's nice driver contributions have led to.

As for the question of ownership, there should be no doubt that I not
only factually contributed meson-gxbb DT files but also figured out how
to get any kernel to boot on S905 with the old vendor U-Boot and fixed
serial output and added earlycon. That was before BayLibre stepped in.
So from my perspective all your contributions are based on my pioneering
S905 work (and my work in turn reused Carlo's S805 meson_uart etc.), and
I will not react well when some BayLibre employee not initially involved
with those efforts (Neil) now tells me to shut up and pretends as if he
makes the rules now just because his colleague stepped up as maintainer.
The ordering rules I applied here are not my invention but things I
picked on from previous reviews. You yourself had stated that I could
resend the patches and explain why - Russell and I already gave some
reasons, and there were no responses, which I took as no opposition and
was frankly surprised about the renewed level of opposition.

More generally, if a fix makes sense on its own and has no dependencies,
then I personally prefer it getting applied after reviews. If there's a
rule/reason for requiring full-series resends in Linux, it hasn't been
explained to me yet.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web