From: Minghao Chi (CGEL ZTE) <[email protected]>
Use of_device_get_match_data() to simplify the code.
Reported-by: Zeal Robot <[email protected]>
Signed-off-by: Minghao Chi (CGEL ZTE) <[email protected]>
---
drivers/clk/rockchip/clk-rk3399.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index 306910a3a0d3..b1b67bfb63b8 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -1634,14 +1634,8 @@ static const struct of_device_id clk_rk3399_match_table[] = {
static int __init clk_rk3399_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- const struct of_device_id *match;
const struct clk_rk3399_inits *init_data;
-
- match = of_match_device(clk_rk3399_match_table, &pdev->dev);
- if (!match || !match->data)
- return -EINVAL;
-
- init_data = match->data;
+ init_data = of_device_get_match_data(&pdev->dev);
if (init_data->inits)
init_data->inits(np);
--
2.25.1
Quoting [email protected] (2022-03-03 17:17:03)
> From: Minghao Chi (CGEL ZTE) <[email protected]>
>
> Use of_device_get_match_data() to simplify the code.
>
> Reported-by: Zeal Robot <[email protected]>
> Signed-off-by: Minghao Chi (CGEL ZTE) <[email protected]>
> ---
> drivers/clk/rockchip/clk-rk3399.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> index 306910a3a0d3..b1b67bfb63b8 100644
> --- a/drivers/clk/rockchip/clk-rk3399.c
> +++ b/drivers/clk/rockchip/clk-rk3399.c
> @@ -1634,14 +1634,8 @@ static const struct of_device_id clk_rk3399_match_table[] = {
> static int __init clk_rk3399_probe(struct platform_device *pdev)
> {
> struct device_node *np = pdev->dev.of_node;
> - const struct of_device_id *match;
> const struct clk_rk3399_inits *init_data;
> -
> - match = of_match_device(clk_rk3399_match_table, &pdev->dev);
> - if (!match || !match->data)
> - return -EINVAL;
> -
> - init_data = match->data;
> + init_data = of_device_get_match_data(&pdev->dev);
The translation doesn't look equivalent. Before we would bail out of
probe if match data isn't there with an error. That isn't possible of
course with further investigation but please make a note of this in the
commit text to aid review. Also, please don't send new versions of
patches in reply to previous versions of patches. It breaks my patch
workflow. Thanks in advance.
> if (init_data->inits)
> init_data->inits(np);
>
Am Mittwoch, 9. M?rz 2022, 19:57:36 CET schrieb Stephen Boyd:
> Quoting [email protected] (2022-03-03 17:17:03)
> > From: Minghao Chi (CGEL ZTE) <[email protected]>
> >
> > Use of_device_get_match_data() to simplify the code.
> >
> > Reported-by: Zeal Robot <[email protected]>
> > Signed-off-by: Minghao Chi (CGEL ZTE) <[email protected]>
> > ---
> > drivers/clk/rockchip/clk-rk3399.c | 8 +-------
> > 1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> > index 306910a3a0d3..b1b67bfb63b8 100644
> > --- a/drivers/clk/rockchip/clk-rk3399.c
> > +++ b/drivers/clk/rockchip/clk-rk3399.c
> > @@ -1634,14 +1634,8 @@ static const struct of_device_id clk_rk3399_match_table[] = {
> > static int __init clk_rk3399_probe(struct platform_device *pdev)
> > {
> > struct device_node *np = pdev->dev.of_node;
> > - const struct of_device_id *match;
> > const struct clk_rk3399_inits *init_data;
> > -
> > - match = of_match_device(clk_rk3399_match_table, &pdev->dev);
> > - if (!match || !match->data)
> > - return -EINVAL;
> > -
> > - init_data = match->data;
> > + init_data = of_device_get_match_data(&pdev->dev);
>
> The translation doesn't look equivalent. Before we would bail out of
> probe if match data isn't there with an error. That isn't possible of
> course with further investigation but please make a note of this in the
> commit text to aid review.
We _do have_ Rockchip clock drivers serving multiple socs already
(rk3188+rk3066 for example) and as patterns are duplicated often from
one existing driver to a new one, I think it makes sense to have the
error handling done correctly.
So doing
init_data = of_device_get_match_data(&pdev->dev);
if (init_data)
return -EINVAL;
as before.
If due to some strange coincidence the condition is triggered
this would cause a null-ptr-dereference with the direct call to
init_data->scripts
below, which might or might not be hidden due to the (early-)console
being up yet, where with correct error handling a system might at
least come up to a point to complain about a missing clock driver.
> Also, please don't send new versions of
> patches in reply to previous versions of patches. It breaks my patch
> workflow. Thanks in advance.
>
> > if (init_data->inits)
> > init_data->inits(np);
> >
>