2019-10-28 18:18:17

by Fancy Fang

Subject: [PATCH v4] clk: imx7ulp: do not export out IMX7ULP_CLK_MIPI_PLL clock

The mipi pll clock comes from the MIPI PHY PLL output, so
it should not be a fixed clock.

MIPI PHY PLL is in the MIPI DSI space, and it is used as
the bit clock for transferring the pixel data out and its
output clock is configured according to the display mode.

So it should be used only for MIPI DSI and not be exported
out for other usages.

Signed-off-by: Fancy Fang <[email protected]>
---
ChangeLog v3->v4:
* Add some comments to 'IMX7ULP_CLK_MIPI_PLL'
clock.

Documentation/devicetree/bindings/clock/imx7ulp-clock.txt | 1 -
drivers/clk/imx/clk-imx7ulp.c | 3 +--
include/dt-bindings/clock/imx7ulp-clock.h | 6 ++++++
3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
index a4f8cd478f92..93d89adb7afe 100644
--- a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
+++ b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
@@ -82,7 +82,6 @@ pcc2: pcc2@403f0000 {
<&scg1 IMX7ULP_CLK_APLL_PFD0>,
<&scg1 IMX7ULP_CLK_UPLL>,
<&scg1 IMX7ULP_CLK_SOSC_BUS_CLK>,
- <&scg1 IMX7ULP_CLK_MIPI_PLL>,
<&scg1 IMX7ULP_CLK_FIRC_BUS_CLK>,
<&scg1 IMX7ULP_CLK_ROSC>,
<&scg1 IMX7ULP_CLK_SPLL_BUS_CLK>;
diff --git a/drivers/clk/imx/clk-imx7ulp.c b/drivers/clk/imx/clk-imx7ulp.c
index 2022d9bead91..459b120b71d5 100644
--- a/drivers/clk/imx/clk-imx7ulp.c
+++ b/drivers/clk/imx/clk-imx7ulp.c
@@ -28,7 +28,7 @@ static const char * const scs_sels[] = { "dummy", "sosc", "sirc", "firc", "dumm
static const char * const ddr_sels[] = { "apll_pfd_sel", "upll", };
static const char * const nic_sels[] = { "firc", "ddr_clk", };
static const char * const periph_plat_sels[] = { "dummy", "nic1_bus_clk", "nic1_clk", "ddr_clk", "apll_pfd2", "apll_pfd1", "apll_pfd0", "upll", };
-static const char * const periph_bus_sels[] = { "dummy", "sosc_bus_clk", "mpll", "firc_bus_clk", "rosc", "nic1_bus_clk", "nic1_clk", "spll_bus_clk", };
+static const char * const periph_bus_sels[] = { "dummy", "sosc_bus_clk", "dummy", "firc_bus_clk", "rosc", "nic1_bus_clk", "nic1_clk", "spll_bus_clk", };
static const char * const arm_sels[] = { "divcore", "dummy", "dummy", "hsrun_divcore", };

/* used by sosc/sirc/firc/ddr/spll/apll dividers */
@@ -75,7 +75,6 @@ static void __init imx7ulp_clk_scg1_init(struct device_node *np)
clks[IMX7ULP_CLK_SOSC] = imx_obtain_fixed_clk_hw(np, "sosc");
clks[IMX7ULP_CLK_SIRC] = imx_obtain_fixed_clk_hw(np, "sirc");
clks[IMX7ULP_CLK_FIRC] = imx_obtain_fixed_clk_hw(np, "firc");
- clks[IMX7ULP_CLK_MIPI_PLL] = imx_obtain_fixed_clk_hw(np, "mpll");
clks[IMX7ULP_CLK_UPLL] = imx_obtain_fixed_clk_hw(np, "upll");

/* SCG1 */
diff --git a/include/dt-bindings/clock/imx7ulp-clock.h b/include/dt-bindings/clock/imx7ulp-clock.h
index 6f66f9005c81..e9ef62f211fe 100644
--- a/include/dt-bindings/clock/imx7ulp-clock.h
+++ b/include/dt-bindings/clock/imx7ulp-clock.h
@@ -49,7 +49,13 @@
#define IMX7ULP_CLK_NIC1_DIV 36
#define IMX7ULP_CLK_NIC1_BUS_DIV 37
#define IMX7ULP_CLK_NIC1_EXT_DIV 38
+
+/* mpll clock is a special clock comes from
+ * mipi DPHY PLL and should be used only for
+ * mipi dsi instead of any other peripheral.
+ */
#define IMX7ULP_CLK_MIPI_PLL 39
+
#define IMX7ULP_CLK_SIRC 40
#define IMX7ULP_CLK_SOSC_BUS_CLK 41
#define IMX7ULP_CLK_FIRC_BUS_CLK 42
--
2.17.1

2019-10-28 19:52:03

by Shawn Guo

Subject: Re: [PATCH v4] clk: imx7ulp: do not export out IMX7ULP_CLK_MIPI_PLL clock

On Mon, Oct 28, 2019 at 08:07:59AM +0000, Fancy Fang wrote:
> The mipi pll clock comes from the MIPI PHY PLL output, so
> it should not be a fixed clock.
>
> MIPI PHY PLL is in the MIPI DSI space, and it is used as
> the bit clock for transferring the pixel data out and its
> output clock is configured according to the display mode.
>
> So it should be used only for MIPI DSI and not be exported
> out for other usages.
>
> Signed-off-by: Fancy Fang <[email protected]>
> ---
> ChangeLog v3->v4:
> * Add some comments to 'IMX7ULP_CLK_MIPI_PLL'
> clock.
>
> Documentation/devicetree/bindings/clock/imx7ulp-clock.txt | 1 -
> drivers/clk/imx/clk-imx7ulp.c | 3 +--
> include/dt-bindings/clock/imx7ulp-clock.h | 6 ++++++
> 3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
> index a4f8cd478f92..93d89adb7afe 100644
> --- a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
> @@ -82,7 +82,6 @@ pcc2: pcc2@403f0000 {
> <&scg1 IMX7ULP_CLK_APLL_PFD0>,
> <&scg1 IMX7ULP_CLK_UPLL>,
> <&scg1 IMX7ULP_CLK_SOSC_BUS_CLK>,
> - <&scg1 IMX7ULP_CLK_MIPI_PLL>,
> <&scg1 IMX7ULP_CLK_FIRC_BUS_CLK>,
> <&scg1 IMX7ULP_CLK_ROSC>,
> <&scg1 IMX7ULP_CLK_SPLL_BUS_CLK>;
> diff --git a/drivers/clk/imx/clk-imx7ulp.c b/drivers/clk/imx/clk-imx7ulp.c
> index 2022d9bead91..459b120b71d5 100644
> --- a/drivers/clk/imx/clk-imx7ulp.c
> +++ b/drivers/clk/imx/clk-imx7ulp.c
> @@ -28,7 +28,7 @@ static const char * const scs_sels[] = { "dummy", "sosc", "sirc", "firc", "dumm
> static const char * const ddr_sels[] = { "apll_pfd_sel", "upll", };
> static const char * const nic_sels[] = { "firc", "ddr_clk", };
> static const char * const periph_plat_sels[] = { "dummy", "nic1_bus_clk", "nic1_clk", "ddr_clk", "apll_pfd2", "apll_pfd1", "apll_pfd0", "upll", };
> -static const char * const periph_bus_sels[] = { "dummy", "sosc_bus_clk", "mpll", "firc_bus_clk", "rosc", "nic1_bus_clk", "nic1_clk", "spll_bus_clk", };
> +static const char * const periph_bus_sels[] = { "dummy", "sosc_bus_clk", "dummy", "firc_bus_clk", "rosc", "nic1_bus_clk", "nic1_clk", "spll_bus_clk", };
> static const char * const arm_sels[] = { "divcore", "dummy", "dummy", "hsrun_divcore", };
>
> /* used by sosc/sirc/firc/ddr/spll/apll dividers */
> @@ -75,7 +75,6 @@ static void __init imx7ulp_clk_scg1_init(struct device_node *np)
> clks[IMX7ULP_CLK_SOSC] = imx_obtain_fixed_clk_hw(np, "sosc");
> clks[IMX7ULP_CLK_SIRC] = imx_obtain_fixed_clk_hw(np, "sirc");
> clks[IMX7ULP_CLK_FIRC] = imx_obtain_fixed_clk_hw(np, "firc");
> - clks[IMX7ULP_CLK_MIPI_PLL] = imx_obtain_fixed_clk_hw(np, "mpll");
> clks[IMX7ULP_CLK_UPLL] = imx_obtain_fixed_clk_hw(np, "upll");
>
> /* SCG1 */
> diff --git a/include/dt-bindings/clock/imx7ulp-clock.h b/include/dt-bindings/clock/imx7ulp-clock.h
> index 6f66f9005c81..e9ef62f211fe 100644
> --- a/include/dt-bindings/clock/imx7ulp-clock.h
> +++ b/include/dt-bindings/clock/imx7ulp-clock.h
> @@ -49,7 +49,13 @@
> #define IMX7ULP_CLK_NIC1_DIV 36
> #define IMX7ULP_CLK_NIC1_BUS_DIV 37
> #define IMX7ULP_CLK_NIC1_EXT_DIV 38
> +
> +/* mpll clock is a special clock comes from
> + * mipi DPHY PLL and should be used only for
> + * mipi dsi instead of any other peripheral.
> + */
> #define IMX7ULP_CLK_MIPI_PLL 39
> +

The point of comment is to tell that the clock ID is unsupported and
shouldn't be used in DT.

I reworded the comment and applied the patch.

Shawn

> #define IMX7ULP_CLK_SIRC 40
> #define IMX7ULP_CLK_SOSC_BUS_CLK 41
> #define IMX7ULP_CLK_FIRC_BUS_CLK 42
> --
> 2.17.1
>

2019-10-29 07:05:16

by Fancy Fang

Subject: RE: [PATCH v4] clk: imx7ulp: do not export out IMX7ULP_CLK_MIPI_PLL clock

> -----Original Message-----
> From: Shawn Guo <[email protected]>
> Sent: Monday, October 28, 2019 6:51 PM
> To: Fancy Fang <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> dl-linux-imx <[email protected]>
> Subject: Re: [PATCH v4] clk: imx7ulp: do not export out IMX7ULP_CLK_MIPI_PLL
> clock
>
> On Mon, Oct 28, 2019 at 08:07:59AM +0000, Fancy Fang wrote:
> > The mipi pll clock comes from the MIPI PHY PLL output, so
> > it should not be a fixed clock.
> >
> > MIPI PHY PLL is in the MIPI DSI space, and it is used as
> > the bit clock for transferring the pixel data out and its
> > output clock is configured according to the display mode.
> >
> > So it should be used only for MIPI DSI and not be exported
> > out for other usages.
> >
> > Signed-off-by: Fancy Fang <[email protected]>
> > ---
> > ChangeLog v3->v4:
> > * Add some comments to 'IMX7ULP_CLK_MIPI_PLL'
> > clock.
> >
> > Documentation/devicetree/bindings/clock/imx7ulp-clock.txt | 1 -
> > drivers/clk/imx/clk-imx7ulp.c | 3 +--
> > include/dt-bindings/clock/imx7ulp-clock.h | 6 ++++++
> > 3 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
> b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
> > index a4f8cd478f92..93d89adb7afe 100644
> > --- a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
> > +++ b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
> > @@ -82,7 +82,6 @@ pcc2: pcc2@403f0000 {
> > <&scg1 IMX7ULP_CLK_APLL_PFD0>,
> > <&scg1 IMX7ULP_CLK_UPLL>,
> > <&scg1 IMX7ULP_CLK_SOSC_BUS_CLK>,
> > - <&scg1 IMX7ULP_CLK_MIPI_PLL>,
> > <&scg1 IMX7ULP_CLK_FIRC_BUS_CLK>,
> > <&scg1 IMX7ULP_CLK_ROSC>,
> > <&scg1 IMX7ULP_CLK_SPLL_BUS_CLK>;
> > diff --git a/drivers/clk/imx/clk-imx7ulp.c b/drivers/clk/imx/clk-imx7ulp.c
> > index 2022d9bead91..459b120b71d5 100644
> > --- a/drivers/clk/imx/clk-imx7ulp.c
> > +++ b/drivers/clk/imx/clk-imx7ulp.c
> > @@ -28,7 +28,7 @@ static const char * const scs_sels[] = { "dummy",
> "sosc", "sirc", "firc", "dumm
> > static const char * const ddr_sels[] = { "apll_pfd_sel", "upll", };
> > static const char * const nic_sels[] = { "firc", "ddr_clk", };
> > static const char * const periph_plat_sels[] = { "dummy", "nic1_bus_clk",
> "nic1_clk", "ddr_clk", "apll_pfd2", "apll_pfd1", "apll_pfd0", "upll", };
> > -static const char * const periph_bus_sels[] = { "dummy", "sosc_bus_clk",
> "mpll", "firc_bus_clk", "rosc", "nic1_bus_clk", "nic1_clk", "spll_bus_clk", };
> > +static const char * const periph_bus_sels[] = { "dummy", "sosc_bus_clk",
> "dummy", "firc_bus_clk", "rosc", "nic1_bus_clk", "nic1_clk", "spll_bus_clk", };
> > static const char * const arm_sels[] = { "divcore", "dummy", "dummy",
> "hsrun_divcore", };
> >
> > /* used by sosc/sirc/firc/ddr/spll/apll dividers */
> > @@ -75,7 +75,6 @@ static void __init imx7ulp_clk_scg1_init(struct
> device_node *np)
> > clks[IMX7ULP_CLK_SOSC] = imx_obtain_fixed_clk_hw(np, "sosc");
> > clks[IMX7ULP_CLK_SIRC] = imx_obtain_fixed_clk_hw(np, "sirc");
> > clks[IMX7ULP_CLK_FIRC] = imx_obtain_fixed_clk_hw(np, "firc");
> > - clks[IMX7ULP_CLK_MIPI_PLL] = imx_obtain_fixed_clk_hw(np, "mpll");
> > clks[IMX7ULP_CLK_UPLL] = imx_obtain_fixed_clk_hw(np, "upll");
> >
> > /* SCG1 */
> > diff --git a/include/dt-bindings/clock/imx7ulp-clock.h
> b/include/dt-bindings/clock/imx7ulp-clock.h
> > index 6f66f9005c81..e9ef62f211fe 100644
> > --- a/include/dt-bindings/clock/imx7ulp-clock.h
> > +++ b/include/dt-bindings/clock/imx7ulp-clock.h
> > @@ -49,7 +49,13 @@
> > #define IMX7ULP_CLK_NIC1_DIV 36
> > #define IMX7ULP_CLK_NIC1_BUS_DIV 37
> > #define IMX7ULP_CLK_NIC1_EXT_DIV 38
> > +
> > +/* mpll clock is a special clock comes from
> > + * mipi DPHY PLL and should be used only for
> > + * mipi dsi instead of any other peripheral.
> > + */
> > #define IMX7ULP_CLK_MIPI_PLL 39
> > +
>
> The point of comment is to tell that the clock ID is unsupported and
> shouldn't be used in DT.
>
> I reworded the comment and applied the patch.

OK. Thank you.

Best regards,
Fancy Fang
>
> Shawn
>
> > #define IMX7ULP_CLK_SIRC 40
> > #define IMX7ULP_CLK_SOSC_BUS_CLK 41
> > #define IMX7ULP_CLK_FIRC_BUS_CLK 42
> > --
> > 2.17.1
> >