Re: [PATCH] TTY: serdev: Replace depricated macros

Re: [PATCH] TTY: serdev: Replace depricated macros

[Greg KH]

On Thu, May 06, 2021 at 08:32:28PM +0200, Ivan Bakula wrote:
> Replace depricated macros ida_simple_get and ida_simple_remove with
> appropriate function calls to ida_alloc and ida_free.
> 
> Signed-off-by: Ivan Bakula <wamreu@gmail.com>
> ---
>  drivers/tty/serdev/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index aead0c0c9..5f873960b 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -75,7 +75,7 @@ static bool is_serdev_device(const struct device *dev)
>  static void serdev_ctrl_release(struct device *dev)
>  {
>  	struct serdev_controller *ctrl = to_serdev_controller(dev);
> -	ida_simple_remove(&ctrl_ida, ctrl->nr);
> +	ida_free(&ctrl_ida, ctrl->nr);
>  	kfree(ctrl);
>  }
>  
> @@ -488,7 +488,7 @@ struct serdev_controller *serdev_controller_alloc(struct device *parent,
>  	if (!ctrl)
>  		return NULL;
>  
> -	id = ida_simple_get(&ctrl_ida, 0, 0, GFP_KERNEL);
> +	id = ida_alloc(&ctrl_ida, GFP_KERNEL);
>  	if (id < 0) {
>  		dev_err(parent,
>  			"unable to allocate serdev controller identifier.\n");
> -- 
> 2.31.1
> 

If these really are "deprecated", why not just do a search/replace
across the whole tree and change them?  Or if not, what is wrong with
the existing ones?

Why shouldn't we be using the old calls?  What is wrong with them?

thanks,

greg k-h

[PATCH] TTY: serdev: Replace depricated macros

[Ivan Bakula]

Replace depricated macros ida_simple_get and ida_simple_remove with
appropriate function calls to ida_alloc and ida_free.

Signed-off-by: Ivan Bakula <wamreu@gmail.com>
---
 drivers/tty/serdev/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index aead0c0c9..5f873960b 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -75,7 +75,7 @@ static bool is_serdev_device(const struct device *dev)
 static void serdev_ctrl_release(struct device *dev)
 {
 	struct serdev_controller *ctrl = to_serdev_controller(dev);
-	ida_simple_remove(&ctrl_ida, ctrl->nr);
+	ida_free(&ctrl_ida, ctrl->nr);
 	kfree(ctrl);
 }
 
@@ -488,7 +488,7 @@ struct serdev_controller *serdev_controller_alloc(struct device *parent,
 	if (!ctrl)
 		return NULL;
 
-	id = ida_simple_get(&ctrl_ida, 0, 0, GFP_KERNEL);
+	id = ida_alloc(&ctrl_ida, GFP_KERNEL);
 	if (id < 0) {
 		dev_err(parent,
 			"unable to allocate serdev controller identifier.\n");
-- 
2.31.1

Re: [PATCH] TTY: serdev: Replace depricated macros

[Greg KH]

On Fri, May 07, 2021 at 01:16:38PM +0200, Ivan Bakula wrote:
> Thank you for your reply.
> 
> > If these really are "deprecated", 
> 
> According to the comment in file "include/linux/idr.h" (line 318) these
> macros are depricated.

So that means what, that no new users should use them?  Or that they
should be replaced?

> > why not just do a search/replace across the whole tree and change them?
> 
> I've stumbled across these macros while trying to figure out how to write 
> device driver using serial device bus. So, I've decided to change them with
> appropriate function calls.
> 
> I didn't do search/replace across the whole tree because it's my first 
> patch and I wanted it to be short. But now, when you mentioned it, 
> I will do it.

No, please do not do that unless you can figure out why this is needed.

Only change code if it has to be changed.  If the idr api needs to be
redone for existing, working code, then that's fine, but do it correctly
and understand why you are doing it.

My point being that if it were as simple as a search/replace, then the
developer who wrote that comment would have done it already, not that
this should be something that you should do :)

hope this helps,

greg k-h

Re: [PATCH] TTY: serdev: Replace depricated macros

[Ivan Bakula]

Thank you for your reply.

> If these really are "deprecated", 

According to the comment in file "include/linux/idr.h" (line 318) these
macros are depricated.

> why not just do a search/replace across the whole tree and change them?

I've stumbled across these macros while trying to figure out how to write 
device driver using serial device bus. So, I've decided to change them with
appropriate function calls.

I didn't do search/replace across the whole tree because it's my first 
patch and I wanted it to be short. But now, when you mentioned it, 
I will do it.

Thank you for your time,

Ivan Bakula

Re: [PATCH] TTY: serdev: Replace depricated macros

[Ivan Bakula]

Ok, got it.

Thank you for your time,

I.B.