Re: [PATCH v3 3/3] arm64: dts: rockchip: k3566-quartz64-a: adds sata variant

[Peter Geis]

On Sat, Sep 17, 2022 at 2:42 AM Heiko Stuebner <heiko@xxxxxxxxx> wrote:

Good Morning Heiko,


>
> Hi Peter,
>
> Am Samstag, 17. September 2022, 03:40:07 CEST schrieb Peter Geis:
> > On Fri, Sep 16, 2022 at 12:06 PM Alessandro Carminati
> > <alessandro.carminati@xxxxxxxxx> wrote:
> > >
> > > The Quartz64 board is built upon Rockchip RK3566.
> > > Rockchip RK3566 has two combo phys.
> > > The first connects USB3 and SATA ctrl1, and the second PCIe lane and SATA
> > > ctrl2.
> > > The second combo phy is hardwired to the PCIe slot, where for the first,
> > > the hardware on the board provides both the USB3 connector and the SATA
> > > connector.
> > > This DT allows the users to switch the combo phy to the SATA connector.
> >
> > Good Evening,
> >
> > NACK to this whole series. Neither works correctly in the hardware as
> > is,
>
> Just for my understanding for the future, sata not working is that a bug
> in the soc or the board?

This is a board level problem. Attempting to build a device that had
both ports electrically connected without a switch chip created a
device where neither worked correctly. The SATA controllers themselves
are amazing. I've used both nvme and sata m2 drives on the model b for
example.

>
> > and USB3 was decided to be left enabled as the SATA port will be
> > removed completely in the next revision.
>
> That is good to know. Thanks for the heads up :-)

In regards to this sort of stuff in the future, we're working on
fragment overlay support in U-Boot to work around the kernel's lack of
support. If I remember correctly EDK2 will be implementing the switch
in firmware as well. Devices that support both (at least ones I
maintain) will have both in the dts, with the less likely use case
left disabled. End users can simply switch which one is enabled if
they want.

Very Respectfully,
Peter

>
> Heiko
>
>
> > > Signed-off-by: Alessandro Carminati <alessandro.carminati@xxxxxxxxx>
> > > ---
> > >  arch/arm64/boot/dts/rockchip/Makefile                   | 1 +
> > >  arch/arm64/boot/dts/rockchip/rk3566-quartz64-a-sata.dts | 9 +++++++++
> > >  2 files changed, 10 insertions(+)
> > >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-quartz64-a-sata.dts
> > >
> > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > > index 8c843f6fc3cc..1d5dd91d1a34 100644
> > > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > > @@ -60,6 +60,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399pro-rock-pi-n10.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.1.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a-usb3.dts
> > > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a-sata.dts
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-b.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-roc-pc.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-soquartz-cm4.dtb
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a-sata.dts b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a-sata.dts
> > > new file mode 100644
> > > index 000000000000..8620df7ec01e
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a-sata.dts
> > > @@ -0,0 +1,9 @@
> > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > +
> > > +/dts-v1/;
> > > +
> > > +#include "rk3566-quartz64-a.dtsi"
> > > +
> > > +&sata1 {
> > > +       status = "okay";
> > > +};
> > > --
> > > 2.34.1
> > >
> > >
> > > _______________________________________________
> > > Linux-rockchip mailing list
> > > Linux-rockchip@xxxxxxxxxxxxxxxxxxx
> > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
> >
>
>
>
>