diff mbox series

[v5,2/5] clk: imx: Mark dram pll on 8mm and 8mn with CLK_GET_RATE_NOCACHE

Message ID 65d08f34741f1ffa94a53bc128433e6c958091d2.1573595319.git.leonard.crestez@nxp.com (mailing list archive)
State Superseded
Headers show
Series PM / devfreq: Add dynamic scaling for imx8m ddr controller | expand

Commit Message

Leonard Crestez Nov. 12, 2019, 9:50 p.m. UTC
DRAM frequency switches are executed in firmware and can change the
configuration of the DRAM PLL outside linux. Mark these CLKs with
CLK_GET_RATE_NOCACHE so we always read back the PLL config registers and
recalculate rates.

In current DRAM frequency tables on 8mm/8mn only the maximum frequency
uses the PLL so it's always configured in the same way. However reading
back the PLL configuration is the correct behavior and allows additional
setpoints in the future.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/clk/imx/clk-imx8mm.c  | 2 +-
 drivers/clk/imx/clk-imx8mn.c  | 2 +-
 drivers/clk/imx/clk-pll14xx.c | 7 +++++++
 drivers/clk/imx/clk.h         | 1 +
 4 files changed, 10 insertions(+), 2 deletions(-)

Comments

Peng Fan Nov. 13, 2019, 7:29 a.m. UTC | #1
Hi Leonard,

> Subject: [PATCH v5 2/5] clk: imx: Mark dram pll on 8mm and 8mn with
> CLK_GET_RATE_NOCACHE

This patch will conflict with https://patchwork.kernel.org/cover/11224933/
And I just post a new patch https://patchwork.kernel.org/patch/11241231/
 
Then no need add imx_1443x_dram_pll

Regards,
Peng.

