Message ID | 20230307193748.25712-2-anarsoul@gmail.com |
---|---|
State | Superseded |
Delegated to: | Kever Yang |
Headers | show |
Series | [1/2] phy: rockchip-inno-usb2: add support for phy-supply | expand |
> From: Vasily Khoruzhick <anarsoul@gmail.com> > Date: Tue, 7 Mar 2023 11:37:48 -0800 > > combphy0 is failing to probe due to unhandled assigned-clocks and > assigned-clocks-rates. That is probably the wrong approach. It should be possible to boot an OS with the device tree provided by U-Boot. Removing these properties means the OS doesn't see them either. But if the assigned-clocks property isn't needed it wouldn't be there would it? > commit 5bec4b0de785 ("arm64: dts: rk356x-u-boot: Drop combphy1 assigned-clocks/rates") > dropped these properties for combphy1, so let's drop them for combphy0 as well. > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > --- > arch/arm/dts/rk3568-evb-u-boot.dtsi | 1 + > arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 1 + > arch/arm/dts/rk3568-u-boot.dtsi | 11 +++++++++++ > 3 files changed, 13 insertions(+) > create mode 100644 arch/arm/dts/rk3568-u-boot.dtsi > > diff --git a/arch/arm/dts/rk3568-evb-u-boot.dtsi b/arch/arm/dts/rk3568-evb-u-boot.dtsi > index 17503d3d27..77430da3ba 100644 > --- a/arch/arm/dts/rk3568-evb-u-boot.dtsi > +++ b/arch/arm/dts/rk3568-evb-u-boot.dtsi > @@ -4,6 +4,7 @@ > */ > > #include "rk356x-u-boot.dtsi" > +#include "rk3568-u-boot.dtsi" > > / { > chosen { > diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi > index ed47efa44b..44cf33ed4b 100644 > --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi > +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi > @@ -5,6 +5,7 @@ > */ > > #include "rk356x-u-boot.dtsi" > +#include "rk3568-u-boot.dtsi" > > / { > chosen { > diff --git a/arch/arm/dts/rk3568-u-boot.dtsi b/arch/arm/dts/rk3568-u-boot.dtsi > new file mode 100644 > index 0000000000..c6776666df > --- /dev/null > +++ b/arch/arm/dts/rk3568-u-boot.dtsi > @@ -0,0 +1,11 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (c) 2023 Vasily Khoruzhick <anarsoul@gmail.com> > + */ > + > +#include "rockchip-u-boot.dtsi" > + > +&combphy0 { > + /delete-property/ assigned-clocks; > + /delete-property/ assigned-clock-rates; > +}; > -- > 2.39.2 > >
On Tue, Mar 7, 2023 at 11:53 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > That is probably the wrong approach. It should be possible to boot an > OS with the device tree provided by U-Boot. Removing these properties > means the OS doesn't see them either. But if the assigned-clocks > property isn't needed it wouldn't be there would it? Well, should we revert 5bec4b0de785 in this case? > > commit 5bec4b0de785 ("arm64: dts: rk356x-u-boot: Drop combphy1 assigned-clocks/rates") > > dropped these properties for combphy1, so let's drop them for combphy0 as well. > > > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > --- > > arch/arm/dts/rk3568-evb-u-boot.dtsi | 1 + > > arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 1 + > > arch/arm/dts/rk3568-u-boot.dtsi | 11 +++++++++++ > > 3 files changed, 13 insertions(+) > > create mode 100644 arch/arm/dts/rk3568-u-boot.dtsi > > > > diff --git a/arch/arm/dts/rk3568-evb-u-boot.dtsi b/arch/arm/dts/rk3568-evb-u-boot.dtsi > > index 17503d3d27..77430da3ba 100644 > > --- a/arch/arm/dts/rk3568-evb-u-boot.dtsi > > +++ b/arch/arm/dts/rk3568-evb-u-boot.dtsi > > @@ -4,6 +4,7 @@ > > */ > > > > #include "rk356x-u-boot.dtsi" > > +#include "rk3568-u-boot.dtsi" > > > > / { > > chosen { > > diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi > > index ed47efa44b..44cf33ed4b 100644 > > --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi > > +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi > > @@ -5,6 +5,7 @@ > > */ > > > > #include "rk356x-u-boot.dtsi" > > +#include "rk3568-u-boot.dtsi" > > > > / { > > chosen { > > diff --git a/arch/arm/dts/rk3568-u-boot.dtsi b/arch/arm/dts/rk3568-u-boot.dtsi > > new file mode 100644 > > index 0000000000..c6776666df > > --- /dev/null > > +++ b/arch/arm/dts/rk3568-u-boot.dtsi > > @@ -0,0 +1,11 @@ > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > +/* > > + * Copyright (c) 2023 Vasily Khoruzhick <anarsoul@gmail.com> > > + */ > > + > > +#include "rockchip-u-boot.dtsi" > > + > > +&combphy0 { > > + /delete-property/ assigned-clocks; > > + /delete-property/ assigned-clock-rates; > > +}; > > -- > > 2.39.2 > > > >
> From: Vasily Khoruzhick <anarsoul@gmail.com> > Date: Tue, 7 Mar 2023 12:34:48 -0800 > > On Tue, Mar 7, 2023 at 11:53 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > That is probably the wrong approach. It should be possible to boot an > > OS with the device tree provided by U-Boot. Removing these properties > > means the OS doesn't see them either. But if the assigned-clocks > > property isn't needed it wouldn't be there would it? > > Well, should we revert 5bec4b0de785 in this case? And implement support for the CLK_PCIEPHYn_REF clocks in drivers/clk/rockchip/clk_rk3568.c:rk3568_pmuclk_set_rate()? Yes, I'd say so. If U-Boot doesn't actually need these clocks to run at the frequency provided by assigned-clock-rates, that could be as simple as returning 0 for these clocks. > > > commit 5bec4b0de785 ("arm64: dts: rk356x-u-boot: Drop combphy1 assigned-clocks/rates") > > > dropped these properties for combphy1, so let's drop them for combphy0 as well. > > > > > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > > --- > > > arch/arm/dts/rk3568-evb-u-boot.dtsi | 1 + > > > arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 1 + > > > arch/arm/dts/rk3568-u-boot.dtsi | 11 +++++++++++ > > > 3 files changed, 13 insertions(+) > > > create mode 100644 arch/arm/dts/rk3568-u-boot.dtsi > > > > > > diff --git a/arch/arm/dts/rk3568-evb-u-boot.dtsi b/arch/arm/dts/rk3568-evb-u-boot.dtsi > > > index 17503d3d27..77430da3ba 100644 > > > --- a/arch/arm/dts/rk3568-evb-u-boot.dtsi > > > +++ b/arch/arm/dts/rk3568-evb-u-boot.dtsi > > > @@ -4,6 +4,7 @@ > > > */ > > > > > > #include "rk356x-u-boot.dtsi" > > > +#include "rk3568-u-boot.dtsi" > > > > > > / { > > > chosen { > > > diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi > > > index ed47efa44b..44cf33ed4b 100644 > > > --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi > > > +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi > > > @@ -5,6 +5,7 @@ > > > */ > > > > > > #include "rk356x-u-boot.dtsi" > > > +#include "rk3568-u-boot.dtsi" > > > > > > / { > > > chosen { > > > diff --git a/arch/arm/dts/rk3568-u-boot.dtsi b/arch/arm/dts/rk3568-u-boot.dtsi > > > new file mode 100644 > > > index 0000000000..c6776666df > > > --- /dev/null > > > +++ b/arch/arm/dts/rk3568-u-boot.dtsi > > > @@ -0,0 +1,11 @@ > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > +/* > > > + * Copyright (c) 2023 Vasily Khoruzhick <anarsoul@gmail.com> > > > + */ > > > + > > > +#include "rockchip-u-boot.dtsi" > > > + > > > +&combphy0 { > > > + /delete-property/ assigned-clocks; > > > + /delete-property/ assigned-clock-rates; > > > +}; > > > -- > > > 2.39.2 > > > > > > >
On Tue, Mar 7, 2023 at 1:04 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > And implement support for the CLK_PCIEPHYn_REF clocks in > drivers/clk/rockchip/clk_rk3568.c:rk3568_pmuclk_set_rate()? > > Yes, I'd say so. > > If U-Boot doesn't actually need these clocks to run at the frequency > provided by assigned-clock-rates, that could be as simple as returning > 0 for these clocks. Sounds good. I'll send v2 that reverts 5bec4b0de785 and implements stubs for CLK_PCIEPHYn_REF clocks. FWIW, I tried booting linux-6.2.2 with u-boot dtb and these clocks dropped, and USB works just fine. > > > > commit 5bec4b0de785 ("arm64: dts: rk356x-u-boot: Drop combphy1 assigned-clocks/rates") > > > > dropped these properties for combphy1, so let's drop them for combphy0 as well. > > > > > > > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > > > --- > > > > arch/arm/dts/rk3568-evb-u-boot.dtsi | 1 + > > > > arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 1 + > > > > arch/arm/dts/rk3568-u-boot.dtsi | 11 +++++++++++ > > > > 3 files changed, 13 insertions(+) > > > > create mode 100644 arch/arm/dts/rk3568-u-boot.dtsi > > > > > > > > diff --git a/arch/arm/dts/rk3568-evb-u-boot.dtsi b/arch/arm/dts/rk3568-evb-u-boot.dtsi > > > > index 17503d3d27..77430da3ba 100644 > > > > --- a/arch/arm/dts/rk3568-evb-u-boot.dtsi > > > > +++ b/arch/arm/dts/rk3568-evb-u-boot.dtsi > > > > @@ -4,6 +4,7 @@ > > > > */ > > > > > > > > #include "rk356x-u-boot.dtsi" > > > > +#include "rk3568-u-boot.dtsi" > > > > > > > > / { > > > > chosen { > > > > diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi > > > > index ed47efa44b..44cf33ed4b 100644 > > > > --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi > > > > +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi > > > > @@ -5,6 +5,7 @@ > > > > */ > > > > > > > > #include "rk356x-u-boot.dtsi" > > > > +#include "rk3568-u-boot.dtsi" > > > > > > > > / { > > > > chosen { > > > > diff --git a/arch/arm/dts/rk3568-u-boot.dtsi b/arch/arm/dts/rk3568-u-boot.dtsi > > > > new file mode 100644 > > > > index 0000000000..c6776666df > > > > --- /dev/null > > > > +++ b/arch/arm/dts/rk3568-u-boot.dtsi > > > > @@ -0,0 +1,11 @@ > > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > +/* > > > > + * Copyright (c) 2023 Vasily Khoruzhick <anarsoul@gmail.com> > > > > + */ > > > > + > > > > +#include "rockchip-u-boot.dtsi" > > > > + > > > > +&combphy0 { > > > > + /delete-property/ assigned-clocks; > > > > + /delete-property/ assigned-clock-rates; > > > > +}; > > > > -- > > > > 2.39.2 > > > > > > > > > >
Hi Vasily, On 2023/3/8 05:34, Vasily Khoruzhick wrote: > On Tue, Mar 7, 2023 at 1:04 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > >> And implement support for the CLK_PCIEPHYn_REF clocks in >> drivers/clk/rockchip/clk_rk3568.c:rk3568_pmuclk_set_rate()? >> >> Yes, I'd say so. >> >> If U-Boot doesn't actually need these clocks to run at the frequency >> provided by assigned-clock-rates, that could be as simple as returning >> 0 for these clocks. > Sounds good. I'll send v2 that reverts 5bec4b0de785 and implements > stubs for CLK_PCIEPHYn_REF clocks. > > FWIW, I tried booting linux-6.2.2 with u-boot dtb and these clocks > dropped, and USB works just fine. The assigned-clock is a software set default value, the hardware can works fine because these clocks have a correct default hardware value. So for the PHY, it works in U-Boot without assigned clock in most case. Returing 0 for those clocks is also OK. Thanks, - Kever > >>>>> commit 5bec4b0de785 ("arm64: dts: rk356x-u-boot: Drop combphy1 assigned-clocks/rates") >>>>> dropped these properties for combphy1, so let's drop them for combphy0 as well. >>>>> >>>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> >>>>> --- >>>>> arch/arm/dts/rk3568-evb-u-boot.dtsi | 1 + >>>>> arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 1 + >>>>> arch/arm/dts/rk3568-u-boot.dtsi | 11 +++++++++++ >>>>> 3 files changed, 13 insertions(+) >>>>> create mode 100644 arch/arm/dts/rk3568-u-boot.dtsi >>>>> >>>>> diff --git a/arch/arm/dts/rk3568-evb-u-boot.dtsi b/arch/arm/dts/rk3568-evb-u-boot.dtsi >>>>> index 17503d3d27..77430da3ba 100644 >>>>> --- a/arch/arm/dts/rk3568-evb-u-boot.dtsi >>>>> +++ b/arch/arm/dts/rk3568-evb-u-boot.dtsi >>>>> @@ -4,6 +4,7 @@ >>>>> */ >>>>> >>>>> #include "rk356x-u-boot.dtsi" >>>>> +#include "rk3568-u-boot.dtsi" >>>>> >>>>> / { >>>>> chosen { >>>>> diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi >>>>> index ed47efa44b..44cf33ed4b 100644 >>>>> --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi >>>>> +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi >>>>> @@ -5,6 +5,7 @@ >>>>> */ >>>>> >>>>> #include "rk356x-u-boot.dtsi" >>>>> +#include "rk3568-u-boot.dtsi" >>>>> >>>>> / { >>>>> chosen { >>>>> diff --git a/arch/arm/dts/rk3568-u-boot.dtsi b/arch/arm/dts/rk3568-u-boot.dtsi >>>>> new file mode 100644 >>>>> index 0000000000..c6776666df >>>>> --- /dev/null >>>>> +++ b/arch/arm/dts/rk3568-u-boot.dtsi >>>>> @@ -0,0 +1,11 @@ >>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) >>>>> +/* >>>>> + * Copyright (c) 2023 Vasily Khoruzhick <anarsoul@gmail.com> >>>>> + */ >>>>> + >>>>> +#include "rockchip-u-boot.dtsi" >>>>> + >>>>> +&combphy0 { >>>>> + /delete-property/ assigned-clocks; >>>>> + /delete-property/ assigned-clock-rates; >>>>> +}; >>>>> -- >>>>> 2.39.2 >>>>> >>>>>
diff --git a/arch/arm/dts/rk3568-evb-u-boot.dtsi b/arch/arm/dts/rk3568-evb-u-boot.dtsi index 17503d3d27..77430da3ba 100644 --- a/arch/arm/dts/rk3568-evb-u-boot.dtsi +++ b/arch/arm/dts/rk3568-evb-u-boot.dtsi @@ -4,6 +4,7 @@ */ #include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi" / { chosen { diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi index ed47efa44b..44cf33ed4b 100644 --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi @@ -5,6 +5,7 @@ */ #include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi" / { chosen { diff --git a/arch/arm/dts/rk3568-u-boot.dtsi b/arch/arm/dts/rk3568-u-boot.dtsi new file mode 100644 index 0000000000..c6776666df --- /dev/null +++ b/arch/arm/dts/rk3568-u-boot.dtsi @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (c) 2023 Vasily Khoruzhick <anarsoul@gmail.com> + */ + +#include "rockchip-u-boot.dtsi" + +&combphy0 { + /delete-property/ assigned-clocks; + /delete-property/ assigned-clock-rates; +};
combphy0 is failing to probe due to unhandled assigned-clocks and assigned-clocks-rates. commit 5bec4b0de785 ("arm64: dts: rk356x-u-boot: Drop combphy1 assigned-clocks/rates") dropped these properties for combphy1, so let's drop them for combphy0 as well. Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> --- arch/arm/dts/rk3568-evb-u-boot.dtsi | 1 + arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 1 + arch/arm/dts/rk3568-u-boot.dtsi | 11 +++++++++++ 3 files changed, 13 insertions(+) create mode 100644 arch/arm/dts/rk3568-u-boot.dtsi