[1/4] rtc: isl1208: Fix unintended clear of SR bits

Message ID 20180123121801.4214-2-m.grzeschik@pengutronix.de
State Accepted
Headers show
Series
  • rtc: isl1208: fixes, documentation and isl1219 support
Related show

Commit Message

Michael Grzeschik Jan. 23, 2018, 12:17 p.m.
From: Denis Osterland <Denis.Osterland@diehl.com>

After successful
sr = isl1208_i2c_set_regs(client, 0, regs, ISL1208_RTC_SECTION_LEN);
sr will be 0.
As a result
sr = i2c_smbus_write_byte_data(client, ISL1208_REG_SR,
			sr & ~ISL1208_REG_SR_WRTC);
is equal to
sr = i2c_smbus_write_byte_data(client, ISL1208_REG_SR, 0);
which clears all flags in SR.

Add an additional read of SR, to have value of SR in sr again.

Signed-off-by: Denis Osterland <Denis.Osterland@diehl.com>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/rtc/rtc-isl1208.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Alexandre Belloni Feb. 14, 2018, 8:26 p.m. | #1
On 23/01/2018 at 13:17:58 +0100, Michael Grzeschik wrote:
> From: Denis Osterland <Denis.Osterland@diehl.com>
> 
> After successful
> sr = isl1208_i2c_set_regs(client, 0, regs, ISL1208_RTC_SECTION_LEN);
> sr will be 0.
> As a result
> sr = i2c_smbus_write_byte_data(client, ISL1208_REG_SR,
> 			sr & ~ISL1208_REG_SR_WRTC);
> is equal to
> sr = i2c_smbus_write_byte_data(client, ISL1208_REG_SR, 0);
> which clears all flags in SR.
> 
> Add an additional read of SR, to have value of SR in sr again.
> 
> Signed-off-by: Denis Osterland <Denis.Osterland@diehl.com>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/rtc/rtc-isl1208.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
Applied, thanks.
Denis OSTERLAND Feb. 15, 2018, 7:27 a.m. | #2
Am Mittwoch, den 14.02.2018, 21:26 +0100 schrieb Alexandre Belloni:
> On 23/01/2018 at 13:17:58 +0100, Michael Grzeschik wrote:

> > 

> > From: Denis Osterland <Denis.Osterland@diehl.com>

> > 

> > After successful

> > sr = isl1208_i2c_set_regs(client, 0, regs, ISL1208_RTC_SECTION_LEN);

> > sr will be 0.

> > As a result

> > sr = i2c_smbus_write_byte_data(client, ISL1208_REG_SR,

> > 			sr & ~ISL1208_REG_SR_WRTC);

> > is equal to

> > sr = i2c_smbus_write_byte_data(client, ISL1208_REG_SR, 0);

> > which clears all flags in SR.

> > 

> > Add an additional read of SR, to have value of SR in sr again.

> > 

> > Signed-off-by: Denis Osterland <Denis.Osterland@diehl.com>

> > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

> > ---

> >  drivers/rtc/rtc-isl1208.c | 5 +++++

> >  1 file changed, 5 insertions(+)

> > 

> Applied, thanks.

> 

You are welcome.

One question, shall we avoid resent this patch in v2 of this series?
I ask because we are pretty far with the suggested changes.

Regards Denis.
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.
Alexandre Belloni Feb. 15, 2018, 8:30 a.m. | #3
On 15/02/2018 at 07:27:47 +0000, Denis OSTERLAND wrote:
> Am Mittwoch, den 14.02.2018, 21:26 +0100 schrieb Alexandre Belloni:
> > On 23/01/2018 at 13:17:58 +0100, Michael Grzeschik wrote:
> > > 
> > > From: Denis Osterland <Denis.Osterland@diehl.com>
> > > 
> > > After successful
> > > sr = isl1208_i2c_set_regs(client, 0, regs, ISL1208_RTC_SECTION_LEN);
> > > sr will be 0.
> > > As a result
> > > sr = i2c_smbus_write_byte_data(client, ISL1208_REG_SR,
> > > 			sr & ~ISL1208_REG_SR_WRTC);
> > > is equal to
> > > sr = i2c_smbus_write_byte_data(client, ISL1208_REG_SR, 0);
> > > which clears all flags in SR.
> > > 
> > > Add an additional read of SR, to have value of SR in sr again.
> > > 
> > > Signed-off-by: Denis Osterland <Denis.Osterland@diehl.com>
> > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> > > ---
> > >  drivers/rtc/rtc-isl1208.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > Applied, thanks.
> > 
> You are welcome.
> 
> One question, shall we avoid resent this patch in v2 of this series?
> I ask because we are pretty far with the suggested changes.
> 

No need to resend. If necessary, you can base v2 on top of rtc-next.

Patch

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index 8dd299c6a1f33..c8b4953482296 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -459,6 +459,11 @@  isl1208_i2c_set_time(struct i2c_client *client, struct rtc_time const *tm)
 	}
 
 	/* clear WRTC again */
+	sr = isl1208_i2c_get_sr(client);
+	if (sr < 0) {
+		dev_err(&client->dev, "%s: reading SR failed\n", __func__);
+		return sr;
+	}
 	sr = i2c_smbus_write_byte_data(client, ISL1208_REG_SR,
 				       sr & ~ISL1208_REG_SR_WRTC);
 	if (sr < 0) {