mbox series

[v2,0/9] Add imx8ulp clock & reset driver support

Message ID 20210810062820.1062884-1-ping.bai@nxp.com
Headers show
Series Add imx8ulp clock & reset driver support | expand

Message

Jacky Bai Aug. 10, 2021, 6:28 a.m. UTC 
This patchset adds the clock & reset driver support for i.MX8ULP.
For some of the PCC slot, As there is a SWRST control bit share
the same pcc register for peripheral reset ccontrol. To simplify
the case, register the pcc reset controller driver when pcc
clock driver is registered.

Patch 1/9  for the dt-bindings part is send out for review previously
with the dts patchset:
https://patchwork.kernel.org/project/linux-arm-kernel/cover/20210607083921.2668568-1-ping.bai@nxp.com/

Shawn suggests to send out the clock driver part firstly, so this
patch is included in this patchset for now.

v2 changes:
  - remove the useless clocks & clock-names from the dt-binding doc
  - remove the redundant fixed clock register.

Anson Huang (1):
  clk: imx: disable i.mx7ulp composite clock during initialization

Jacky Bai (8):
  dt-bindings: clock: Add imx8ulp clock support
  clk: imx: Update the pllv4 to support imx8ulp
  clk: imx: Update the compsite driver to support imx8ulp
  clk: imx: Add 'CLK_SET_RATE_NO_REPARENT' for composite-7ulp
  clk: imx: disable the pfd when set pfdv2 clock rate
  clk: imx: Update the pfdv2 for 8ulp specific support
  clk: imx: Add clock driver for imx8ulp
  clk: imx: Add the pcc reset controller support on imx8ulp

 .../bindings/clock/imx8ulp-clock.yaml         |  71 +++
 drivers/clk/imx/Kconfig                       |   7 +
 drivers/clk/imx/Makefile                      |   2 +
 drivers/clk/imx/clk-composite-7ulp.c          |  87 ++-
 drivers/clk/imx/clk-imx7ulp.c                 |  20 +-
 drivers/clk/imx/clk-imx8ulp.c                 | 568 ++++++++++++++++++
 drivers/clk/imx/clk-pfdv2.c                   |  22 +-
 drivers/clk/imx/clk-pllv4.c                   |  34 +-
 drivers/clk/imx/clk.h                         |  24 +-
 include/dt-bindings/clock/imx8ulp-clock.h     | 258 ++++++++
 include/dt-bindings/reset/imx8ulp-pcc-reset.h |  59 ++
 11 files changed, 1120 insertions(+), 32 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/imx8ulp-clock.yaml
 create mode 100644 drivers/clk/imx/clk-imx8ulp.c
 create mode 100644 include/dt-bindings/clock/imx8ulp-clock.h
 create mode 100644 include/dt-bindings/reset/imx8ulp-pcc-reset.h

Comments

Philipp Zabel Aug. 23, 2021, 11:23 a.m. UTC | #1 
Hi Jacky,

On Tue, 2021-08-10 at 14:28 +0800, Jacky Bai wrote:
> On i.MX8ULP, for some of the PCCs, it has a peripheral SW RST bit
> resides in the same registers as the clock controller. So add this
> SW RST controller support alongs with the pcc clock initialization.
> 
> the reset and clock shared the same register, to avoid  accessing
> the same register by reset control and clock control concurrently,
> locking is necessary, so reuse the imx_ccm_lock spinlock to simplify
> the code.
> 
> Suggested-by: Liu Ying <victor.liu@nxp.com>
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> ---
>   v2 changes:
>     - add 'Suggested-by' as suggested by Victor Liu
> ---
>  drivers/clk/imx/Kconfig              |   1 +
>  drivers/clk/imx/clk-composite-7ulp.c |  10 +++
>  drivers/clk/imx/clk-imx8ulp.c        | 115 ++++++++++++++++++++++++++-
>  3 files changed, 123 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
> index b81d6437ed95..0d1e3a6ac32a 100644
> --- a/drivers/clk/imx/Kconfig
> +++ b/drivers/clk/imx/Kconfig
> @@ -102,5 +102,6 @@ config CLK_IMX8QXP
>  config CLK_IMX8ULP
>  	tristate "IMX8ULP CCM Clock Driver"
>  	depends on ARCH_MXC || COMPILE_TEST
> +	select RESET_CONTROLLER

This shouldn't be required anymore, devm_reset_controller_register() has
a stub since commit 48a74b1147f7 ("reset: Add compile-test stubs").

