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@xxxxxxxxxxxx> > > Reviewed-by: AngeloGioacchino Del Regno < > > angelogioacchino.delregno@xxxxxxxxxxxxx> > > --- > > <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