Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1427405 > unrolled thread

[PATCH v2 02/17] RTC: ds1307: Disable square wave and timers as default

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 02/17] RTC: ds1307: Disable square wave and timers as default Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-06-21 09:30 +0200
    [PATCH v2 13/17] RTC: ds1307: Report oscillator problems more intelligently Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-06-21 09:30 +0200
      Re: [PATCH v2 13/17] RTC: ds1307: Report oscillator problems more  intelligently Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2016-06-21 23:30 +0200
        Re: [PATCH v2 13/17] RTC: ds1307: Report oscillator problems more intelligently Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-06-22 01:10 +0200
    [PATCH v2 10/17] RTC: ds1307: Constify struct ds1307 where possible Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-06-21 09:30 +0200
    Re: [PATCH v2 02/17] RTC: ds1307: Disable square wave and timers as  default Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2016-06-21 23:20 +0200
      Re: [PATCH v2 02/17] RTC: ds1307: Disable square wave and timers as default Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-06-22 01:50 +0200

#1427405 — [PATCH v2 02/17] RTC: ds1307: Disable square wave and timers as default

Disable square wave and timers as default for DS1337/39/41 and
DS3231. The rationale being that configuring a chip this way puts it
into a known state with lower power consumption. While it is not very
likely it is still possible that the code controlling RTCs that ran
before this driver configured it to produce square wave and left it in
such a state.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 7e65e2e..c618c22 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1320,19 +1320,17 @@ static int ds1307_probe(struct i2c_client *client,
 			ds1307->regs[0] &= ~DS1337_BIT_nEOSC;
 
 		/*
-		 * Using IRQ or defined as wakeup-source?
 		 * Disable the square wave and both alarms.
 		 * For some variants, be sure alarms can trigger when we're
 		 * running on Vbackup (BBSQI/BBSQW)
 		 */
-		if (chip->alarm && (ds1307->client->irq > 0 ||
-						ds1307_can_wakeup_device)) {
-			ds1307->regs[0] |= DS1337_BIT_INTCN
-					| bbsqi_bitpos[ds1307->type];
-			ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
+		ds1307->regs[0] |= DS1337_BIT_INTCN
+			| bbsqi_bitpos[ds1307->type];
+		ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
 
+		if (chip->alarm && (ds1307->client->irq > 0 ||
+				    ds1307_can_wakeup_device))
 			want_irq = true;
-		}
 
 		i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
 							ds1307->regs[0]);
-- 
2.5.5

[toc] | [next] | [standalone]

#1427406 — [PATCH v2 13/17] RTC: ds1307: Report oscillator problems more intelligently

Report oscillator problems more intelligently, by printing more
information about what cause the issue and not yelling "SET TIME!" at
the user.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 2af9c00..2169e5b 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1424,6 +1424,19 @@ static int ds1307_chip_configure(const struct ds1307 *ds1307)
 	return 0;
 }
 
+static void ds1307_report_clock_halt(const struct ds1307 *ds1307)
+{
+	dev_warn(&ds1307->client->dev, "RTC's oscillator is turned off. "
+		 "Turning it on. Please set time");
+}
+
+static void ds1307_report_oscillator_fault(const struct ds1307 *ds1307)
+{
+	dev_warn(&ds1307->client->dev, "RTC's reported oscillator fault. "
+		 "Clearing the fault flag and re-reading RTC status. "
+		 "Please set time");
+}
+
 static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 {
 	int tmp, retries;
@@ -1453,7 +1466,7 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 			if (tmp & DS1307_BIT_CH) {
 				i2c_smbus_write_byte_data(client,
 							  DS1307_REG_SECS, 0);
-				dev_warn(&client->dev, "SET TIME!\n");
+				ds1307_report_clock_halt(ds1307);
 				continue;
 			}
 			break;
@@ -1469,15 +1482,17 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 							  DS1307_REG_CONTROL,
 							  regs[DS1307_REG_CONTROL]
 							  & ~DS1338_BIT_OSF);
-				dev_warn(&client->dev, "SET TIME!\n");
+				ds1307_report_oscillator_fault(ds1307);
 				continue;
 			}
 			break;
 		case ds_1340:
 			/* clock halted?  turn it on, so clock can tick. */
