[v2,0/3] riscv: dts: starfive: add Milkv Mars board device tree

  The Milkv Mars is a development board based on the Starfive JH7110 SoC.
The board features:

- JH7110 SoC
- 1/2/4/8 GiB LPDDR4 DRAM
- AXP15060 PMIC
- 40 pin GPIO header
- 3x USB 3.0 host port
- 1x USB 2.0 host port
- 1x M.2 E-Key
- 1x eMMC slot
- 1x MicroSD slot
- 1x QSPI Flash
- 1x 1Gbps Ethernet port
- 1x HDMI port
- 1x 2-lane DSI and 1x 4-lane DSI
- 1x 2-lane CSI

patch1 adds 'cpus' label
patch2 adds "milkv,mars" board dt-binding
patch3 adds the devicetree file describing the currently supported
features:
Namely PMIC, UART, I2C, GPIO, SD card, QSPI Flash, eMMC and Ethernet.

Since v1:
 - add two new patches which add "cpus" label and board dt-binding
 - adopt Krzysztof's suggestions, thanks

Hi Conor,

I see you have sent a patch which moves the timebase-frequency property
to soc dtsi, but this series doesn't rebase on that. I can update it
once your patch is merged.

Thanks

Jisheng Zhang (3):
  riscv: dts: starfive: add 'cpus' label to jh7110 and jh7100 soc dtsi
  dt-bindings: riscv: starfive: add Milkv Mars board
  riscv: dts: starfive: add Milkv Mars board device tree

 .../devicetree/bindings/riscv/starfive.yaml   |   1 +
 arch/riscv/boot/dts/starfive/Makefile         |   1 +
 arch/riscv/boot/dts/starfive/jh7100.dtsi      |   2 +-
 .../boot/dts/starfive/jh7110-milkv-mars.dts   | 637 ++++++++++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |   2 +-
 5 files changed, 641 insertions(+), 2 deletions(-)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-milkv-mars.dts
  

Jisheng Zhang wrote:
> The Milkv Mars is a development board based on the Starfive JH7110 SoC.
> The board features:
>
> - JH7110 SoC
> - 1/2/4/8 GiB LPDDR4 DRAM
> - AXP15060 PMIC
> - 40 pin GPIO header
> - 3x USB 3.0 host port
> - 1x USB 2.0 host port
> - 1x M.2 E-Key
> - 1x eMMC slot
> - 1x MicroSD slot
> - 1x QSPI Flash
> - 1x 1Gbps Ethernet port
> - 1x HDMI port
> - 1x 2-lane DSI and 1x 4-lane DSI
> - 1x 2-lane CSI
>
> patch1 adds 'cpus' label
> patch2 adds "milkv,mars" board dt-binding
> patch3 adds the devicetree file describing the currently supported
> features:
> Namely PMIC, UART, I2C, GPIO, SD card, QSPI Flash, eMMC and Ethernet.
>
> Since v1:
>  - add two new patches which add "cpus" label and board dt-binding
>  - adopt Krzysztof's suggestions, thanks
>
> Hi Conor,
>
> I see you have sent a patch which moves the timebase-frequency property
> to soc dtsi, but this series doesn't rebase on that. I can update it
> once your patch is merged.

Hi Jisheng,

Thanks for working on this! On the JH7110 the mtime derives almost directly
from the external oscillator like this:

osc (24MHz) -> rtc_toggle (div 6) -> mtime (4MHz)

So to me it makes sense to define the timebase-frequency in the same file as
the frequency of the external oscillator.

In general it looks good, but if you do

  diff -Naur jh7110-{starfive-visionfive-2.dtsi,milkv-mars.dts}

you'll see that those two files are almost identical. Even external clock
speeds and all the pin configuration are the same. I'd strongly prefer to have
all that factored out in a common .dtsi so fixes don't get out of sync.

/Emil
  

