* [PATCH] imx8mm-venice-gw7902: update pci refclk
@ 2022-04-05 20:06 Tim Harvey
2022-04-11 1:31 ` Shawn Guo
0 siblings, 1 reply; 5+ messages in thread
From: Tim Harvey @ 2022-04-05 20:06 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, NXP Linux Team, devicetree, linux-arm-kernel,
linux-kernel
Cc: Tim Harvey
Use the correct PCI clock bindings.
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
index 6aa0eb463647..f71416be29a7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
@@ -595,7 +595,7 @@
&pcie_phy {
fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>;
fsl,clkreq-unsupported;
- clocks = <&clk IMX8MM_CLK_DUMMY>;
+ clocks = <&pcie0_refclk>;
status = "okay";
};
@@ -604,8 +604,8 @@
pinctrl-0 = <&pinctrl_pcie0>;
reset-gpio = <&gpio4 5 GPIO_ACTIVE_LOW>;
clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>, <&clk IMX8MM_CLK_PCIE1_AUX>,
- <&clk IMX8MM_CLK_DUMMY>, <&pcie0_refclk>;
- clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus";
+ <&pcie0_refclk>;
+ clock-names = "pcie", "pcie_aux", "pcie_bus";
assigned-clocks = <&clk IMX8MM_CLK_PCIE1_AUX>,
<&clk IMX8MM_CLK_PCIE1_CTRL>;
assigned-clock-rates = <10000000>, <250000000>;
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] imx8mm-venice-gw7902: update pci refclk
2022-04-05 20:06 [PATCH] imx8mm-venice-gw7902: update pci refclk Tim Harvey
@ 2022-04-11 1:31 ` Shawn Guo
2022-04-11 19:44 ` Tim Harvey
0 siblings, 1 reply; 5+ messages in thread
From: Shawn Guo @ 2022-04-11 1:31 UTC (permalink / raw)
To: Tim Harvey
Cc: Rob Herring, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, NXP Linux Team, devicetree, linux-arm-kernel,
linux-kernel
On Tue, Apr 05, 2022 at 01:06:25PM -0700, Tim Harvey wrote:
> Use the correct PCI clock bindings.
Please improve the commit log to explain why clock "pcie_phy" can be
dropped.
Shawn
>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
> arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
> index 6aa0eb463647..f71416be29a7 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
> @@ -595,7 +595,7 @@
> &pcie_phy {
> fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>;
> fsl,clkreq-unsupported;
> - clocks = <&clk IMX8MM_CLK_DUMMY>;
> + clocks = <&pcie0_refclk>;
> status = "okay";
> };
>
> @@ -604,8 +604,8 @@
> pinctrl-0 = <&pinctrl_pcie0>;
> reset-gpio = <&gpio4 5 GPIO_ACTIVE_LOW>;
> clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>, <&clk IMX8MM_CLK_PCIE1_AUX>,
> - <&clk IMX8MM_CLK_DUMMY>, <&pcie0_refclk>;
> - clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus";
> + <&pcie0_refclk>;
> + clock-names = "pcie", "pcie_aux", "pcie_bus";
> assigned-clocks = <&clk IMX8MM_CLK_PCIE1_AUX>,
> <&clk IMX8MM_CLK_PCIE1_CTRL>;
> assigned-clock-rates = <10000000>, <250000000>;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] imx8mm-venice-gw7902: update pci refclk
2022-04-11 1:31 ` Shawn Guo
@ 2022-04-11 19:44 ` Tim Harvey
2022-04-18 7:57 ` Shawn Guo
0 siblings, 1 reply; 5+ messages in thread
From: Tim Harvey @ 2022-04-11 19:44 UTC (permalink / raw)
To: Shawn Guo
Cc: Rob Herring, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, NXP Linux Team, Device Tree Mailing List,
Linux ARM Mailing List, open list
On Sun, Apr 10, 2022 at 6:31 PM Shawn Guo <shawnguo@kernel.org> wrote:
>
> On Tue, Apr 05, 2022 at 01:06:25PM -0700, Tim Harvey wrote:
> > Use the correct PCI clock bindings.
>
> Please improve the commit log to explain why clock "pcie_phy" can be
> dropped.
>
Shawn,
The original PCIe bindings for this board were wrong - they were from
a version of the bindings that was not yet approved (my mistake) and
I'm just trying to bring them up to date.
That said, I looked at the latest fsl,imx6q-pcie.yaml dt-bindings [1]
and see that there should be a min of 3 clocks called 'pcie',
'pcie_bus', and 'pcie_phy'. However I notice that all of the current
imx8mm boards that enable PCI have clock-names of 'pcie', 'pcie_aux',
and 'pcie_bus'. It seems like all the imx8mm boards having pcie have
clock-names this way:
arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
arch/arm64/boot/dts/freescale/imx8mm-tqma8mqml-mba8mx.dts
arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi
arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi
arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi
arch/arm64/boot/dts/freescale/imx8mm-venice-gw7901.dts
arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
arch/arm64/boot/dts/freescale/imx8mm-venice-gw7903.dts
Does the binding need to change or do the clock names need to change
in the above?
Best Regards,
Tim
[1]
> Shawn
>
> >
> > Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> > ---
> > arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
> > index 6aa0eb463647..f71416be29a7 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
> > @@ -595,7 +595,7 @@
> > &pcie_phy {
> > fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>;
> > fsl,clkreq-unsupported;
> > - clocks = <&clk IMX8MM_CLK_DUMMY>;
> > + clocks = <&pcie0_refclk>;
> > status = "okay";
> > };
> >
> > @@ -604,8 +604,8 @@
> > pinctrl-0 = <&pinctrl_pcie0>;
> > reset-gpio = <&gpio4 5 GPIO_ACTIVE_LOW>;
> > clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>, <&clk IMX8MM_CLK_PCIE1_AUX>,
> > - <&clk IMX8MM_CLK_DUMMY>, <&pcie0_refclk>;
> > - clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus";
> > + <&pcie0_refclk>;
> > + clock-names = "pcie", "pcie_aux", "pcie_bus";
> > assigned-clocks = <&clk IMX8MM_CLK_PCIE1_AUX>,
> > <&clk IMX8MM_CLK_PCIE1_CTRL>;
> > assigned-clock-rates = <10000000>, <250000000>;
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] imx8mm-venice-gw7902: update pci refclk
2022-04-11 19:44 ` Tim Harvey
@ 2022-04-18 7:57 ` Shawn Guo
2022-04-29 16:04 ` Tim Harvey
0 siblings, 1 reply; 5+ messages in thread
From: Shawn Guo @ 2022-04-18 7:57 UTC (permalink / raw)
To: Tim Harvey
Cc: Rob Herring, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, NXP Linux Team, Device Tree Mailing List,
Linux ARM Mailing List, open list
On Mon, Apr 11, 2022 at 12:44:23PM -0700, Tim Harvey wrote:
> On Sun, Apr 10, 2022 at 6:31 PM Shawn Guo <shawnguo@kernel.org> wrote:
> >
> > On Tue, Apr 05, 2022 at 01:06:25PM -0700, Tim Harvey wrote:
> > > Use the correct PCI clock bindings.
> >
> > Please improve the commit log to explain why clock "pcie_phy" can be
> > dropped.
> >
>
> Shawn,
>
> The original PCIe bindings for this board were wrong - they were from
> a version of the bindings that was not yet approved (my mistake) and
> I'm just trying to bring them up to date.
>
> That said, I looked at the latest fsl,imx6q-pcie.yaml dt-bindings [1]
> and see that there should be a min of 3 clocks called 'pcie',
> 'pcie_bus', and 'pcie_phy'. However I notice that all of the current
> imx8mm boards that enable PCI have clock-names of 'pcie', 'pcie_aux',
> and 'pcie_bus'. It seems like all the imx8mm boards having pcie have
> clock-names this way:
>
> arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
> arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> arch/arm64/boot/dts/freescale/imx8mm-tqma8mqml-mba8mx.dts
> arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi
> arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi
> arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi
> arch/arm64/boot/dts/freescale/imx8mm-venice-gw7901.dts
> arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
> arch/arm64/boot/dts/freescale/imx8mm-venice-gw7903.dts
>
> Does the binding need to change or do the clock names need to change
> in the above?
If the bindings is approved/correct, device tree should match bindings.
Shawn
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] imx8mm-venice-gw7902: update pci refclk
2022-04-18 7:57 ` Shawn Guo
@ 2022-04-29 16:04 ` Tim Harvey
0 siblings, 0 replies; 5+ messages in thread
From: Tim Harvey @ 2022-04-29 16:04 UTC (permalink / raw)
To: Shawn Guo
Cc: Rob Herring, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, NXP Linux Team, Device Tree Mailing List,
Linux ARM Mailing List, open list, Richard Zhu
On Mon, Apr 18, 2022 at 12:57 AM Shawn Guo <shawnguo@kernel.org> wrote:
>
> On Mon, Apr 11, 2022 at 12:44:23PM -0700, Tim Harvey wrote:
> > On Sun, Apr 10, 2022 at 6:31 PM Shawn Guo <shawnguo@kernel.org> wrote:
> > >
> > > On Tue, Apr 05, 2022 at 01:06:25PM -0700, Tim Harvey wrote:
> > > > Use the correct PCI clock bindings.
> > >
> > > Please improve the commit log to explain why clock "pcie_phy" can be
> > > dropped.
> > >
> >
> > Shawn,
> >
> > The original PCIe bindings for this board were wrong - they were from
> > a version of the bindings that was not yet approved (my mistake) and
> > I'm just trying to bring them up to date.
> >
> > That said, I looked at the latest fsl,imx6q-pcie.yaml dt-bindings [1]
> > and see that there should be a min of 3 clocks called 'pcie',
> > 'pcie_bus', and 'pcie_phy'. However I notice that all of the current
> > imx8mm boards that enable PCI have clock-names of 'pcie', 'pcie_aux',
> > and 'pcie_bus'. It seems like all the imx8mm boards having pcie have
> > clock-names this way:
> >
> > arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
> > arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> > arch/arm64/boot/dts/freescale/imx8mm-tqma8mqml-mba8mx.dts
> > arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi
> > arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi
> > arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi
> > arch/arm64/boot/dts/freescale/imx8mm-venice-gw7901.dts
> > arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
> > arch/arm64/boot/dts/freescale/imx8mm-venice-gw7903.dts
> >
> > Does the binding need to change or do the clock names need to change
> > in the above?
>
> If the bindings is approved/correct, device tree should match bindings.
>
Shawn,
I think the bindings are wrong.
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml has [1]:
clocks:
minItems: 3
items:
- description: PCIe bridge clock.
- description: PCIe bus clock.
- description: PCIe PHY clock.
- description: Additional required clock entry for imx6sx-pcie,
imx8mq-pcie.
clock-names:
minItems: 3
items:
- const: pcie
- const: pcie_bus
- const: pcie_phy
- const: pcie_inbound_axi for imx6sx-pcie, pcie_aux for imx8mq-pcie
This indicates the "pcie_phy" clock is required yet
drivers/pci/controller/dwc/pci-imx6.c [2] doesn't require it if it has
an abstract PHY driver which is the case for IMX8M (and that's why my
patch drops it)
Additionally I note that the 4th clock described in the bindings could
use some clarification for imx8mm-pcie as for this "pcie_aux" is
required.
Best Regards,
Tim
[1] https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
[2] https://elixir.bootlin.com/linux/v5.18-rc4/source/drivers/pci/controller/dwc/pci-imx6.c#L1140
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-04-29 16:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-05 20:06 [PATCH] imx8mm-venice-gw7902: update pci refclk Tim Harvey
2022-04-11 1:31 ` Shawn Guo
2022-04-11 19:44 ` Tim Harvey
2022-04-18 7:57 ` Shawn Guo
2022-04-29 16:04 ` Tim Harvey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).