diff mbox series

[V2,1/2,RESEND] arm64: dts: rockchip: Add DTS for FriendlyARM NanoPi R2S Plus

Message ID 20240801175736.16591-2-jin@mediatomb.cc (mailing list archive)
State New, archived
Headers show
Series [V2,1/2,RESEND] arm64: dts: rockchip: Add DTS for FriendlyARM NanoPi R2S Plus | expand

Commit Message

Sergey 'Jin' Bostandzhyan Aug. 1, 2024, 5:57 p.m. UTC
The R2S Plus is basically an R2S with additional eMMC.

The eMMC configuration for the DTS has been extracted and copied from
rk3328-nanopi-r2.dts, v2017.09 branch from the friendlyarm/uboot-rockchip
repository.

Signed-off-by: Sergey Bostandzhyan <jin@mediatomb.cc>
---
 arch/arm64/boot/dts/rockchip/Makefile         |  1 +
 .../dts/rockchip/rk3328-nanopi-r2s-plus.dts   | 31 +++++++++++++++++++
 2 files changed, 32 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts

Comments

Daniel Golle Aug. 4, 2024, 12:27 a.m. UTC | #1
On Thu, Aug 01, 2024 at 05:57:35PM +0000, Sergey Bostandzhyan wrote:
> The R2S Plus is basically an R2S with additional eMMC.
> 
> The eMMC configuration for the DTS has been extracted and copied from
> rk3328-nanopi-r2.dts, v2017.09 branch from the friendlyarm/uboot-rockchip
> repository.
> 
> Signed-off-by: Sergey Bostandzhyan <jin@mediatomb.cc>
> ---
>  arch/arm64/boot/dts/rockchip/Makefile         |  1 +
>  .../dts/rockchip/rk3328-nanopi-r2s-plus.dts   | 31 +++++++++++++++++++
>  2 files changed, 32 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> 
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index fda1b980eb4b..36258dc8dafd 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-evb.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c-plus.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s-plus.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus-lts.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-rock64.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> new file mode 100644
> index 000000000000..7b83090a2145
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2018 FriendlyElec Computer Tech. Co., Ltd.
> + * (http://www.friendlyarm.com)
> + *
> + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
> + */
> +
> +/dts-v1/;
> +#include "rk3328-nanopi-r2s.dts"
> +
> +/ {
> +	model = "FriendlyElec NanoPi R2S Plus";
> +	compatible = "friendlyarm,nanopi-r2s-plus", "rockchip,rk3328";
> +
> +	aliases {
> +		mmc1 = &emmc;
> +	};
> +};
> +
> +&emmc {
> +	bus-width = <8>;
> +	cap-mmc-highspeed;
> +	supports-emmc;
> +	disable-wp;
> +	non-removable;
> +	num-slots = <1>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&emmc_clk &emmc_cmd &emmc_bus8>;

I think it's worth adding

	mmc-hs200-1_8v;


I've tried getting the best speed possible and while HS400 with and
without enhanced strobe did NOT work, hs200 works just fine.
[    0.459863] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
[    0.460884] mmc_host mmc1: Bus speed (slot 0) = 150000000Hz (slot req 150000000Hz, actual 150000000HZ div = 0)
...
[    0.728220] dwmmc_rockchip ff520000.mmc: Successfully tuned phase to 194
[    0.728940] mmc1: new HS200 MMC card at address 0001
[    0.730774] mmcblk1: mmc1:0001 A3A551 28.9 GiB
[    0.733262]  mmcblk1: p1 p2
[    0.734562] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
[    0.736818] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
[    0.738503] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)

root@OpenWrt:/# hdparm -t /dev/mmcblk1

/dev/mmcblk1:
 Timing buffered disk reads: 342 MB in  3.00 seconds = 113.81 MB/sec


Without 'mmc-hs200-1_8v' property in DT the eMMC is detected as
[    0.440465] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
[    0.442032] mmc1: new high speed MMC card at address 0001
[    0.444261] mmcblk1: mmc1:0001 A3A551 28.9 GiB
[    0.447388]  mmcblk1: p1 p2
[    0.448744] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
[    0.451065] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
[    0.452871] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)


root@OpenWrt:/# hdparm -t /dev/mmcblk1

/dev/mmcblk1:
 Timing buffered disk reads: 134 MB in  3.03 seconds =  44.18 MB/sec


> +	status = "okay";
> +};

I'm right now trying to get SDIO RTL8822CS working, so far I'm out of luck,
but it can be added later once we got it working.
Sergey 'Jin' Bostandzhyan Aug. 5, 2024, 8:59 a.m. UTC | #2
Daniel,

On Sun, Aug 04, 2024 at 01:27:50AM +0100, Daniel Golle wrote:
> On Thu, Aug 01, 2024 at 05:57:35PM +0000, Sergey Bostandzhyan wrote:
> > The R2S Plus is basically an R2S with additional eMMC.
> > 
> > The eMMC configuration for the DTS has been extracted and copied from
> > rk3328-nanopi-r2.dts, v2017.09 branch from the friendlyarm/uboot-rockchip
> > repository.
> > 
> > Signed-off-by: Sergey Bostandzhyan <jin@mediatomb.cc>
> > ---
> >  arch/arm64/boot/dts/rockchip/Makefile         |  1 +
> >  .../dts/rockchip/rk3328-nanopi-r2s-plus.dts   | 31 +++++++++++++++++++
> >  2 files changed, 32 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > index fda1b980eb4b..36258dc8dafd 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-evb.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c-plus.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s.dtb
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s-plus.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus-lts.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-rock64.dtb
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > new file mode 100644
> > index 000000000000..7b83090a2145
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > @@ -0,0 +1,31 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2018 FriendlyElec Computer Tech. Co., Ltd.
> > + * (http://www.friendlyarm.com)
> > + *
> > + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
> > + */
> > +
> > +/dts-v1/;
> > +#include "rk3328-nanopi-r2s.dts"
> > +
> > +/ {
> > +	model = "FriendlyElec NanoPi R2S Plus";
> > +	compatible = "friendlyarm,nanopi-r2s-plus", "rockchip,rk3328";
> > +
> > +	aliases {
> > +		mmc1 = &emmc;
> > +	};
> > +};
> > +
> > +&emmc {
> > +	bus-width = <8>;
> > +	cap-mmc-highspeed;
> > +	supports-emmc;
> > +	disable-wp;
> > +	non-removable;
> > +	num-slots = <1>;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&emmc_clk &emmc_cmd &emmc_bus8>;
> 
> I think it's worth adding
> 
> 	mmc-hs200-1_8v;
> 
> 
> I've tried getting the best speed possible and while HS400 with and
> without enhanced strobe did NOT work, hs200 works just fine.
> [    0.459863] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> [    0.460884] mmc_host mmc1: Bus speed (slot 0) = 150000000Hz (slot req 150000000Hz, actual 150000000HZ div = 0)
> ...
> [    0.728220] dwmmc_rockchip ff520000.mmc: Successfully tuned phase to 194
> [    0.728940] mmc1: new HS200 MMC card at address 0001
> [    0.730774] mmcblk1: mmc1:0001 A3A551 28.9 GiB
> [    0.733262]  mmcblk1: p1 p2
> [    0.734562] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
> [    0.736818] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
> [    0.738503] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)
> 
> root@OpenWrt:/# hdparm -t /dev/mmcblk1
> 
> /dev/mmcblk1:
>  Timing buffered disk reads: 342 MB in  3.00 seconds = 113.81 MB/sec
> 
> 
> Without 'mmc-hs200-1_8v' property in DT the eMMC is detected as
> [    0.440465] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> [    0.442032] mmc1: new high speed MMC card at address 0001
> [    0.444261] mmcblk1: mmc1:0001 A3A551 28.9 GiB
> [    0.447388]  mmcblk1: p1 p2
> [    0.448744] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
> [    0.451065] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
> [    0.452871] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)
> 
> 
> root@OpenWrt:/# hdparm -t /dev/mmcblk1
> 
> /dev/mmcblk1:
>  Timing buffered disk reads: 134 MB in  3.03 seconds =  44.18 MB/sec
> 
> 
> > +	status = "okay";
> > +};
> 
> I'm right now trying to get SDIO RTL8822CS working, so far I'm out of luck,
> but it can be added later once we got it working.

