2022-04-10 02:38:38

by Peter Geis

[permalink] [raw]
Subject: Re: Aw: Re: Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb

On Sat, Apr 9, 2022 at 7:01 AM Heiko Stuebner <[email protected]> wrote:
>
> Am Samstag, 9. April 2022, 12:57:39 CEST schrieb Frank Wunderlich:
> > Hi
> > > Gesendet: Samstag, 09. April 2022 um 12:40 Uhr
> > > Von: "Dan Johansen" <[email protected]>
> >
> > > So the issue is only with usb 3 ports, not usb 2 ports?
> >
> > my board has no standalone usb2-ports. usb2 is integrated into the usb3 ports (dual phy). here both were not working.
> >
> > afaik rk3566 has standalone usb2 ports that may not be broken, but i have no such board for testing.

Good Morning,

>
> As far as I understand the issue now after checking the code, this
> patch actually fixes the usb3 series from Peter, right?
>
> I.e. the usb-nodes that are fixed in this patch are not yet present
> in the main rk356x dtsi and only get added in
> "arm64: dts: rockchip: add rk356x dwc3 usb3 nodes" [0]
>
> As we don't want to add broken changes, this fix should squashed
> into a next version of the patch adding the nodes.

Thank you for reporting this, I will squash this fix in and add your signed-off.

However the offending patch is in fact the clock separation patch, and
it breaks backwards compatibility with the rk3328 dtsi which is why my
series also is broken.

The rockchip,dwc3.yaml needs to be fixed to align with the
snps,dwc3.yaml, and both the rk3328 and rk3399 clock names updated.
Also the offending clock separation patch needs a fix to grab the old
clock names for rk3328 backwards compatibility to be retained.

This might also be a good time to look into moving rk3399 to the core
dwc3 driver?

This is a delightful mess.

>
>
> Heiko
>
> [0] https://lore.kernel.org/r/[email protected]
>
>

Very Respectfully,
Peter Geis


2022-04-11 14:17:23

by Peter Geis

[permalink] [raw]
Subject: Re: Aw: Re: Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb

On Sat, Apr 9, 2022 at 7:14 AM Peter Geis <[email protected]> wrote:
>
> On Sat, Apr 9, 2022 at 7:01 AM Heiko Stuebner <[email protected]> wrote:
> >
> > Am Samstag, 9. April 2022, 12:57:39 CEST schrieb Frank Wunderlich:
> > > Hi
> > > > Gesendet: Samstag, 09. April 2022 um 12:40 Uhr
> > > > Von: "Dan Johansen" <[email protected]>
> > >
> > > > So the issue is only with usb 3 ports, not usb 2 ports?
> > >
> > > my board has no standalone usb2-ports. usb2 is integrated into the usb3 ports (dual phy). here both were not working.
> > >
> > > afaik rk3566 has standalone usb2 ports that may not be broken, but i have no such board for testing.
>
> Good Morning,
>
> >
> > As far as I understand the issue now after checking the code, this
> > patch actually fixes the usb3 series from Peter, right?
> >
> > I.e. the usb-nodes that are fixed in this patch are not yet present
> > in the main rk356x dtsi and only get added in
> > "arm64: dts: rockchip: add rk356x dwc3 usb3 nodes" [0]
> >
> > As we don't want to add broken changes, this fix should squashed
> > into a next version of the patch adding the nodes.
>
> Thank you for reporting this, I will squash this fix in and add your signed-off.
>
> However the offending patch is in fact the clock separation patch, and
> it breaks backwards compatibility with the rk3328 dtsi which is why my
> series also is broken.
>
> The rockchip,dwc3.yaml needs to be fixed to align with the
> snps,dwc3.yaml, and both the rk3328 and rk3399 clock names updated.
> Also the offending clock separation patch needs a fix to grab the old
> clock names for rk3328 backwards compatibility to be retained.
>
> This might also be a good time to look into moving rk3399 to the core
> dwc3 driver?
>
> This is a delightful mess.

In the idea of getting this series to land, if all parties agree, I'll
submit a patch that fixes the clock separation patch with this series
and leave the naming as is for now.
The renaming of clocks and alignment of everything can be addressed in
a future series once discussion on how best to handle it has happened.

Do you concur with this?

>
> >
> >
> > Heiko
> >
> > [0] https://lore.kernel.org/r/[email protected]
> >
> >
>
> Very Respectfully,
> Peter Geis

2022-04-12 07:57:46

by Heiko Stuebner

[permalink] [raw]
Subject: Re: Aw: Re: Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb

Am Samstag, 9. April 2022, 13:30:44 CEST schrieb Peter Geis:
> On Sat, Apr 9, 2022 at 7:14 AM Peter Geis <[email protected]> wrote:
> >
> > On Sat, Apr 9, 2022 at 7:01 AM Heiko Stuebner <[email protected]> wrote:
> > >
> > > Am Samstag, 9. April 2022, 12:57:39 CEST schrieb Frank Wunderlich:
> > > > Hi
> > > > > Gesendet: Samstag, 09. April 2022 um 12:40 Uhr
> > > > > Von: "Dan Johansen" <[email protected]>
> > > >
> > > > > So the issue is only with usb 3 ports, not usb 2 ports?
> > > >
> > > > my board has no standalone usb2-ports. usb2 is integrated into the usb3 ports (dual phy). here both were not working.
> > > >
> > > > afaik rk3566 has standalone usb2 ports that may not be broken, but i have no such board for testing.
> >
> > Good Morning,
> >
> > >
> > > As far as I understand the issue now after checking the code, this
> > > patch actually fixes the usb3 series from Peter, right?
> > >
> > > I.e. the usb-nodes that are fixed in this patch are not yet present
> > > in the main rk356x dtsi and only get added in
> > > "arm64: dts: rockchip: add rk356x dwc3 usb3 nodes" [0]
> > >
> > > As we don't want to add broken changes, this fix should squashed
> > > into a next version of the patch adding the nodes.
> >
> > Thank you for reporting this, I will squash this fix in and add your signed-off.
> >
> > However the offending patch is in fact the clock separation patch, and
> > it breaks backwards compatibility with the rk3328 dtsi which is why my
> > series also is broken.
> >
> > The rockchip,dwc3.yaml needs to be fixed to align with the
> > snps,dwc3.yaml, and both the rk3328 and rk3399 clock names updated.
> > Also the offending clock separation patch needs a fix to grab the old
> > clock names for rk3328 backwards compatibility to be retained.
> >
> > This might also be a good time to look into moving rk3399 to the core
> > dwc3 driver?
> >
> > This is a delightful mess.
>
> In the idea of getting this series to land, if all parties agree, I'll
> submit a patch that fixes the clock separation patch with this series
> and leave the naming as is for now.
> The renaming of clocks and alignment of everything can be addressed in
> a future series once discussion on how best to handle it has happened.
>
> Do you concur with this?

I'm not sure about that ... i.e. adding known-broken changes
(for the rk356x) feels somewhat wrong to me.

Heiko