Messages in this thread | | | Date | Thu, 7 Nov 2019 17:49:28 +0100 | From | Michael Tretter <> | Subject | Re: [PATCH] clk: zynqmp: Add support for custom type flags |
| |
On Thu, 07 Nov 2019 00:58:06 -0800, Rajan Vaja wrote: > Store extra custom type flags received from firmware. > > Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com> > Signed-off-by: Jolly Shah <jolly.shah@xilinx.com> > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > drivers/clk/zynqmp/clkc.c | 8 +++++++- > drivers/clk/zynqmp/divider.c | 4 ++-- > 2 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c > index a11f93e..0dea55e 100644 > --- a/drivers/clk/zynqmp/clkc.c > +++ b/drivers/clk/zynqmp/clkc.c > @@ -2,7 +2,7 @@ > /* > * Zynq UltraScale+ MPSoC clock controller > * > - * Copyright (C) 2016-2018 Xilinx > + * Copyright (C) 2016-2019 Xilinx > * > * Based on drivers/clk/zynq/clkc.c > */ > @@ -86,6 +86,8 @@ struct topology_resp { > #define CLK_TOPOLOGY_TYPE GENMASK(3, 0) > #define CLK_TOPOLOGY_FLAGS GENMASK(23, 8) > #define CLK_TOPOLOGY_TYPE_FLAGS GENMASK(31, 24) > +#define CLK_TOPOLOGY_TYPE_FLAG2 GENMASK(7, 4)
What kind of function do these flags indicate? The name is really not obvious to me.
I would prefer if the defines are kept in the order of the bits in the field, i.e., the new define should go between CLK_TOPOLOGY_TYPE and CLK_TOPOLOGY_FLAGS.
> +#define CLK_TOPOLOGY_TYPE_FLAG_BITS 8 > u32 topology[CLK_GET_TOPOLOGY_RESP_WORDS]; > }; > > @@ -396,6 +398,10 @@ static int __zynqmp_clock_get_topology(struct clock_topology *topology, > topology[*nnodes].type_flag = > FIELD_GET(CLK_TOPOLOGY_TYPE_FLAGS, > response->topology[i]); > + topology[*nnodes].type_flag |= > + FIELD_GET(CLK_TOPOLOGY_TYPE_FLAG2, > + response->topology[i]) << > + CLK_TOPOLOGY_TYPE_FLAG_BITS;
Shifting the new flags into the existing type_flag field seems like a source for code that is really difficult to read. Maybe use a new field in the topology for the new flags with a proper name?
> (*nnodes)++; > } > > diff --git a/drivers/clk/zynqmp/divider.c b/drivers/clk/zynqmp/divider.c > index d8f5b70d..d376529 100644 > --- a/drivers/clk/zynqmp/divider.c > +++ b/drivers/clk/zynqmp/divider.c > @@ -2,7 +2,7 @@ > /* > * Zynq UltraScale+ MPSoC Divider support > * > - * Copyright (C) 2016-2018 Xilinx > + * Copyright (C) 2016-2019 Xilinx > * > * Adjustable divider clock implementation > */ > @@ -37,7 +37,7 @@ > */ > struct zynqmp_clk_divider { > struct clk_hw hw; > - u8 flags; > + u16 flags;
This change looks unrelated to the remaining patch.
Michael
> bool is_frac; > u32 clk_id; > u32 div_type;
| |