Re: [PATCH 3/3] staging: most: aim-cdev: relocate call to ida_init()

[Greg KH]

On Mon, Aug 22, 2016 at 10:58:07AM +0200, Christian Gromm wrote:
> On Sun, 21 Aug 2016 16:50:20 +0200
> Greg KH  wrote:
> 
> > On Fri, Aug 19, 2016 at 01:09:35PM +0200, Christian Gromm wrote:
> > > This patch moves the initialization of the idr structure towards the end
> > > of the module's init routine. This keeps the code compact and eliminates
> > > the need of having to call ida_destroy() in case the function exits with
> > > an exception.
> > > 
> > > Signed-off-by: Christian Gromm 
> > > ---
> > >  drivers/staging/most/aim-cdev/cdev.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/most/aim-cdev/cdev.c 
> > > b/drivers/staging/most/aim-cdev/cdev.c
> > > index db6f458..3864009 100644
> > > --- a/drivers/staging/most/aim-cdev/cdev.c
> > > +++ b/drivers/staging/most/aim-cdev/cdev.c
> > > @@ -505,7 +505,6 @@ static int __init mod_init(void)
> > >  
> > >   INIT_LIST_HEAD(_list);
> > >   spin_lock_init(_list_lock);
> > > - ida_init(_id);
> > >  
> > >   err = alloc_chrdev_region(_devno, 0, 50, "cdev");
> > >   if (err < 0)
> > > @@ -521,6 +520,7 @@ static int __init mod_init(void)
> > >   err = most_register_aim(_aim);
> > >   if (err)
> > >   goto dest_class;
> > > + ida_init(_id);
> > 
> > But can't the minor_id be used before this call?  most_register_aim()
> > could call into a driver and have a cdev be wanted, right?
> 
> No. After calling most_register_aim() the device appears in sysfs. Then
> the user needs to establish a link to a certain channel of the hardware
> by writing to the sysfs attribute add_link. And this is when minor_id is
> being used to create a new device that will show up in /dev.
> 
> See store_add_link() function of the core module.

Ok, I'll trust you :)

Can you resend this, it's gone from my queue now...

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/3] staging: most: aim-cdev: relocate call to ida_init()

[Christian Gromm]

On Sun, 21 Aug 2016 16:50:20 +0200
Greg KH  wrote:

> On Fri, Aug 19, 2016 at 01:09:35PM +0200, Christian Gromm wrote:
> > This patch moves the initialization of the idr structure towards the end
> > of the module's init routine. This keeps the code compact and eliminates
> > the need of having to call ida_destroy() in case the function exits with
> > an exception.
> > 
> > Signed-off-by: Christian Gromm 
> > ---
> >  drivers/staging/most/aim-cdev/cdev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/most/aim-cdev/cdev.c 
> > b/drivers/staging/most/aim-cdev/cdev.c
> > index db6f458..3864009 100644
> > --- a/drivers/staging/most/aim-cdev/cdev.c
> > +++ b/drivers/staging/most/aim-cdev/cdev.c
> > @@ -505,7 +505,6 @@ static int __init mod_init(void)
> >  
> > INIT_LIST_HEAD(_list);
> > spin_lock_init(_list_lock);
> > -   ida_init(_id);
> >  
> > err = alloc_chrdev_region(_devno, 0, 50, "cdev");
> > if (err < 0)
> > @@ -521,6 +520,7 @@ static int __init mod_init(void)
> > err = most_register_aim(_aim);
> > if (err)
> > goto dest_class;
> > +   ida_init(_id);
> 
> But can't the minor_id be used before this call?  most_register_aim()
> could call into a driver and have a cdev be wanted, right?

No. After calling most_register_aim() the device appears in sysfs. Then
the user needs to establish a link to a certain channel of the hardware
by writing to the sysfs attribute add_link. And this is when minor_id is
being used to create a new device that will show up in /dev.

See store_add_link() function of the core module.


regards,
Chris

> 
> thanks,
> 
> greg k-h

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/3] staging: most: aim-cdev: relocate call to ida_init()

[Greg KH]

On Fri, Aug 19, 2016 at 01:09:35PM +0200, Christian Gromm wrote:
> This patch moves the initialization of the idr structure towards the end
> of the module's init routine. This keeps the code compact and eliminates
> the need of having to call ida_destroy() in case the function exits with
> an exception.
> 
> Signed-off-by: Christian Gromm 
> ---
>  drivers/staging/most/aim-cdev/cdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/most/aim-cdev/cdev.c 
> b/drivers/staging/most/aim-cdev/cdev.c
> index db6f458..3864009 100644
> --- a/drivers/staging/most/aim-cdev/cdev.c
> +++ b/drivers/staging/most/aim-cdev/cdev.c
> @@ -505,7 +505,6 @@ static int __init mod_init(void)
>  
>   INIT_LIST_HEAD(_list);
>   spin_lock_init(_list_lock);
> - ida_init(_id);
>  
>   err = alloc_chrdev_region(_devno, 0, 50, "cdev");
>   if (err < 0)
> @@ -521,6 +520,7 @@ static int __init mod_init(void)
>   err = most_register_aim(_aim);
>   if (err)
>   goto dest_class;
> + ida_init(_id);

But can't the minor_id be used before this call?  most_register_aim()
could call into a driver and have a cdev be wanted, right?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel