diff mbox series

[v2,2/4] rtc: isl1208: switch to rtc_register_device

Message ID 1519821214-22379-3-git-send-email-Denis.Osterland@diehl.com
State Superseded
Headers show
Series rtc: isl1208: fixes, documentation and isl1219 support | expand

Commit Message

Denis Osterland-Heim Feb. 28, 2018, 12:45 p.m. UTC 
From: Denis Osterland <Denis.Osterland@diehl.com>

Fix possible race condition.
It is not allowed to return with an error code after RTC is registered.

Suggested-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Signed-off-by: Denis Osterland <Denis.Osterland@diehl.com>
Reviewed-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/rtc/rtc-isl1208.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

kernel test robot March 3, 2018, 2:35 a.m. UTC | #1 
Hi Denis,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v4.16-rc3 next-20180302]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Denis-OSTERLAND/rtc-isl1208-fixes-documentation-and-isl1219-support/20180303-093405
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
config: x86_64-randconfig-x017-201808 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Denis-OSTERLAND/rtc-isl1208-fixes-documentation-and-isl1219-support/20180303-093405 HEAD 1d3cf882ecd061847968da1511ec956b9bf70d33 builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   In file included from drivers/rtc/rtc-isl1208.c:16:0:
   drivers/rtc/rtc-isl1208.c: In function 'isl1208_probe':
>> drivers/rtc/rtc-isl1208.c:677:29: error: 'isl1208' undeclared (first use in this function)
     return rtc_register_device(isl1208->rtc);
                                ^
   include/linux/rtc.h:261:37: note: in definition of macro 'rtc_register_device'
     __rtc_register_device(THIS_MODULE, device)
                                        ^~~~~~
   drivers/rtc/rtc-isl1208.c:677:29: note: each undeclared identifier is reported only once for each function it appears in
     return rtc_register_device(isl1208->rtc);
                                ^
   include/linux/rtc.h:261:37: note: in definition of macro 'rtc_register_device'
     __rtc_register_device(THIS_MODULE, device)
                                        ^~~~~~
>> drivers/rtc/rtc-isl1208.c:678:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^

vim +/isl1208 +677 drivers/rtc/rtc-isl1208.c

   625	
   626	static int
   627	isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
   628	{
   629		int rc = 0;
   630		struct rtc_device *rtc;
   631	
   632		if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
   633			return -ENODEV;
   634	
   635		if (isl1208_i2c_validate_client(client) < 0)
   636			return -ENODEV;
   637	
   638		rtc = devm_rtc_allocate_device(&client->dev);
   639		if (IS_ERR(rtc))
   640			return PTR_ERR(rtc);
   641	
   642		rtc->ops = &isl1208_rtc_ops;
   643	
   644		i2c_set_clientdata(client, rtc);
   645	
   646		rc = isl1208_i2c_get_sr(client);
   647		if (rc < 0) {
   648			dev_err(&client->dev, "reading status failed\n");
   649			return rc;
   650		}
   651	
   652		if (rc & ISL1208_REG_SR_RTCF)
   653			dev_warn(&client->dev, "rtc power failure detected, "
   654				 "please set clock.\n");
   655	
   656		rc = sysfs_create_group(&client->dev.kobj, &isl1208_rtc_sysfs_files);
   657		if (rc)
   658			return rc;
   659	
   660		if (client->irq > 0) {
   661			rc = devm_request_threaded_irq(&client->dev, client->irq, NULL,
   662						       isl1208_rtc_interrupt,
   663						       IRQF_SHARED | IRQF_ONESHOT,
   664						       isl1208_driver.driver.name,
   665						       client);
   666			if (!rc) {
   667				device_init_wakeup(&client->dev, 1);
   668				enable_irq_wake(client->irq);
   669			} else {
   670				dev_err(&client->dev,
   671					"Unable to request irq %d, no alarm support\n",
   672					client->irq);
   673				client->irq = 0;
   674			}
   675		}
   676	
 > 677		return rtc_register_device(isl1208->rtc);
 > 678	}
   679	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Denis Osterland-Heim March 5, 2018, 7:44 a.m. UTC | #2 
Hi,

an obvious mistake by me.
isl1208 is introduced in patch 3/4.
I will send v3.
Sorry for that.

Regards Denis

Am Samstag, den 03.03.2018, 10:35 +0800 schrieb kbuild test robot:
> Hi Denis,

> 

> Thank you for the patch! Yet something to improve:

> 

> [auto build test ERROR on abelloni/rtc-next]

> [also build test ERROR on v4.16-rc3 next-20180302]

> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

> 

> url:    https://github.com/0day-ci/linux/commits/Denis-OSTERLAND/rtc-isl1208-fixes-documentation-and-isl1219-support/20180303-093405

> base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next

> config: x86_64-randconfig-x017-201808 (attached as .config)

> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0

> reproduce:

>         # save the attached .config to linux build tree

>         make ARCH=x86_64 

> 

> Note: the linux-review/Denis-OSTERLAND/rtc-isl1208-fixes-documentation-and-isl1219-support/20180303-093405 HEAD 1d3cf882ecd061847968da1511ec956b9bf70d33 builds fine.

>       It only hurts bisectibility.

> 

> All error/warnings (new ones prefixed by >>):

> 

