Re: [PATCH v2 2/2] clk: gate: Add missing fw_name for clk_gate_register_test_parent_data_legacy

[Christian Marangi]

On Fri, Feb 10, 2023 at 04:52:36PM -0800, Stephen Boyd wrote:
> Quoting Christian Marangi (2023-01-31 08:08:29)
> > Fix warning for missing .fw_name in parent_data based on names.
> > It's wrong to define only .name since clk core expect always .fw_name to
> > be defined.
> > 
> > Reported-by: kernel test robot <oliver.sang@xxxxxxxxx>
> 
> What was the report?
>

With the previous patch applied kernel test robot report the WARN for
declaring a parent_data with .name but no .fw_name.

> > Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx>
> > ---
> >  drivers/clk/clk-gate_test.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/clk/clk-gate_test.c b/drivers/clk/clk-gate_test.c
> > index e136aaad48bf..a0a63cd4ce0b 100644
> > --- a/drivers/clk/clk-gate_test.c
> > +++ b/drivers/clk/clk-gate_test.c
> > @@ -74,6 +74,7 @@ static void clk_gate_register_test_parent_data_legacy(struct kunit *test)
> >                                             1000000);
> >         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent);
> >         pdata.name = "test_parent";
> > +       pdata.fw_name = "test_parent";
> >  
> >         ret = clk_hw_register_gate_parent_data(NULL, "test_gate", &pdata, 0,
> 
> We don't pass a 'dev' here, so the pdata.index isn't looked at. I
> suppose we can assign .index to -1 to be more explicit, but because
> there isn't a device used for registering, we won't try to use the
> .index. Instead we'll try to use .fw_name for clkdev, of which there
> won't be a clkdev lookup either. Eventually we'll fallback to the .name
> lookup, and it will be fine.

Problem is that from what we observed, it won't fallback to .name if
.fw_name is not declared.

But it will work if .fw_name is declared but not exposed by DT. (and
will correctly fallback to .name as .fw_name is not found)
(but this is to explain why the change in the other patch is needed so I
may be OT here)

> 
> We need tests that exercises the 'dev' path and also the DT path and the
> clkdev path. I was thinking about working on that outside of the gate
> test though, and just having a generic clk test for that with simple
> clk_ops that do basically nothing.

-- 
	Ansuel