Re: [PATCH] drivers: input: Directly use ida_alloc()/free()

[Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>]

Hi,

Le 27/05/2022 à 12:37, keliu a écrit :
> Use ida_alloc()/ida_free() instead of deprecated
> ida_simple_get()/ida_simple_remove() .
>
> Signed-off-by: keliu <liuke94@xxxxxxxxxx>
> ---
>   drivers/input/input.c         | 8 ++++----
>   drivers/input/joystick/xpad.c | 6 +++---
>   2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 1365c9dfb5f2..1e4a275795f9 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -2618,15 +2618,15 @@ int input_get_new_minor(int legacy_base, unsigned int legacy_num,
>   	 * locking is needed here.
>   	 */
>   	if (legacy_base >= 0) {
> -		int minor = ida_simple_get(&input_ida,
> +		int minor = ida_alloc_range(&input_ida,
>   					   legacy_base,
> -					   legacy_base + legacy_num,
> +					   legacy_base + legacy_num - 1,

You got my point, things are going in the right direction.
This one is correct...

>   					   GFP_KERNEL);
>   		if (minor >= 0 || !allow_dynamic)
>   			return minor;
>   	}
>   
> -	return ida_simple_get(&input_ida,
> +	return ida_alloc_range(&input_ida,
>   			      INPUT_FIRST_DYNAMIC_DEV, INPUT_MAX_CHAR_DEVICES,

... but you missed the -1 here.

Also maybe an explanation of why this -1 are introduced would help 
reviewers. (if needed, I think I already wrote some, just ask)

CJ

>   			      GFP_KERNEL);
>   }
> @@ -2641,7 +2641,7 @@ EXPORT_SYMBOL(input_get_new_minor);
>    */
>   void input_free_minor(unsigned int minor)
>   {
> -	ida_simple_remove(&input_ida, minor);
> +	ida_free(&input_ida, minor);
>   }
>   EXPORT_SYMBOL(input_free_minor);
>   
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 18190b529bca..fafc0d5703dc 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -1456,7 +1456,7 @@ static int xpad_led_probe(struct usb_xpad *xpad)
>   	if (!led)
>   		return -ENOMEM;
>   
> -	xpad->pad_nr = ida_simple_get(&xpad_pad_seq, 0, 0, GFP_KERNEL);
> +	xpad->pad_nr = ida_alloc(&xpad_pad_seq, GFP_KERNEL);
>   	if (xpad->pad_nr < 0) {
>   		error = xpad->pad_nr;
>   		goto err_free_mem;
> @@ -1479,7 +1479,7 @@ static int xpad_led_probe(struct usb_xpad *xpad)
>   	return 0;
>   
>   err_free_id:
> -	ida_simple_remove(&xpad_pad_seq, xpad->pad_nr);
> +	ida_free(&xpad_pad_seq, xpad->pad_nr);
>   err_free_mem:
>   	kfree(led);
>   	xpad->led = NULL;
> @@ -1492,7 +1492,7 @@ static void xpad_led_disconnect(struct usb_xpad *xpad)
>   
>   	if (xpad_led) {
>   		led_classdev_unregister(&xpad_led->led_cdev);
> -		ida_simple_remove(&xpad_pad_seq, xpad->pad_nr);
> +		ida_free(&xpad_pad_seq, xpad->pad_nr);
>   		kfree(xpad_led);
>   	}
>   }