Messages in this thread | | | Date | Mon, 6 Jun 2022 08:49:05 +0300 | From | Matti Vaittinen <> | Subject | Re: [RFC PATCH 1/3] clk: bd718x7: Clean up the code, no functional changes |
| |
On 6/5/22 19:57, Michael Trimarchi wrote: > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> > --- > drivers/clk/clk-bd718x7.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/drivers/clk/clk-bd718x7.c b/drivers/clk/clk-bd718x7.c > index ac40b669d60b..04cc0beb67df 100644 > --- a/drivers/clk/clk-bd718x7.c > +++ b/drivers/clk/clk-bd718x7.c > @@ -81,27 +81,28 @@ static int bd71837_clk_probe(struct platform_device *pdev) > struct bd718xx_clk *c; > int rval = -ENOMEM; > const char *parent_clk; > + struct device *dev = &pdev->dev;
I am not a fan of assigning pointers to struct members to local variables unless they're shortening lines to fit on one row instead of using two. Whenever we add such a variable we hide information. After that being said - in this particular case the device 'dev' points to is quite obvious so I am not completely against the change if other see the value.
> struct device *parent = pdev->dev.parent; > struct clk_init_data init = { > .name = "bd718xx-32k-out", > .ops = &bd71837_clk_ops, > + .num_parents = 1,
I like this. Thanks.
> };
Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>
Best Regards -- Matti
-- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Discuss - Estimate - Plan - Report and finally accomplish this: void do_work(int time) __attribute__ ((const));
| |