[v7,00/19] Cleanup MediaTek clk reset drivers and support SoCs

In this series, we cleanup MediaTek clock reset drivers in clk/mediatek
folder. MediaTek clock reset driver is used to provide reset control
of modules controlled in clk, like infra_ao.

Changes for v7:
1. v7 is based on linux-next next-20220519 and Chen-Yu's series[1].
2. Add support for MT8186.

[1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=643003

Changes for v6:
1. Add a new patch to support inuput argument index mode.
2. Revise definition in reset.h to index.

Rex-BC Chen (19):
  clk: mediatek: reset: Add reset.h
  clk: mediatek: reset: Fix written reset bit offset
  clk: mediatek: reset: Refine and reorder functions in reset.c
  clk: mediatek: reset: Extract common drivers to update function
  clk: mediatek: reset: Merge and revise reset register function
  clk: mediatek: reset: Revise structure to control reset register
  clk: mediatek: reset: Support nonsequence base offsets of reset
    registers
  clk: mediatek: reset: Support inuput argument index mode
  clk: mediatek: reset: Change return type for clock reset register
    function
  clk: mediatek: reset: Add new register reset function with device
  clk: mediatek: reset: Add reset support for simple probe
  dt-bindings: arm: mediatek: Add #reset-cells property for
    MT8192/MT8195
  dt-bindings: reset: mediatek: Add infra_ao reset index for
    MT8192/MT8195
  clk: mediatek: reset: Add infra_ao reset support for MT8192/MT8195
  arm64: dts: mediatek: Add infra #reset-cells property for MT8192
  arm64: dts: mediatek: Add infra #reset-cells property for MT8195
  dt-bindings: reset: mediatek: Add infra_ao reset index for MT8186
  dt-bindings: arm: mediatek: Add #reset-cells property for MT8186
  clk: mediatek: reset: Add infra_ao reset support for MT8186

 .../mediatek/mediatek,mt8186-sys-clock.yaml   |   3 +
 .../mediatek/mediatek,mt8192-sys-clock.yaml   |   3 +
 .../mediatek/mediatek,mt8195-sys-clock.yaml   |   3 +
 arch/arm64/boot/dts/mediatek/mt8192.dtsi      |   1 +
 arch/arm64/boot/dts/mediatek/mt8195.dtsi      |  13 +-
 drivers/clk/mediatek/clk-mt2701-eth.c         |  10 +-
 drivers/clk/mediatek/clk-mt2701-g3d.c         |  10 +-
 drivers/clk/mediatek/clk-mt2701-hif.c         |  10 +-
 drivers/clk/mediatek/clk-mt2701.c             |  22 +-
 drivers/clk/mediatek/clk-mt2712.c             |  22 +-
 drivers/clk/mediatek/clk-mt7622-eth.c         |  10 +-
 drivers/clk/mediatek/clk-mt7622-hif.c         |  12 +-
 drivers/clk/mediatek/clk-mt7622.c             |  22 +-
 drivers/clk/mediatek/clk-mt7629-eth.c         |  10 +-
 drivers/clk/mediatek/clk-mt7629-hif.c         |  12 +-
 drivers/clk/mediatek/clk-mt8135.c             |  22 +-
 drivers/clk/mediatek/clk-mt8173.c             |  22 +-
 drivers/clk/mediatek/clk-mt8183.c             |  18 +-
 drivers/clk/mediatek/clk-mt8186-infra_ao.c    |  23 ++
 drivers/clk/mediatek/clk-mt8192.c             |  29 +++
 drivers/clk/mediatek/clk-mt8195-infra_ao.c    |  24 +++
 drivers/clk/mediatek/clk-mtk.c                |   7 +
 drivers/clk/mediatek/clk-mtk.h                |   9 +-
 drivers/clk/mediatek/reset.c                  | 198 +++++++++++++-----
 drivers/clk/mediatek/reset.h                  |  82 ++++++++
 include/dt-bindings/reset/mt8186-resets.h     |   5 +
 include/dt-bindings/reset/mt8192-resets.h     |   8 +
 include/dt-bindings/reset/mt8195-resets.h     |   6 +
 28 files changed, 522 insertions(+), 94 deletions(-)
 create mode 100644 drivers/clk/mediatek/reset.h

On Thu, May 19, 2022 at 08:55:12PM +0800, Rex-BC Chen wrote:
> To make drivers more clear and readable, we extract common code
> within assert and deassert to mtk_reset_update_set_clr() and
> mtk_reset_update().
> 
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/clk/mediatek/reset.c | 38 +++++++++++++++++++++---------------
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
> index 5cbbcc22a4fc..22fa9f09752c 100644
> --- a/drivers/clk/mediatek/reset.c
> +++ b/drivers/clk/mediatek/reset.c
> @@ -12,24 +12,27 @@
>  
>  #include "reset.h"
>  
> -static int mtk_reset_assert(struct reset_controller_dev *rcdev,
> -			    unsigned long id)
> +static int mtk_reset_update(struct reset_controller_dev *rcdev,
> +			    unsigned long id, bool deassert)

I'd have called the bool 'assert', and then passed true on assert and false on
deassert, as I think that's slightly more intuitive, but that's just personal
preference. It's fine like this as well.

Thanks,
Nícolas

>  {
>  	struct mtk_reset *data = container_of(rcdev, struct mtk_reset, rcdev);
> +	unsigned int val = deassert ? 0 : ~0;
>  
>  	return regmap_update_bits(data->regmap,
>  				  data->regofs + ((id / 32) << 2),
> -				  BIT(id % 32), ~0);
> +				  BIT(id % 32), val);
> +}
> +
> +static int mtk_reset_assert(struct reset_controller_dev *rcdev,
> +			    unsigned long id)
> +{
> +	return mtk_reset_update(rcdev, id, false);
>  }
>  
>  static int mtk_reset_deassert(struct reset_controller_dev *rcdev,
>  			      unsigned long id)
>  {
> -	struct mtk_reset *data = container_of(rcdev, struct mtk_reset, rcdev);
> -
> -	return regmap_update_bits(data->regmap,
> -				  data->regofs + ((id / 32) << 2),
> -				  BIT(id % 32), 0);
> +	return mtk_reset_update(rcdev, id, true);
>  }
>  
>  static int mtk_reset(struct reset_controller_dev *rcdev, unsigned long id)
> @@ -43,24 +46,27 @@ static int mtk_reset(struct reset_controller_dev *rcdev, unsigned long id)
>  	return mtk_reset_deassert(rcdev, id);
>  }
>  
> -static int mtk_reset_assert_set_clr(struct reset_controller_dev *rcdev,
> -				    unsigned long id)
> +static int mtk_reset_update_set_clr(struct reset_controller_dev *rcdev,
> +				    unsigned long id, bool deassert)
>  {
>  	struct mtk_reset *data = container_of(rcdev, struct mtk_reset, rcdev);
> +	unsigned int deassert_ofs = deassert ? 0x4 : 0;
>  
>  	return regmap_write(data->regmap,
> -			    data->regofs + ((id / 32) << 4),
> +			    data->regofs + ((id / 32) << 4) + deassert_ofs,
>  			    BIT(id % 32));
>  }
>  
> +static int mtk_reset_assert_set_clr(struct reset_controller_dev *rcdev,
> +				    unsigned long id)
> +{
> +	return mtk_reset_update_set_clr(rcdev, id, false);
> +}
> +
>  static int mtk_reset_deassert_set_clr(struct reset_controller_dev *rcdev,
>  				      unsigned long id)
>  {
> -	struct mtk_reset *data = container_of(rcdev, struct mtk_reset, rcdev);
> -
> -	return regmap_write(data->regmap,
> -			    data->regofs + ((id / 32) << 4) + 0x4,
> -			    BIT(id % 32));
> +	return mtk_reset_update_set_clr(rcdev, id, true);
>  }
>  
>  static int mtk_reset_set_clr(struct reset_controller_dev *rcdev,
> -- 
> 2.18.0
> 
>

Hi Rex,

On Thu, May 19, 2022 at 08:55:13PM +0800, Rex-BC Chen wrote:
> There are two versions for clock reset register control for MediaTek
> SoCs. The old hardware is one bit per reset control, and does not
> have separate registers for bit set, clear and read-back operations.
> This matches the scheme supported by the simple reset driver.
> 
> However, because we need to use different data structure from
> reset_simple_data, we can not use the operation of simple reset
> driver.
> For this reason, we keep the original functions and name this version
> as "MTK_RST_SIMPLE".
> 
> In this patch:
> - Add a version enumeration to separate different reset hardware.
> - Merge the reset register function of simple and set_clr into one
>   function "mtk_register_reset_controller".
> - Rename input variable "num_regs" to "rst_bank_nr" to avoid
>   confusion. This variable is used to define the quantity of reset bank.
> - Document mtk_reset_version and mtk_register_reset_controller.
> 
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---

<snip>

> index 764a8affe206..2a39eec9cff7 100644
> --- a/drivers/clk/mediatek/reset.h
> +++ b/drivers/clk/mediatek/reset.h
> @@ -9,16 +9,32 @@
>  #include <linux/reset-controller.h>
>  #include <linux/types.h>
>  
> +/**
> + * enum mtk_reset_version - Version of MediaTek clock reset controller.
> + * @MTK_RST_SIMPLE: Use the same registers for bit set and clear.
> + * @MTK_RST_SET_CLR: Use separate registers for bit set and clear.
> + * @MTK_RST_MAX: Total quantity of version for MediaTek clock reset controller.
> + */
> +enum mtk_reset_version {
> +	MTK_RST_SIMPLE = 0,
> +	MTK_RST_SET_CLR,
> +	MTK_RST_MAX,
> +};
> +
>  struct mtk_reset {
>  	struct regmap *regmap;
>  	int regofs;
>  	struct reset_controller_dev rcdev;
>  };
>  
> +/**
> + * mtk_register_reset_controller - Register MediaTek clock reset controller
> + * @np: Pointer to device node.
> + * @rst_bank_nr: Quantity of reset bank.
> + * @reg_ofs: Base offset of the reset register.
> + * @version: Version of MediaTek clock reset controller.
> + */
>  void mtk_register_reset_controller(struct device_node *np,
> -				   unsigned int num_regs, int regofs);
> -
> -void mtk_register_reset_controller_set_clr(struct device_node *np,
> -					   unsigned int num_regs, int regofs);
> +				   u32 rst_bank_nr, u16 reg_ofs, u8 version);

