Add a new file "reset.h" to place some definitions for clock reset.

Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
---
 drivers/clk/mediatek/clk-mtk.h | 11 ++---------
 drivers/clk/mediatek/reset.h   | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 9 deletions(-)
 create mode 100644 drivers/clk/mediatek/reset.h

diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index 399f1b2dc7d0..a6d0f24c62fa 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -13,6 +13,8 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
+#include "reset.h"
+
 #define MAX_MUX_GATE_BIT	31
 #define INVALID_MUX_GATE_BIT	(MAX_MUX_GATE_BIT + 1)
 
@@ -178,12 +180,6 @@ struct mtk_clk_divider {
 		.div_width = _width,				\
 }
 
-enum mtk_reset_version {
-	MTK_RST_SIMPLE = 0,
-	MTK_RST_SET_CLR,
-	MTK_RST_MAX,
-};
-
 int mtk_clk_register_dividers(const struct mtk_clk_divider *mcds, int num,
 			      void __iomem *base, spinlock_t *lock,
 			      struct clk_onecell_data *clk_data);
@@ -196,9 +192,6 @@ void mtk_free_clk_data(struct clk_onecell_data *clk_data);
 struct clk *mtk_clk_register_ref2usb_tx(const char *name,
 			const char *parent_name, void __iomem *reg);
 
-void mtk_clk_register_rst_ctrl(struct device_node *np,
-			       u32 reg_num, u16 reg_ofs, u8 version);
-
 struct mtk_clk_desc {
 	const struct mtk_gate *clks;
 	size_t num_clks;
diff --git a/drivers/clk/mediatek/reset.h b/drivers/clk/mediatek/reset.h
new file mode 100644
index 000000000000..e4081c7217e3
--- /dev/null
+++ b/drivers/clk/mediatek/reset.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2022 MediaTek Inc.
+ */
+
+#ifndef __DRV_CLK_MTK_RESET_H
+#define __DRV_CLK_MTK_RESET_H
+
+#include <linux/types.h>
+
+enum mtk_reset_version {
+	MTK_RST_SIMPLE = 0,
+	MTK_RST_SET_CLR,
+	MTK_RST_MAX,
+};
+
+void mtk_clk_register_rst_ctrl(struct device_node *np,
+			       u32 reg_num, u16 reg_ofs, u8 version);
+
+#endif /* __DRV_CLK_MTK_RESET_H */
-- 
2.18.0

Il 20/04/22 15:05, Rex-BC Chen ha scritto:
> Add a new file "reset.h" to place some definitions for clock reset.
> 
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>

Right now, you're adding the enum mtk_reset_version and *then* you're
moving it to the new reset.h header, but does that really make sense?

I think that this series would be cleaner if you add this header from
the start, so that you place the aforementioned enumeration directly
in here...

...so we would have a commit that moves the mtk_clk_register_rst_ctrl()
function from clk-mtk.h to a newly created reset.h, mentioning in the
commit description that it's all about preparing for a coming cleanup,
then the addition of enum mtk_reset_version would be in
`clk: mediatek: reset: Merge and revise reset register function` directly
into reset.h.

Does that sound right to you?

Cheers,
Angelo

On Thu, 2022-04-21 at 11:07 +0200, AngeloGioacchino Del Regno wrote:
> Il 20/04/22 15:05, Rex-BC Chen ha scritto:
> > Add a new file "reset.h" to place some definitions for clock reset.
> > 
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> 
> Right now, you're adding the enum mtk_reset_version and *then* you're
> moving it to the new reset.h header, but does that really make sense?
> 
> I think that this series would be cleaner if you add this header from
> the start, so that you place the aforementioned enumeration directly
> in here...
> 
> ...so we would have a commit that moves the
> mtk_clk_register_rst_ctrl()
> function from clk-mtk.h to a newly created reset.h, mentioning in the
> commit description that it's all about preparing for a coming
> cleanup,
> then the addition of enum mtk_reset_version would be in
> `clk: mediatek: reset: Merge and revise reset register function`
> directly
> into reset.h.
> 
> Does that sound right to you?
> 
> Cheers,
> Angelo
> 

Hello Angelo,

yes, I think it's more reasonable to move this modification to the
first order of this series.
I will do this in next version.

BRs,
Rex

On Thu, Apr 21, 2022 at 5:07 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 20/04/22 15:05, Rex-BC Chen ha scritto:
> > Add a new file "reset.h" to place some definitions for clock reset.
> >
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
>
> Right now, you're adding the enum mtk_reset_version and *then* you're
> moving it to the new reset.h header, but does that really make sense?
>
> I think that this series would be cleaner if you add this header from
> the start, so that you place the aforementioned enumeration directly
> in here...
>
> ...so we would have a commit that moves the mtk_clk_register_rst_ctrl()
> function from clk-mtk.h to a newly created reset.h, mentioning in the
> commit description that it's all about preparing for a coming cleanup,
> then the addition of enum mtk_reset_version would be in
> `clk: mediatek: reset: Merge and revise reset register function` directly
> into reset.h.

And probably name it mtk-reset.h ? 'reset.h' is a bit too generic, and
I'm sure there are multiple files with the same name throughout the
kernel source tree.

ChenYu

On Thu, Apr 21, 2022 at 5:14 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Thu, Apr 21, 2022 at 5:07 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
> >
> > Il 20/04/22 15:05, Rex-BC Chen ha scritto:
> > > Add a new file "reset.h" to place some definitions for clock reset.
> > >
> > > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> >
> > Right now, you're adding the enum mtk_reset_version and *then* you're
> > moving it to the new reset.h header, but does that really make sense?
> >
> > I think that this series would be cleaner if you add this header from
> > the start, so that you place the aforementioned enumeration directly
> > in here...
> >
> > ...so we would have a commit that moves the mtk_clk_register_rst_ctrl()
> > function from clk-mtk.h to a newly created reset.h, mentioning in the
> > commit description that it's all about preparing for a coming cleanup,
> > then the addition of enum mtk_reset_version would be in
> > `clk: mediatek: reset: Merge and revise reset register function` directly
> > into reset.h.
>
> And probably name it mtk-reset.h ? 'reset.h' is a bit too generic, and
> I'm sure there are multiple files with the same name throughout the
> kernel source tree.

On second thought, please ignore my comment.

ChenYu