[PATCH v5 0/3] Add DeepComputing FML13V01 board dts

[PATCH v5 0/3] Add DeepComputing FML13V01 board dts

[Guodong Xu]

This series updates Device Tree related files to introduce the
FML13V10 board from DeepComputing, which incorporates a StarFive
JH7110 SoC.  This board is designed for use on the Framework Laptop 13
Chassis, which has (Framework) SKU FRANHQ0001.

The original three versions of this series were posted by Sandie Cao
from DeepComputing.  Her mailer configuration caused mail threading
errors, which led to some confusion.  After some discussion, we have
agreed to take over moving this series toward acceptance.

Changes from v4:

Remove the extra "From:" line in commit messages of patch 1 & 2.

Changes from v3:

In this version, the board name is changed from FM7110 to FML13V10.

The descriptions for all patches in this series now have been updated
(slightly).  The first patch received an ack from Krzysztof in v2, and
the second patch received an ack from Rob on v3, and both are included
here. Other than the board name, the content of the first and second
patches are the same as before.

The third patch has been modified.  pcie0 was marked as disabled in
patch 3, and Krzysztof asked why pcie0 was enabled in the first place.
In fact, it is *not* enabled, and the node disabling it in patch 3
was not required.  Similarly, gmac1 is already disabled.  So these nodes
have been removed in patch 3.

However, gmac0 and pwmdac *are* enabled (in "jh7110-common.dtsi"),
but they should not be (for now) on this board.  The same is true
for spi0, csi2rx, and camss.  There are a few other nodes added in
patch 3 to ensure things are disabled that should be. Finally, usb0
remains enabled, but with a different operational mode (dr_mode =
"host").

The proper fix is to disable things except where they're needed rather
than enabling them in jh7110-common.dtsi and disabling them for this
board.  We propose to fix this in a follow-on patch (or series), but
we can do this in a new version of this series if requested.