would you be interested in taking over my attempted patches? Thing is,
that I am a userspace guy who only copy-pasted some entries from
FriendlyElec and things happened to work, but I really have no clue what I am
doing when it comes to hardware and DTS. I see that some changes were suggested, 
not only by you above, but also by others earlier and I have little
understanding of where I should be inserting what and how.

At this point I think it would make more sense if someone who actually
understands what they are doing would continue to tune the DTS :)

So it'd be great if either you or anyone else would be willing to take
over?

Kind regards,
Sergey
Heiko Stuebner Aug. 10, 2024, 7:11 p.m. UTC | #3
Hi,

Am Montag, 5. August 2024, 10:59:35 CEST schrieb Sergey 'Jin' Bostandzhyan:
> On Sun, Aug 04, 2024 at 01:27:50AM +0100, Daniel Golle wrote:
> > On Thu, Aug 01, 2024 at 05:57:35PM +0000, Sergey Bostandzhyan wrote:
> > > The R2S Plus is basically an R2S with additional eMMC.
> > > 
> > > The eMMC configuration for the DTS has been extracted and copied from
> > > rk3328-nanopi-r2.dts, v2017.09 branch from the friendlyarm/uboot-rockchip
> > > repository.
> > > 
> > > Signed-off-by: Sergey Bostandzhyan <jin@mediatomb.cc>
> > > ---
> > >  arch/arm64/boot/dts/rockchip/Makefile         |  1 +
> > >  .../dts/rockchip/rk3328-nanopi-r2s-plus.dts   | 31 +++++++++++++++++++
> > >  2 files changed, 32 insertions(+)
> > >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > 
> > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > > index fda1b980eb4b..36258dc8dafd 100644
> > > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-evb.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c-plus.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s.dtb
> > > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s-plus.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus-lts.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-rock64.dtb
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > new file mode 100644
> > > index 000000000000..7b83090a2145
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > @@ -0,0 +1,31 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * (C) Copyright 2018 FriendlyElec Computer Tech. Co., Ltd.
> > > + * (http://www.friendlyarm.com)
> > > + *
> > > + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
> > > + */
> > > +
> > > +/dts-v1/;
> > > +#include "rk3328-nanopi-r2s.dts"
> > > +
> > > +/ {
> > > +	model = "FriendlyElec NanoPi R2S Plus";
> > > +	compatible = "friendlyarm,nanopi-r2s-plus", "rockchip,rk3328";
> > > +
> > > +	aliases {
> > > +		mmc1 = &emmc;
> > > +	};
> > > +};
> > > +
> > > +&emmc {
> > > +	bus-width = <8>;
> > > +	cap-mmc-highspeed;
> > > +	supports-emmc;
> > > +	disable-wp;
> > > +	non-removable;
> > > +	num-slots = <1>;
> > > +	pinctrl-names = "default";
> > > +	pinctrl-0 = <&emmc_clk &emmc_cmd &emmc_bus8>;
> > 
> > I think it's worth adding
> > 
> > 	mmc-hs200-1_8v;
> > 
> > 
> > I've tried getting the best speed possible and while HS400 with and
> > without enhanced strobe did NOT work, hs200 works just fine.
> > [    0.459863] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> > [    0.460884] mmc_host mmc1: Bus speed (slot 0) = 150000000Hz (slot req 150000000Hz, actual 150000000HZ div = 0)
> > ...
> > [    0.728220] dwmmc_rockchip ff520000.mmc: Successfully tuned phase to 194
> > [    0.728940] mmc1: new HS200 MMC card at address 0001
> > [    0.730774] mmcblk1: mmc1:0001 A3A551 28.9 GiB
> > [    0.733262]  mmcblk1: p1 p2
> > [    0.734562] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
> > [    0.736818] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
> > [    0.738503] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)
> > 
> > root@OpenWrt:/# hdparm -t /dev/mmcblk1
> > 
> > /dev/mmcblk1:
> >  Timing buffered disk reads: 342 MB in  3.00 seconds = 113.81 MB/sec
> > 
> > 
> > Without 'mmc-hs200-1_8v' property in DT the eMMC is detected as
> > [    0.440465] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> > [    0.442032] mmc1: new high speed MMC card at address 0001
> > [    0.444261] mmcblk1: mmc1:0001 A3A551 28.9 GiB
> > [    0.447388]  mmcblk1: p1 p2
> > [    0.448744] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
> > [    0.451065] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
> > [    0.452871] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)
> > 
> > 
> > root@OpenWrt:/# hdparm -t /dev/mmcblk1
> > 
> > /dev/mmcblk1:
> >  Timing buffered disk reads: 134 MB in  3.03 seconds =  44.18 MB/sec
> > 
> > 
> > > +	status = "okay";
> > > +};
> > 
> > I'm right now trying to get SDIO RTL8822CS working, so far I'm out of luck,
> > but it can be added later once we got it working.
> 
> would you be interested in taking over my attempted patches? Thing is,
> that I am a userspace guy who only copy-pasted some entries from
> FriendlyElec and things happened to work, but I really have no clue what I am
> doing when it comes to hardware and DTS. I see that some changes were suggested, 
> not only by you above, but also by others earlier and I have little
> understanding of where I should be inserting what and how.
> 
> At this point I think it would make more sense if someone who actually
> understands what they are doing would continue to tune the DTS :)
> 
> So it'd be great if either you or anyone else would be willing to take
> over?