Why not use 'enum mtk_reset_version' instead of a generic u8? Same thing when
you move it to a struct in patch 6.

Thanks,
Nícolas

Hi Rex,

On Thu, May 19, 2022 at 08:55:15PM +0800, Rex-BC Chen wrote:
> The bank offsets are not serial for all reset registers.
> For example, there are five infra reset banks for MT8192: 0x120, 0x130,
> 0x140, 0x150 and 0x730.
> 
> To support this,
> - Change reg_ofs to rst_bank_ofs which is a pointer to base offsets of
>   the reset register.
> - Add a new define RST_NR_PER_BANK to define reset number for each
>   reset bank.
> 
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---

<snip>

> diff --git a/drivers/clk/mediatek/clk-mt2701-g3d.c b/drivers/clk/mediatek/clk-mt2701-g3d.c
> index 9cfd589939e5..5cbc5c42204d 100644
> --- a/drivers/clk/mediatek/clk-mt2701-g3d.c
> +++ b/drivers/clk/mediatek/clk-mt2701-g3d.c
> @@ -35,10 +35,12 @@ static const struct mtk_gate g3d_clks[] = {
>  	GATE_G3D(CLK_G3DSYS_CORE, "g3d_core", "mfg_sel", 0),
>  };
>  
> +static u16 rst_ofs[] = { 0xC, };

