| Message ID | 6e17f2145ce2bbc12af6700c8bd56a8a7bdb103d.1705738045.git.christophe.jaillet@wanadoo.fr |
|---|---|
| State | New |
| Headers | show |
| Series | fsi: occ: Remove usage of the deprecated ida_simple_xx() API | expand |
Le 20/01/2024 à 09:07, Christophe JAILLET a écrit : > ida_alloc() and ida_free() should be preferred to the deprecated > ida_simple_get() and ida_simple_remove(). > > Note that the upper limit of ida_simple_get() is exclusive, but the one of > ida_alloc_range() is inclusive. So, this upper limit, INT_MAX, should have > been changed to INT_MAX-1. > > But, it is likely that the INT_MAX 'idx' is valid that the max value passed > to ida_simple_get() should have been 0. > > So, allow this INT_MAX 'idx' value now. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > The change related to the INT_MAX value is speculative. > Review with care. (or I can re-submit with INT_MAX-1, to be safe :)) > --- Hi, polite reminder. CJ > drivers/fsi/fsi-occ.c | 17 ++++++++--------- > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c > index da35ca9e84a6..f7157c1d77d8 100644 > --- a/drivers/fsi/fsi-occ.c > +++ b/drivers/fsi/fsi-occ.c > @@ -656,17 +656,16 @@ static int occ_probe(struct platform_device *pdev) > rc = of_property_read_u32(dev->of_node, "reg", ®); > if (!rc) { > /* make sure we don't have a duplicate from dts */ > - occ->idx = ida_simple_get(&occ_ida, reg, reg + 1, > - GFP_KERNEL); > + occ->idx = ida_alloc_range(&occ_ida, reg, reg, > + GFP_KERNEL); > if (occ->idx < 0) > - occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX, > - GFP_KERNEL); > + occ->idx = ida_alloc_min(&occ_ida, 1, > + GFP_KERNEL); > } else { > - occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX, > - GFP_KERNEL); > + occ->idx = ida_alloc_min(&occ_ida, 1, GFP_KERNEL); > } > } else { > - occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX, GFP_KERNEL); > + occ->idx = ida_alloc_min(&occ_ida, 1, GFP_KERNEL); > } > > platform_set_drvdata(pdev, occ); > @@ -680,7 +679,7 @@ static int occ_probe(struct platform_device *pdev) > rc = misc_register(&occ->mdev); > if (rc) { > dev_err(dev, "failed to register miscdevice: %d\n", rc); > - ida_simple_remove(&occ_ida, occ->idx); > + ida_free(&occ_ida, occ->idx); > kvfree(occ->buffer); > return rc; > } > @@ -719,7 +718,7 @@ static int occ_remove(struct platform_device *pdev) > else > device_for_each_child(&pdev->dev, NULL, occ_unregister_of_child); > > - ida_simple_remove(&occ_ida, occ->idx); > + ida_free(&occ_ida, occ->idx); > > return 0; > }
On 4/14/24 04:00, Christophe JAILLET wrote: > Le 20/01/2024 à 09:07, Christophe JAILLET a écrit : >> ida_alloc() and ida_free() should be preferred to the deprecated >> ida_simple_get() and ida_simple_remove(). >> >> Note that the upper limit of ida_simple_get() is exclusive, but the >> one of >> ida_alloc_range() is inclusive. So, this upper limit, INT_MAX, should >> have >> been changed to INT_MAX-1. >> >> But, it is likely that the INT_MAX 'idx' is valid that the max value >> passed >> to ida_simple_get() should have been 0. >> >> So, allow this INT_MAX 'idx' value now. Reviewed-by: Eddie James <eajames@linux.ibm.com> >> >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >> --- >> The change related to the INT_MAX value is speculative. >> Review with care. (or I can re-submit with INT_MAX-1, to be safe :)) >> --- > > Hi, > > polite reminder. Thank you! Looks good. > > CJ > > > >> drivers/fsi/fsi-occ.c | 17 ++++++++--------- >> 1 file changed, 8 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c >> index da35ca9e84a6..f7157c1d77d8 100644 >> --- a/drivers/fsi/fsi-occ.c >> +++ b/drivers/fsi/fsi-occ.c >> @@ -656,17 +656,16 @@ static int occ_probe(struct platform_device *pdev) >> rc = of_property_read_u32(dev->of_node, "reg", ®); >> if (!rc) { >> /* make sure we don't have a duplicate from dts */ >> - occ->idx = ida_simple_get(&occ_ida, reg, reg + 1, >> - GFP_KERNEL); >> + occ->idx = ida_alloc_range(&occ_ida, reg, reg, >> + GFP_KERNEL); >> if (occ->idx < 0) >> - occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX, >> - GFP_KERNEL); >> + occ->idx = ida_alloc_min(&occ_ida, 1, >> + GFP_KERNEL); >> } else { >> - occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX, >> - GFP_KERNEL); >> + occ->idx = ida_alloc_min(&occ_ida, 1, GFP_KERNEL); >> } >> } else { >> - occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX, GFP_KERNEL); >> + occ->idx = ida_alloc_min(&occ_ida, 1, GFP_KERNEL); >> } >> platform_set_drvdata(pdev, occ); >> @@ -680,7 +679,7 @@ static int occ_probe(struct platform_device *pdev) >> rc = misc_register(&occ->mdev); >> if (rc) { >> dev_err(dev, "failed to register miscdevice: %d\n", rc); >> - ida_simple_remove(&occ_ida, occ->idx); >> + ida_free(&occ_ida, occ->idx); >> kvfree(occ->buffer); >> return rc; >> } >> @@ -719,7 +718,7 @@ static int occ_remove(struct platform_device *pdev) >> else >> device_for_each_child(&pdev->dev, NULL, >> occ_unregister_of_child); >> - ida_simple_remove(&occ_ida, occ->idx); >> + ida_free(&occ_ida, occ->idx); >> return 0; >> } >
diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c index da35ca9e84a6..f7157c1d77d8 100644 --- a/drivers/fsi/fsi-occ.c +++ b/drivers/fsi/fsi-occ.c @@ -656,17 +656,16 @@ static int occ_probe(struct platform_device *pdev) rc = of_property_read_u32(dev->of_node, "reg", ®); if (!rc) { /* make sure we don't have a duplicate from dts */ - occ->idx = ida_simple_get(&occ_ida, reg, reg + 1, - GFP_KERNEL); + occ->idx = ida_alloc_range(&occ_ida, reg, reg, + GFP_KERNEL); if (occ->idx < 0) - occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX, - GFP_KERNEL); + occ->idx = ida_alloc_min(&occ_ida, 1, + GFP_KERNEL); } else { - occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX, - GFP_KERNEL); + occ->idx = ida_alloc_min(&occ_ida, 1, GFP_KERNEL); } } else { - occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX, GFP_KERNEL); + occ->idx = ida_alloc_min(&occ_ida, 1, GFP_KERNEL); } platform_set_drvdata(pdev, occ); @@ -680,7 +679,7 @@ static int occ_probe(struct platform_device *pdev) rc = misc_register(&occ->mdev); if (rc) { dev_err(dev, "failed to register miscdevice: %d\n", rc); - ida_simple_remove(&occ_ida, occ->idx); + ida_free(&occ_ida, occ->idx); kvfree(occ->buffer); return rc; } @@ -719,7 +718,7 @@ static int occ_remove(struct platform_device *pdev) else device_for_each_child(&pdev->dev, NULL, occ_unregister_of_child); - ida_simple_remove(&occ_ida, occ->idx); + ida_free(&occ_ida, occ->idx); return 0; }
ida_alloc() and ida_free() should be preferred to the deprecated ida_simple_get() and ida_simple_remove(). Note that the upper limit of ida_simple_get() is exclusive, but the one of ida_alloc_range() is inclusive. So, this upper limit, INT_MAX, should have been changed to INT_MAX-1. But, it is likely that the INT_MAX 'idx' is valid that the max value passed to ida_simple_get() should have been 0. So, allow this INT_MAX 'idx' value now. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- The change related to the INT_MAX value is speculative. Review with care. (or I can re-submit with INT_MAX-1, to be safe :)) --- drivers/fsi/fsi-occ.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)