Though, a board devicetree is a nice way to get "your feet wet" in the
kernel :-) and for a lot of people scratching ones own itches gets them
started.

The devicetree is easy enough, also looks correct and you even got the
binding change correct - and you're the person with the actual board :-) .

Could you possibly test if the   mmc-hs200-1_8v; property works for you?


Thanks a lot
Heiko
Sergey 'Jin' Bostandzhyan Aug. 14, 2024, 11:21 a.m. UTC | #4
Hi,

On Sat, Aug 10, 2024 at 09:11:56PM +0200, Heiko Stübner wrote:
> Am Montag, 5. August 2024, 10:59:35 CEST schrieb Sergey 'Jin' Bostandzhyan:
> > On Sun, Aug 04, 2024 at 01:27:50AM +0100, Daniel Golle wrote:
> > > On Thu, Aug 01, 2024 at 05:57:35PM +0000, Sergey Bostandzhyan wrote:
> > > > The R2S Plus is basically an R2S with additional eMMC.
> > > > 
> > > > The eMMC configuration for the DTS has been extracted and copied from
> > > > rk3328-nanopi-r2.dts, v2017.09 branch from the friendlyarm/uboot-rockchip
> > > > repository.
> > > > 
> > > > Signed-off-by: Sergey Bostandzhyan <jin@mediatomb.cc>
> > > > ---
> > > >  arch/arm64/boot/dts/rockchip/Makefile         |  1 +
> > > >  .../dts/rockchip/rk3328-nanopi-r2s-plus.dts   | 31 +++++++++++++++++++
> > > >  2 files changed, 32 insertions(+)
> > > >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > > > index fda1b980eb4b..36258dc8dafd 100644
> > > > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > > > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > > > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-evb.dtb
> > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c.dtb
> > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c-plus.dtb
> > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s.dtb
> > > > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s-plus.dtb
> > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus.dtb
> > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus-lts.dtb
> > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-rock64.dtb
> > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > > new file mode 100644
> > > > index 000000000000..7b83090a2145
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > > @@ -0,0 +1,31 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * (C) Copyright 2018 FriendlyElec Computer Tech. Co., Ltd.
> > > > + * (http://www.friendlyarm.com)
> > > > + *
> > > > + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
> > > > + */
> > > > +
> > > > +/dts-v1/;
> > > > +#include "rk3328-nanopi-r2s.dts"
> > > > +
> > > > +/ {
> > > > +	model = "FriendlyElec NanoPi R2S Plus";
> > > > +	compatible = "friendlyarm,nanopi-r2s-plus", "rockchip,rk3328";
> > > > +
> > > > +	aliases {
> > > > +		mmc1 = &emmc;
> > > > +	};
> > > > +};
> > > > +
> > > > +&emmc {
> > > > +	bus-width = <8>;
> > > > +	cap-mmc-highspeed;
> > > > +	supports-emmc;
> > > > +	disable-wp;
> > > > +	non-removable;
> > > > +	num-slots = <1>;
> > > > +	pinctrl-names = "default";
> > > > +	pinctrl-0 = <&emmc_clk &emmc_cmd &emmc_bus8>;
> > > 
> > > I think it's worth adding
> > > 
> > > 	mmc-hs200-1_8v;
> > > 
> > > 
> > > I've tried getting the best speed possible and while HS400 with and
> > > without enhanced strobe did NOT work, hs200 works just fine.
> > > [    0.459863] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> > > [    0.460884] mmc_host mmc1: Bus speed (slot 0) = 150000000Hz (slot req 150000000Hz, actual 150000000HZ div = 0)
> > > ...
> > > [    0.728220] dwmmc_rockchip ff520000.mmc: Successfully tuned phase to 194
> > > [    0.728940] mmc1: new HS200 MMC card at address 0001
> > > [    0.730774] mmcblk1: mmc1:0001 A3A551 28.9 GiB
> > > [    0.733262]  mmcblk1: p1 p2
> > > [    0.734562] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
> > > [    0.736818] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
> > > [    0.738503] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)
> > > 
> > > root@OpenWrt:/# hdparm -t /dev/mmcblk1
> > > 
> > > /dev/mmcblk1:
> > >  Timing buffered disk reads: 342 MB in  3.00 seconds = 113.81 MB/sec
> > > 
> > > 
> > > Without 'mmc-hs200-1_8v' property in DT the eMMC is detected as
> > > [    0.440465] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> > > [    0.442032] mmc1: new high speed MMC card at address 0001
> > > [    0.444261] mmcblk1: mmc1:0001 A3A551 28.9 GiB
> > > [    0.447388]  mmcblk1: p1 p2
> > > [    0.448744] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
> > > [    0.451065] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
> > > [    0.452871] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)
> > > 
> > > 
> > > root@OpenWrt:/# hdparm -t /dev/mmcblk1
> > > 
> > > /dev/mmcblk1:
> > >  Timing buffered disk reads: 134 MB in  3.03 seconds =  44.18 MB/sec
> > > 
> > > 
> > > > +	status = "okay";
> > > > +};
> > > 
> > > I'm right now trying to get SDIO RTL8822CS working, so far I'm out of luck,
> > > but it can be added later once we got it working.
> > 
> > would you be interested in taking over my attempted patches? Thing is,
> > that I am a userspace guy who only copy-pasted some entries from
> > FriendlyElec and things happened to work, but I really have no clue what I am
> > doing when it comes to hardware and DTS. I see that some changes were suggested, 
> > not only by you above, but also by others earlier and I have little
> > understanding of where I should be inserting what and how.
> > 
> > At this point I think it would make more sense if someone who actually
> > understands what they are doing would continue to tune the DTS :)
> > 
> > So it'd be great if either you or anyone else would be willing to take
> > over?
> 
> Though, a board devicetree is a nice way to get "your feet wet" in the
> kernel :-) and for a lot of people scratching ones own itches gets them
> started.

