| Message ID | 20230131160829.23369-2-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | [v2,1/2] clk: Warn and add workaround on misuse of .parent_data with .name only | expand |
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@intel.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@gmail.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.
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@intel.com> What was the report? > Signed-off-by: Christian Marangi <ansuelsmth@gmail.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. 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.
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, NULL, 0, 0, NULL);
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@intel.com> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- drivers/clk/clk-gate_test.c | 1 + 1 file changed, 1 insertion(+)