linux-kernel - Re: [PATCH v2 2/2] clk: gate: Add missing fw_name for clk_gate_register_test_parent_data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 10 Feb 2023 19:39:07 +0100
From:   Christian Marangi <ansuelsmth@...il.com>
To:     Stephen Boyd <sboyd@...nel.org>
Cc:     Michael Turquette <mturquette@...libre.com>,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel test robot <oliver.sang@...el.com>
Subject: Re: [PATCH v2 2/2] clk: gate: Add missing fw_name for
 clk_gate_register_test_parent_data_legacy

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@...el.com>
> 
> 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@...il.com>
> > ---
> >  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