While this may very well be true, my main issue is not the DT syntax,
but the lack of understanding of the underlying hardware and also a lack of
enthusiasm to dive into the hardware topics - I prefer to stay in
userspace where the kernel provides a very nice abstraction to all those 
details ;)

> The devicetree is easy enough, also looks correct and you even got the
> binding change correct - and you're the person with the actual board :-) .
> 
> Could you possibly test if the   mmc-hs200-1_8v; property works for you?

It does, I get pretty much the same results as Daniel:

root@nanopi-r2s-plus:~# hdparm -t /dev/mmcblk1
/dev/mmcblk1:
 Timing buffered disk reads: 134 MB in  3.04 seconds =  44.13 MB/sec

With mmc-hs200-1_8v:

root@nanopi-r2s-plus:~# hdparm -t /dev/mmcblk1
 /dev/mmcblk1:
  Timing buffered disk reads: 340 MB in  3.01 seconds = 113.08 MB/sec

Should I add a commit on top with this change and submit a v3 patchset?

On Thu, Aug 01, 2024 at 11:22:27PM +0200, Heiko Stübner wrote:
> general remark, please don't send new versions as threaded replies to
> old
> versions. The normal case for git-send-email is to create a new thread
> and this continuing inside the old thread confues tooling.

In case you tell me to go ahead with a v3 set, should it be in this
thread or not? I understood RESEND's should be new, but updates should
stay in the thread, right?

Sorry, I actually did read the guides, but seems misunderstood what I should
be doing as I inserted the in-reply-to header in my last RESEND.

Kind regards,
Sergey
Diederik de Haas Aug. 14, 2024, 11:30 a.m. UTC | #5
On Wed Aug 14, 2024 at 1:21 PM CEST, Sergey 'Jin' Bostandzhyan wrote:
> In case you tell me to go ahead with a v3 set, should it be in this
> thread or not? I understood RESEND's should be new, but updates should
> stay in the thread, right?

No, a new series should be its own thread too.
Diederik de Haas Aug. 14, 2024, 11:34 a.m. UTC | #6
On Wed Aug 14, 2024 at 1:30 PM CEST, Diederik de Haas wrote:
> On Wed Aug 14, 2024 at 1:21 PM CEST, Sergey 'Jin' Bostandzhyan wrote:
> > In case you tell me to go ahead with a v3 set, should it be in this
> > thread or not? I understood RESEND's should be new, but updates should
> > stay in the thread, right?
>
> No, a new series should be its own thread too.

More correctly and hopefully more clearly:

No, a new patch version (series) should be its own thread too.
Heiko Stuebner Aug. 14, 2024, 11:36 a.m. UTC | #7
Am Mittwoch, 14. August 2024, 13:21:38 CEST schrieb Sergey 'Jin' Bostandzhyan:
> Hi,
> 
> On Sat, Aug 10, 2024 at 09:11:56PM +0200, Heiko Stübner wrote:
> > Am Montag, 5. August 2024, 10:59:35 CEST schrieb Sergey 'Jin' Bostandzhyan:
> > > On Sun, Aug 04, 2024 at 01:27:50AM +0100, Daniel Golle wrote:
> > > > On Thu, Aug 01, 2024 at 05:57:35PM +0000, Sergey Bostandzhyan wrote:
> > > > > The R2S Plus is basically an R2S with additional eMMC.
> > > > > 
> > > > > The eMMC configuration for the DTS has been extracted and copied from
> > > > > rk3328-nanopi-r2.dts, v2017.09 branch from the friendlyarm/uboot-rockchip
> > > > > repository.
> > > > > 
> > > > > Signed-off-by: Sergey Bostandzhyan <jin@mediatomb.cc>
> > > > > ---
> > > > >  arch/arm64/boot/dts/rockchip/Makefile         |  1 +
> > > > >  .../dts/rockchip/rk3328-nanopi-r2s-plus.dts   | 31 +++++++++++++++++++
> > > > >  2 files changed, 32 insertions(+)
> > > > >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > > > > index fda1b980eb4b..36258dc8dafd 100644
> > > > > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > > > > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > > > > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-evb.dtb
> > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c.dtb
> > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c-plus.dtb
> > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s.dtb
> > > > > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s-plus.dtb
> > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus.dtb
> > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus-lts.dtb
> > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-rock64.dtb
> > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > > > new file mode 100644
> > > > > index 000000000000..7b83090a2145
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > > > @@ -0,0 +1,31 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +/*
> > > > > + * (C) Copyright 2018 FriendlyElec Computer Tech. Co., Ltd.
> > > > > + * (http://www.friendlyarm.com)
> > > > > + *
> > > > > + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
> > > > > + */
> > > > > +
> > > > > +/dts-v1/;
> > > > > +#include "rk3328-nanopi-r2s.dts"
> > > > > +
> > > > > +/ {
> > > > > +	model = "FriendlyElec NanoPi R2S Plus";
> > > > > +	compatible = "friendlyarm,nanopi-r2s-plus", "rockchip,rk3328";
> > > > > +
> > > > > +	aliases {
> > > > > +		mmc1 = &emmc;
> > > > > +	};
> > > > > +};
> > > > > +
> > > > > +&emmc {
> > > > > +	bus-width = <8>;
> > > > > +	cap-mmc-highspeed;
> > > > > +	supports-emmc;
> > > > > +	disable-wp;
> > > > > +	non-removable;
> > > > > +	num-slots = <1>;
> > > > > +	pinctrl-names = "default";
> > > > > +	pinctrl-0 = <&emmc_clk &emmc_cmd &emmc_bus8>;
> > > > 
> > > > I think it's worth adding
> > > > 
> > > > 	mmc-hs200-1_8v;
> > > > 
> > > > 
> > > > I've tried getting the best speed possible and while HS400 with and
> > > > without enhanced strobe did NOT work, hs200 works just fine.
> > > > [    0.459863] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> > > > [    0.460884] mmc_host mmc1: Bus speed (slot 0) = 150000000Hz (slot req 150000000Hz, actual 150000000HZ div = 0)
> > > > ...
> > > > [    0.728220] dwmmc_rockchip ff520000.mmc: Successfully tuned phase to 194
> > > > [    0.728940] mmc1: new HS200 MMC card at address 0001
> > > > [    0.730774] mmcblk1: mmc1:0001 A3A551 28.9 GiB
> > > > [    0.733262]  mmcblk1: p1 p2
> > > > [    0.734562] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
> > > > [    0.736818] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
> > > > [    0.738503] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)
> > > > 
> > > > root@OpenWrt:/# hdparm -t /dev/mmcblk1
> > > > 
> > > > /dev/mmcblk1:
> > > >  Timing buffered disk reads: 342 MB in  3.00 seconds = 113.81 MB/sec
> > > > 
> > > > 
> > > > Without 'mmc-hs200-1_8v' property in DT the eMMC is detected as
> > > > [    0.440465] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> > > > [    0.442032] mmc1: new high speed MMC card at address 0001
> > > > [    0.444261] mmcblk1: mmc1:0001 A3A551 28.9 GiB
> > > > [    0.447388]  mmcblk1: p1 p2
> > > > [    0.448744] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
> > > > [    0.451065] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
> > > > [    0.452871] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)
> > > > 
> > > > 
> > > > root@OpenWrt:/# hdparm -t /dev/mmcblk1
> > > > 
> > > > /dev/mmcblk1:
> > > >  Timing buffered disk reads: 134 MB in  3.03 seconds =  44.18 MB/sec
> > > > 
> > > > 
> > > > > +	status = "okay";
> > > > > +};
> > > > 
> > > > I'm right now trying to get SDIO RTL8822CS working, so far I'm out of luck,
> > > > but it can be added later once we got it working.
> > > 
> > > would you be interested in taking over my attempted patches? Thing is,
> > > that I am a userspace guy who only copy-pasted some entries from
> > > FriendlyElec and things happened to work, but I really have no clue what I am
> > > doing when it comes to hardware and DTS. I see that some changes were suggested, 
> > > not only by you above, but also by others earlier and I have little
> > > understanding of where I should be inserting what and how.
> > > 
> > > At this point I think it would make more sense if someone who actually
> > > understands what they are doing would continue to tune the DTS :)
> > > 
> > > So it'd be great if either you or anyone else would be willing to take
> > > over?
> > 
> > Though, a board devicetree is a nice way to get "your feet wet" in the
> > kernel :-) and for a lot of people scratching ones own itches gets them
> > started.
> 
> While this may very well be true, my main issue is not the DT syntax,
> but the lack of understanding of the underlying hardware and also a lack of
> enthusiasm to dive into the hardware topics - I prefer to stay in
> userspace where the kernel provides a very nice abstraction to all those 
> details ;)

