Message ID | 1476712411-20970-1-git-send-email-mirza.krak@gmail.com |
---|---|
State | Accepted |
Headers | show |
Hi, On 17/10/2016 at 15:53:31 +0200, Mirza Krak wrote : > From: Mirza Krak <mirza.krak@hostmobility.com> > > Add a sanity check to see if chip is present. If we can not communicate > with the chip there is no point in registering a RTC device. > Does it makes sense? If the device is not present, why has it been registered in the first place? > Signed-off-by: Mirza Krak <mirza.krak@hostmobility.com> > --- > drivers/rtc/rtc-pcf85063.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c > index efb0a08..a06dff9 100644 > --- a/drivers/rtc/rtc-pcf85063.c > +++ b/drivers/rtc/rtc-pcf85063.c > @@ -191,12 +191,19 @@ static int pcf85063_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > struct rtc_device *rtc; > + int err; > > dev_dbg(&client->dev, "%s\n", __func__); > > if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) > return -ENODEV; > > + err = i2c_smbus_read_byte_data(client, PCF85063_REG_CTRL1); > + if (err < 0) { > + dev_err(&client->dev, "RTC chip is not present\n"); > + return err; > + } > + > rtc = devm_rtc_device_register(&client->dev, > pcf85063_driver.driver.name, > &pcf85063_rtc_ops, THIS_MODULE); > -- > 2.1.4 >
2016-10-17 18:23 GMT+02:00 Alexandre Belloni <alexandre.belloni@free-electrons.com>: > Hi, > > On 17/10/2016 at 15:53:31 +0200, Mirza Krak wrote : >> From: Mirza Krak <mirza.krak@hostmobility.com> >> >> Add a sanity check to see if chip is present. If we can not communicate >> with the chip there is no point in registering a RTC device. >> > > Does it makes sense? If the device is not present, why has it been > registered in the first place? It made sense to me, that if you can not communicated with a chip at probe time for whatever reason (chip fault, bus problem, "not present") then probe should fail. Maybe I shouldn't have used the word "present", because obviously we expect the chip to be there, but I experienced this on a board where the chip was not mounted and the driver produces a lot of errors because it registered an RTC device for something that is not there. Looking around in other rtc drivers, it is common to do a communication check at probe time and fail probe if the communication check does not go trough.
On 17/10/2016 at 15:53:31 +0200, Mirza Krak wrote : > From: Mirza Krak <mirza.krak@hostmobility.com> > > Add a sanity check to see if chip is present. If we can not communicate > with the chip there is no point in registering a RTC device. > > Signed-off-by: Mirza Krak <mirza.krak@hostmobility.com> > --- > drivers/rtc/rtc-pcf85063.c | 7 +++++++ > 1 file changed, 7 insertions(+) > Applied, thanks.
diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c index efb0a08..a06dff9 100644 --- a/drivers/rtc/rtc-pcf85063.c +++ b/drivers/rtc/rtc-pcf85063.c @@ -191,12 +191,19 @@ static int pcf85063_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct rtc_device *rtc; + int err; dev_dbg(&client->dev, "%s\n", __func__); if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) return -ENODEV; + err = i2c_smbus_read_byte_data(client, PCF85063_REG_CTRL1); + if (err < 0) { + dev_err(&client->dev, "RTC chip is not present\n"); + return err; + } + rtc = devm_rtc_device_register(&client->dev, pcf85063_driver.driver.name, &pcf85063_rtc_ops, THIS_MODULE);