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@xxxxxxx>
> Signed-off-by: Jacky Bai <ping.bai@xxxxxxx>
> ---
> 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@xxxxxxxxxxxxxx>
regards
Philipp