No worries :-) .

Though in this case you're "on the hook" for the board devicetree :-D .


> > The devicetree is easy enough, also looks correct and you even got the
> > binding change correct - and you're the person with the actual board :-) .
> > 
> > Could you possibly test if the   mmc-hs200-1_8v; property works for you?
> 
> It does, I get pretty much the same results as Daniel:
> 
> root@nanopi-r2s-plus:~# hdparm -t /dev/mmcblk1
> /dev/mmcblk1:
>  Timing buffered disk reads: 134 MB in  3.04 seconds =  44.13 MB/sec
> 
> With mmc-hs200-1_8v:
> 
> root@nanopi-r2s-plus:~# hdparm -t /dev/mmcblk1
>  /dev/mmcblk1:
>   Timing buffered disk reads: 340 MB in  3.01 seconds = 113.08 MB/sec
> 
> Should I add a commit on top with this change and submit a v3 patchset?
> 
> On Thu, Aug 01, 2024 at 11:22:27PM +0200, Heiko Stübner wrote:
> > general remark, please don't send new versions as threaded replies to
> > old
> > versions. The normal case for git-send-email is to create a new thread
> > and this continuing inside the old thread confues tooling.
> 
> In case you tell me to go ahead with a v3 set, should it be in this
> thread or not? I understood RESEND's should be new, but updates should
> stay in the thread, right?
> 
> Sorry, I actually did read the guides, but seems misunderstood what I should
> be doing as I inserted the in-reply-to header in my last RESEND.

Please do a v3 ... in a new thread.

Also for the process, please add the Ack you received for patch 2
in that v3.