Very nitpicky, but you could have left the hex lowercase '0xc'.

Thanks,
Nícolas

Hi Rex,

On Thu, May 19, 2022 at 08:55:24PM +0800, Rex-BC Chen wrote:
> We will use mediatek clock reset as infracfg_ao reset instead of
> ti-syscon. To support this, remove property of ti reset and add
> property of #reset-cells for mediatek clock reset.
> 
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index b57e620c2c72..8e5ac11b19f1 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -10,7 +10,6 @@
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/phy/phy.h>
>  #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
> -#include <dt-bindings/reset/ti-syscon.h>
>  
>  / {
>  	compatible = "mediatek,mt8195";
> @@ -295,17 +294,7 @@
>  			compatible = "mediatek,mt8195-infracfg_ao", "syscon", "simple-mfd";

I believe the 'simple-mfd' compatible was added only to make the
reset-controller subnode probe (at least this was the case for mt8192), so it
might make sense to drop it here as well.

Thanks,
Nícolas

>  			reg = <0 0x10001000 0 0x1000>;
>  			#clock-cells = <1>;
> -
> -			infracfg_rst: reset-controller {
> -				compatible = "ti,syscon-reset";
> -				#reset-cells = <1>;
> -				ti,reset-bits = <
> -					0x140 18 0x144 18 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* pcie */
> -					0x120 0  0x124 0  0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* thermal */
> -					0x730 10 0x734 10 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* thermal */
> -					0x150 5  0x154 5  0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* svs gpu */
> -				>;
> -			};
> +			#reset-cells = <1>;
>  		};
>  
>  		pericfg: syscon@10003000 {
> -- 
> 2.18.0
> 
>

On Thu, May 19, 2022 at 08:55:08PM +0800, Rex-BC Chen wrote:
> In this series, we cleanup MediaTek clock reset drivers in clk/mediatek
> folder. MediaTek clock reset driver is used to provide reset control
> of modules controlled in clk, like infra_ao.
> 
> Changes for v7:
> 1. v7 is based on linux-next next-20220519 and Chen-Yu's series[1].
> 2. Add support for MT8186.
> 
> [1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=643003
> 
> Changes for v6:
> 1. Add a new patch to support inuput argument index mode.
> 2. Revise definition in reset.h to index.
> 
> Rex-BC Chen (19):
>   clk: mediatek: reset: Add reset.h
>   clk: mediatek: reset: Fix written reset bit offset
>   clk: mediatek: reset: Refine and reorder functions in reset.c
>   clk: mediatek: reset: Extract common drivers to update function
>   clk: mediatek: reset: Merge and revise reset register function
>   clk: mediatek: reset: Revise structure to control reset register
>   clk: mediatek: reset: Support nonsequence base offsets of reset
>     registers
>   clk: mediatek: reset: Support inuput argument index mode
>   clk: mediatek: reset: Change return type for clock reset register
>     function
>   clk: mediatek: reset: Add new register reset function with device
>   clk: mediatek: reset: Add reset support for simple probe
>   dt-bindings: arm: mediatek: Add #reset-cells property for
>     MT8192/MT8195
>   dt-bindings: reset: mediatek: Add infra_ao reset index for
>     MT8192/MT8195
>   clk: mediatek: reset: Add infra_ao reset support for MT8192/MT8195
>   arm64: dts: mediatek: Add infra #reset-cells property for MT8192
>   arm64: dts: mediatek: Add infra #reset-cells property for MT8195
>   dt-bindings: reset: mediatek: Add infra_ao reset index for MT8186
>   dt-bindings: arm: mediatek: Add #reset-cells property for MT8186
>   clk: mediatek: reset: Add infra_ao reset support for MT8186

For the whole series:

Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

And also

Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

on mt8192-asurada-spherion. PCIe resets work as intended by adding on the pcie
node 

+                       resets = <&infracfg MT8192_INFRA_RST2_PEXTP_PHY_SWRST>,
+                                <&infracfg MT8192_INFRA_RST4_PCIE_TOP_SWRST>;
+                       reset-names = "phy", "mac";

Thanks for the great work on this series, Rex!

Nícolas

On Fri, 2022-05-20 at 10:55 -0400, Nícolas F. R. A. Prado wrote:
> On Thu, May 19, 2022 at 08:55:12PM +0800, Rex-BC Chen wrote:
> > To make drivers more clear and readable, we extract common code
> > within assert and deassert to mtk_reset_update_set_clr() and
> > mtk_reset_update().
> > 
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > ---
> >  drivers/clk/mediatek/reset.c | 38 +++++++++++++++++++++-----------
> > ----
> >  1 file changed, 22 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/clk/mediatek/reset.c
> > b/drivers/clk/mediatek/reset.c
> > index 5cbbcc22a4fc..22fa9f09752c 100644
> > --- a/drivers/clk/mediatek/reset.c
> > +++ b/drivers/clk/mediatek/reset.c
> > @@ -12,24 +12,27 @@
> >  
> >  #include "reset.h"
> >  
> > -static int mtk_reset_assert(struct reset_controller_dev *rcdev,
> > -			    unsigned long id)
> > +static int mtk_reset_update(struct reset_controller_dev *rcdev,
> > +			    unsigned long id, bool deassert)
> 
> I'd have called the bool 'assert', and then passed true on assert and
> false on
> deassert, as I think that's slightly more intuitive, but that's just
> personal
> preference. It's fine like this as well.
> 
> Thanks,
> Nícolas
> 

Hello Nícolas,

Thanks for your advice, but I think I will keep the original logic in
next version.

BRs,
Rex


> >  {
> >  	struct mtk_reset *data = container_of(rcdev, struct mtk_reset,
> > rcdev);
> > +	unsigned int val = deassert ? 0 : ~0;
> >  
> >  	return regmap_update_bits(data->regmap,
> >  				  data->regofs + ((id / 32) << 2),
> > -				  BIT(id % 32), ~0);
> > +				  BIT(id % 32), val);
> > +}
> > +
> > +static int mtk_reset_assert(struct reset_controller_dev *rcdev,
> > +			    unsigned long id)
> > +{
> > +	return mtk_reset_update(rcdev, id, false);
> >  }
> >  
> >  static int mtk_reset_deassert(struct reset_controller_dev *rcdev,
> >  			      unsigned long id)
> >  {
> > -	struct mtk_reset *data = container_of(rcdev, struct mtk_reset,
> > rcdev);
> > -
> > -	return regmap_update_bits(data->regmap,
> > -				  data->regofs + ((id / 32) << 2),
> > -				  BIT(id % 32), 0);
> > +	return mtk_reset_update(rcdev, id, true);
> >  }
> >  
> >  static int mtk_reset(struct reset_controller_dev *rcdev, unsigned
> > long id)
> > @@ -43,24 +46,27 @@ static int mtk_reset(struct
> > reset_controller_dev *rcdev, unsigned long id)
> >  	return mtk_reset_deassert(rcdev, id);
> >  }
> >  
> > -static int mtk_reset_assert_set_clr(struct reset_controller_dev
> > *rcdev,
> > -				    unsigned long id)
> > +static int mtk_reset_update_set_clr(struct reset_controller_dev
> > *rcdev,
> > +				    unsigned long id, bool deassert)
> >  {
> >  	struct mtk_reset *data = container_of(rcdev, struct mtk_reset,
> > rcdev);
> > +	unsigned int deassert_ofs = deassert ? 0x4 : 0;
> >  
> >  	return regmap_write(data->regmap,
> > -			    data->regofs + ((id / 32) << 4),
> > +			    data->regofs + ((id / 32) << 4) +
> > deassert_ofs,
> >  			    BIT(id % 32));
> >  }
> >  
> > +static int mtk_reset_assert_set_clr(struct reset_controller_dev
> > *rcdev,
> > +				    unsigned long id)
> > +{
> > +	return mtk_reset_update_set_clr(rcdev, id, false);
> > +}
> > +
> >  static int mtk_reset_deassert_set_clr(struct reset_controller_dev
> > *rcdev,
> >  				      unsigned long id)
> >  {
> > -	struct mtk_reset *data = container_of(rcdev, struct mtk_reset,
> > rcdev);
> > -
> > -	return regmap_write(data->regmap,
> > -			    data->regofs + ((id / 32) << 4) + 0x4,
> > -			    BIT(id % 32));
> > +	return mtk_reset_update_set_clr(rcdev, id, true);
> >  }
> >  
> >  static int mtk_reset_set_clr(struct reset_controller_dev *rcdev,
> > -- 
> > 2.18.0
> > 
> >

On Fri, 2022-05-20 at 11:12 -0400, Nícolas F. R. A. Prado wrote:
> Hi Rex,
> 
> On Thu, May 19, 2022 at 08:55:13PM +0800, Rex-BC Chen wrote:
> > There are two versions for clock reset register control for
> > MediaTek
> > SoCs. The old hardware is one bit per reset control, and does not
> > have separate registers for bit set, clear and read-back
> > operations.
> > This matches the scheme supported by the simple reset driver.
> > 
> > However, because we need to use different data structure from
> > reset_simple_data, we can not use the operation of simple reset
> > driver.
> > For this reason, we keep the original functions and name this
> > version
> > as "MTK_RST_SIMPLE".
> > 
> > In this patch:
> > - Add a version enumeration to separate different reset hardware.
> > - Merge the reset register function of simple and set_clr into one
> >   function "mtk_register_reset_controller".
> > - Rename input variable "num_regs" to "rst_bank_nr" to avoid
> >   confusion. This variable is used to define the quantity of reset
> > bank.
> > - Document mtk_reset_version and mtk_register_reset_controller.
> > 
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > ---
> 
> <snip>
> 
> > index 764a8affe206..2a39eec9cff7 100644
> > --- a/drivers/clk/mediatek/reset.h
> > +++ b/drivers/clk/mediatek/reset.h
> > @@ -9,16 +9,32 @@
> >  #include <linux/reset-controller.h>
> >  #include <linux/types.h>
> >  
> > +/**
> > + * enum mtk_reset_version - Version of MediaTek clock reset
> > controller.
> > + * @MTK_RST_SIMPLE: Use the same registers for bit set and clear.
> > + * @MTK_RST_SET_CLR: Use separate registers for bit set and clear.
> > + * @MTK_RST_MAX: Total quantity of version for MediaTek clock
> > reset controller.
> > + */
> > +enum mtk_reset_version {
> > +	MTK_RST_SIMPLE = 0,
> > +	MTK_RST_SET_CLR,
> > +	MTK_RST_MAX,
> > +};
> > +
> >  struct mtk_reset {
> >  	struct regmap *regmap;
> >  	int regofs;
> >  	struct reset_controller_dev rcdev;
> >  };
> >  
> > +/**
> > + * mtk_register_reset_controller - Register MediaTek clock reset
> > controller
> > + * @np: Pointer to device node.
> > + * @rst_bank_nr: Quantity of reset bank.
> > + * @reg_ofs: Base offset of the reset register.
> > + * @version: Version of MediaTek clock reset controller.
> > + */
> >  void mtk_register_reset_controller(struct device_node *np,
> > -				   unsigned int num_regs, int regofs);
> > -
> > -void mtk_register_reset_controller_set_clr(struct device_node *np,
> > -					   unsigned int num_regs, int
> > regofs);
> > +				   u32 rst_bank_nr, u16 reg_ofs, u8
> > version);
> 
> Why not use 'enum mtk_reset_version' instead of a generic u8? Same
> thing when
> you move it to a struct in patch 6.
> 
> Thanks,
> Nícolas