[...]
> diff --git a/drivers/clk/imx/clk-imx8ulp.c b/drivers/clk/imx/clk-imx8ulp.c
> index 6aad04114658..ea596cd6855a 100644
> --- a/drivers/clk/imx/clk-imx8ulp.c
> +++ b/drivers/clk/imx/clk-imx8ulp.c
> @@ -9,6 +9,7 @@
>  #include <linux/module.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
>  #include <linux/slab.h>
>  
>  #include "clk.h"
> @@ -48,6 +49,98 @@ static const char * const nic_per_divplat[] = { "nic_per_divplat" };
>  static const char * const lpav_axi_div[] = { "lpav_axi_div" };
>  static const char * const lpav_bus_div[] = { "lpav_bus_div" };
>  
> +struct pcc_reset_dev {
> +	void __iomem *base;
> +	struct reset_controller_dev rcdev;
> +	const u32 *resets;
> +	spinlock_t *lock;

I'd add a comment to this lock, stating that it is set to imx_ccm_lock
and protects access to registers shared with clock control.

With these addressed,

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp
Jacky Bai Aug. 23, 2021, 11:58 p.m. UTC | #2 
> Subject: Re: [PATCH v2 9/9] clk: imx: Add the pcc reset controller support on
> imx8ulp
> 
> Hi Jacky,
> 
> On Tue, 2021-08-10 at 14:28 +0800, Jacky Bai wrote:
> > On i.MX8ULP, for some of the PCCs, it has a peripheral SW RST bit
> > resides in the same registers as the clock controller. So add this SW
> > RST controller support alongs with the pcc clock initialization.
> >
> > the reset and clock shared the same register, to avoid  accessing the
> > same register by reset control and clock control concurrently, locking
> > is necessary, so reuse the imx_ccm_lock spinlock to simplify the code.
> >
> > Suggested-by: Liu Ying <victor.liu@nxp.com>
> > Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> > ---
> >   v2 changes:
> >     - add 'Suggested-by' as suggested by Victor Liu
> > ---
> >  drivers/clk/imx/Kconfig              |   1 +
> >  drivers/clk/imx/clk-composite-7ulp.c |  10 +++
> >  drivers/clk/imx/clk-imx8ulp.c        | 115
> ++++++++++++++++++++++++++-
> >  3 files changed, 123 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index
> > b81d6437ed95..0d1e3a6ac32a 100644
> > --- a/drivers/clk/imx/Kconfig
> > +++ b/drivers/clk/imx/Kconfig
> > @@ -102,5 +102,6 @@ config CLK_IMX8QXP  config CLK_IMX8ULP
> >  	tristate "IMX8ULP CCM Clock Driver"
> >  	depends on ARCH_MXC || COMPILE_TEST
> > +	select RESET_CONTROLLER
> 
> This shouldn't be required anymore, devm_reset_controller_register() has a
> stub since commit 48a74b1147f7 ("reset: Add compile-test stubs").
> 

So we don't need to select 'RESET_CONTROLLER' explicitly, right?

> [...]
> > diff --git a/drivers/clk/imx/clk-imx8ulp.c
> > b/drivers/clk/imx/clk-imx8ulp.c index 6aad04114658..ea596cd6855a
> > 100644
> > --- a/drivers/clk/imx/clk-imx8ulp.c
> > +++ b/drivers/clk/imx/clk-imx8ulp.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/module.h>
> >  #include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/reset-controller.h>
> >  #include <linux/slab.h>
> >
> >  #include "clk.h"
> > @@ -48,6 +49,98 @@ static const char * const nic_per_divplat[] = {
> > "nic_per_divplat" };  static const char * const lpav_axi_div[] = {
> > "lpav_axi_div" };  static const char * const lpav_bus_div[] = {
> > "lpav_bus_div" };
> >
> > +struct pcc_reset_dev {
> > +	void __iomem *base;
> > +	struct reset_controller_dev rcdev;
> > +	const u32 *resets;
> > +	spinlock_t *lock;
> 
> I'd add a comment to this lock, stating that it is set to imx_ccm_lock and
> protects access to registers shared with clock control.
> 

Ok, will add some comments, thx.

BR
Jacky Bai

> With these addressed,
> 
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> regards
> Philipp
Philipp Zabel Aug. 25, 2021, 10:54 a.m. UTC | #3 
On Mon, 2021-08-23 at 23:58 +0000, Jacky Bai wrote:
[...]
> > > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index
> > > b81d6437ed95..0d1e3a6ac32a 100644
> > > --- a/drivers/clk/imx/Kconfig
> > > +++ b/drivers/clk/imx/Kconfig
> > > @@ -102,5 +102,6 @@ config CLK_IMX8QXP  config CLK_IMX8ULP
> > >  	tristate "IMX8ULP CCM Clock Driver"
> > >  	depends on ARCH_MXC || COMPILE_TEST
> > > +	select RESET_CONTROLLER
> > 
> > This shouldn't be required anymore, devm_reset_controller_register() has a
> > stub since commit 48a74b1147f7 ("reset: Add compile-test stubs").
> 
> So we don't need to select 'RESET_CONTROLLER' explicitly, right?

Right.

regards
Philipp