Thanks
Heiko
Sergey 'Jin' Bostandzhyan Aug. 14, 2024, 12:24 p.m. UTC | #8
On Wed, Aug 14, 2024 at 01:36:43PM +0200, Heiko Stübner wrote:
> Am Mittwoch, 14. August 2024, 13:21:38 CEST schrieb Sergey 'Jin' Bostandzhyan:
> > Hi,
> > 
> > On Sat, Aug 10, 2024 at 09:11:56PM +0200, Heiko Stübner wrote:
> > > Am Montag, 5. August 2024, 10:59:35 CEST schrieb Sergey 'Jin' Bostandzhyan:
> > > > On Sun, Aug 04, 2024 at 01:27:50AM +0100, Daniel Golle wrote:
> > > > > On Thu, Aug 01, 2024 at 05:57:35PM +0000, Sergey Bostandzhyan wrote:
> > > > > > The R2S Plus is basically an R2S with additional eMMC.
> > > > > > 
> > > > > > The eMMC configuration for the DTS has been extracted and copied from
> > > > > > rk3328-nanopi-r2.dts, v2017.09 branch from the friendlyarm/uboot-rockchip
> > > > > > repository.
> > > > > > 
> > > > > > Signed-off-by: Sergey Bostandzhyan <jin@mediatomb.cc>
> > > > > > ---
> > > > > >  arch/arm64/boot/dts/rockchip/Makefile         |  1 +
> > > > > >  .../dts/rockchip/rk3328-nanopi-r2s-plus.dts   | 31 +++++++++++++++++++
> > > > > >  2 files changed, 32 insertions(+)
> > > > > >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > > > > 
> > > > > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > > > > > index fda1b980eb4b..36258dc8dafd 100644
> > > > > > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > > > > > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > > > > > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-evb.dtb
> > > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c.dtb
> > > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c-plus.dtb
> > > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s.dtb
> > > > > > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s-plus.dtb
> > > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus.dtb
> > > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus-lts.dtb
> > > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-rock64.dtb
> > > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > > > > new file mode 100644
> > > > > > index 000000000000..7b83090a2145
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > > > > @@ -0,0 +1,31 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > +/*
> > > > > > + * (C) Copyright 2018 FriendlyElec Computer Tech. Co., Ltd.
> > > > > > + * (http://www.friendlyarm.com)
> > > > > > + *
> > > > > > + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
> > > > > > + */
> > > > > > +
> > > > > > +/dts-v1/;
> > > > > > +#include "rk3328-nanopi-r2s.dts"
> > > > > > +
> > > > > > +/ {
> > > > > > +	model = "FriendlyElec NanoPi R2S Plus";
> > > > > > +	compatible = "friendlyarm,nanopi-r2s-plus", "rockchip,rk3328";
> > > > > > +
> > > > > > +	aliases {
> > > > > > +		mmc1 = &emmc;
> > > > > > +	};
> > > > > > +};
> > > > > > +
> > > > > > +&emmc {
> > > > > > +	bus-width = <8>;
> > > > > > +	cap-mmc-highspeed;
> > > > > > +	supports-emmc;
> > > > > > +	disable-wp;
> > > > > > +	non-removable;
> > > > > > +	num-slots = <1>;
> > > > > > +	pinctrl-names = "default";
> > > > > > +	pinctrl-0 = <&emmc_clk &emmc_cmd &emmc_bus8>;
> > > > > 
> > > > > I think it's worth adding
> > > > > 
> > > > > 	mmc-hs200-1_8v;
> > > > > 
> > > > > 
> > > > > I've tried getting the best speed possible and while HS400 with and
> > > > > without enhanced strobe did NOT work, hs200 works just fine.
> > > > > [    0.459863] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> > > > > [    0.460884] mmc_host mmc1: Bus speed (slot 0) = 150000000Hz (slot req 150000000Hz, actual 150000000HZ div = 0)
> > > > > ...
> > > > > [    0.728220] dwmmc_rockchip ff520000.mmc: Successfully tuned phase to 194
> > > > > [    0.728940] mmc1: new HS200 MMC card at address 0001
> > > > > [    0.730774] mmcblk1: mmc1:0001 A3A551 28.9 GiB
> > > > > [    0.733262]  mmcblk1: p1 p2
> > > > > [    0.734562] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
> > > > > [    0.736818] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
> > > > > [    0.738503] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)
> > > > > 
> > > > > root@OpenWrt:/# hdparm -t /dev/mmcblk1
> > > > > 
> > > > > /dev/mmcblk1:
> > > > >  Timing buffered disk reads: 342 MB in  3.00 seconds = 113.81 MB/sec
> > > > > 
> > > > > 
> > > > > Without 'mmc-hs200-1_8v' property in DT the eMMC is detected as
> > > > > [    0.440465] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> > > > > [    0.442032] mmc1: new high speed MMC card at address 0001
> > > > > [    0.444261] mmcblk1: mmc1:0001 A3A551 28.9 GiB
> > > > > [    0.447388]  mmcblk1: p1 p2
> > > > > [    0.448744] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
> > > > > [    0.451065] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
> > > > > [    0.452871] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)
> > > > > 
> > > > > 
> > > > > root@OpenWrt:/# hdparm -t /dev/mmcblk1
> > > > > 
> > > > > /dev/mmcblk1:
> > > > >  Timing buffered disk reads: 134 MB in  3.03 seconds =  44.18 MB/sec
> > > > > 
> > > > > 
> > > > > > +	status = "okay";
> > > > > > +};
> > > > > 
> > > > > I'm right now trying to get SDIO RTL8822CS working, so far I'm out of luck,
> > > > > but it can be added later once we got it working.
> > > > 
> > > > would you be interested in taking over my attempted patches? Thing is,
> > > > that I am a userspace guy who only copy-pasted some entries from
> > > > FriendlyElec and things happened to work, but I really have no clue what I am
> > > > doing when it comes to hardware and DTS. I see that some changes were suggested, 
> > > > not only by you above, but also by others earlier and I have little
> > > > understanding of where I should be inserting what and how.
> > > > 
> > > > At this point I think it would make more sense if someone who actually
> > > > understands what they are doing would continue to tune the DTS :)
> > > > 
> > > > So it'd be great if either you or anyone else would be willing to take
> > > > over?
> > > 
> > > Though, a board devicetree is a nice way to get "your feet wet" in the
> > > kernel :-) and for a lot of people scratching ones own itches gets them
> > > started.
> > 
> > While this may very well be true, my main issue is not the DT syntax,
> > but the lack of understanding of the underlying hardware and also a lack of
> > enthusiasm to dive into the hardware topics - I prefer to stay in
> > userspace where the kernel provides a very nice abstraction to all those 
> > details ;)
> 
> No worries :-) .
> 
> Though in this case you're "on the hook" for the board devicetree :-D .
> 
> 
> > > The devicetree is easy enough, also looks correct and you even got the
> > > binding change correct - and you're the person with the actual board :-) .
> > > 
> > > Could you possibly test if the   mmc-hs200-1_8v; property works for you?
> > 
> > It does, I get pretty much the same results as Daniel:
> > 
> > root@nanopi-r2s-plus:~# hdparm -t /dev/mmcblk1
> > /dev/mmcblk1:
> >  Timing buffered disk reads: 134 MB in  3.04 seconds =  44.13 MB/sec
> > 
> > With mmc-hs200-1_8v:
> > 
> > root@nanopi-r2s-plus:~# hdparm -t /dev/mmcblk1
> >  /dev/mmcblk1:
> >   Timing buffered disk reads: 340 MB in  3.01 seconds = 113.08 MB/sec
> > 
> > Should I add a commit on top with this change and submit a v3 patchset?
> > 
> > On Thu, Aug 01, 2024 at 11:22:27PM +0200, Heiko Stübner wrote:
> > > general remark, please don't send new versions as threaded replies to
> > > old
> > > versions. The normal case for git-send-email is to create a new thread
> > > and this continuing inside the old thread confues tooling.
> > 
> > In case you tell me to go ahead with a v3 set, should it be in this
> > thread or not? I understood RESEND's should be new, but updates should
> > stay in the thread, right?
> > 
> > Sorry, I actually did read the guides, but seems misunderstood what I should
> > be doing as I inserted the in-reply-to header in my last RESEND.
> 
> Please do a v3 ... in a new thread.

There was one other note though to which I did not receive a clear
repsonse. Bjoern A. Zeeb noticed, that the newer version from the
rockhip repo has // SPDX-License-Identifier: (GPL-2.0+ OR MIT) while the
one which I copied the code from did not have the "OR MIT" part, hence I
also did not have it in my patch.

Am I supposed to leave it as is, since I copied the block from the
sources which indeed were GP-2.0 only or should I add the "OR MIT" part
as it is apparently the case in newer versions of the dts file from
rockhcip?

> Also for the process, please add the Ack you received for patch 2
> in that v3.

