[PATCH] tlclk: fix platform_device_register_simple() error check

[PATCH] tlclk: fix platform_device_register_simple() error check

[Akinobu Mita]

The return value of platform_device_register_simple() should be
checked by IS_ERR().

This patch also fix misc_register() error case. Because misc_register()
returns error code.

Cc: Sebastien Bouchard <sebastien.bouchard@ca.kontron.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>

---
 drivers/char/tlclk.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Index: work-fault-inject/drivers/char/tlclk.c
===================================================================
--- work-fault-inject.orig/drivers/char/tlclk.c
+++ work-fault-inject/drivers/char/tlclk.c
@@ -792,15 +792,14 @@ static int __init tlclk_init(void)
 	ret = misc_register(&tlclk_miscdev);
 	if (ret < 0) {
 		printk(KERN_ERR "tlclk: misc_register returns %d.\n", ret);
-		ret = -EBUSY;
 		goto out3;
 	}
 
 	tlclk_device = platform_device_register_simple("telco_clock",
 				-1, NULL, 0);
-	if (!tlclk_device) {
+	if (IS_ERR(tlclk_device)) {
 		printk(KERN_ERR "tlclk: platform_device_register failed.\n");
-		ret = -EBUSY;
+		ret = PTR_ERR(tlclk_device);
 		goto out4;
 	}
 

Re: [PATCH] tlclk: fix platform_device_register_simple() error check

[Mark Gross]

On Thu, Nov 23, 2006 at 03:41:11AM +0900, Akinobu Mita wrote:
> The return value of platform_device_register_simple() should be
> checked by IS_ERR().
> 
> This patch also fix misc_register() error case. Because misc_register()
> returns error code.
> 
> Cc: Sebastien Bouchard <sebastien.bouchard@ca.kontron.com>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> 
> ---
>  drivers/char/tlclk.c |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> Index: work-fault-inject/drivers/char/tlclk.c
> ===================================================================
> --- work-fault-inject.orig/drivers/char/tlclk.c
> +++ work-fault-inject/drivers/char/tlclk.c
> @@ -792,15 +792,14 @@ static int __init tlclk_init(void)
>  	ret = misc_register(&tlclk_miscdev);
>  	if (ret < 0) {
>  		printk(KERN_ERR "tlclk: misc_register returns %d.\n", ret);
> -		ret = -EBUSY;

results in an non-error return when there this device isn't on the
system.

* NAK *
>  		goto out3;
>  	}
>  
>  	tlclk_device = platform_device_register_simple("telco_clock",
>  				-1, NULL, 0);
> -	if (!tlclk_device) {
> +	if (IS_ERR(tlclk_device)) {
ok

>  		printk(KERN_ERR "tlclk: platform_device_register failed.\n");
> -		ret = -EBUSY;
> +		ret = PTR_ERR(tlclk_device);

I don't know about this but I could be wrong.  Please convince me.

>  		goto out4;
>  	}
>  
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tlclk: fix platform_device_register_simple() error check

[Mark Gross]

On Mon, Nov 27, 2006 at 12:34:52PM -0800, Mark Gross wrote:
> On Thu, Nov 23, 2006 at 03:41:11AM +0900, Akinobu Mita wrote:
> > The return value of platform_device_register_simple() should be
> > checked by IS_ERR().
> > 
> > This patch also fix misc_register() error case. Because misc_register()
> > returns error code.
> > 
> > Cc: Sebastien Bouchard <sebastien.bouchard@ca.kontron.com>
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > 
> > ---
> >  drivers/char/tlclk.c |    5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > Index: work-fault-inject/drivers/char/tlclk.c
> > ===================================================================
> > --- work-fault-inject.orig/drivers/char/tlclk.c
> > +++ work-fault-inject/drivers/char/tlclk.c
> > @@ -792,15 +792,14 @@ static int __init tlclk_init(void)
> >  	ret = misc_register(&tlclk_miscdev);
> >  	if (ret < 0) {
> >  		printk(KERN_ERR "tlclk: misc_register returns %d.\n", ret);
> > -		ret = -EBUSY;
> 
> results in an non-error return when there this device isn't on the
> system.
> 
> * NAK *
My bad.  I was looking at the earlier ret = EBUSY, this is ok.

> >  		goto out3;
> >  	}
> >  
> >  	tlclk_device = platform_device_register_simple("telco_clock",
> >  				-1, NULL, 0);
> > -	if (!tlclk_device) {
> > +	if (IS_ERR(tlclk_device)) {
> ok
> 
> >  		printk(KERN_ERR "tlclk: platform_device_register failed.\n");
> > -		ret = -EBUSY;
> > +		ret = PTR_ERR(tlclk_device);
> 
> I don't know about this but I could be wrong.  Please convince me.
> 
> >  		goto out4;
> >  	}
> >  
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tlclk: fix platform_device_register_simple() error check

[Akinobu Mita]

On Mon, Nov 27, 2006 at 12:34:52PM -0800, Mark Gross wrote:
> >  
> >  	tlclk_device = platform_device_register_simple("telco_clock",
> >  				-1, NULL, 0);
> > -	if (!tlclk_device) {
> > +	if (IS_ERR(tlclk_device)) {
> ok
> 
> >  		printk(KERN_ERR "tlclk: platform_device_register failed.\n");
> > -		ret = -EBUSY;
> > +		ret = PTR_ERR(tlclk_device);
> 
> I don't know about this but I could be wrong.  Please convince me.

We expect platform_device_register_simple() returns proper errno as pointer
when it fails.

Re: [PATCH] tlclk: fix platform_device_register_simple() error check

[Mark Gross]

On Tue, Nov 28, 2006 at 03:08:30PM +0900, Akinobu Mita wrote:
> On Mon, Nov 27, 2006 at 12:34:52PM -0800, Mark Gross wrote:
> > >  
> > >  	tlclk_device = platform_device_register_simple("telco_clock",
> > >  				-1, NULL, 0);
> > > -	if (!tlclk_device) {
> > > +	if (IS_ERR(tlclk_device)) {
> > ok
> > 
> > >  		printk(KERN_ERR "tlclk: platform_device_register failed.\n");
> > > -		ret = -EBUSY;
> > > +		ret = PTR_ERR(tlclk_device);
> > 
> > I don't know about this but I could be wrong.  Please convince me.
> 
> We expect platform_device_register_simple() returns proper errno as pointer
> when it fails.

What's wrong with EBUSY?

--mgross

Re: [PATCH] tlclk: fix platform_device_register_simple() error check

[Akinobu Mita]

On Wed, Nov 29, 2006 at 01:17:57PM -0800, Mark Gross wrote:
> > We expect platform_device_register_simple() returns proper errno as pointer
> > when it fails.
> 
> What's wrong with EBUSY?

-ENOMEM or -EINVAL could be returned by platform_device_register_simple()
logically. And we don't know what kind of change will be made into driver
core in future.

Re: [PATCH] tlclk: fix platform_device_register_simple() error check

[Mark Gross]

On Sat, Dec 02, 2006 at 01:03:32PM +0900, Akinobu Mita wrote:
> On Wed, Nov 29, 2006 at 01:17:57PM -0800, Mark Gross wrote:
> > > We expect platform_device_register_simple() returns proper errno as pointer
> > > when it fails.
> > 
> > What's wrong with EBUSY?
> 
> -ENOMEM or -EINVAL could be returned by platform_device_register_simple()
> logically. And we don't know what kind of change will be made into driver
> core in future.

Ok, would you like me to ack your earlier patch or will you send a new
one?

--mgross

Re: [PATCH] tlclk: fix platform_device_register_simple() error check

[Mark Gross]

On Tue, Dec 05, 2006 at 10:25:43AM -0800, Mark Gross wrote:
> On Sat, Dec 02, 2006 at 01:03:32PM +0900, Akinobu Mita wrote:
> > On Wed, Nov 29, 2006 at 01:17:57PM -0800, Mark Gross wrote:
> > > > We expect platform_device_register_simple() returns proper errno as pointer
> > > > when it fails.
> > > 
> > > What's wrong with EBUSY?
> > 
> > -ENOMEM or -EINVAL could be returned by platform_device_register_simple()
> > logically. And we don't know what kind of change will be made into driver
> > core in future.
> 
> Ok, would you like me to ack your earlier patch or will you send a new
> one?

I'm working on an update to the tlclk driver to avoid a race in the open
and read functions.  Do you have any changes I should roll into the
driver?

--mgross