| Message ID | 20221213160112.1900410-1-l.stach@pengutronix.de |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/4] dt-bindings: soc: imx8mp-hsio-blk-ctrl: add clock cells | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
Am Dienstag, dem 13.12.2022 um 17:01 +0100 schrieb Lucas Stach: > Expose the high performance PLL as a regular Linux clock, so the > PCIe PHY can use it when there is no external refclock provided. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > drivers/soc/imx/imx8mp-blk-ctrl.c | 99 +++++++++++++++++++++++++++++++ > 1 file changed, 99 insertions(+) > > diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c > index b3d9f6e083ba..ad5aebd640eb 100644 > --- a/drivers/soc/imx/imx8mp-blk-ctrl.c > +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c > @@ -5,6 +5,7 @@ > */ > > #include <linux/clk.h> > +#include <linux/clk-provider.h> > #include <linux/device.h> > #include <linux/interconnect.h> > #include <linux/module.h> > @@ -21,6 +22,15 @@ > #define USB_CLOCK_MODULE_EN BIT(1) > #define PCIE_PHY_APB_RST BIT(4) > #define PCIE_PHY_INIT_RST BIT(5) > +#define GPR_REG1 0x4 > +#define PLL_LOCK BIT(13) > +#define GPR_REG2 0x8 > +#define P_PLL_MASK GENMASK(5, 0) > +#define M_PLL_MASK GENMASK(15, 6) > +#define S_PLL_MASK GENMASK(18, 16) > +#define GPR_REG3 0xc > +#define PLL_CKE BIT(17) > +#define PLL_RST BIT(31) > > struct imx8mp_blk_ctrl_domain; > > @@ -74,6 +84,94 @@ to_imx8mp_blk_ctrl_domain(struct generic_pm_domain *genpd) > return container_of(genpd, struct imx8mp_blk_ctrl_domain, genpd); > } > > +struct clk_hsio_pll { > + struct clk_hw hw; > + struct regmap *regmap; > +}; > + > +static inline struct clk_hsio_pll *to_clk_hsio_pll(struct clk_hw *hw) > +{ > + return container_of(hw, struct clk_hsio_pll, hw); > +} > + > +static int clk_hsio_pll_prepare(struct clk_hw *hw) > +{ > + struct clk_hsio_pll *clk = to_clk_hsio_pll(hw); > + u32 val; > + > + /* set the PLL configuration */ > + regmap_update_bits(clk->regmap, GPR_REG2, > + P_PLL_MASK | M_PLL_MASK | S_PLL_MASK, > + FIELD_PREP(P_PLL_MASK, 12) | > + FIELD_PREP(M_PLL_MASK, 800) | > + FIELD_PREP(S_PLL_MASK, 4)); > + > + /* de-assert PLL reset */ > + regmap_update_bits(clk->regmap, GPR_REG3, PLL_RST, PLL_RST); > + > + /* enable PLL */ > + regmap_update_bits(clk->regmap, GPR_REG3, PLL_CKE, PLL_CKE); > + > + return regmap_read_poll_timeout(clk->regmap, GPR_REG1, val, > + val & PLL_LOCK, 10, 100); > +} > + > +static void clk_hsio_pll_unprepare(struct clk_hw *hw) > +{ > + struct clk_hsio_pll *clk = to_clk_hsio_pll(hw); > + > + regmap_update_bits(clk->regmap, GPR_REG3, PLL_RST | PLL_CKE, 0); > +} > + > +static int clk_hsio_pll_is_prepared(struct clk_hw *hw) > +{ > + struct clk_hsio_pll *clk = to_clk_hsio_pll(hw); > + > + return regmap_test_bits(clk->regmap, GPR_REG1, PLL_LOCK); > +} > + > +static unsigned long clk_hsio_pll_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + return 100000000; > +} > + > +static const struct clk_ops clk_hsio_pll_ops = { > + .prepare = clk_hsio_pll_prepare, > + .unprepare = clk_hsio_pll_unprepare, > + .is_prepared = clk_hsio_pll_is_prepared, > + .recalc_rate = clk_hsio_pll_recalc_rate, > +}; > + > +int imx8mp_hsio_blk_ctrl_probe(struct imx8mp_blk_ctrl *bc) > +{ > + struct clk_hsio_pll *clk_hsio_pll; > + struct clk_hw *hw; > + struct clk_init_data init = {}; > + int ret; > + > + printk("%s\n", __func__); This printk should obviously not be here. Removed locally. I'll wait for some feedback before sending v2. Regards, Lucas > + > + clk_hsio_pll = devm_kzalloc(bc->dev, sizeof(*clk_hsio_pll), GFP_KERNEL); > + if (!clk_hsio_pll) > + return -ENOMEM; > + > + init.name = "hsio_pll"; > + init.ops = &clk_hsio_pll_ops; > + init.parent_names = (const char *[]){"osc_24m"}; > + init.num_parents = 1; > + > + clk_hsio_pll->regmap = bc->regmap; > + clk_hsio_pll->hw.init = &init; > + > + hw = &clk_hsio_pll->hw; > + ret = devm_clk_hw_register(bc->dev, hw); > + if (ret) > + return ret; > + > + return devm_of_clk_add_hw_provider(bc->dev, of_clk_hw_simple_get, hw); > +} > + > static void imx8mp_hsio_blk_ctrl_power_on(struct imx8mp_blk_ctrl *bc, > struct imx8mp_blk_ctrl_domain *domain) > { > @@ -188,6 +286,7 @@ static const struct imx8mp_blk_ctrl_domain_data imx8mp_hsio_domain_data[] = { > > static const struct imx8mp_blk_ctrl_data imx8mp_hsio_blk_ctl_dev_data = { > .max_reg = 0x24, > + .probe = imx8mp_hsio_blk_ctrl_probe, > .power_on = imx8mp_hsio_blk_ctrl_power_on, > .power_off = imx8mp_hsio_blk_ctrl_power_off, > .power_notifier_fn = imx8mp_hsio_power_notifier,
On Tue, 13 Dec 2022 17:01:09 +0100, Lucas Stach wrote: > The HSIO blk-ctrl has a internal PLL, which can be used as a reference > clock for the PCIe PHY. Add clock-cells to the binding to allow the > driver to expose this PLL. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > .../devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.example.dtb: blk-ctrl@32f10000: #clock-cells:0:0: 1 was expected From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221213160112.1900410-1-l.stach@pengutronix.de The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Sali Lucas On Tue, 2022-12-13 at 17:01 +0100, Lucas Stach wrote: > The HSIO blk-ctrl has a internal PLL, which can be used as a reference > clock for the PCIe PHY. Add clock-cells to the binding to allow the > driver to expose this PLL. Nice! This indeed works fine on Verdin iMX8M Plus as well. Thanks! > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> Entire series: Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > --- > .../devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml > b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml > index c29181a9745b..1cc7c2bdf2bb 100644 > --- a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml > +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml > @@ -39,6 +39,9 @@ properties: > - const: pcie > - const: pcie-phy > > + '#clock-cells': > + const: 1 > + > clocks: > minItems: 2 > maxItems: 2 > @@ -85,4 +88,5 @@ examples: > power-domain-names = "bus", "usb", "usb-phy1", > "usb-phy2", "pcie", "pcie-phy"; > #power-domain-cells = <1>; > + #clock-cells = <0>; > };
On Wed, 2022-12-14 at 05:51 +0000, Hongxing Zhu wrote: > Hi Lucas: > Thanks a lot for your help about this series. > > Should the clocks of the pcie_phy should be changed as below when internal > PLL is used as PCIe reference clock on i.MX8MP EVK board? > --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts > +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts > @@ -370,7 +370,7 @@ &i2c5 { > > &pcie_phy { > fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>; > - clocks = <&pcie0_refclk>; > + clocks = <&hsio_blk_ctrl>; Yes, exactly. See e.g. also [1]. But don't forget to also change the fsl,refclk-pad-mode to IMX8_PCIE_REFCLK_PAD_OUTPUT (;-p). [1] https://lore.kernel.org/all/20221214061354.174072-1-marcel@ziswiler.com/ > clock-names = "ref"; > status = "okay"; > }; > > Best Regards > Richard Zhu > > > -----Original Message----- > > From: Lucas Stach <l.stach@pengutronix.de> > > Sent: 2022年12月14日 0:01 > > To: Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > > <krzysztof.kozlowski+dt@linaro.org>; Shawn Guo <shawnguo@kernel.org>; > > Hongxing Zhu <hongxing.zhu@nxp.com> > > Cc: dl-linux-imx <linux-imx@nxp.com>; Pengutronix Kernel Team > > <kernel@pengutronix.de>; Marcel Ziswiler <marcel.ziswiler@toradex.com>; > > marex@denx.de; tharvey@gateworks.com; alexander.stein@ew.tq-group.com; > > richard.leitner@linux.dev; lukas@mntre.com; patchwork-lst@pengutronix.de; > > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org > > Subject: [PATCH 1/4] dt-bindings: soc: imx8mp-hsio-blk-ctrl: add clock cells > > > > The HSIO blk-ctrl has a internal PLL, which can be used as a reference clock for > > the PCIe PHY. Add clock-cells to the binding to allow the driver to expose this > > PLL. > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > --- > > .../devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml > > b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml > > index c29181a9745b..1cc7c2bdf2bb 100644 > > --- > > a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml > > +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl > > +++ .yaml > > @@ -39,6 +39,9 @@ properties: > > - const: pcie > > - const: pcie-phy > > > > + '#clock-cells': > > + const: 1 > > + > > clocks: > > minItems: 2 > > maxItems: 2 > > @@ -85,4 +88,5 @@ examples: > > power-domain-names = "bus", "usb", "usb-phy1", > > "usb-phy2", "pcie", "pcie-phy"; > > #power-domain-cells = <1>; > > + #clock-cells = <0>; > > }; > > -- > > 2.30.2
Hi Lucas, Am Dienstag, 13. Dezember 2022, 17:01:09 CET schrieb Lucas Stach: > The HSIO blk-ctrl has a internal PLL, which can be used as a reference > clock for the PCIe PHY. Add clock-cells to the binding to allow the > driver to expose this PLL. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > .../devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml > b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml > index c29181a9745b..1cc7c2bdf2bb 100644 > --- > a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml > +++ > b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml > @@ -39,6 +39,9 @@ properties: > - const: pcie > - const: pcie-phy > > + '#clock-cells': > + const: 1 Rob's bot already noticed the mismatch between this and the example. Best regards, Alexander > + > clocks: > minItems: 2 > maxItems: 2 > @@ -85,4 +88,5 @@ examples: > power-domain-names = "bus", "usb", "usb-phy1", > "usb-phy2", "pcie", "pcie-phy"; > #power-domain-cells = <1>; > + #clock-cells = <0>; > };
> -----Original Message----- > From: Marcel Ziswiler <marcel.ziswiler@toradex.com> > Sent: 2022年12月14日 14:22 > To: l.stach@pengutronix.de; krzysztof.kozlowski+dt@linaro.org; Hongxing Zhu > <hongxing.zhu@nxp.com>; robh+dt@kernel.org; shawnguo@kernel.org > Cc: dl-linux-imx <linux-imx@nxp.com>; kernel@pengutronix.de; > richard.leitner@linux.dev; alexander.stein@ew.tq-group.com; > patchwork-lst@pengutronix.de; tharvey@gateworks.com; marex@denx.de; > lukas@mntre.com; devicetree@vger.kernel.org; > linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH 1/4] dt-bindings: soc: imx8mp-hsio-blk-ctrl: add clock cells > > On Wed, 2022-12-14 at 05:51 +0000, Hongxing Zhu wrote: > > Hi Lucas: > > Thanks a lot for your help about this series. > > > > Should the clocks of the pcie_phy should be changed as below when > > internal > > PLL is used as PCIe reference clock on i.MX8MP EVK board? > > --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts > > @@ -370,7 +370,7 @@ &i2c5 { > > > > &pcie_phy { > > fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>; > > - clocks = <&pcie0_refclk>; > > + clocks = <&hsio_blk_ctrl>; > > Yes, exactly. See e.g. also [1]. But don't forget to also change the > fsl,refclk-pad-mode to IMX8_PCIE_REFCLK_PAD_OUTPUT (;-p). > Got that. Thanks for your reminder. Best Regards Richard Zhu > [1] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker > nel.org%2Fall%2F20221214061354.174072-1-marcel%40ziswiler.com%2F&am > p;data=05%7C01%7Chongxing.zhu%40nxp.com%7Cacc7c14718fe45028c2808 > dadd9b8ae7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63806 > 5957361141610%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC > JQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&am > p;sdata=jK7t%2Bk6EZRS8oqHzRzsHR%2FImM2RGYRp7dIwc9pX2fqE%3D& > reserved=0 > > > clock-names = "ref"; > > status = "okay"; > > }; > > > > Best Regards > > Richard Zhu > > > > > -----Original Message----- > > > From: Lucas Stach <l.stach@pengutronix.de> > > > Sent: 2022年12月14日 0:01 > > > To: Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > > > <krzysztof.kozlowski+dt@linaro.org>; Shawn Guo > > > <shawnguo@kernel.org>; Hongxing Zhu <hongxing.zhu@nxp.com> > > > Cc: dl-linux-imx <linux-imx@nxp.com>; Pengutronix Kernel Team > > > <kernel@pengutronix.de>; Marcel Ziswiler > > > <marcel.ziswiler@toradex.com>; marex@denx.de; > tharvey@gateworks.com; > > > alexander.stein@ew.tq-group.com; richard.leitner@linux.dev; > > > lukas@mntre.com; patchwork-lst@pengutronix.de; > > > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org > > > Subject: [PATCH 1/4] dt-bindings: soc: imx8mp-hsio-blk-ctrl: add > > > clock cells > > > > > > The HSIO blk-ctrl has a internal PLL, which can be used as a > > > reference clock for the PCIe PHY. Add clock-cells to the binding to > > > allow the driver to expose this PLL. > > > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > > --- > > > .../devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml | 4 > > > ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl > > > .yaml > > > b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl > > > .yaml index c29181a9745b..1cc7c2bdf2bb 100644 > > > --- > > > a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl > > > .yaml > > > +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk- > > > +++ ctrl > > > +++ .yaml > > > @@ -39,6 +39,9 @@ properties: > > > - const: pcie > > > - const: pcie-phy > > > > > > + '#clock-cells': > > > + const: 1 > > > + > > > clocks: > > > minItems: 2 > > > maxItems: 2 > > > @@ -85,4 +88,5 @@ examples: > > > power-domain-names = "bus", "usb", "usb-phy1", > > > "usb-phy2", "pcie", > "pcie-phy"; > > > #power-domain-cells = <1>; > > > + #clock-cells = <0>; > > > }; > > > -- > > > 2.30.2
diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml index c29181a9745b..1cc7c2bdf2bb 100644 --- a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml @@ -39,6 +39,9 @@ properties: - const: pcie - const: pcie-phy + '#clock-cells': + const: 1 + clocks: minItems: 2 maxItems: 2 @@ -85,4 +88,5 @@ examples: power-domain-names = "bus", "usb", "usb-phy1", "usb-phy2", "pcie", "pcie-phy"; #power-domain-cells = <1>; + #clock-cells = <0>; };
The HSIO blk-ctrl has a internal PLL, which can be used as a reference clock for the PCIe PHY. Add clock-cells to the binding to allow the driver to expose this PLL. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- .../devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml | 4 ++++ 1 file changed, 4 insertions(+)