Below is a version history, including direct links to all of the patches
(because a single link to each series doesn't work).

Best regards,
Guodong, Alex

v5:
- Remove the extra "From:" line in commit messages of patch 1 & 2.

v4:
- Board name was changed from FM7110 to FML13V10
- Descriptions for all patches in this series were updated slightly
- Add Rob's ack on patch 2
- In patch 3, device nodes were updated to reflect their proper status

https://lore.kernel.org/linux-riscv/20241019162605.308475-1-guodong@riscstar.com/

v3:
- Update board features into description
- Add Krzysztof's ack on patch 1

https://lore.kernel.org/all/20240925053123.1364574-1-sandie.cao@deepcomputing.io/
https://lore.kernel.org/all/20240925053123.1364574-2-sandie.cao@deepcomputing.io/
https://lore.kernel.org/all/20240925053123.1364574-3-sandie.cao@deepcomputing.io/
https://lore.kernel.org/all/20240925053123.1364574-4-sandie.cao@deepcomputing.io/

v2:
- Add deepcomputing into vendor list.
- Add deepcomputing,fm7110 into model compatible list.
- Framework Config will be included in later a patch.

https://lore.kernel.org/all/20240924080650.1345485-1-sandie.cao@deepcomputing.io/
https://lore.kernel.org/all/20240924080650.1345485-2-sandie.cao@deepcomputing.io/
https://lore.kernel.org/all/20240924080650.1345485-3-sandie.cao@deepcomputing.io/
https://lore.kernel.org/all/20240924080650.1345485-4-sandie.cao@deepcomputing.io/

v1:
- Add framework dts and config.

https://lore.kernel.org/all/20240923053621.1585972-1-sandie.cao@deepcomputing.io/
https://lore.kernel.org/all/20240923053621.1585972-2-sandie.cao@deepcomputing.io/
https://lore.kernel.org/all/20240923053806.1586080-1-sandie.cao@deepcomputing.io/

Guodong Xu (1):
  riscv: dts: starfive: add DeepComputing FML13V01 board device tree

Sandie Cao (2):
  dt-bindings: vendor: add deepcomputing
  dt-bindings: riscv: starfive: add deepcomputing,fml13v01

 .../devicetree/bindings/riscv/starfive.yaml   |  1 +
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 arch/riscv/boot/dts/starfive/Makefile         |  1 +
 .../jh7110-deepcomputing-fml13v01.dts         | 44 +++++++++++++++++++
 4 files changed, 48 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-deepcomputing-fml13v01.dts

-- 
2.34.1

[PATCH v5 1/3] dt-bindings: vendor: add deepcomputing

[Guodong Xu]

From: Sandie Cao <sandie.cao@deepcomputing.io>

Add "deepcomputing" to the Devicetree Vendor Prefix Registry.

Signed-off-by: Sandie Cao <sandie.cao@deepcomputing.io>
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
[elder@riscstar.com: revised the description]
Signed-off-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Guodong Xu <guodong@riscstar.com>
---
v5: Removed extra "From:" line in the commit message
v4: Updated description
v3: Add Krzysztof's ack
v2: Add deepcomputing into Vendor Prefix Registery

 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index b320a39de7fe..f8bb3df65797 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -356,6 +356,8 @@ patternProperties:
     description: DataImage, Inc.
   "^davicom,.*":
     description: DAVICOM Semiconductor, Inc.
+  "^deepcomputing,.*":
+    description: DeepComputing (HK) Limited
   "^dell,.*":
     description: Dell Inc.
   "^delta,.*":
-- 
2.34.1

[PATCH v5 2/3] dt-bindings: riscv: starfive: add deepcomputing,fml13v01

[Guodong Xu]

From: Sandie Cao <sandie.cao@deepcomputing.io>

Add "deepcomputing,fml13v01" as a StarFive SoC-based board.

The DeepComputing FML13V01 board incorporates a StarFive JH7110 SoC, and
it's designed for the Framework Laptop 13 Chassis, which has (Framework)
SKU FRANHQ0001.

Signed-off-by: Sandie Cao <sandie.cao@deepcomputing.io>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
[elder@riscstar.com: considerably shortened the description]
Signed-off-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Guodong Xu <guodong@riscstar.com>
---
v5: Removed extra "From:" line in the commit message
v4: Added Rob's Ack
    Updated description
    Changed board name from fm7110 to fml13v01
v3: No change
v2: Add deepcomputing,fm7110 into model compatible list

 Documentation/devicetree/bindings/riscv/starfive.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/riscv/starfive.yaml b/Documentation/devicetree/bindings/riscv/starfive.yaml
index 4d5c857b3cac..7ef85174353d 100644
--- a/Documentation/devicetree/bindings/riscv/starfive.yaml
+++ b/Documentation/devicetree/bindings/riscv/starfive.yaml
@@ -26,6 +26,7 @@ properties:
 
       - items:
           - enum:
+              - deepcomputing,fml13v01
               - milkv,mars
               - pine64,star64
               - starfive,visionfive-2-v1.2a
-- 
2.34.1

[PATCH v5 3/3] riscv: dts: starfive: add DeepComputing FML13V01 board device tree

[Guodong Xu]

From: Sandie Cao <sandie.cao@deepcomputing.io>

The FML13V01 board from DeepComputing incorporates a StarFive JH7110 SoC.
It is a mainboard designed for the Framework Laptop 13 Chassis, which has
(Framework) SKU FRANHQ0001.

The FML13V01 board features:
- StarFive JH7110 SoC
- LPDDR4 8GB
- eMMC 32GB or 128GB
- QSPI Flash
- MicroSD Slot
- PCIe-based Wi-Fi
- 4 USB-C Ports
 - Port 1: PD 3.0 (60W Max), USB 3.2 Gen 1, DP 1.4 (4K@30Hz/2.5K@60Hz)
 - Port 2: PD 3.0 (60W Max), USB 3.2 Gen 1
 - Port 3 & 4: USB 3.2 Gen 1

Create the DTS file for the DeepComputing FML13V01 board. Seven device
nodes have been verified functional and remain enabled: i2c2, i2c5, i2c6
qspi, mmc0, mmc1 and usb0.  All others remain disabled, or are disabled
by nodes in "jh7110-deepcomputing-fml13v01.dts".

Signed-off-by: Sandie Cao <sandie.cao@deepcomputing.io>
[elder@riscstar.com: revised the description, updated some nodes]
Signed-off-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Guodong Xu <guodong@riscstar.com>
---
v5: No change
v4: Changed model string to "DeepComputing FML13V01"
    Changed dts filename and Makefile accordingly to reflect the change
    Updated device nodes status, and verified functional
    Revised the commit message
v3: Updated the commit message
v2: Changed the model and copmatible strings
    Updated the commit message with board features

 arch/riscv/boot/dts/starfive/Makefile         |  1 +
 .../jh7110-deepcomputing-fml13v01.dts         | 44 +++++++++++++++++++
 2 files changed, 45 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-deepcomputing-fml13v01.dts

diff --git a/arch/riscv/boot/dts/starfive/Makefile b/arch/riscv/boot/dts/starfive/Makefile
index 7a163a7d6ba3..b3bb12f78e7d 100644
--- a/arch/riscv/boot/dts/starfive/Makefile
+++ b/arch/riscv/boot/dts/starfive/Makefile
@@ -8,6 +8,7 @@ DTC_FLAGS_jh7110-starfive-visionfive-2-v1.3b := -@
 dtb-$(CONFIG_ARCH_STARFIVE) += jh7100-beaglev-starlight.dtb
 dtb-$(CONFIG_ARCH_STARFIVE) += jh7100-starfive-visionfive-v1.dtb
 
+dtb-$(CONFIG_ARCH_STARFIVE) += jh7110-deepcomputing-fml13v01.dtb
 dtb-$(CONFIG_ARCH_STARFIVE) += jh7110-milkv-mars.dtb
 dtb-$(CONFIG_ARCH_STARFIVE) += jh7110-pine64-star64.dtb
 dtb-$(CONFIG_ARCH_STARFIVE) += jh7110-starfive-visionfive-2-v1.2a.dtb
diff --git a/arch/riscv/boot/dts/starfive/jh7110-deepcomputing-fml13v01.dts b/arch/riscv/boot/dts/starfive/jh7110-deepcomputing-fml13v01.dts
new file mode 100644
index 000000000000..b515b7d04c37
--- /dev/null
+++ b/arch/riscv/boot/dts/starfive/jh7110-deepcomputing-fml13v01.dts
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2024 DeepComputing (HK) Limited
+ */
+
+/dts-v1/;
+#include "jh7110-common.dtsi"
+
+/ {
+	model = "DeepComputing FML13V01";
+	compatible = "deepcomputing,fml13v01", "starfive,jh7110";
+};
+
+&camss {
+	status = "disabled";
+};
+
+&csi2rx {
+	status = "disabled";
+};
+
+&gmac0 {
+	status = "disabled";
+};
+
+&i2c0 {
+	status = "disabled";
+};
+
+&pwm {
+	status = "disabled";
+};
+
+&pwmdac {
+	status = "disabled";
+};
+
+&spi0 {
+	status = "disabled";
+};
+
+&usb0 {
+	dr_mode = "host";
+};
-- 
2.34.1

Re: [PATCH v5 3/3] riscv: dts: starfive: add DeepComputing FML13V01 board device tree

[Krzysztof Kozlowski]

On Sun, Oct 20, 2024 at 09:49:59PM +0800, Guodong Xu wrote:
> From: Sandie Cao <sandie.cao@deepcomputing.io>
> 
> The FML13V01 board from DeepComputing incorporates a StarFive JH7110 SoC.
> It is a mainboard designed for the Framework Laptop 13 Chassis, which has
> (Framework) SKU FRANHQ0001.
> 
> The FML13V01 board features:
> - StarFive JH7110 SoC
> - LPDDR4 8GB
> - eMMC 32GB or 128GB
> - QSPI Flash
> - MicroSD Slot
> - PCIe-based Wi-Fi
> - 4 USB-C Ports
>  - Port 1: PD 3.0 (60W Max), USB 3.2 Gen 1, DP 1.4 (4K@30Hz/2.5K@60Hz)
>  - Port 2: PD 3.0 (60W Max), USB 3.2 Gen 1
>  - Port 3 & 4: USB 3.2 Gen 1
> 
> Create the DTS file for the DeepComputing FML13V01 board. Seven device
> nodes have been verified functional and remain enabled: i2c2, i2c5, i2c6
> qspi, mmc0, mmc1 and usb0.  All others remain disabled, or are disabled
> by nodes in "jh7110-deepcomputing-fml13v01.dts".
> 
> Signed-off-by: Sandie Cao <sandie.cao@deepcomputing.io>
> [elder@riscstar.com: revised the description, updated some nodes]
> Signed-off-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Guodong Xu <guodong@riscstar.com>
> ---
> v5: No change
> v4: Changed model string to "DeepComputing FML13V01"
>     Changed dts filename and Makefile accordingly to reflect the change
>     Updated device nodes status, and verified functional
>     Revised the commit message
> v3: Updated the commit message
> v2: Changed the model and copmatible strings
>     Updated the commit message with board features
> 
>  arch/riscv/boot/dts/starfive/Makefile         |  1 +
>  .../jh7110-deepcomputing-fml13v01.dts         | 44 +++++++++++++++++++
>  2 files changed, 45 insertions(+)
>  create mode 100644 arch/riscv/boot/dts/starfive/jh7110-deepcomputing-fml13v01.dts
> 
> diff --git a/arch/riscv/boot/dts/starfive/Makefile b/arch/riscv/boot/dts/starfive/Makefile
> index 7a163a7d6ba3..b3bb12f78e7d 100644
> --- a/arch/riscv/boot/dts/starfive/Makefile
> +++ b/arch/riscv/boot/dts/starfive/Makefile
> @@ -8,6 +8,7 @@ DTC_FLAGS_jh7110-starfive-visionfive-2-v1.3b := -@
>  dtb-$(CONFIG_ARCH_STARFIVE) += jh7100-beaglev-starlight.dtb
>  dtb-$(CONFIG_ARCH_STARFIVE) += jh7100-starfive-visionfive-v1.dtb
>  
> +dtb-$(CONFIG_ARCH_STARFIVE) += jh7110-deepcomputing-fml13v01.dtb
>  dtb-$(CONFIG_ARCH_STARFIVE) += jh7110-milkv-mars.dtb
>  dtb-$(CONFIG_ARCH_STARFIVE) += jh7110-pine64-star64.dtb
>  dtb-$(CONFIG_ARCH_STARFIVE) += jh7110-starfive-visionfive-2-v1.2a.dtb
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-deepcomputing-fml13v01.dts b/arch/riscv/boot/dts/starfive/jh7110-deepcomputing-fml13v01.dts
> new file mode 100644
> index 000000000000..b515b7d04c37
> --- /dev/null
> +++ b/arch/riscv/boot/dts/starfive/jh7110-deepcomputing-fml13v01.dts
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright (C) 2024 DeepComputing (HK) Limited
> + */
> +
> +/dts-v1/;
> +#include "jh7110-common.dtsi"
> +
> +/ {
> +	model = "DeepComputing FML13V01";
> +	compatible = "deepcomputing,fml13v01", "starfive,jh7110";
> +};
> +
> +&camss {
> +	status = "disabled";
> +};
> +
> +&csi2rx {
> +	status = "disabled";
> +};
> +
> +&gmac0 {
> +	status = "disabled";
> +};
> +
> +&i2c0 {
> +	status = "disabled";
> +};
> +
> +&pwm {
> +	status = "disabled";
> +};
> +
> +&pwmdac {
> +	status = "disabled";
> +};
> +
> +&spi0 {
> +	status = "disabled";

If your board has to disable all these, then they should not have been
enabled in DTSI in the first place. Only blocks present and working in
the SoC (without amny external needs) should be enabled.

I suggest to fix that aspect first.

Best regards,
Krzysztof

Re: [PATCH v5 3/3] riscv: dts: starfive: add DeepComputing FML13V01 board device tree

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1085 bytes --]

On Mon, Oct 21, 2024 at 09:17:59AM +0200, Krzysztof Kozlowski wrote:
> On Sun, Oct 20, 2024 at 09:49:59PM +0800, Guodong Xu wrote:
> > From: Sandie Cao <sandie.cao@deepcomputing.io>
> > +&camss {
> > +	status = "disabled";
> > +};
> > +
> > +&csi2rx {
> > +	status = "disabled";
> > +};

You can drop these two, I marked them disabled in the common file
earlier this week.
1
> > +
> > +&gmac0 {
> > +	status = "disabled";
> > +};
> > +
> > +&i2c0 {
> > +	status = "disabled";
> > +};
> > +
> > +&pwm {
> > +	status = "disabled";
> > +};
> > +
> > +&pwmdac {
> > +	status = "disabled";
> > +};
> > +
> > +&spi0 {
> > +	status = "disabled";
> 
> If your board has to disable all these, then they should not have been
> enabled in DTSI in the first place. Only blocks present and working in
> the SoC (without amny external needs) should be enabled.
> 
> I suggest to fix that aspect first.

Eh, I don't think I agree. Having 5 disables here is a lesser evil than
reproducing 90% of jh7110-common.dtsi or shunting a bunch of stuff
around. Emil?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 3/3] riscv: dts: starfive: add DeepComputing FML13V01 board device tree

[Krzysztof Kozlowski]

On 21/10/2024 13:16, Conor Dooley wrote:
> On Mon, Oct 21, 2024 at 09:17:59AM +0200, Krzysztof Kozlowski wrote:
>> On Sun, Oct 20, 2024 at 09:49:59PM +0800, Guodong Xu wrote:
>>> From: Sandie Cao <sandie.cao@deepcomputing.io>
>>> +&camss {
>>> +	status = "disabled";
>>> +};
>>> +
>>> +&csi2rx {
>>> +	status = "disabled";
>>> +};
> 
> You can drop these two, I marked them disabled in the common file
> earlier this week.
> 1
>>> +
>>> +&gmac0 {
>>> +	status = "disabled";
>>> +};
>>> +
>>> +&i2c0 {
>>> +	status = "disabled";
>>> +};
>>> +
>>> +&pwm {
>>> +	status = "disabled";
>>> +};
>>> +
>>> +&pwmdac {
>>> +	status = "disabled";
>>> +};
>>> +
>>> +&spi0 {
>>> +	status = "disabled";
>>
>> If your board has to disable all these, then they should not have been
>> enabled in DTSI in the first place. Only blocks present and working in
>> the SoC (without amny external needs) should be enabled.
>>
>> I suggest to fix that aspect first.
> 
> Eh, I don't think I agree. Having 5 disables here is a lesser evil than
> reproducing 90% of jh7110-common.dtsi or shunting a bunch of stuff
> around. Emil?

Why reproducing 90%? Only enable would be here, no? Or you want to say
the common DTSI has things which do not exist?

Best regards,
Krzysztof

Re: [PATCH v5 3/3] riscv: dts: starfive: add DeepComputing FML13V01 board device tree

[Alex Elder]

On 10/21/24 7:47 AM, Krzysztof Kozlowski wrote:
> On 21/10/2024 13:16, Conor Dooley wrote:
>> On Mon, Oct 21, 2024 at 09:17:59AM +0200, Krzysztof Kozlowski wrote:
>>> On Sun, Oct 20, 2024 at 09:49:59PM +0800, Guodong Xu wrote:
>>>> From: Sandie Cao <sandie.cao@deepcomputing.io>
>>>> +&camss {
>>>> +	status = "disabled";
>>>> +};
>>>> +
>>>> +&csi2rx {
>>>> +	status = "disabled";
>>>> +};
>>
>> You can drop these two, I marked them disabled in the common file
>> earlier this week.
>> 1
>>>> +
>>>> +&gmac0 {
>>>> +	status = "disabled";
>>>> +};
>>>> +
>>>> +&i2c0 {
>>>> +	status = "disabled";
>>>> +};
>>>> +
>>>> +&pwm {
>>>> +	status = "disabled";
>>>> +};
>>>> +
>>>> +&pwmdac {
>>>> +	status = "disabled";
>>>> +};
>>>> +
>>>> +&spi0 {
>>>> +	status = "disabled";
>>>
>>> If your board has to disable all these, then they should not have been
>>> enabled in DTSI in the first place. Only blocks present and working in
>>> the SoC (without amny external needs) should be enabled.
>>>
>>> I suggest to fix that aspect first.
>>
>> Eh, I don't think I agree. Having 5 disables here is a lesser evil than
>> reproducing 90% of jh7110-common.dtsi or shunting a bunch of stuff
>> around. Emil?
> 
> Why reproducing 90%? Only enable would be here, no? Or you want to say
> the common DTSI has things which do not exist?

For what it's worth, I agree with Krzysztof.  In the (long) cover
page we pointed this out, and offered to do it in a followup patch.
But if requested we can do it now.

So in v6, a new patch would be inserted before the other three,
and it would:
- Remove the status = "okay" lines for those nodes that are not enabled
   in this new platform, in "jh7110-common.dtsi"
- Add nodes where appropriate in:
     jh7110-milkv-mars.dts
     jh7110-pine64-star64.dts
     jh7110-starfive-visionfive-2.dtsi
   They'll look like this, to enable the ones disabled above, e.g.:
     &gmac0 {
         status = "okay";
     };

     &i2c0 {
         status = "okay";
     };

You guys should come to agreement, but I do think what Krzysztof says
is the right approach.  And unless convinced otherwise, this will be
what shows up in the next version of this series.

					-Alex

> Best regards,
> Krzysztof
> 

Re: [PATCH v5 3/3] riscv: dts: starfive: add DeepComputing FML13V01 board device tree

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2614 bytes --]

On Mon, Oct 21, 2024 at 08:44:16AM -0500, Alex Elder wrote:
> On 10/21/24 7:47 AM, Krzysztof Kozlowski wrote:
> > On 21/10/2024 13:16, Conor Dooley wrote:
> > > On Mon, Oct 21, 2024 at 09:17:59AM +0200, Krzysztof Kozlowski wrote:
> > > > On Sun, Oct 20, 2024 at 09:49:59PM +0800, Guodong Xu wrote:
> > > > > From: Sandie Cao <sandie.cao@deepcomputing.io>
> > > > > +&camss {
> > > > > +	status = "disabled";
> > > > > +};
> > > > > +
> > > > > +&csi2rx {
> > > > > +	status = "disabled";
> > > > > +};
> > > 
> > > You can drop these two, I marked them disabled in the common file
> > > earlier this week.
> > > 1
> > > > > +
> > > > > +&gmac0 {
> > > > > +	status = "disabled";
> > > > > +};
> > > > > +
> > > > > +&i2c0 {
> > > > > +	status = "disabled";
> > > > > +};
> > > > > +
> > > > > +&pwm {
> > > > > +	status = "disabled";
> > > > > +};
> > > > > +
> > > > > +&pwmdac {
> > > > > +	status = "disabled";
> > > > > +};
> > > > > +
> > > > > +&spi0 {
> > > > > +	status = "disabled";
> > > > 
> > > > If your board has to disable all these, then they should not have been
> > > > enabled in DTSI in the first place. Only blocks present and working in
> > > > the SoC (without amny external needs) should be enabled.
> > > > 
> > > > I suggest to fix that aspect first.
> > > 
> > > Eh, I don't think I agree. Having 5 disables here is a lesser evil than
> > > reproducing 90% of jh7110-common.dtsi or shunting a bunch of stuff
> > > around. Emil?
> > 
> > Why reproducing 90%? Only enable would be here, no? Or you want to say
> > the common DTSI has things which do not exist?
> 
> For what it's worth, I agree with Krzysztof.  In the (long) cover
> page we pointed this out, and offered to do it in a followup patch.
> But if requested we can do it now.
> 
> So in v6, a new patch would be inserted before the other three,
> and it would:
> - Remove the status = "okay" lines for those nodes that are not enabled
>   in this new platform, in "jh7110-common.dtsi"
> - Add nodes where appropriate in:
>     jh7110-milkv-mars.dts
>     jh7110-pine64-star64.dts
>     jh7110-starfive-visionfive-2.dtsi
>   They'll look like this, to enable the ones disabled above, e.g.:
>     &gmac0 {
>         status = "okay";
>     };
> 
>     &i2c0 {
>         status = "okay";
>     };
> 
> You guys should come to agreement, but I do think what Krzysztof says
> is the right approach.  And unless convinced otherwise, this will be
> what shows up in the next version of this series.

Ultimately, it is up to Emil how he wants these laid out.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 2/3] dt-bindings: riscv: starfive: add deepcomputing,fml13v01

[Emil Renner Berthing]

Guodong Xu wrote:
> From: Sandie Cao <sandie.cao@deepcomputing.io>
>
> Add "deepcomputing,fml13v01" as a StarFive SoC-based board.
>
> The DeepComputing FML13V01 board incorporates a StarFive JH7110 SoC, and
> it's designed for the Framework Laptop 13 Chassis, which has (Framework)
> SKU FRANHQ0001.
>
> Signed-off-by: Sandie Cao <sandie.cao@deepcomputing.io>
> Acked-by: Rob Herring (Arm) <robh@kernel.org>
> [elder@riscstar.com: considerably shortened the description]
> Signed-off-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Guodong Xu <guodong@riscstar.com>

Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

Re: [PATCH v5 3/3] riscv: dts: starfive: add DeepComputing FML13V01 board device tree

[Emil Renner Berthing]

Conor Dooley wrote:
> On Mon, Oct 21, 2024 at 08:44:16AM -0500, Alex Elder wrote:
> > On 10/21/24 7:47 AM, Krzysztof Kozlowski wrote:
> > > On 21/10/2024 13:16, Conor Dooley wrote:
> > > > On Mon, Oct 21, 2024 at 09:17:59AM +0200, Krzysztof Kozlowski wrote:
> > > > > On Sun, Oct 20, 2024 at 09:49:59PM +0800, Guodong Xu wrote:
> > > > > > From: Sandie Cao <sandie.cao@deepcomputing.io>
> > > > > > +&camss {
> > > > > > +	status = "disabled";
> > > > > > +};
> > > > > > +
> > > > > > +&csi2rx {
> > > > > > +	status = "disabled";
> > > > > > +};
> > > >
> > > > You can drop these two, I marked them disabled in the common file
> > > > earlier this week.
> > > > 1
> > > > > > +
> > > > > > +&gmac0 {
> > > > > > +	status = "disabled";
> > > > > > +};
> > > > > > +
> > > > > > +&i2c0 {
> > > > > > +	status = "disabled";
> > > > > > +};
> > > > > > +
> > > > > > +&pwm {
> > > > > > +	status = "disabled";
> > > > > > +};
> > > > > > +
> > > > > > +&pwmdac {
> > > > > > +	status = "disabled";
> > > > > > +};
> > > > > > +
> > > > > > +&spi0 {
> > > > > > +	status = "disabled";
> > > > >
> > > > > If your board has to disable all these, then they should not have been
> > > > > enabled in DTSI in the first place. Only blocks present and working in
> > > > > the SoC (without amny external needs) should be enabled.
> > > > >
> > > > > I suggest to fix that aspect first.
> > > >
> > > > Eh, I don't think I agree. Having 5 disables here is a lesser evil than
> > > > reproducing 90% of jh7110-common.dtsi or shunting a bunch of stuff
> > > > around. Emil?
> > >
> > > Why reproducing 90%? Only enable would be here, no? Or you want to say
> > > the common DTSI has things which do not exist?
> >
> > For what it's worth, I agree with Krzysztof.  In the (long) cover
> > page we pointed this out, and offered to do it in a followup patch.
> > But if requested we can do it now.
> >
> > So in v6, a new patch would be inserted before the other three,
> > and it would:
> > - Remove the status = "okay" lines for those nodes that are not enabled
> >   in this new platform, in "jh7110-common.dtsi"
> > - Add nodes where appropriate in:
> >     jh7110-milkv-mars.dts
> >     jh7110-pine64-star64.dts
> >     jh7110-starfive-visionfive-2.dtsi
> >   They'll look like this, to enable the ones disabled above, e.g.:
> >     &gmac0 {
> >         status = "okay";
> >     };
> >
> >     &i2c0 {
> >         status = "okay";
> >     };
> >
> > You guys should come to agreement, but I do think what Krzysztof says
> > is the right approach.  And unless convinced otherwise, this will be
> > what shows up in the next version of this series.
>
> Ultimately, it is up to Emil how he wants these laid out.

Thanks, but I agree. Please begin with a patch moving the nodes that are no
longer common out of jh7110-common.dtsi and into the vf2, mars and pine64
consumers. You should probably do the same with the &usb0 node instead
of overriding the dr_mode property.

/Emil

Re: [PATCH v5 3/3] riscv: dts: starfive: add DeepComputing FML13V01 board device tree

[Alex Elder]

On 10/23/24 11:49 AM, Emil Renner Berthing wrote:
>> Ultimately, it is up to Emil how he wants these laid out.
> Thanks, but I agree. Please begin with a patch moving the nodes that are no
> longer common out of jh7110-common.dtsi and into the vf2, mars and pine64
> consumers. You should probably do the same with the &usb0 node instead
> of overriding the dr_mode property.
> 
> /Emil

Thenk you Emil, we will prepare the next version of the series
to do these things.

					-Alex