>    In file included from drivers/rtc/rtc-isl1208.c:16:0:

>    drivers/rtc/rtc-isl1208.c: In function 'isl1208_probe':

> > 

> > > 

> > > drivers/rtc/rtc-isl1208.c:677:29: error: 'isl1208' undeclared (first use in this function)

>      return rtc_register_device(isl1208->rtc);

>                                 ^

>    include/linux/rtc.h:261:37: note: in definition of macro 'rtc_register_device'

>      __rtc_register_device(THIS_MODULE, device)

>                                         ^~~~~~

>    drivers/rtc/rtc-isl1208.c:677:29: note: each undeclared identifier is reported only once for each function it appears in

>      return rtc_register_device(isl1208->rtc);

>                                 ^

>    include/linux/rtc.h:261:37: note: in definition of macro 'rtc_register_device'

>      __rtc_register_device(THIS_MODULE, device)

>                                         ^~~~~~

> > 

> > > 

> > > drivers/rtc/rtc-isl1208.c:678:1: warning: control reaches end of non-void function [-Wreturn-type]

>     }

>     ^

> 

> vim +/isl1208 +677 drivers/rtc/rtc-isl1208.c

> 

>    625	

>    626	static int

>    627	isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)

>    628	{

>    629		int rc = 0;

>    630		struct rtc_device *rtc;

>    631	

>    632		if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))

>    633			return -ENODEV;

>    634	

>    635		if (isl1208_i2c_validate_client(client) < 0)

>    636			return -ENODEV;

>    637	

>    638		rtc = devm_rtc_allocate_device(&client->dev);

>    639		if (IS_ERR(rtc))

>    640			return PTR_ERR(rtc);

>    641	

>    642		rtc->ops = &isl1208_rtc_ops;

>    643	

>    644		i2c_set_clientdata(client, rtc);

>    645	

>    646		rc = isl1208_i2c_get_sr(client);

>    647		if (rc < 0) {

>    648			dev_err(&client->dev, "reading status failed\n");

>    649			return rc;

>    650		}

>    651	

>    652		if (rc & ISL1208_REG_SR_RTCF)

>    653			dev_warn(&client->dev, "rtc power failure detected, "

>    654				 "please set clock.\n");

>    655	

>    656		rc = sysfs_create_group(&client->dev.kobj, &isl1208_rtc_sysfs_files);

>    657		if (rc)

>    658			return rc;

>    659	

>    660		if (client->irq > 0) {

>    661			rc = devm_request_threaded_irq(&client->dev, client->irq, NULL,

>    662						       isl1208_rtc_interrupt,

>    663						       IRQF_SHARED | IRQF_ONESHOT,

>    664						       isl1208_driver.driver.name,

>    665						       client);

>    666			if (!rc) {

>    667				device_init_wakeup(&client->dev, 1);

>    668				enable_irq_wake(client->irq);

>    669			} else {

>    670				dev_err(&client->dev,

>    671					"Unable to request irq %d, no alarm support\n",

>    672					client->irq);

>    673				client->irq = 0;

>    674			}

>    675		}

>    676	

>  > 677		return rtc_register_device(isl1208->rtc);

>  > 678	}

>    679	

> 

> ---

> 0-DAY kernel test infrastructure                Open Source Technology Center

> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Diehl AKO Stiftung & Co. KG, Pfannerstraße 75-83, 88239 Wangen im Allgäu
Bereichsvorstand: Dr.-Ing. Michael Siedentop (Sprecher), Josef Fellner (Mitglied)
Sitz der Gesellschaft: Wangen i.A. – Registergericht: Amtsgericht Ulm HRA 620609 – Persönlich haftende Gesellschafterin: Diehl Verwaltungs-Stiftung – Sitz: Nürnberg – Registergericht: Amtsgericht Nürnberg HRA 11756 –
Vorstand: Dr.-Ing. E.h. Thomas Diehl (†) (Vorsitzender), Herr Dipl.-Wirtsch.-Ing. Wolfgang Weggen (stellvertretender Vorsitzender), Dipl.-Kfm. Claus Günther, Dipl.-Kfm. Frank Gutzeit, Dr.-Ing. Heinrich Schunk, Dr.-Ing. Michael Siedentop , Dipl.-Kfm. Dr.-Ing. Martin Sommer, Dipl.-Ing. (FH) Rainer von Borstel, Vorsitzender des Aufsichtsrates: Dr. Klaus Maier
___________________________________________________________________________________________________
Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.
diff mbox series

Patch

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index a13a4ba..908fb4d 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -635,12 +635,12 @@  isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	if (isl1208_i2c_validate_client(client) < 0)
 		return -ENODEV;
 
-	rtc = devm_rtc_device_register(&client->dev, isl1208_driver.driver.name,
-				  &isl1208_rtc_ops,
-				  THIS_MODULE);
+	rtc = devm_rtc_allocate_device(&client->dev);
 	if (IS_ERR(rtc))
 		return PTR_ERR(rtc);
 
+	rtc->ops = &isl1208_rtc_ops;
+
 	i2c_set_clientdata(client, rtc);
 
 	rc = isl1208_i2c_get_sr(client);
@@ -674,7 +674,7 @@  isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		}
 	}
 
-	return 0;
+	return rtc_register_device(isl1208->rtc);
 }
 
 static int