HEllo Nícolas,

Thanks for the review.
I will modify for both patch 5 and 6 for this in next version.

BRs,
Rex

On Fri, 2022-05-20 at 11:18 -0400, Nícolas F. R. A. Prado wrote:
> Hi Rex,
> 
> On Thu, May 19, 2022 at 08:55:15PM +0800, Rex-BC Chen wrote:
> > The bank offsets are not serial for all reset registers.
> > For example, there are five infra reset banks for MT8192: 0x120,
> > 0x130,
> > 0x140, 0x150 and 0x730.
> > 
> > To support this,
> > - Change reg_ofs to rst_bank_ofs which is a pointer to base offsets
> > of
> >   the reset register.
> > - Add a new define RST_NR_PER_BANK to define reset number for each
> >   reset bank.
> > 
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > ---
> 
> <snip>
> 
> > diff --git a/drivers/clk/mediatek/clk-mt2701-g3d.c
> > b/drivers/clk/mediatek/clk-mt2701-g3d.c
> > index 9cfd589939e5..5cbc5c42204d 100644
> > --- a/drivers/clk/mediatek/clk-mt2701-g3d.c
> > +++ b/drivers/clk/mediatek/clk-mt2701-g3d.c
> > @@ -35,10 +35,12 @@ static const struct mtk_gate g3d_clks[] = {
> >  	GATE_G3D(CLK_G3DSYS_CORE, "g3d_core", "mfg_sel", 0),
> >  };
> >  
> > +static u16 rst_ofs[] = { 0xC, };
> 
> Very nitpicky, but you could have left the hex lowercase '0xc'.
> 
> Thanks,
> Nícolas