> 
> DRAM frequency switches are executed in firmware and can change the
> configuration of the DRAM PLL outside linux. Mark these CLKs with
> CLK_GET_RATE_NOCACHE so we always read back the PLL config registers
> and recalculate rates.
> 
> In current DRAM frequency tables on 8mm/8mn only the maximum frequency
> uses the PLL so it's always configured in the same way. However reading back
> the PLL configuration is the correct behavior and allows additional setpoints in
> the future.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Reviewed-by: Abel Vesa <abel.vesa@nxp.com>
> ---
>  drivers/clk/imx/clk-imx8mm.c  | 2 +-
>  drivers/clk/imx/clk-imx8mn.c  | 2 +-
>  drivers/clk/imx/clk-pll14xx.c | 7 +++++++
>  drivers/clk/imx/clk.h         | 1 +
>  4 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
> index e2bc3c90d93c..9246e89bb5fd 100644
> --- a/drivers/clk/imx/clk-imx8mm.c
> +++ b/drivers/clk/imx/clk-imx8mm.c
> @@ -326,11 +326,11 @@ static int imx8mm_clocks_probe(struct
> platform_device *pdev)
>  	clks[IMX8MM_SYS_PLL3_REF_SEL] = imx_clk_mux("sys_pll3_ref_sel",
> base + 0x114, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
> 
>  	clks[IMX8MM_AUDIO_PLL1] = imx_clk_pll14xx("audio_pll1",
> "audio_pll1_ref_sel", base, &imx_1443x_pll);
>  	clks[IMX8MM_AUDIO_PLL2] = imx_clk_pll14xx("audio_pll2",
> "audio_pll2_ref_sel", base + 0x14, &imx_1443x_pll);
>  	clks[IMX8MM_VIDEO_PLL1] = imx_clk_pll14xx("video_pll1",
> "video_pll1_ref_sel", base + 0x28, &imx_1443x_pll);
> -	clks[IMX8MM_DRAM_PLL] = imx_clk_pll14xx("dram_pll",
> "dram_pll_ref_sel", base + 0x50, &imx_1443x_pll);
> +	clks[IMX8MM_DRAM_PLL] = imx_clk_pll14xx("dram_pll",
> +"dram_pll_ref_sel", base + 0x50, &imx_1443x_dram_pll);
>  	clks[IMX8MM_GPU_PLL] = imx_clk_pll14xx("gpu_pll", "gpu_pll_ref_sel",
> base + 0x64, &imx_1416x_pll);
>  	clks[IMX8MM_VPU_PLL] = imx_clk_pll14xx("vpu_pll", "vpu_pll_ref_sel",
> base + 0x74, &imx_1416x_pll);
>  	clks[IMX8MM_ARM_PLL] = imx_clk_pll14xx("arm_pll", "arm_pll_ref_sel",
> base + 0x84, &imx_1416x_pll);
>  	clks[IMX8MM_SYS_PLL1] = imx_clk_fixed("sys_pll1", 800000000);
>  	clks[IMX8MM_SYS_PLL2] = imx_clk_fixed("sys_pll2", 1000000000); diff
> --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c index
> de905e278b80..4749beab9fc8 100644
> --- a/drivers/clk/imx/clk-imx8mn.c
> +++ b/drivers/clk/imx/clk-imx8mn.c
> @@ -323,11 +323,11 @@ static int imx8mn_clocks_probe(struct
> platform_device *pdev)
>  	clks[IMX8MN_SYS_PLL3_REF_SEL] = imx_clk_mux("sys_pll3_ref_sel",
> base + 0x114, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
> 
>  	clks[IMX8MN_AUDIO_PLL1] = imx_clk_pll14xx("audio_pll1",
> "audio_pll1_ref_sel", base, &imx_1443x_pll);
>  	clks[IMX8MN_AUDIO_PLL2] = imx_clk_pll14xx("audio_pll2",
> "audio_pll2_ref_sel", base + 0x14, &imx_1443x_pll);
>  	clks[IMX8MN_VIDEO_PLL1] = imx_clk_pll14xx("video_pll1",
> "video_pll1_ref_sel", base + 0x28, &imx_1443x_pll);
> -	clks[IMX8MN_DRAM_PLL] = imx_clk_pll14xx("dram_pll",
> "dram_pll_ref_sel", base + 0x50, &imx_1443x_pll);
> +	clks[IMX8MN_DRAM_PLL] = imx_clk_pll14xx("dram_pll",
> +"dram_pll_ref_sel", base + 0x50, &imx_1443x_dram_pll);
>  	clks[IMX8MN_GPU_PLL] = imx_clk_pll14xx("gpu_pll", "gpu_pll_ref_sel",
> base + 0x64, &imx_1416x_pll);
>  	clks[IMX8MN_VPU_PLL] = imx_clk_pll14xx("vpu_pll", "vpu_pll_ref_sel",
> base + 0x74, &imx_1416x_pll);
>  	clks[IMX8MN_ARM_PLL] = imx_clk_pll14xx("arm_pll", "arm_pll_ref_sel",
> base + 0x84, &imx_1416x_pll);
>  	clks[IMX8MN_SYS_PLL1] = imx_clk_fixed("sys_pll1", 800000000);
>  	clks[IMX8MN_SYS_PLL2] = imx_clk_fixed("sys_pll2", 1000000000); diff
> --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c index
> 5c458199060a..a6d31a7262ef 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -65,10 +65,17 @@ struct imx_pll14xx_clk imx_1443x_pll = {
>  	.type = PLL_1443X,
>  	.rate_table = imx_pll1443x_tbl,
>  	.rate_count = ARRAY_SIZE(imx_pll1443x_tbl),  };
> 
> +struct imx_pll14xx_clk imx_1443x_dram_pll = {
> +	.type = PLL_1443X,
> +	.rate_table = imx_pll1443x_tbl,
> +	.rate_count = ARRAY_SIZE(imx_pll1443x_tbl),
> +	.flags = CLK_GET_RATE_NOCACHE,
> +};
> +
>  struct imx_pll14xx_clk imx_1416x_pll = {
>  	.type = PLL_1416X,
>  	.rate_table = imx_pll1416x_tbl,
>  	.rate_count = ARRAY_SIZE(imx_pll1416x_tbl),  }; diff --git
> a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h index
> bc5bb6ac8636..81122c9ab842 100644
> --- a/drivers/clk/imx/clk.h
> +++ b/drivers/clk/imx/clk.h
> @@ -50,10 +50,11 @@ struct imx_pll14xx_clk {
>  	int flags;
>  };
> 
>  extern struct imx_pll14xx_clk imx_1416x_pll;  extern struct
> imx_pll14xx_clk imx_1443x_pll;
> +extern struct imx_pll14xx_clk imx_1443x_dram_pll;
> 
>  #define imx_clk_cpu(name, parent_name, div, mux, pll, step) \
>  	imx_clk_hw_cpu(name, parent_name, div, mux, pll, step)->clk
> 
>  #define clk_register_gate2(dev, name, parent_name, flags, reg, bit_idx, \
> --
> 2.17.1
Leonard Crestez Nov. 13, 2019, 12:02 p.m. UTC | #2
On 13.11.2019 09:29, Peng Fan wrote:
> Hi Leonard,
> 
>> Subject: [PATCH v5 2/5] clk: imx: Mark dram pll on 8mm and 8mn with
>> CLK_GET_RATE_NOCACHE
> 
> This patch will conflict with https://patchwork.kernel.org/cover/11224933/
> And I just post a new patch https://patchwork.kernel.org/patch/11241231/
>   
> Then no need add imx_1443x_dram_pll

I saw those patches and the conflicts are minor (API cleanups, no 
functionality changes).

I usually send patches against latest linux-next/master and this usually 
includes all accepted patches. If after the clk_hw refactorings are 
accepted I will rebase and resend.

> Regards,
> Peng.
> 
>>
>> DRAM frequency switches are executed in firmware and can change the
>> configuration of the DRAM PLL outside linux. Mark these CLKs with
>> CLK_GET_RATE_NOCACHE so we always read back the PLL config registers
>> and recalculate rates.
>>
>> In current DRAM frequency tables on 8mm/8mn only the maximum frequency
>> uses the PLL so it's always configured in the same way. However reading back
>> the PLL configuration is the correct behavior and allows additional setpoints in
>> the future.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>> Reviewed-by: Abel Vesa <abel.vesa@nxp.com>
>> ---
>>   drivers/clk/imx/clk-imx8mm.c  | 2 +-
>>   drivers/clk/imx/clk-imx8mn.c  | 2 +-
>>   drivers/clk/imx/clk-pll14xx.c | 7 +++++++
>>   drivers/clk/imx/clk.h         | 1 +
>>   4 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
>> index e2bc3c90d93c..9246e89bb5fd 100644
>> --- a/drivers/clk/imx/clk-imx8mm.c
>> +++ b/drivers/clk/imx/clk-imx8mm.c
>> @@ -326,11 +326,11 @@ static int imx8mm_clocks_probe(struct
>> platform_device *pdev)
>>   	clks[IMX8MM_SYS_PLL3_REF_SEL] = imx_clk_mux("sys_pll3_ref_sel",
>> base + 0x114, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
>>
>>   	clks[IMX8MM_AUDIO_PLL1] = imx_clk_pll14xx("audio_pll1",
>> "audio_pll1_ref_sel", base, &imx_1443x_pll);
>>   	clks[IMX8MM_AUDIO_PLL2] = imx_clk_pll14xx("audio_pll2",
>> "audio_pll2_ref_sel", base + 0x14, &imx_1443x_pll);
>>   	clks[IMX8MM_VIDEO_PLL1] = imx_clk_pll14xx("video_pll1",
>> "video_pll1_ref_sel", base + 0x28, &imx_1443x_pll);
>> -	clks[IMX8MM_DRAM_PLL] = imx_clk_pll14xx("dram_pll",
>> "dram_pll_ref_sel", base + 0x50, &imx_1443x_pll);
>> +	clks[IMX8MM_DRAM_PLL] = imx_clk_pll14xx("dram_pll",
>> +"dram_pll_ref_sel", base + 0x50, &imx_1443x_dram_pll);
>>   	clks[IMX8MM_GPU_PLL] = imx_clk_pll14xx("gpu_pll", "gpu_pll_ref_sel",
>> base + 0x64, &imx_1416x_pll);
>>   	clks[IMX8MM_VPU_PLL] = imx_clk_pll14xx("vpu_pll", "vpu_pll_ref_sel",
>> base + 0x74, &imx_1416x_pll);
>>   	clks[IMX8MM_ARM_PLL] = imx_clk_pll14xx("arm_pll", "arm_pll_ref_sel",
>> base + 0x84, &imx_1416x_pll);
>>   	clks[IMX8MM_SYS_PLL1] = imx_clk_fixed("sys_pll1", 800000000);
>>   	clks[IMX8MM_SYS_PLL2] = imx_clk_fixed("sys_pll2", 1000000000); diff
>> --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c index
>> de905e278b80..4749beab9fc8 100644
>> --- a/drivers/clk/imx/clk-imx8mn.c
>> +++ b/drivers/clk/imx/clk-imx8mn.c
>> @@ -323,11 +323,11 @@ static int imx8mn_clocks_probe(struct
>> platform_device *pdev)
>>   	clks[IMX8MN_SYS_PLL3_REF_SEL] = imx_clk_mux("sys_pll3_ref_sel",
>> base + 0x114, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
>>
>>   	clks[IMX8MN_AUDIO_PLL1] = imx_clk_pll14xx("audio_pll1",
>> "audio_pll1_ref_sel", base, &imx_1443x_pll);
>>   	clks[IMX8MN_AUDIO_PLL2] = imx_clk_pll14xx("audio_pll2",
>> "audio_pll2_ref_sel", base + 0x14, &imx_1443x_pll);
>>   	clks[IMX8MN_VIDEO_PLL1] = imx_clk_pll14xx("video_pll1",
>> "video_pll1_ref_sel", base + 0x28, &imx_1443x_pll);
>> -	clks[IMX8MN_DRAM_PLL] = imx_clk_pll14xx("dram_pll",
>> "dram_pll_ref_sel", base + 0x50, &imx_1443x_pll);
>> +	clks[IMX8MN_DRAM_PLL] = imx_clk_pll14xx("dram_pll",
>> +"dram_pll_ref_sel", base + 0x50, &imx_1443x_dram_pll);
>>   	clks[IMX8MN_GPU_PLL] = imx_clk_pll14xx("gpu_pll", "gpu_pll_ref_sel",
>> base + 0x64, &imx_1416x_pll);
>>   	clks[IMX8MN_VPU_PLL] = imx_clk_pll14xx("vpu_pll", "vpu_pll_ref_sel",
>> base + 0x74, &imx_1416x_pll);
>>   	clks[IMX8MN_ARM_PLL] = imx_clk_pll14xx("arm_pll", "arm_pll_ref_sel",
>> base + 0x84, &imx_1416x_pll);
>>   	clks[IMX8MN_SYS_PLL1] = imx_clk_fixed("sys_pll1", 800000000);
>>   	clks[IMX8MN_SYS_PLL2] = imx_clk_fixed("sys_pll2", 1000000000); diff
>> --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c index
>> 5c458199060a..a6d31a7262ef 100644
>> --- a/drivers/clk/imx/clk-pll14xx.c
>> +++ b/drivers/clk/imx/clk-pll14xx.c
>> @@ -65,10 +65,17 @@ struct imx_pll14xx_clk imx_1443x_pll = {
>>   	.type = PLL_1443X,
>>   	.rate_table = imx_pll1443x_tbl,
>>   	.rate_count = ARRAY_SIZE(imx_pll1443x_tbl),  };
>>
>> +struct imx_pll14xx_clk imx_1443x_dram_pll = {
>> +	.type = PLL_1443X,
>> +	.rate_table = imx_pll1443x_tbl,
>> +	.rate_count = ARRAY_SIZE(imx_pll1443x_tbl),
>> +	.flags = CLK_GET_RATE_NOCACHE,
>> +};
>> +
>>   struct imx_pll14xx_clk imx_1416x_pll = {
>>   	.type = PLL_1416X,
>>   	.rate_table = imx_pll1416x_tbl,
>>   	.rate_count = ARRAY_SIZE(imx_pll1416x_tbl),  }; diff --git
>> a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h index
>> bc5bb6ac8636..81122c9ab842 100644
>> --- a/drivers/clk/imx/clk.h
>> +++ b/drivers/clk/imx/clk.h
>> @@ -50,10 +50,11 @@ struct imx_pll14xx_clk {
>>   	int flags;
>>   };
>>
>>   extern struct imx_pll14xx_clk imx_1416x_pll;  extern struct
>> imx_pll14xx_clk imx_1443x_pll;
>> +extern struct imx_pll14xx_clk imx_1443x_dram_pll;
>>
>>   #define imx_clk_cpu(name, parent_name, div, mux, pll, step) \
>>   	imx_clk_hw_cpu(name, parent_name, div, mux, pll, step)->clk
>>
>>   #define clk_register_gate2(dev, name, parent_name, flags, reg, bit_idx, \
>> --
>> 2.17.1
> 
>
diff mbox series

Patch

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index e2bc3c90d93c..9246e89bb5fd 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -326,11 +326,11 @@  static int imx8mm_clocks_probe(struct platform_device *pdev)
 	clks[IMX8MM_SYS_PLL3_REF_SEL] = imx_clk_mux("sys_pll3_ref_sel", base + 0x114, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
 
 	clks[IMX8MM_AUDIO_PLL1] = imx_clk_pll14xx("audio_pll1", "audio_pll1_ref_sel", base, &imx_1443x_pll);
 	clks[IMX8MM_AUDIO_PLL2] = imx_clk_pll14xx("audio_pll2", "audio_pll2_ref_sel", base + 0x14, &imx_1443x_pll);
 	clks[IMX8MM_VIDEO_PLL1] = imx_clk_pll14xx("video_pll1", "video_pll1_ref_sel", base + 0x28, &imx_1443x_pll);
-	clks[IMX8MM_DRAM_PLL] = imx_clk_pll14xx("dram_pll", "dram_pll_ref_sel", base + 0x50, &imx_1443x_pll);
+	clks[IMX8MM_DRAM_PLL] = imx_clk_pll14xx("dram_pll", "dram_pll_ref_sel", base + 0x50, &imx_1443x_dram_pll);
 	clks[IMX8MM_GPU_PLL] = imx_clk_pll14xx("gpu_pll", "gpu_pll_ref_sel", base + 0x64, &imx_1416x_pll);
 	clks[IMX8MM_VPU_PLL] = imx_clk_pll14xx("vpu_pll", "vpu_pll_ref_sel", base + 0x74, &imx_1416x_pll);
 	clks[IMX8MM_ARM_PLL] = imx_clk_pll14xx("arm_pll", "arm_pll_ref_sel", base + 0x84, &imx_1416x_pll);
 	clks[IMX8MM_SYS_PLL1] = imx_clk_fixed("sys_pll1", 800000000);
 	clks[IMX8MM_SYS_PLL2] = imx_clk_fixed("sys_pll2", 1000000000);
diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
index de905e278b80..4749beab9fc8 100644
--- a/drivers/clk/imx/clk-imx8mn.c
+++ b/drivers/clk/imx/clk-imx8mn.c
@@ -323,11 +323,11 @@  static int imx8mn_clocks_probe(struct platform_device *pdev)
 	clks[IMX8MN_SYS_PLL3_REF_SEL] = imx_clk_mux("sys_pll3_ref_sel", base + 0x114, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
 
 	clks[IMX8MN_AUDIO_PLL1] = imx_clk_pll14xx("audio_pll1", "audio_pll1_ref_sel", base, &imx_1443x_pll);
 	clks[IMX8MN_AUDIO_PLL2] = imx_clk_pll14xx("audio_pll2", "audio_pll2_ref_sel", base + 0x14, &imx_1443x_pll);
 	clks[IMX8MN_VIDEO_PLL1] = imx_clk_pll14xx("video_pll1", "video_pll1_ref_sel", base + 0x28, &imx_1443x_pll);
-	clks[IMX8MN_DRAM_PLL] = imx_clk_pll14xx("dram_pll", "dram_pll_ref_sel", base + 0x50, &imx_1443x_pll);
+	clks[IMX8MN_DRAM_PLL] = imx_clk_pll14xx("dram_pll", "dram_pll_ref_sel", base + 0x50, &imx_1443x_dram_pll);
 	clks[IMX8MN_GPU_PLL] = imx_clk_pll14xx("gpu_pll", "gpu_pll_ref_sel", base + 0x64, &imx_1416x_pll);
 	clks[IMX8MN_VPU_PLL] = imx_clk_pll14xx("vpu_pll", "vpu_pll_ref_sel", base + 0x74, &imx_1416x_pll);
 	clks[IMX8MN_ARM_PLL] = imx_clk_pll14xx("arm_pll", "arm_pll_ref_sel", base + 0x84, &imx_1416x_pll);
 	clks[IMX8MN_SYS_PLL1] = imx_clk_fixed("sys_pll1", 800000000);
 	clks[IMX8MN_SYS_PLL2] = imx_clk_fixed("sys_pll2", 1000000000);
diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 5c458199060a..a6d31a7262ef 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -65,10 +65,17 @@  struct imx_pll14xx_clk imx_1443x_pll = {
 	.type = PLL_1443X,
 	.rate_table = imx_pll1443x_tbl,
 	.rate_count = ARRAY_SIZE(imx_pll1443x_tbl),
 };
 
+struct imx_pll14xx_clk imx_1443x_dram_pll = {
+	.type = PLL_1443X,
+	.rate_table = imx_pll1443x_tbl,
+	.rate_count = ARRAY_SIZE(imx_pll1443x_tbl),
+	.flags = CLK_GET_RATE_NOCACHE,
+};
+
 struct imx_pll14xx_clk imx_1416x_pll = {
 	.type = PLL_1416X,
 	.rate_table = imx_pll1416x_tbl,
 	.rate_count = ARRAY_SIZE(imx_pll1416x_tbl),
 };
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index bc5bb6ac8636..81122c9ab842 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -50,10 +50,11 @@  struct imx_pll14xx_clk {
 	int flags;
 };
 
 extern struct imx_pll14xx_clk imx_1416x_pll;
 extern struct imx_pll14xx_clk imx_1443x_pll;
+extern struct imx_pll14xx_clk imx_1443x_dram_pll;
 
 #define imx_clk_cpu(name, parent_name, div, mux, pll, step) \
 	imx_clk_hw_cpu(name, parent_name, div, mux, pll, step)->clk
 
 #define clk_register_gate2(dev, name, parent_name, flags, reg, bit_idx, \