[0/5] Add reset support to EN7581 clk driver

Introduce reset-controller support to the Airoha EN7581 clock module.

Lorenzo Bianconi (5):
  dt-bindings: clock: airoha: Add reset support to EN7581 clock binding
  dt-bindings: reset: Add reset definitions for EN7581 SoC.
  arm64: dts: airoha: Add reset-controller support to EN7581 clock node
  clk: en7523: Add reset-controller support for EN7581 SoC
  clk: en7523: Remove pcie prepare/unpreare callbacks for EN7581 SoC

 .../bindings/clock/airoha,en7523-scu.yaml     |  21 +++
 arch/arm64/boot/dts/airoha/en7581.dtsi        |   2 +
 drivers/clk/clk-en7523.c                      | 137 ++++++++++++------
 .../dt-bindings/reset/airoha,en7581-reset.h   |  66 +++++++++
 4 files changed, 185 insertions(+), 41 deletions(-)
 create mode 100644 include/dt-bindings/reset/airoha,en7581-reset.h

Il 15/05/24 14:58, Lorenzo Bianconi ha scritto:
> Introduce reset API support to EN7581 clock driver.
> 
> Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Heh, that's exactly the usual MediaTek reset controller :-D

> ---
>   drivers/clk/clk-en7523.c | 96 +++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 94 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
> index 381605be333f..18798b692b68 100644
> --- a/drivers/clk/clk-en7523.c
> +++ b/drivers/clk/clk-en7523.c
> @@ -6,6 +6,7 @@
>   #include <linux/of.h>
>   #include <linux/of_device.h>
>   #include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
>   #include <dt-bindings/clock/en7523-clk.h>
>   
>   #define REG_PCI_CONTROL			0x88
> @@ -65,8 +66,18 @@ struct en_clk_gate {
>   	struct clk_hw hw;
>   };
>   
> +#define RST_NR_PER_BANK		32

Please move this definition at the beginning of this file, grouping that with
the others.

..snip...

> @@ -456,12 +542,14 @@ static int en7523_clk_probe(struct platform_device *pdev)
>   	en7523_register_clocks(&pdev->dev, clk_data, base, np_base);
>   
>   	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> -	if (r)
> +	if (r) {
>   		dev_err(&pdev->dev,
>   			"could not register clock provider: %s: %d\n",
>   			pdev->name, r);
> +		return r;
> +	}
>   
> -	return r;
> +	return en7523_reset_register(&pdev->dev, np_base, soc_data);

If en7523_reset_register fails, you want to call of_clk_del_provider(), so
you can't just return like this...

Cheers,
Angelo

>   }
>   
>   static const struct en_clk_soc_data en7523_data = {
> @@ -480,6 +568,10 @@ static const struct en_clk_soc_data en7581_data = {
>   		.unprepare = en7581_pci_unprepare,
>   		.disable = en7581_pci_disable,
>   	},
> +	.reset_data = {
> +		.base_addr = REG_RESET_CONTROL2,
> +		.n_banks = 2,
> +	},
>   	.hw_init = en7581_clk_hw_init,
>   };
>

Il 15/05/24 14:58, Lorenzo Bianconi ha scritto:
> Get rid of prepare and unpreare callbacks for PCIe clock since they can
> be modeled as a reset line cosumed by the PCIe driver
> (pcie-mediatek-gen3)
> 
> Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> Il 15/05/24 14:58, Lorenzo Bianconi ha scritto:
> > Introduce reset API support to EN7581 clock driver.
> > 
> > Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> Heh, that's exactly the usual MediaTek reset controller :-D
> 
> > ---
> >   drivers/clk/clk-en7523.c | 96 +++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 94 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
> > index 381605be333f..18798b692b68 100644
> > --- a/drivers/clk/clk-en7523.c
> > +++ b/drivers/clk/clk-en7523.c
> > @@ -6,6 +6,7 @@
> >   #include <linux/of.h>
> >   #include <linux/of_device.h>
> >   #include <linux/platform_device.h>
> > +#include <linux/reset-controller.h>
> >   #include <dt-bindings/clock/en7523-clk.h>
> >   #define REG_PCI_CONTROL			0x88
> > @@ -65,8 +66,18 @@ struct en_clk_gate {
> >   	struct clk_hw hw;
> >   };
> > +#define RST_NR_PER_BANK		32
> 
> Please move this definition at the beginning of this file, grouping that with
> the others.

ack, I will fix it.

> 
> ..snip...
> 
> > @@ -456,12 +542,14 @@ static int en7523_clk_probe(struct platform_device *pdev)
> >   	en7523_register_clocks(&pdev->dev, clk_data, base, np_base);
> >   	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> > -	if (r)
> > +	if (r) {
> >   		dev_err(&pdev->dev,
> >   			"could not register clock provider: %s: %d\n",
> >   			pdev->name, r);
> > +		return r;
> > +	}
> > -	return r;
> > +	return en7523_reset_register(&pdev->dev, np_base, soc_data);
> 
> If en7523_reset_register fails, you want to call of_clk_del_provider(), so
> you can't just return like this...

ack, I will fix it.

Regards,
Lorenzo

> 
> Cheers,
> Angelo
> 
> >   }
> >   static const struct en_clk_soc_data en7523_data = {
> > @@ -480,6 +568,10 @@ static const struct en_clk_soc_data en7581_data = {
> >   		.unprepare = en7581_pci_unprepare,
> >   		.disable = en7581_pci_disable,
> >   	},
> > +	.reset_data = {
> > +		.base_addr = REG_RESET_CONTROL2,
> > +		.n_banks = 2,
> > +	},
> >   	.hw_init = en7581_clk_hw_init,
> >   };
> 
> 
>