Hello Nícolas,

ok, I will modify it in next version.

BRs,
Rex

On Fri, 2022-05-20 at 23:30 +0800, Nícolas F. R. A. Prado wrote:
> Hi Rex,
> 
> On Thu, May 19, 2022 at 08:55:24PM +0800, Rex-BC Chen wrote:
> > We will use mediatek clock reset as infracfg_ao reset instead of
> > ti-syscon. To support this, remove property of ti reset and add
> > property of #reset-cells for mediatek clock reset.
> > 
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 13 +------------
> >  1 file changed, 1 insertion(+), 12 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > index b57e620c2c72..8e5ac11b19f1 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > @@ -10,7 +10,6 @@
> >  #include <dt-bindings/interrupt-controller/irq.h>
> >  #include <dt-bindings/phy/phy.h>
> >  #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
> > -#include <dt-bindings/reset/ti-syscon.h>
> >  
> >  / {
> >  	compatible = "mediatek,mt8195";
> > @@ -295,17 +294,7 @@
> >  			compatible = "mediatek,mt8195-infracfg_ao",
> > "syscon", "simple-mfd";
> 
> I believe the 'simple-mfd' compatible was added only to make the
> reset-controller subnode probe (at least this was the case for
> mt8192), so it
> might make sense to drop it here as well.
> 
> Thanks,
> Nícolas
> 

Hello Nícolas,
ok, I will drop 'simple-mfd' in next version.

BRs,
Rex

> >  			reg = <0 0x10001000 0 0x1000>;
> >  			#clock-cells = <1>;
> > -
> > -			infracfg_rst: reset-controller {
> > -				compatible = "ti,syscon-reset";
> > -				#reset-cells = <1>;
> > -				ti,reset-bits = <
> > -					0x140 18 0x144 18 0 0
> > (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* pcie */
> > -					0x120 0  0x124 0  0 0
> > (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* thermal */
> > -					0x730 10 0x734 10 0 0
> > (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* thermal */
> > -					0x150 5  0x154 5  0 0
> > (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* svs gpu */
> > -				>;
> > -			};
> > +			#reset-cells = <1>;
> >  		};
> >  
> >  		pericfg: syscon@10003000 {
> > -- 
> > 2.18.0
> > 
> >

On Fri, 2022-05-20 at 11:40 -0400, Nícolas F. R. A. Prado wrote:
> On Thu, May 19, 2022 at 08:55:08PM +0800, Rex-BC Chen wrote:
> > In this series, we cleanup MediaTek clock reset drivers in
> > clk/mediatek
> > folder. MediaTek clock reset driver is used to provide reset
> > control
> > of modules controlled in clk, like infra_ao.
> > 
> > Changes for v7:
> > 1. v7 is based on linux-next next-20220519 and Chen-Yu's series[1].
> > 2. Add support for MT8186.
> > 
> > [1]: 
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/list/?series=643003__;!!CTRNKA9wMg0ARbw!3N_R1R2qXYd-vLUE-Bzrc1ZD_39liFO6Vz_RyJdPiuAyoMHO4TuaoOwWk1ka50bVe_0fdyJakB-FzCLGjzdVtsd6sQ$
> >  
> > 
> > Changes for v6:
> > 1. Add a new patch to support inuput argument index mode.
> > 2. Revise definition in reset.h to index.
> > 
> > Rex-BC Chen (19):
> >   clk: mediatek: reset: Add reset.h
> >   clk: mediatek: reset: Fix written reset bit offset
> >   clk: mediatek: reset: Refine and reorder functions in reset.c
> >   clk: mediatek: reset: Extract common drivers to update function
> >   clk: mediatek: reset: Merge and revise reset register function
> >   clk: mediatek: reset: Revise structure to control reset register
> >   clk: mediatek: reset: Support nonsequence base offsets of reset
> >     registers
> >   clk: mediatek: reset: Support inuput argument index mode
> >   clk: mediatek: reset: Change return type for clock reset register
> >     function
> >   clk: mediatek: reset: Add new register reset function with device
> >   clk: mediatek: reset: Add reset support for simple probe
> >   dt-bindings: arm: mediatek: Add #reset-cells property for
> >     MT8192/MT8195
> >   dt-bindings: reset: mediatek: Add infra_ao reset index for
> >     MT8192/MT8195
> >   clk: mediatek: reset: Add infra_ao reset support for
> > MT8192/MT8195
> >   arm64: dts: mediatek: Add infra #reset-cells property for MT8192
> >   arm64: dts: mediatek: Add infra #reset-cells property for MT8195
> >   dt-bindings: reset: mediatek: Add infra_ao reset index for MT8186
> >   dt-bindings: arm: mediatek: Add #reset-cells property for MT8186
> >   clk: mediatek: reset: Add infra_ao reset support for MT8186
> 
> For the whole series:
> 
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> And also
> 
> Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> on mt8192-asurada-spherion. PCIe resets work as intended by adding on
> the pcie
> node 
> 
> +                       resets = <&infracfg
> MT8192_INFRA_RST2_PEXTP_PHY_SWRST>,
> +                                <&infracfg
> MT8192_INFRA_RST4_PCIE_TOP_SWRST>;
> +                       reset-names = "phy", "mac";
> 
> Thanks for the great work on this series, Rex!
> 
> Nícolas

Hello Nícolas,

Thanks for helping me to test this series in MT8192.

BRs,
Rex