You mean, ammend the appropriate commit and add the Acked-By to the
commit message? OK, will do.

On Wed, Aug 14, 2024 at 01:34:13PM +0200, Diederik de Haas wrote:
> On Wed Aug 14, 2024 at 1:30 PM CEST, Diederik de Haas wrote:
> > On Wed Aug 14, 2024 at 1:21 PM CEST, Sergey 'Jin' Bostandzhyan
> > wrote:
> > > In case you tell me to go ahead with a v3 set, should it be in
> > > this
> > > thread or not? I understood RESEND's should be new, but updates
> > > should
> > > stay in the thread, right?
> >
> > No, a new series should be its own thread too.
> 
> More correctly and hopefully more clearly:

Understood, thank you!

Kind regards,
Sergey
Heiko Stuebner Aug. 14, 2024, 1:53 p.m. UTC | #9
Am Mittwoch, 14. August 2024, 14:24:03 CEST schrieb Sergey 'Jin' Bostandzhyan:
> On Wed, Aug 14, 2024 at 01:36:43PM +0200, Heiko Stübner wrote:
> > Am Mittwoch, 14. August 2024, 13:21:38 CEST schrieb Sergey 'Jin' Bostandzhyan:
> > > Hi,
> > > 
> > > On Sat, Aug 10, 2024 at 09:11:56PM +0200, Heiko Stübner wrote:
> > > > Am Montag, 5. August 2024, 10:59:35 CEST schrieb Sergey 'Jin' Bostandzhyan:
> > > > > On Sun, Aug 04, 2024 at 01:27:50AM +0100, Daniel Golle wrote:
> > > > > > On Thu, Aug 01, 2024 at 05:57:35PM +0000, Sergey Bostandzhyan wrote:
> > > > > > > The R2S Plus is basically an R2S with additional eMMC.
> > > > > > > 
> > > > > > > The eMMC configuration for the DTS has been extracted and copied from
> > > > > > > rk3328-nanopi-r2.dts, v2017.09 branch from the friendlyarm/uboot-rockchip
> > > > > > > repository.
> > > > > > > 
> > > > > > > Signed-off-by: Sergey Bostandzhyan <jin@mediatomb.cc>
> > > > > > > ---
> > > > > > >  arch/arm64/boot/dts/rockchip/Makefile         |  1 +
> > > > > > >  .../dts/rockchip/rk3328-nanopi-r2s-plus.dts   | 31 +++++++++++++++++++
> > > > > > >  2 files changed, 32 insertions(+)
> > > > > > >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > > > > > 
> > > > > > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > > > > > > index fda1b980eb4b..36258dc8dafd 100644
> > > > > > > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > > > > > > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > > > > > > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-evb.dtb
> > > > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c.dtb
> > > > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c-plus.dtb
> > > > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s.dtb
> > > > > > > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s-plus.dtb
> > > > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus.dtb
> > > > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus-lts.dtb
> > > > > > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-rock64.dtb
> > > > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > > > > > new file mode 100644
> > > > > > > index 000000000000..7b83090a2145
> > > > > > > --- /dev/null
> > > > > > > +++ b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
> > > > > > > @@ -0,0 +1,31 @@
> > > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > > +/*
> > > > > > > + * (C) Copyright 2018 FriendlyElec Computer Tech. Co., Ltd.
> > > > > > > + * (http://www.friendlyarm.com)
> > > > > > > + *
> > > > > > > + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
> > > > > > > + */
> > > > > > > +
> > > > > > > +/dts-v1/;
> > > > > > > +#include "rk3328-nanopi-r2s.dts"
> > > > > > > +
> > > > > > > +/ {
> > > > > > > +	model = "FriendlyElec NanoPi R2S Plus";
> > > > > > > +	compatible = "friendlyarm,nanopi-r2s-plus", "rockchip,rk3328";
> > > > > > > +
> > > > > > > +	aliases {
> > > > > > > +		mmc1 = &emmc;
> > > > > > > +	};
> > > > > > > +};
> > > > > > > +
> > > > > > > +&emmc {
> > > > > > > +	bus-width = <8>;
> > > > > > > +	cap-mmc-highspeed;
> > > > > > > +	supports-emmc;
> > > > > > > +	disable-wp;
> > > > > > > +	non-removable;
> > > > > > > +	num-slots = <1>;
> > > > > > > +	pinctrl-names = "default";
> > > > > > > +	pinctrl-0 = <&emmc_clk &emmc_cmd &emmc_bus8>;
> > > > > > 
> > > > > > I think it's worth adding
> > > > > > 
> > > > > > 	mmc-hs200-1_8v;
> > > > > > 
> > > > > > 
> > > > > > I've tried getting the best speed possible and while HS400 with and
> > > > > > without enhanced strobe did NOT work, hs200 works just fine.
> > > > > > [    0.459863] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> > > > > > [    0.460884] mmc_host mmc1: Bus speed (slot 0) = 150000000Hz (slot req 150000000Hz, actual 150000000HZ div = 0)
> > > > > > ...
> > > > > > [    0.728220] dwmmc_rockchip ff520000.mmc: Successfully tuned phase to 194
> > > > > > [    0.728940] mmc1: new HS200 MMC card at address 0001
> > > > > > [    0.730774] mmcblk1: mmc1:0001 A3A551 28.9 GiB
> > > > > > [    0.733262]  mmcblk1: p1 p2
> > > > > > [    0.734562] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
> > > > > > [    0.736818] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
> > > > > > [    0.738503] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)
> > > > > > 
> > > > > > root@OpenWrt:/# hdparm -t /dev/mmcblk1
> > > > > > 
> > > > > > /dev/mmcblk1:
> > > > > >  Timing buffered disk reads: 342 MB in  3.00 seconds = 113.81 MB/sec
> > > > > > 
> > > > > > 
> > > > > > Without 'mmc-hs200-1_8v' property in DT the eMMC is detected as
> > > > > > [    0.440465] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> > > > > > [    0.442032] mmc1: new high speed MMC card at address 0001
> > > > > > [    0.444261] mmcblk1: mmc1:0001 A3A551 28.9 GiB
> > > > > > [    0.447388]  mmcblk1: p1 p2
> > > > > > [    0.448744] mmcblk1boot0: mmc1:0001 A3A551 4.00 MiB
> > > > > > [    0.451065] mmcblk1boot1: mmc1:0001 A3A551 4.00 MiB
> > > > > > [    0.452871] mmcblk1rpmb: mmc1:0001 A3A551 16.0 MiB, chardev (245:0)
> > > > > > 
> > > > > > 
> > > > > > root@OpenWrt:/# hdparm -t /dev/mmcblk1
> > > > > > 
> > > > > > /dev/mmcblk1:
> > > > > >  Timing buffered disk reads: 134 MB in  3.03 seconds =  44.18 MB/sec
> > > > > > 
> > > > > > 
> > > > > > > +	status = "okay";
> > > > > > > +};
> > > > > > 
> > > > > > I'm right now trying to get SDIO RTL8822CS working, so far I'm out of luck,
> > > > > > but it can be added later once we got it working.
> > > > > 
> > > > > would you be interested in taking over my attempted patches? Thing is,
> > > > > that I am a userspace guy who only copy-pasted some entries from
> > > > > FriendlyElec and things happened to work, but I really have no clue what I am
> > > > > doing when it comes to hardware and DTS. I see that some changes were suggested, 
> > > > > not only by you above, but also by others earlier and I have little
> > > > > understanding of where I should be inserting what and how.
> > > > > 
> > > > > At this point I think it would make more sense if someone who actually
> > > > > understands what they are doing would continue to tune the DTS :)
> > > > > 
> > > > > So it'd be great if either you or anyone else would be willing to take
> > > > > over?
> > > > 
> > > > Though, a board devicetree is a nice way to get "your feet wet" in the
> > > > kernel :-) and for a lot of people scratching ones own itches gets them
> > > > started.
> > > 
> > > While this may very well be true, my main issue is not the DT syntax,
> > > but the lack of understanding of the underlying hardware and also a lack of
> > > enthusiasm to dive into the hardware topics - I prefer to stay in
> > > userspace where the kernel provides a very nice abstraction to all those 
> > > details ;)
> > 
> > No worries :-) .
> > 
> > Though in this case you're "on the hook" for the board devicetree :-D .
> > 
> > 
> > > > The devicetree is easy enough, also looks correct and you even got the
> > > > binding change correct - and you're the person with the actual board :-) .
> > > > 
> > > > Could you possibly test if the   mmc-hs200-1_8v; property works for you?
> > > 
> > > It does, I get pretty much the same results as Daniel:
> > > 
> > > root@nanopi-r2s-plus:~# hdparm -t /dev/mmcblk1
> > > /dev/mmcblk1:
> > >  Timing buffered disk reads: 134 MB in  3.04 seconds =  44.13 MB/sec
> > > 
> > > With mmc-hs200-1_8v:
> > > 
> > > root@nanopi-r2s-plus:~# hdparm -t /dev/mmcblk1
> > >  /dev/mmcblk1:
> > >   Timing buffered disk reads: 340 MB in  3.01 seconds = 113.08 MB/sec
> > > 
> > > Should I add a commit on top with this change and submit a v3 patchset?
> > > 
> > > On Thu, Aug 01, 2024 at 11:22:27PM +0200, Heiko Stübner wrote:
> > > > general remark, please don't send new versions as threaded replies to
> > > > old
> > > > versions. The normal case for git-send-email is to create a new thread
> > > > and this continuing inside the old thread confues tooling.
> > > 
> > > In case you tell me to go ahead with a v3 set, should it be in this
> > > thread or not? I understood RESEND's should be new, but updates should
> > > stay in the thread, right?
> > > 
> > > Sorry, I actually did read the guides, but seems misunderstood what I should
> > > be doing as I inserted the in-reply-to header in my last RESEND.
> > 
> > Please do a v3 ... in a new thread.
> 
> There was one other note though to which I did not receive a clear
> repsonse. Bjoern A. Zeeb noticed, that the newer version from the
> rockhip repo has // SPDX-License-Identifier: (GPL-2.0+ OR MIT) while the
> one which I copied the code from did not have the "OR MIT" part, hence I
> also did not have it in my patch.
> 
> Am I supposed to leave it as is, since I copied the block from the
> sources which indeed were GP-2.0 only or should I add the "OR MIT" part
> as it is apparently the case in newer versions of the dts file from
> rockhcip?