On Sat, Dec 02, 2023 at 02:07:50PM -0800, Emil Renner Berthing wrote:
> Jisheng Zhang wrote:
> > The Milkv Mars is a development board based on the Starfive JH7110 SoC.
> > The board features:
> >
> > - JH7110 SoC
> > - 1/2/4/8 GiB LPDDR4 DRAM
> > - AXP15060 PMIC
> > - 40 pin GPIO header
> > - 3x USB 3.0 host port
> > - 1x USB 2.0 host port
> > - 1x M.2 E-Key
> > - 1x eMMC slot
> > - 1x MicroSD slot
> > - 1x QSPI Flash
> > - 1x 1Gbps Ethernet port
> > - 1x HDMI port
> > - 1x 2-lane DSI and 1x 4-lane DSI
> > - 1x 2-lane CSI
> >
> > patch1 adds 'cpus' label
> > patch2 adds "milkv,mars" board dt-binding
> > patch3 adds the devicetree file describing the currently supported
> > features:
> > Namely PMIC, UART, I2C, GPIO, SD card, QSPI Flash, eMMC and Ethernet.
> >
> > Since v1:
> >  - add two new patches which add "cpus" label and board dt-binding
> >  - adopt Krzysztof's suggestions, thanks
> >
> > Hi Conor,
> >
> > I see you have sent a patch which moves the timebase-frequency property
> > to soc dtsi, but this series doesn't rebase on that. I can update it
> > once your patch is merged.
> 
> Hi Jisheng,
> 
> Thanks for working on this! On the JH7110 the mtime derives almost directly
> from the external oscillator like this:
> 
> osc (24MHz) -> rtc_toggle (div 6) -> mtime (4MHz)
> 
> So to me it makes sense to define the timebase-frequency in the same file as
> the frequency of the external oscillator.
> 
> In general it looks good, but if you do
> 
>   diff -Naur jh7110-{starfive-visionfive-2.dtsi,milkv-mars.dts}
> 
> you'll see that those two files are almost identical. Even external clock
> speeds and all the pin configuration are the same. I'd strongly prefer to have
> all that factored out in a common .dtsi so fixes don't get out of sync.

I'm gonna mark this as changes requested on patchwork because of this
comment. LMK if you don't think this is worth another version Emil.

Cheers,
Conor.
  

On Wed, Jan 10, 2024 at 01:53:11PM +0000, Conor Dooley wrote:
> On Sat, Dec 02, 2023 at 02:07:50PM -0800, Emil Renner Berthing wrote:
> > Jisheng Zhang wrote:
> > > The Milkv Mars is a development board based on the Starfive JH7110 SoC.
> > > The board features:
> > >
> > > - JH7110 SoC
> > > - 1/2/4/8 GiB LPDDR4 DRAM
> > > - AXP15060 PMIC
> > > - 40 pin GPIO header
> > > - 3x USB 3.0 host port
> > > - 1x USB 2.0 host port
> > > - 1x M.2 E-Key
> > > - 1x eMMC slot
> > > - 1x MicroSD slot
> > > - 1x QSPI Flash
> > > - 1x 1Gbps Ethernet port
> > > - 1x HDMI port
> > > - 1x 2-lane DSI and 1x 4-lane DSI
> > > - 1x 2-lane CSI
> > >
> > > patch1 adds 'cpus' label
> > > patch2 adds "milkv,mars" board dt-binding
> > > patch3 adds the devicetree file describing the currently supported
> > > features:
> > > Namely PMIC, UART, I2C, GPIO, SD card, QSPI Flash, eMMC and Ethernet.
> > >
> > > Since v1:
> > >  - add two new patches which add "cpus" label and board dt-binding
> > >  - adopt Krzysztof's suggestions, thanks
> > >
> > > Hi Conor,
> > >
> > > I see you have sent a patch which moves the timebase-frequency property
> > > to soc dtsi, but this series doesn't rebase on that. I can update it
> > > once your patch is merged.
> > 
> > Hi Jisheng,
> > 
> > Thanks for working on this! On the JH7110 the mtime derives almost directly
> > from the external oscillator like this:
> > 
> > osc (24MHz) -> rtc_toggle (div 6) -> mtime (4MHz)
> > 
> > So to me it makes sense to define the timebase-frequency in the same file as
> > the frequency of the external oscillator.
> > 
> > In general it looks good, but if you do
> > 
> >   diff -Naur jh7110-{starfive-visionfive-2.dtsi,milkv-mars.dts}
> > 
> > you'll see that those two files are almost identical. Even external clock
> > speeds and all the pin configuration are the same. I'd strongly prefer to have
> > all that factored out in a common .dtsi so fixes don't get out of sync.
> 
> I'm gonna mark this as changes requested on patchwork because of this
> comment. LMK if you don't think this is worth another version Emil.

Yeah a new version is needed. And I think it's a bit late for this
window, so the new version will be out once v6.8-rc1 is out.

Thanks