-			if (tmp & DS1340_BIT_nEOSC)
+			if (tmp & DS1340_BIT_nEOSC) {
 				i2c_smbus_write_byte_data(client,
 							  DS1307_REG_SECS, 0);
+				ds1307_report_clock_halt(ds1307);
+			}
 
 			tmp = i2c_smbus_read_byte_data(client, DS1340_REG_FLAG);
 			if (tmp < 0) {
@@ -1489,7 +1504,7 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 			if (tmp & DS1340_BIT_OSF) {
 				i2c_smbus_write_byte_data(client,
 							  DS1340_REG_FLAG, 0);
-				dev_warn(&client->dev, "SET TIME!\n");
+				ds1307_report_oscillator_fault(ds1307);
 			}
 			return 0;
 
@@ -1500,6 +1515,9 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 							  DS1307_REG_WDAY,
 							  regs[DS1307_REG_WDAY]
 							  | MCP794XX_BIT_VBATEN);
+				dev_warn(&client->dev,
+					 "battery backup was disabled. "
+					 "Re-enabling it\n");
 			}
 
 			/* clock halted?  turn it on, so clock can tick. */
@@ -1507,7 +1525,7 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 				i2c_smbus_write_byte_data(client,
 							  DS1307_REG_SECS,
 							  MCP794XX_BIT_ST);
-				dev_warn(&client->dev, "SET TIME!\n");
+				ds1307_report_clock_halt(ds1307);
 				continue;
 			}
 
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1428180 — Re: [PATCH v2 13/17] RTC: ds1307: Report oscillator problems more intelligently

On 21/06/2016 at 00:22:46 -0700, Andrey Smirnov wrote :
> Report oscillator problems more intelligently, by printing more
> information about what cause the issue and not yelling "SET TIME!" at
> the user.
> 

Well, the proper way of doing that is to ensure that -EINVAL is returned
when reading the time until it has been set once instead of starting the
oscillator and forgetting about that useful information.

> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  drivers/rtc/rtc-ds1307.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index 2af9c00..2169e5b 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -1424,6 +1424,19 @@ static int ds1307_chip_configure(const struct ds1307 *ds1307)
>  	return 0;
>  }
>  
> +static void ds1307_report_clock_halt(const struct ds1307 *ds1307)
> +{
> +	dev_warn(&ds1307->client->dev, "RTC's oscillator is turned off. "
> +		 "Turning it on. Please set time");
> +}
> +
> +static void ds1307_report_oscillator_fault(const struct ds1307 *ds1307)
> +{
> +	dev_warn(&ds1307->client->dev, "RTC's reported oscillator fault. "
> +		 "Clearing the fault flag and re-reading RTC status. "
> +		 "Please set time");
> +}
> +
>  static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
>  {
>  	int tmp, retries;
> @@ -1453,7 +1466,7 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
>  			if (tmp & DS1307_BIT_CH) {
>  				i2c_smbus_write_byte_data(client,
>  							  DS1307_REG_SECS, 0);
> -				dev_warn(&client->dev, "SET TIME!\n");
> +				ds1307_report_clock_halt(ds1307);
>  				continue;
>  			}
>  			break;
> @@ -1469,15 +1482,17 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
>  							  DS1307_REG_CONTROL,
>  							  regs[DS1307_REG_CONTROL]
>  							  & ~DS1338_BIT_OSF);
> -				dev_warn(&client->dev, "SET TIME!\n");
> +				ds1307_report_oscillator_fault(ds1307);
>  				continue;
>  			}
>  			break;
>  		case ds_1340:
>  			/* clock halted?  turn it on, so clock can tick. */
> -			if (tmp & DS1340_BIT_nEOSC)
> +			if (tmp & DS1340_BIT_nEOSC) {
>  				i2c_smbus_write_byte_data(client,
>  							  DS1307_REG_SECS, 0);
> +				ds1307_report_clock_halt(ds1307);
> +			}
>  
>  			tmp = i2c_smbus_read_byte_data(client, DS1340_REG_FLAG);
>  			if (tmp < 0) {
> @@ -1489,7 +1504,7 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
>  			if (tmp & DS1340_BIT_OSF) {
>  				i2c_smbus_write_byte_data(client,
>  							  DS1340_REG_FLAG, 0);
> -				dev_warn(&client->dev, "SET TIME!\n");
> +				ds1307_report_oscillator_fault(ds1307);
>  			}
>  			return 0;
>  
> @@ -1500,6 +1515,9 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
>  							  DS1307_REG_WDAY,
>  							  regs[DS1307_REG_WDAY]
>  							  | MCP794XX_BIT_VBATEN);
> +				dev_warn(&client->dev,
> +					 "battery backup was disabled. "
> +					 "Re-enabling it\n");
>  			}
>  
>  			/* clock halted?  turn it on, so clock can tick. */
> @@ -1507,7 +1525,7 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
>  				i2c_smbus_write_byte_data(client,
>  							  DS1307_REG_SECS,
>  							  MCP794XX_BIT_ST);
> -				dev_warn(&client->dev, "SET TIME!\n");
> +				ds1307_report_clock_halt(ds1307);
>  				continue;
>  			}
>  
> -- 
> 2.5.5
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1428254 — Re: [PATCH v2 13/17] RTC: ds1307: Report oscillator problems more intelligently

