Re: [PATCH v3 9/9] clk: imx: Add the pcc reset controller support on imx8ulp

[Abel Vesa <abel.vesa@nxp.com>]

On 21-09-14 14:52:08, Jacky Bai wrote:
> diff --git a/drivers/clk/imx/clk-imx8ulp.c b/drivers/clk/imx/clk-imx8ulp.c
> index 6aad04114658..6699437e17b8 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,99 @@ 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;
> +	/* Set to imx_ccm_lock to protect register access shared with clock control */
> +	spinlock_t *lock;
> +};
> +
> +#define PCC_SW_RST	BIT(28)
> +#define to_pcc_reset_dev(_rcdev)	container_of(_rcdev, struct pcc_reset_dev, rcdev)
> +
> +static const u32 pcc3_resets[] = {
> +	0xa8, 0xac, 0xc8, 0xcc, 0xd0,
> +	0xd4, 0xd8, 0xdc, 0xe0, 0xe4,
> +	0xe8, 0xec, 0xf0
> +};
> +
> +static const u32 pcc4_resets[] = {
> +	0x4, 0x8, 0xc, 0x10, 0x14,
> +	0x18, 0x1c, 0x20, 0x24, 0x34,
> +	0x38, 0x3c, 0x40, 0x44, 0x48,
> +	0x4c, 0x54
> +};
> +
> +static const u32 pcc5_resets[] = {
> +	0xa0, 0xa4, 0xa8, 0xac, 0xb0,
> +	0xb4, 0xbc, 0xc0, 0xc8, 0xcc,
> +	0xd0, 0xf0, 0xf4, 0xf8
> +};
> +

I believe this could be avoided entirely by having something like:

PCCx_RESETS_OFFSET + (index * 4)

and then:

#define PCC3_RESETS_OFFSET	0xa8
#define PCC4_RESETS_OFFSET	0x4
#define PCC5_RESETS_OFFSET	0xa0

#define PCC3_RESETS_NUM		13
#define PCC4_RESETS_NUM		17
#define PCC5_RESETS_NUM		14

Then we could pass on the PCCx_RESETS_OFFSET and the PCCx_RESETS_NUM and
that's it.

> +static int imx8ulp_pcc_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	struct pcc_reset_dev *pcc_reset = to_pcc_reset_dev(rcdev);
> +	u32 offset = pcc_reset->resets[id];
> +	unsigned long flags;
> +	u32 val;
> +
> +	spin_lock_irqsave(pcc_reset->lock, flags);
> +
> +	val = readl(pcc_reset->base + offset);
> +	val &= ~PCC_SW_RST;
> +	writel(val, pcc_reset->base + offset);
> +
> +	spin_unlock_irqrestore(pcc_reset->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int imx8ulp_pcc_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	struct pcc_reset_dev *pcc_reset = to_pcc_reset_dev(rcdev);
> +	u32 offset = pcc_reset->resets[id];
> +	unsigned long flags;
> +	u32 val;
> +
> +	spin_lock_irqsave(pcc_reset->lock, flags);
> +
> +	val = readl(pcc_reset->base + offset);
> +	val |= PCC_SW_RST;
> +	writel(val, pcc_reset->base + offset);
> +
> +	spin_unlock_irqrestore(pcc_reset->lock, flags);
> +
> +	return 0;
> +}
> +
> +static const struct reset_control_ops imx8ulp_pcc_reset_ops = {
> +	.assert = imx8ulp_pcc_assert,
> +	.deassert = imx8ulp_pcc_deassert,
> +};
> +
> +static int imx8ulp_pcc_reset_init(struct platform_device *pdev, void __iomem *base,
> +	 const u32 *resets, unsigned int nr_resets)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device *dev = &pdev->dev;
> +	struct pcc_reset_dev *pcc_reset;
> +
> +	pcc_reset = devm_kzalloc(dev, sizeof(*pcc_reset), GFP_KERNEL);
> +	if (!pcc_reset)
> +		return -ENOMEM;
> +
> +	pcc_reset->base = base;
> +	pcc_reset->lock = &imx_ccm_lock;
> +	pcc_reset->resets = resets;
> +	pcc_reset->rcdev.owner = THIS_MODULE;
> +	pcc_reset->rcdev.nr_resets = nr_resets;
> +	pcc_reset->rcdev.ops = &imx8ulp_pcc_reset_ops;
> +	pcc_reset->rcdev.of_node = np;
> +
> +	return devm_reset_controller_register(dev, &pcc_reset->rcdev);
> +}
> +

...

> -- 
> 2.26.2
>