the code you based your dts on changed licenses, so I guess you're also
allowed to change. You could very well also just have "copied it again"
from those new sources under GPL+MIT ;-)

And yep, dual licensing is preferred.

> > Also for the process, please add the Ack you received for patch 2
> > in that v3.
> 
> You mean, ammend the appropriate commit and add the Acked-By to the
> commit message? OK, will do.

correct. That is the expecting thing to do. DT maintainers see so many
patches that they won't keep track of "oh I have seen that already", so
would in the worst case, re-review the binding patch. By adding the Ack
they can just see (and probably tooling can simply filter out) those that
are already done.


> On Wed, Aug 14, 2024 at 01:34:13PM +0200, Diederik de Haas wrote:
> > On Wed Aug 14, 2024 at 1:30 PM CEST, Diederik de Haas wrote:
> > > On Wed Aug 14, 2024 at 1:21 PM CEST, Sergey 'Jin' Bostandzhyan
> > > wrote:
> > > > In case you tell me to go ahead with a v3 set, should it be in
> > > > this
> > > > thread or not? I understood RESEND's should be new, but updates
> > > > should
> > > > stay in the thread, right?
> > >
> > > No, a new series should be its own thread too.
> > 
> > More correctly and hopefully more clearly:
> 
> Understood, thank you!
> 
> Kind regards,
> Sergey
> 
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index fda1b980eb4b..36258dc8dafd 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -20,6 +20,7 @@  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-evb.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2c-plus.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s-plus.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-orangepi-r1-plus-lts.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-rock64.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
new file mode 100644
index 000000000000..7b83090a2145
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s-plus.dts
@@ -0,0 +1,31 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2018 FriendlyElec Computer Tech. Co., Ltd.
+ * (http://www.friendlyarm.com)
+ *
+ * (C) Copyright 2016 Rockchip Electronics Co., Ltd
+ */
+
+/dts-v1/;
+#include "rk3328-nanopi-r2s.dts"
+
+/ {
+	model = "FriendlyElec NanoPi R2S Plus";
+	compatible = "friendlyarm,nanopi-r2s-plus", "rockchip,rk3328";
+
+	aliases {
+		mmc1 = &emmc;
+	};
+};
+
+&emmc {
+	bus-width = <8>;
+	cap-mmc-highspeed;
+	supports-emmc;
+	disable-wp;
+	non-removable;
+	num-slots = <1>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&emmc_clk &emmc_cmd &emmc_bus8>;
+	status = "okay";
+};