On Tue, Jun 21, 2016 at 2:22 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> On 21/06/2016 at 00:22:46 -0700, Andrey Smirnov wrote :
>> Report oscillator problems more intelligently, by printing more
>> information about what cause the issue and not yelling "SET TIME!" at
>> the user.
>>
>
> Well, the proper way of doing that is to ensure that -EINVAL is returned
> when reading the time until it has been set once instead of starting the
> oscillator and forgetting about that useful information.

Agreed, will change in v3.

[toc] | [prev] | [next] | [standalone]

#1427407 — [PATCH v2 10/17] RTC: ds1307: Constify struct ds1307 where possible

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 50 ++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 66e7168..67907f1 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -333,7 +333,7 @@ static s32 ds1307_native_smbus_read_block_data(const struct i2c_client *client,
 static irqreturn_t ds1307_irq(int irq, void *dev_id)
 {
 	struct i2c_client	*client = dev_id;
-	struct ds1307		*ds1307 = i2c_get_clientdata(client);
+	const struct ds1307	*ds1307 = i2c_get_clientdata(client);
 	struct mutex		*lock = &ds1307->rtc->ops_lock;
 	int			stat, control;
 
@@ -367,8 +367,8 @@ out:
 static int ds1307_get_time(struct device *dev, struct rtc_time *t)
 {
 	u8              regs[DS1307_REG_COUNT];
-	struct ds1307	*ds1307 = dev_get_drvdata(dev);
 	int		tmp;
+	const struct ds1307 *ds1307 = dev_get_drvdata(dev);
 
 	/* read the RTC date and time registers all at once */
 	tmp = ds1307->read_block_data(ds1307->client,
@@ -404,7 +404,7 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)
 
 static int ds1307_set_time(struct device *dev, struct rtc_time *t)
 {
-	struct ds1307	*ds1307 = dev_get_drvdata(dev);
+	const struct ds1307 *ds1307 = dev_get_drvdata(dev);
 	int		result;
 	int		tmp;
 	u8		regs[DS1307_REG_COUNT];
@@ -464,7 +464,7 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)
 static int ds1337_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client       *client = to_i2c_client(dev);
-	struct ds1307		*ds1307 = i2c_get_clientdata(client);
+	const struct ds1307	*ds1307 = i2c_get_clientdata(client);
 	int			ret;
 	u8			regs[DS1307_REG_COUNT];
 
@@ -512,7 +512,7 @@ static int ds1337_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 static int ds1337_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client	*client = to_i2c_client(dev);
-	struct ds1307		*ds1307 = i2c_get_clientdata(client);
+	const struct ds1307	*ds1307 = i2c_get_clientdata(client);
 	u8			regs[DS1307_REG_COUNT];
 	u8			control, status;
 	int			ret;
@@ -574,7 +574,7 @@ static int ds1337_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 static int ds1307_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
 	struct i2c_client	*client = to_i2c_client(dev);
-	struct ds1307		*ds1307 = i2c_get_clientdata(client);
+	const struct ds1307	*ds1307 = i2c_get_clientdata(client);
 	int			ret;
 
 	if (!test_bit(HAS_ALARM, &ds1307->flags))
@@ -629,7 +629,7 @@ static const struct rtc_class_ops ds13xx_rtc_ops = {
 static irqreturn_t mcp794xx_irq(int irq, void *dev_id)
 {
 	struct i2c_client       *client = dev_id;
-	struct ds1307           *ds1307 = i2c_get_clientdata(client);
+	const struct ds1307     *ds1307 = i2c_get_clientdata(client);
 	struct mutex            *lock = &ds1307->rtc->ops_lock;
 	int reg, ret;
 
@@ -666,7 +666,7 @@ out:
 static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client *client = to_i2c_client(dev);
-	struct ds1307 *ds1307 = i2c_get_clientdata(client);
+	const struct ds1307 *ds1307 = i2c_get_clientdata(client);
 	u8  regs[DS1307_REG_COUNT];
 	int ret;
 
@@ -705,7 +705,7 @@ static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client *client = to_i2c_client(dev);
-	struct ds1307 *ds1307 = i2c_get_clientdata(client);
+	const struct ds1307 *ds1307 = i2c_get_clientdata(client);
 	u8  regs[DS1307_REG_COUNT];
 	int ret;
 
@@ -751,7 +751,7 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 static int mcp794xx_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
 	struct i2c_client *client = to_i2c_client(dev);
-	struct ds1307 *ds1307 = i2c_get_clientdata(client);
+	const struct ds1307 *ds1307 = i2c_get_clientdata(client);
 	int reg;
 
 	if (!test_bit(HAS_ALARM, &ds1307->flags))
@@ -785,7 +785,7 @@ ds1307_nvram_read(struct file *filp, struct kobject *kobj,
 		char *buf, loff_t off, size_t count)
 {
 	struct i2c_client	*client;
-	struct ds1307		*ds1307;
+	const struct ds1307	*ds1307;
 	int			result;
 
 	client = kobj_to_i2c_client(kobj);
@@ -804,7 +804,7 @@ ds1307_nvram_write(struct file *filp, struct kobject *kobj,
 		char *buf, loff_t off, size_t count)
 {
 	struct i2c_client	*client;
-	struct ds1307		*ds1307;
+	const struct ds1307	*ds1307;
 	int			result;
 
 	client = kobj_to_i2c_client(kobj);
@@ -880,7 +880,7 @@ out:
  */
 static int ds3231_hwmon_read_temp(struct device *dev, s32 *mC)
 {
-	struct ds1307 *ds1307 = dev_get_drvdata(dev);
+	const struct ds1307 *ds1307 = dev_get_drvdata(dev);
 	u8 temp_buf[2];
 	s16 temp;
 	int ret;
@@ -973,7 +973,7 @@ static int ds3231_clk_sqw_rates[] = {
 	8192,
 };
 
-static int ds1337_write_control(struct ds1307 *ds1307, u8 mask, u8 value)
+static int ds1337_write_control(const struct ds1307 *ds1307, u8 mask, u8 value)
 {
 	struct i2c_client *client = ds1307->client;
 	struct mutex *lock = &ds1307->rtc->ops_lock;
@@ -1001,7 +1001,7 @@ out:
 static unsigned long ds3231_clk_sqw_recalc_rate(struct clk_hw *hw,
 						unsigned long parent_rate)
 {
-	struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
 	int control;
 	int rate_sel = 0;
 
@@ -1032,7 +1032,7 @@ static long ds3231_clk_sqw_round_rate(struct clk_hw *hw, unsigned long rate,
 static int ds3231_clk_sqw_set_rate(struct clk_hw *hw, unsigned long rate,
 					unsigned long parent_rate)
 {
-	struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
 	int control = 0;
 	int rate_sel;
 
@@ -1056,21 +1056,21 @@ static int ds3231_clk_sqw_set_rate(struct clk_hw *hw, unsigned long rate,
 
 static int ds3231_clk_sqw_prepare(struct clk_hw *hw)
 {
-	struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
 
 	return ds1337_write_control(ds1307, DS1337_BIT_INTCN, 0);
 }
 
 static void ds3231_clk_sqw_unprepare(struct clk_hw *hw)
 {
-	struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
 
 	ds1337_write_control(ds1307, DS1337_BIT_INTCN, DS1337_BIT_INTCN);
 }
 
 static int ds3231_clk_sqw_is_prepared(struct clk_hw *hw)
 {
-	struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
 	int control;
 
 	control = i2c_smbus_read_byte_data(ds1307->client, DS1337_REG_CONTROL);
@@ -1095,7 +1095,7 @@ static unsigned long ds3231_clk_32khz_recalc_rate(struct clk_hw *hw,
 	return 32768;
 }
 
-static int ds3231_clk_32khz_control(struct ds1307 *ds1307, bool enable)
+static int ds3231_clk_32khz_control(const struct ds1307 *ds1307, bool enable)
 {
 	struct i2c_client *client = ds1307->client;
 	struct mutex *lock = &ds1307->rtc->ops_lock;
@@ -1124,21 +1124,21 @@ out:
 
 static int ds3231_clk_32khz_prepare(struct clk_hw *hw)
 {
-	struct ds1307 *ds1307 = clk_32khz_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_32khz_to_ds1307(hw);
 
 	return ds3231_clk_32khz_control(ds1307, true);
 }
 
 static void ds3231_clk_32khz_unprepare(struct clk_hw *hw)
 {
-	struct ds1307 *ds1307 = clk_32khz_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_32khz_to_ds1307(hw);
 
 	ds3231_clk_32khz_control(ds1307, false);
 }
 
 static int ds3231_clk_32khz_is_prepared(struct clk_hw *hw)
 {
-	struct ds1307 *ds1307 = clk_32khz_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_32khz_to_ds1307(hw);
 	int status;
 
 	status = i2c_smbus_read_byte_data(ds1307->client, DS1337_REG_STATUS);
@@ -1273,7 +1273,7 @@ static bool ds1307_want_irq(const struct ds1307 *ds1307,
 	return false;
 }
 
-static int ds1307_chip_configure(struct ds1307 *ds1307)
+static int ds1307_chip_configure(const struct ds1307 *ds1307)
 {
 	int tmp;
 	u8  regs[DS1307_REG_COUNT];
@@ -1422,7 +1422,7 @@ static int ds1307_chip_configure(struct ds1307 *ds1307)
 	return 0;
 }
 
-static int ds1307_chip_sanity_check(struct ds1307 *ds1307)
+static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 {
 	int tmp;
 	u8  regs[DS1307_REG_COUNT];
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1428177 — Re: [PATCH v2 02/17] RTC: ds1307: Disable square wave and timers as default

On 21/06/2016 at 00:22:35 -0700, Andrey Smirnov wrote :
> Disable square wave and timers as default for DS1337/39/41 and
> DS3231. The rationale being that configuring a chip this way puts it
> into a known state with lower power consumption. While it is not very
> likely it is still possible that the code controlling RTCs that ran
> before this driver configured it to produce square wave and left it in
> such a state.
> 

The main issue being that if it has been configured that way (from the
bootloader for example). It is probably necessary for the board. There
may be users of that clock and this patch definitively breaks them.
The proper way of doing that is to add CCF support in the driver. See:
http://patchwork.ozlabs.org/patch/576201/

> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  drivers/rtc/rtc-ds1307.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index 7e65e2e..c618c22 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -1320,19 +1320,17 @@ static int ds1307_probe(struct i2c_client *client,
>  			ds1307->regs[0] &= ~DS1337_BIT_nEOSC;
>  
>  		/*
> -		 * Using IRQ or defined as wakeup-source?
>  		 * Disable the square wave and both alarms.
>  		 * For some variants, be sure alarms can trigger when we're
>  		 * running on Vbackup (BBSQI/BBSQW)
>  		 */
> -		if (chip->alarm && (ds1307->client->irq > 0 ||
> -						ds1307_can_wakeup_device)) {
> -			ds1307->regs[0] |= DS1337_BIT_INTCN
> -					| bbsqi_bitpos[ds1307->type];
> -			ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
> +		ds1307->regs[0] |= DS1337_BIT_INTCN
> +			| bbsqi_bitpos[ds1307->type];
> +		ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
>  
> +		if (chip->alarm && (ds1307->client->irq > 0 ||
> +				    ds1307_can_wakeup_device))
>  			want_irq = true;
> -		}
>  
>  		i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
>  							ds1307->regs[0]);
> -- 
> 2.5.5
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1428265

On Tue, Jun 21, 2016 at 2:17 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> On 21/06/2016 at 00:22:35 -0700, Andrey Smirnov wrote :
>> Disable square wave and timers as default for DS1337/39/41 and
>> DS3231. The rationale being that configuring a chip this way puts it
>> into a known state with lower power consumption. While it is not very
>> likely it is still possible that the code controlling RTCs that ran
>> before this driver configured it to produce square wave and left it in
>> such a state.
>>
>
> The main issue being that if it has been configured that way (from the
> bootloader for example). It is probably necessary for the board. There
> may be users of that clock and this patch definitively breaks them.
> The proper way of doing that is to add CCF support in the driver. See:
> http://patchwork.ozlabs.org/patch/576201/

Implementing CCF support would still break users who rely on this
particular behavior and they would still have to spend effort
modifying their boards' device tree blob, so doing it the way you
propose wouldn't really save those users from pain, it would just give
them a way out.

Unfortunately, I don't have a bandwidth to develop and test a feature
that I don't have a use-case for, so I'll drop this patch from v3.

Andrey

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web