* [RFCv2] rtc: ds1307: add square wave output from ds1308
From: Sean Nyekjaer @ 2017-08-02 5:27 UTC (permalink / raw)
To: linux-rtc; +Cc: Sean Nyekjaer, linux-clk, alexandre.belloni
Add square wave output to generic clock framework
Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
---
This contains some code duplication(of my opinion) I guess it could be done more
generic together with the ds3231.
Changes from v1:
- Removed explicit disabling of square wave output when running from battery.
When the clock is disabled using the SQWE bit it's also disabled when running
from battery.
drivers/rtc/rtc-ds1307.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 165 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 1cedb21ba792..0c0484260411 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -79,6 +79,7 @@ enum ds_type {
# define DS1307_BIT_OUT 0x80
# define DS1338_BIT_OSF 0x20
# define DS1307_BIT_SQWE 0x10
+# define DS1308_BIT_BBCLK 0x04
# define DS1307_BIT_RS1 0x02
# define DS1307_BIT_RS0 0x01
#define DS1337_REG_CONTROL 0x0e
@@ -1056,6 +1057,13 @@ enum {
#define clk_32khz_to_ds1307(clk) \
container_of(clk, struct ds1307, clks[DS3231_CLK_32KHZ])
+static int ds1308_clk_sqw_rates[] = {
+ 1,
+ 4096,
+ 8192,
+ 32768,
+};
+
static int ds3231_clk_sqw_rates[] = {
1,
1024,
@@ -1063,6 +1071,19 @@ static int ds3231_clk_sqw_rates[] = {
8192,
};
+static int ds1308_write_control(struct ds1307 *ds1307, u8 mask, u8 value)
+{
+ struct mutex *lock = &ds1307->rtc->ops_lock;
+ int ret;
+
+ mutex_lock(lock);
+ ret = regmap_update_bits(ds1307->regmap, DS1307_REG_CONTROL,
+ mask, value);
+ mutex_unlock(lock);
+
+ return ret;
+}
+
static int ds1337_write_control(struct ds1307 *ds1307, u8 mask, u8 value)
{
struct mutex *lock = &ds1307->rtc->ops_lock;
@@ -1076,6 +1097,24 @@ static int ds1337_write_control(struct ds1307 *ds1307, u8 mask, u8 value)
return ret;
}
+static unsigned long ds1308_clk_sqw_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+ int control, ret;
+ int rate_sel = 0;
+
+ ret = regmap_read(ds1307->regmap, DS1307_REG_CONTROL, &control);
+ if (ret)
+ return ret;
+ if (control & DS1307_BIT_RS0)
+ rate_sel += 1;
+ if (control & DS1307_BIT_RS1)
+ rate_sel += 2;
+
+ return ds1308_clk_sqw_rates[rate_sel];
+}
+
static unsigned long ds3231_clk_sqw_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
@@ -1094,6 +1133,19 @@ static unsigned long ds3231_clk_sqw_recalc_rate(struct clk_hw *hw,
return ds3231_clk_sqw_rates[rate_sel];
}
+static long ds1308_clk_sqw_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ int i;
+
+ for (i = ARRAY_SIZE(ds1308_clk_sqw_rates) - 1; i >= 0; i--) {
+ if (ds1308_clk_sqw_rates[i] <= rate)
+ return ds1308_clk_sqw_rates[i];
+ }
+
+ return 0;
+}
+
static long ds3231_clk_sqw_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long *prate)
{
@@ -1107,6 +1159,31 @@ static long ds3231_clk_sqw_round_rate(struct clk_hw *hw, unsigned long rate,
return 0;
}
+static int ds1308_clk_sqw_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+ int control = 0;
+ int rate_sel;
+
+ for (rate_sel = 0; rate_sel < ARRAY_SIZE(ds1308_clk_sqw_rates);
+ rate_sel++) {
+ if (ds1308_clk_sqw_rates[rate_sel] == rate)
+ break;
+ }
+
+ if (rate_sel == ARRAY_SIZE(ds1308_clk_sqw_rates))
+ return -EINVAL;
+
+ if (rate_sel & 1)
+ control |= DS1307_BIT_RS0;
+ if (rate_sel & 2)
+ control |= DS1307_BIT_RS1;
+
+ return ds1337_write_control(ds1307, DS1307_BIT_RS0 | DS1307_BIT_RS1,
+ control);
+}
+
static int ds3231_clk_sqw_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)
{
@@ -1132,6 +1209,13 @@ static int ds3231_clk_sqw_set_rate(struct clk_hw *hw, unsigned long rate,
control);
}
+static int ds1308_clk_sqw_prepare(struct clk_hw *hw)
+{
+ struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+
+ return ds1308_write_control(ds1307, DS1307_BIT_SQWE, 1);
+}
+
static int ds3231_clk_sqw_prepare(struct clk_hw *hw)
{
struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
@@ -1139,6 +1223,13 @@ static int ds3231_clk_sqw_prepare(struct clk_hw *hw)
return ds1337_write_control(ds1307, DS1337_BIT_INTCN, 0);
}
+static void ds1308_clk_sqw_unprepare(struct clk_hw *hw)
+{
+ struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+
+ ds1308_write_control(ds1307, DS1307_BIT_SQWE, 0);
+}
+
static void ds3231_clk_sqw_unprepare(struct clk_hw *hw)
{
struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
@@ -1146,6 +1237,18 @@ static void ds3231_clk_sqw_unprepare(struct clk_hw *hw)
ds1337_write_control(ds1307, DS1337_BIT_INTCN, DS1337_BIT_INTCN);
}
+static int ds1308_clk_sqw_is_prepared(struct clk_hw *hw)
+{
+ struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+ int control, ret;
+
+ ret = regmap_read(ds1307->regmap, DS1307_REG_CONTROL, &control);
+ if (ret)
+ return ret;
+
+ return control & DS1307_BIT_SQWE;
+}
+
static int ds3231_clk_sqw_is_prepared(struct clk_hw *hw)
{
struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
@@ -1158,6 +1261,15 @@ static int ds3231_clk_sqw_is_prepared(struct clk_hw *hw)
return !(control & DS1337_BIT_INTCN);
}
+static const struct clk_ops ds1308_clk_sqw_ops = {
+ .prepare = ds1308_clk_sqw_prepare,
+ .unprepare = ds1308_clk_sqw_unprepare,
+ .is_prepared = ds1308_clk_sqw_is_prepared,
+ .recalc_rate = ds1308_clk_sqw_recalc_rate,
+ .round_rate = ds1308_clk_sqw_round_rate,
+ .set_rate = ds1308_clk_sqw_set_rate,
+};
+
static const struct clk_ops ds3231_clk_sqw_ops = {
.prepare = ds3231_clk_sqw_prepare,
.unprepare = ds3231_clk_sqw_unprepare,
@@ -1220,6 +1332,13 @@ static const struct clk_ops ds3231_clk_32khz_ops = {
.recalc_rate = ds3231_clk_32khz_recalc_rate,
};
+static struct clk_init_data ds1308_clks_init[] = {
+ {
+ .name = "ds1308_clk_sqw",
+ .ops = &ds1308_clk_sqw_ops,
+ },
+};
+
static struct clk_init_data ds3231_clks_init[] = {
[DS3231_CLK_SQW] = {
.name = "ds3231_clk_sqw",
@@ -1231,6 +1350,44 @@ static struct clk_init_data ds3231_clks_init[] = {
},
};
+static int ds1308_clks_register(struct ds1307 *ds1307)
+{
+ struct device_node *node = ds1307->dev->of_node;
+ struct clk_onecell_data *onecell;
+ int i;
+
+ onecell = devm_kzalloc(ds1307->dev, sizeof(*onecell), GFP_KERNEL);
+ if (!onecell)
+ return -ENOMEM;
+
+ onecell->clk_num = ARRAY_SIZE(ds1308_clks_init);
+ onecell->clks = devm_kcalloc(ds1307->dev, onecell->clk_num,
+ sizeof(onecell->clks[0]), GFP_KERNEL);
+ if (!onecell->clks)
+ return -ENOMEM;
+
+ for (i = 0; i < ARRAY_SIZE(ds1308_clks_init); i++) {
+ struct clk_init_data init = ds1308_clks_init[i];
+
+ /* optional override of the clockname */
+ of_property_read_string_index(node, "clock-output-names", i,
+ &init.name);
+ ds1307->clks[i].init = &init;
+
+ onecell->clks[i] = devm_clk_register(ds1307->dev,
+ &ds1307->clks[i]);
+ if (IS_ERR(onecell->clks[i]))
+ return PTR_ERR(onecell->clks[i]);
+ }
+
+ if (!node)
+ return 0;
+
+ of_clk_add_provider(node, of_clk_src_onecell_get, onecell);
+
+ return 0;
+}
+
static int ds3231_clks_register(struct ds1307 *ds1307)
{
struct device_node *node = ds1307->dev->of_node;
@@ -1280,10 +1437,16 @@ static void ds1307_clks_register(struct ds1307 *ds1307)
{
int ret;
- if (ds1307->type != ds_3231)
+ switch (ds1307->type) {
+ case ds_1308:
+ ret = ds1308_clks_register(ds1307);
+ case ds_3231:
+ ret = ds3231_clks_register(ds1307);
+ break;
+ default:
return;
+ }
- ret = ds3231_clks_register(ds1307);
if (ret) {
dev_warn(ds1307->dev, "unable to register clock device %d\n",
ret);
--
2.13.3
^ permalink raw reply related
* [rtc-linux] Re: [RFC] rtc: ds1307: add square wave output from ds1308
From: Alexandre Belloni @ 2017-07-31 14:02 UTC (permalink / raw)
To: Sean Nyekjaer; +Cc: rtc-linux
In-Reply-To: <20170731133013.22347-1-sean.nyekjaer@prevas.dk>
Hi Sean,
The mailing list address changed, can you resend there. Also, please
copy the CCF maintainers.
A quick comment below:
On 31/07/2017 at 15:30:13 +0200, Sean Nyekjaer wrote:
> - ret = ds3231_clks_register(ds1307);
> if (ret) {
> dev_warn(ds1307->dev, "unable to register clock device %d\n",
> ret);
> @@ -1545,6 +1708,11 @@ static int ds1307_probe(struct i2c_client *client,
> }
> break;
> case ds_1308:
> + /* disable sqw when driven from battery backup */
> + if (ds1307->regs[DS1307_REG_CONTROL] & DS1308_BIT_BBCLK)
> + i2c_smbus_write_byte_data(client, DS1307_REG_CONTROL,
> + ds1307->regs[DS1307_REG_CONTROL]
> + & ~DS1308_BIT_BBCLK);
That should not be needed, the CCF will disable any unused clock.
> case ds_1338:
> /* clock halted? turn it on, so clock can tick. */
> if (tmp & DS1307_BIT_CH)
> --
> 2.13.3
>
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] [RFC] rtc: ds1307: add square wave output from ds1308
From: Sean Nyekjaer @ 2017-07-31 13:30 UTC (permalink / raw)
To: rtc-linux; +Cc: Sean Nyekjaer, alexandre.belloni
add square wave output to generic clock framework and
disable sqw output from ds1308 in order to reduce
the power consumption by ~50%.
Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
---
This contains some code duplication(of my opinion) I guess it could be done more
generic together with the ds3231.
drivers/rtc/rtc-ds1307.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 170 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 1cedb21ba792..f92c99d7dd2e 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -79,6 +79,7 @@ enum ds_type {
# define DS1307_BIT_OUT 0x80
# define DS1338_BIT_OSF 0x20
# define DS1307_BIT_SQWE 0x10
+# define DS1308_BIT_BBCLK 0x04
# define DS1307_BIT_RS1 0x02
# define DS1307_BIT_RS0 0x01
#define DS1337_REG_CONTROL 0x0e
@@ -1056,6 +1057,13 @@ enum {
#define clk_32khz_to_ds1307(clk) \
container_of(clk, struct ds1307, clks[DS3231_CLK_32KHZ])
+static int ds1308_clk_sqw_rates[] = {
+ 1,
+ 4096,
+ 8192,
+ 32768,
+};
+
static int ds3231_clk_sqw_rates[] = {
1,
1024,
@@ -1063,6 +1071,19 @@ static int ds3231_clk_sqw_rates[] = {
8192,
};
+static int ds1308_write_control(struct ds1307 *ds1307, u8 mask, u8 value)
+{
+ struct mutex *lock = &ds1307->rtc->ops_lock;
+ int ret;
+
+ mutex_lock(lock);
+ ret = regmap_update_bits(ds1307->regmap, DS1307_REG_CONTROL,
+ mask, value);
+ mutex_unlock(lock);
+
+ return ret;
+}
+
static int ds1337_write_control(struct ds1307 *ds1307, u8 mask, u8 value)
{
struct mutex *lock = &ds1307->rtc->ops_lock;
@@ -1076,6 +1097,24 @@ static int ds1337_write_control(struct ds1307 *ds1307, u8 mask, u8 value)
return ret;
}
+static unsigned long ds1308_clk_sqw_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+ int control, ret;
+ int rate_sel = 0;
+
+ ret = regmap_read(ds1307->regmap, DS1307_REG_CONTROL, &control);
+ if (ret)
+ return ret;
+ if (control & DS1307_BIT_RS0)
+ rate_sel += 1;
+ if (control & DS1307_BIT_RS1)
+ rate_sel += 2;
+
+ return ds1308_clk_sqw_rates[rate_sel];
+}
+
static unsigned long ds3231_clk_sqw_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
@@ -1094,6 +1133,19 @@ static unsigned long ds3231_clk_sqw_recalc_rate(struct clk_hw *hw,
return ds3231_clk_sqw_rates[rate_sel];
}
+static long ds1308_clk_sqw_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ int i;
+
+ for (i = ARRAY_SIZE(ds1308_clk_sqw_rates) - 1; i >= 0; i--) {
+ if (ds1308_clk_sqw_rates[i] <= rate)
+ return ds1308_clk_sqw_rates[i];
+ }
+
+ return 0;
+}
+
static long ds3231_clk_sqw_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long *prate)
{
@@ -1107,6 +1159,31 @@ static long ds3231_clk_sqw_round_rate(struct clk_hw *hw, unsigned long rate,
return 0;
}
+static int ds1308_clk_sqw_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+ int control = 0;
+ int rate_sel;
+
+ for (rate_sel = 0; rate_sel < ARRAY_SIZE(ds1308_clk_sqw_rates);
+ rate_sel++) {
+ if (ds1308_clk_sqw_rates[rate_sel] == rate)
+ break;
+ }
+
+ if (rate_sel == ARRAY_SIZE(ds1308_clk_sqw_rates))
+ return -EINVAL;
+
+ if (rate_sel & 1)
+ control |= DS1307_BIT_RS0;
+ if (rate_sel & 2)
+ control |= DS1307_BIT_RS1;
+
+ return ds1337_write_control(ds1307, DS1307_BIT_RS0 | DS1307_BIT_RS1,
+ control);
+}
+
static int ds3231_clk_sqw_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)
{
@@ -1132,6 +1209,13 @@ static int ds3231_clk_sqw_set_rate(struct clk_hw *hw, unsigned long rate,
control);
}
+static int ds1308_clk_sqw_prepare(struct clk_hw *hw)
+{
+ struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+
+ return ds1308_write_control(ds1307, DS1307_BIT_SQWE, 1);
+}
+
static int ds3231_clk_sqw_prepare(struct clk_hw *hw)
{
struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
@@ -1139,6 +1223,13 @@ static int ds3231_clk_sqw_prepare(struct clk_hw *hw)
return ds1337_write_control(ds1307, DS1337_BIT_INTCN, 0);
}
+static void ds1308_clk_sqw_unprepare(struct clk_hw *hw)
+{
+ struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+
+ ds1308_write_control(ds1307, DS1307_BIT_SQWE, 0);
+}
+
static void ds3231_clk_sqw_unprepare(struct clk_hw *hw)
{
struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
@@ -1146,6 +1237,18 @@ static void ds3231_clk_sqw_unprepare(struct clk_hw *hw)
ds1337_write_control(ds1307, DS1337_BIT_INTCN, DS1337_BIT_INTCN);
}
+static int ds1308_clk_sqw_is_prepared(struct clk_hw *hw)
+{
+ struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+ int control, ret;
+
+ ret = regmap_read(ds1307->regmap, DS1307_REG_CONTROL, &control);
+ if (ret)
+ return ret;
+
+ return control & DS1307_BIT_SQWE;
+}
+
static int ds3231_clk_sqw_is_prepared(struct clk_hw *hw)
{
struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
@@ -1158,6 +1261,15 @@ static int ds3231_clk_sqw_is_prepared(struct clk_hw *hw)
return !(control & DS1337_BIT_INTCN);
}
+static const struct clk_ops ds1308_clk_sqw_ops = {
+ .prepare = ds1308_clk_sqw_prepare,
+ .unprepare = ds1308_clk_sqw_unprepare,
+ .is_prepared = ds1308_clk_sqw_is_prepared,
+ .recalc_rate = ds1308_clk_sqw_recalc_rate,
+ .round_rate = ds1308_clk_sqw_round_rate,
+ .set_rate = ds1308_clk_sqw_set_rate,
+};
+
static const struct clk_ops ds3231_clk_sqw_ops = {
.prepare = ds3231_clk_sqw_prepare,
.unprepare = ds3231_clk_sqw_unprepare,
@@ -1220,6 +1332,13 @@ static const struct clk_ops ds3231_clk_32khz_ops = {
.recalc_rate = ds3231_clk_32khz_recalc_rate,
};
+static struct clk_init_data ds1308_clks_init[] = {
+ {
+ .name = "ds1308_clk_sqw",
+ .ops = &ds1308_clk_sqw_ops,
+ },
+};
+
static struct clk_init_data ds3231_clks_init[] = {
[DS3231_CLK_SQW] = {
.name = "ds3231_clk_sqw",
@@ -1231,6 +1350,44 @@ static struct clk_init_data ds3231_clks_init[] = {
},
};
+static int ds1308_clks_register(struct ds1307 *ds1307)
+{
+ struct device_node *node = ds1307->dev->of_node;
+ struct clk_onecell_data *onecell;
+ int i;
+
+ onecell = devm_kzalloc(ds1307->dev, sizeof(*onecell), GFP_KERNEL);
+ if (!onecell)
+ return -ENOMEM;
+
+ onecell->clk_num = ARRAY_SIZE(ds1308_clks_init);
+ onecell->clks = devm_kcalloc(ds1307->dev, onecell->clk_num,
+ sizeof(onecell->clks[0]), GFP_KERNEL);
+ if (!onecell->clks)
+ return -ENOMEM;
+
+ for (i = 0; i < ARRAY_SIZE(ds1308_clks_init); i++) {
+ struct clk_init_data init = ds1308_clks_init[i];
+
+ /* optional override of the clockname */
+ of_property_read_string_index(node, "clock-output-names", i,
+ &init.name);
+ ds1307->clks[i].init = &init;
+
+ onecell->clks[i] = devm_clk_register(ds1307->dev,
+ &ds1307->clks[i]);
+ if (IS_ERR(onecell->clks[i]))
+ return PTR_ERR(onecell->clks[i]);
+ }
+
+ if (!node)
+ return 0;
+
+ of_clk_add_provider(node, of_clk_src_onecell_get, onecell);
+
+ return 0;
+}
+
static int ds3231_clks_register(struct ds1307 *ds1307)
{
struct device_node *node = ds1307->dev->of_node;
@@ -1280,10 +1437,16 @@ static void ds1307_clks_register(struct ds1307 *ds1307)
{
int ret;
- if (ds1307->type != ds_3231)
+ switch (ds1307->type) {
+ case ds_1308:
+ ret = ds1308_clks_register(ds1307);
+ case ds_3231:
+ ret = ds3231_clks_register(ds1307);
+ break;
+ default:
return;
+ }
- ret = ds3231_clks_register(ds1307);
if (ret) {
dev_warn(ds1307->dev, "unable to register clock device %d\n",
ret);
@@ -1545,6 +1708,11 @@ static int ds1307_probe(struct i2c_client *client,
}
break;
case ds_1308:
+ /* disable sqw when driven from battery backup */
+ if (ds1307->regs[DS1307_REG_CONTROL] & DS1308_BIT_BBCLK)
+ i2c_smbus_write_byte_data(client, DS1307_REG_CONTROL,
+ ds1307->regs[DS1307_REG_CONTROL]
+ & ~DS1308_BIT_BBCLK);
case ds_1338:
/* clock halted? turn it on, so clock can tick. */
if (tmp & DS1307_BIT_CH)
--
2.13.3
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply related
* Re: [PATCH] rtc: rtc-max8925: remove redundant check on ret
From: Alexandre Belloni @ 2017-07-30 15:06 UTC (permalink / raw)
To: Colin King; +Cc: Alessandro Zummo, linux-rtc, kernel-janitors, linux-kernel
In-Reply-To: <20170607152615.2072-1-colin.king@canonical.com>
On 07/06/2017 at 16:26:15 +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The check on ret < 0 is redundant as the goto destination is the
> next statment. Remove this redudant check and goto.
>
> Detected by CoverityScan, CID#1268785 ("Identical code for different
> branches")
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/rtc/rtc-max8925.c | 2 --
> 1 file changed, 2 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH] rtc: sun6i: ensure clk_data is kfree'd on error
From: Alexandre Belloni @ 2017-07-30 14:51 UTC (permalink / raw)
To: Colin Ian King
Cc: Alessandro Zummo, Maxime Ripard, Chen-Yu Tsai, linux-rtc,
linux-arm-kernel, kernel-janitors, linux-kernel
In-Reply-To: <7412e733-e552-e280-6ea4-aa53b14a333a@canonical.com>
On 19/07/2017 at 18:48:27 +0100, Colin Ian King wrote:
> On 19/07/17 18:32, Alexandre Belloni wrote:
> > Hi,
> >
> > On 19/07/2017 at 17:57:02 +0100, Colin King wrote:
> >> From: Colin Ian King <colin.king@canonical.com>
> >>
> >> There are two error return paths that do not kfree clk_data and
> >> we end up with a memory leak. Fix these with a kfree error exit
> >> path.
> >>
> >> Detected by CoverityScan, CID#1402959 ("Resource Leak")
> >>
> >
> > I think that patch fixes the same issue (and more):
> > http://patchwork.ozlabs.org/patch/787151/
>
> Yep, that's true.
>
Actually, I'm taking your patch now because it is the correct thing to
do.
> >
> >> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >> ---
> >> drivers/rtc/rtc-sun6i.c | 8 ++++++--
> >> 1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> >> index 39cbc1238b92..61502221ab6e 100644
> >> --- a/drivers/rtc/rtc-sun6i.c
> >> +++ b/drivers/rtc/rtc-sun6i.c
> >> @@ -204,7 +204,7 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
> >> rtc->base = of_io_request_and_map(node, 0, of_node_full_name(node));
> >> if (IS_ERR(rtc->base)) {
> >> pr_crit("Can't map RTC registers");
> >> - return;
> >> + goto err;
> >> }
> >>
> >> /* Switch to the external, more precise, oscillator */
> >> @@ -216,7 +216,7 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
> >>
> >> /* Deal with old DTs */
> >> if (!of_get_property(node, "clocks", NULL))
> >> - return;
> >> + goto err;
> >>
> >> rtc->int_osc = clk_hw_register_fixed_rate_with_accuracy(NULL,
> >> "rtc-int-osc",
> >> @@ -246,6 +246,10 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
> >> clk_data->num = 1;
> >> clk_data->hws[0] = &rtc->hw;
> >> of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> >> + return;
> >> +
> >> +err:
> >> + kfree(clk_data);
> >> }
> >> CLK_OF_DECLARE_DRIVER(sun6i_rtc_clk, "allwinner,sun6i-a31-rtc",
> >> sun6i_rtc_clk_init);
> >> --
> >> 2.11.0
> >>
> >
>
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 3/3] rtc: sun6i: Remove unneeded initalization of ret in sun6i_rtc_setalarm()
From: Alexandre Belloni @ 2017-07-30 14:41 UTC (permalink / raw)
To: Alexey Klimov; +Cc: linux-rtc, a.zummo, linux-kernel, maxime.ripard, wens, robh
In-Reply-To: <1499857190-30849-3-git-send-email-alexey.klimov@arm.com>
Hi,
On 12/07/2017 at 11:59:50 +0100, Alexey Klimov wrote:
> Signed-off-by: Alexey Klimov <alexey.klimov@arm.com>
> ---
> drivers/rtc/rtc-sun6i.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> index 77bc4d3..1886b85 100644
> --- a/drivers/rtc/rtc-sun6i.c
> +++ b/drivers/rtc/rtc-sun6i.c
> @@ -362,7 +362,7 @@ static int sun6i_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> unsigned long time_now = 0;
> unsigned long time_set = 0;
> unsigned long time_gap = 0;
> - int ret = 0;
> + int ret;
>
I don't see any reason that would justify that change.
> ret = sun6i_rtc_gettime(dev, &tm_now);
> if (ret < 0) {
> --
> 1.9.1
>
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 1/3] rtc: sun6i: Remove double init of spinlock in sun6i_rtc_clk_init()
From: Alexandre Belloni @ 2017-07-30 14:40 UTC (permalink / raw)
To: Alexey Klimov; +Cc: linux-rtc, a.zummo, linux-kernel, maxime.ripard, wens, robh
In-Reply-To: <1499857190-30849-1-git-send-email-alexey.klimov@arm.com>
On 12/07/2017 at 11:59:48 +0100, Alexey Klimov wrote:
> Fixes: 847b8bf62eb4 ("rtc: sun6i: Expose the 32kHz oscillator")
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Alexey Klimov <alexey.klimov@arm.com>
> ---
> drivers/rtc/rtc-sun6i.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Applied, after adding a proper commit message.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH] rtc: ds1307: remove legacy check for "isil,irq2-can-wakeup-machine" property
From: Alexandre Belloni @ 2017-07-30 14:30 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: linux-rtc, devicetree@vger.kernel.org
In-Reply-To: <ff4ea92b-4af3-1a59-2ea3-056a43125c20@gmail.com>
On 06/07/2017 at 22:40:03 +0200, Heiner Kallweit wrote:
> Commit 8b44f5be20fd ("ARM: dts: armada: replace isil,irq2-can-wakeup-machine with wakeup-source property")
> removed the last usage of "isil,irq2-can-wakeup-machine" almost
> two years ago. So I think we can get rid of supporting this
> legacy binding.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> Documentation/devicetree/bindings/power/wakeup-source.txt | 9 ++++-----
> Documentation/devicetree/bindings/rtc/isil,isl12057.txt | 1 -
> drivers/rtc/rtc-ds1307.c | 8 +-------
> 3 files changed, 5 insertions(+), 13 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [rtc-linux] Re: [PATCH v2 2/2] RTC: s35390a: implement ioctls
From: Alexandre Belloni @ 2017-07-30 14:30 UTC (permalink / raw)
To: Fabien Lahoudere; +Cc: a.zummo, rtc-linux
In-Reply-To: <bc0dd162f9b89349b41087f1fd710409cd9aaa8f.1499240924.git.fabien.lahoudere@collabora.co.uk>
On 05/07/2017 at 10:02:30 +0200, Fabien Lahoudere wrote:
> Implements RTC_VL_READ and RTC_VL_CLR ioctls.
>
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> ---
> drivers/rtc/rtc-s35390a.c | 32 +++++++++++++++++++++++++++++++-
> 1 file changed, 31 insertions(+), 1 deletion(-)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] Re: [PATCH v2 1/2] RTC: s35390a: handle invalid RTC time
From: Alexandre Belloni @ 2017-07-30 14:30 UTC (permalink / raw)
To: Fabien Lahoudere; +Cc: a.zummo, rtc-linux
In-Reply-To: <0ee84ea918faab368ed36b73dfc7fdff07c4b4b8.1499240924.git.fabien.lahoudere@collabora.co.uk>
On 05/07/2017 at 10:02:29 +0200, Fabien Lahoudere wrote:
> If RTC time have been altered by low voltage, we notify users
> that RTC time is invalid by returning -EINVAL.
> The RTC time needs to be set correctly to clear the invalid flag.
> If the RTC is not set before restarting, the information will be lost.
>
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> ---
> drivers/rtc/rtc-s35390a.c | 72 +++++++++++++++++++++++++++--------------------
> 1 file changed, 42 insertions(+), 30 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] [PATCH] rtc: m41t80: enable wakealarm when "wakeup-source" is specified
From: Eric Cooper @ 2017-07-30 0:10 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni, rtc-linux; +Cc: Eric Cooper
Don't require an IRQ if the wakeup-source device-tree property is present.
Signed-off-by: Eric Cooper <ecc@cmu.edu>
---
drivers/rtc/rtc-m41t80.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
index 58698d21c2c3..85ecb7043dc5 100644
--- a/drivers/rtc/rtc-m41t80.c
+++ b/drivers/rtc/rtc-m41t80.c
@@ -774,6 +774,7 @@ static int m41t80_probe(struct i2c_client *client,
struct rtc_device *rtc = NULL;
struct rtc_time tm;
struct m41t80_data *m41t80_data = NULL;
+ bool wakeup_source = false;
if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_I2C_BLOCK |
I2C_FUNC_SMBUS_BYTE_DATA)) {
@@ -789,6 +790,10 @@ static int m41t80_probe(struct i2c_client *client,
m41t80_data->features = id->driver_data;
i2c_set_clientdata(client, m41t80_data);
+#ifdef CONFIG_OF
+ wakeup_source = of_property_read_bool(client->dev.of_node,
+ "wakeup-source");
+#endif
if (client->irq > 0) {
rc = devm_request_threaded_irq(&client->dev, client->irq,
NULL, m41t80_handle_irq,
@@ -797,14 +802,16 @@ static int m41t80_probe(struct i2c_client *client,
if (rc) {
dev_warn(&client->dev, "unable to request IRQ, alarms disabled\n");
client->irq = 0;
- } else {
- m41t80_rtc_ops.read_alarm = m41t80_read_alarm;
- m41t80_rtc_ops.set_alarm = m41t80_set_alarm;
- m41t80_rtc_ops.alarm_irq_enable = m41t80_alarm_irq_enable;
- /* Enable the wakealarm */
- device_init_wakeup(&client->dev, true);
+ wakeup_source = false;
}
}
+ if (client->irq > 0 || wakeup_source) {
+ m41t80_rtc_ops.read_alarm = m41t80_read_alarm;
+ m41t80_rtc_ops.set_alarm = m41t80_set_alarm;
+ m41t80_rtc_ops.alarm_irq_enable = m41t80_alarm_irq_enable;
+ /* Enable the wakealarm */
+ device_init_wakeup(&client->dev, true);
+ }
rtc = devm_rtc_device_register(&client->dev, client->name,
&m41t80_rtc_ops, THIS_MODULE);
@@ -812,6 +819,10 @@ static int m41t80_probe(struct i2c_client *client,
return PTR_ERR(rtc);
m41t80_data->rtc = rtc;
+ if (client->irq <= 0) {
+ /* We cannot support UIE mode if we do not have an IRQ line */
+ rtc->uie_unsupported = 1;
+ }
/* Make sure HT (Halt Update) bit is cleared */
rc = i2c_smbus_read_byte_data(client, M41T80_REG_ALARM_HOUR);
--
2.13.2
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply related
* Re: [PATCH] rtc: rtc-stmp3xxx: detect power failure on backup power domain
From: Alexandre Belloni @ 2017-07-29 22:38 UTC (permalink / raw)
To: Michael Thalmeier; +Cc: Alessandro Zummo, linux-rtc, linux-kernel
In-Reply-To: <20170607130503.GA32452@entw49.hale>
On 07/06/2017 at 15:05:03 +0200, Michael Thalmeier wrote:
> On Thu, May 18, 2017 at 05:56:31PM +0200, Alexandre Belloni wrote:
> > Hi,
> >
> > On 18/05/2017 at 16:45:21 +0200, Michael Thalmeier wrote:
> > > To detect when the backup power domain has lost power a software defined bit
> > > is set in one of the general purpose persistent registers when writing a new
> > > time into the rtc.
> > > When reading the time this bit is checked to determine if a power fail has
> > > happened since the last time the rtc time was set.
> > >
> >
> > I'm kind of concerned that other people may want to use those register
> > for something else but I don't currently have anything better to
> > suggest.
> >
> > The other concern is that when updating the kernel, this will make amm
> > the rtc report that the time is invalid until the next update. So this
> > should be disabled by default.
>
> How would you think this is best to disable by default? With a config
> option or with a module parameter?
> Either way would also allow to make the bit mask in the PERSISTENT2
> register configurable, so one can use whichever bit is still unused by
> some other applications.
>
What you could do is export the persistent registers with nvmem. Then
you can define a cell in DT using a bit in persistent2 and use it from
the rtc driver itself.
If the cell is defined, then the driver know it has been configured to
detect power failure.
> >
> > > When we detect a power fail we return -ENODATA.
> > >
> >
> > All the other drivers return -EINVAL in that case.
> >
> > > Signed-off-by: Michael Thalmeier <michael.thalmeier@hale.at>
> > > ---
> > > drivers/rtc/rtc-stmp3xxx.c | 9 +++++++++
> > > 1 file changed, 9 insertions(+)
> > >
> > > diff --git a/drivers/rtc/rtc-stmp3xxx.c b/drivers/rtc/rtc-stmp3xxx.c
> > > index d578e40..51330ec 100644
> > > --- a/drivers/rtc/rtc-stmp3xxx.c
> > > +++ b/drivers/rtc/rtc-stmp3xxx.c
> > > @@ -62,6 +62,9 @@
> > > /* missing bitmask in headers */
> > > #define STMP3XXX_RTC_PERSISTENT1_FORCE_UPDATER 0x80000000
> > >
> > > +#define STMP3XXX_RTC_PERSISTENT2 0x80
> > > +#define STMP3XXX_RTC_PERSISTENT2_VALID_TIME 0x01
> > > +
> > > struct stmp3xxx_rtc_data {
> > > struct rtc_device *rtc;
> > > void __iomem *io;
> > > @@ -160,6 +163,10 @@ static int stmp3xxx_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
> > > if (ret)
> > > return ret;
> > >
> > > + if (!(readl(rtc_data->io + STMP3XXX_RTC_PERSISTENT2) &
> > > + STMP3XXX_RTC_PERSISTENT2_VALID_TIME))
> > > + return -ENODATA;
> > > +
> > > rtc_time_to_tm(readl(rtc_data->io + STMP3XXX_RTC_SECONDS), rtc_tm);
> > > return 0;
> > > }
> > > @@ -169,6 +176,8 @@ static int stmp3xxx_rtc_set_mmss(struct device *dev, unsigned long t)
> > > struct stmp3xxx_rtc_data *rtc_data = dev_get_drvdata(dev);
> > >
> > > writel(t, rtc_data->io + STMP3XXX_RTC_SECONDS);
> > > + writel(STMP3XXX_RTC_PERSISTENT2_VALID_TIME,
> > > + rtc_data->io + STMP3XXX_RTC_PERSISTENT2 + STMP_OFFSET_REG_SET);
> > > return stmp3xxx_wait_time(rtc_data);
> > > }
> > >
> > > --
> > > 2.9.2
> > >
> >
> > --
> > Alexandre Belloni, Free Electrons
> > Embedded Linux and Kernel engineering
> > http://free-electrons.com
>
> --
>
> Michael Thalmeier
> (Entwicklung)
> HALE electronic GmbH
> Eugen-Müller-Straße 18, 5020 Salzburg, Austria
> Tel: +43 (662) 439011 0
> Fax: +43 (662) 439011 9
> michael.thalmeier@hale.at
> Firmenbuchnummer: FN 66801m HG Salzburg
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 2/2] mfd: ds1374: Add Dallas/Maxim DS1374 Multi Function Device
From: Alexandre Belloni @ 2017-07-29 22:26 UTC (permalink / raw)
To: Moritz Fischer
Cc: lee.jones, robh+dt, mark.rutland, a.zummo, wim, linux, devicetree,
linux-kernel, linux-watchdog, linux-rtc, Moritz Fischer
In-Reply-To: <1499975665-14581-2-git-send-email-mdf@kernel.org>
On 13/07/2017 at 12:54:25 -0700, Moritz Fischer wrote:
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 52a70ee..1703611 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -120,6 +120,16 @@ config DA9062_WATCHDOG
>
> This driver can be built as a module. The module name is da9062_wdt.
>
> +config DS1374_WATCHDOG
> + tristate "Maxim/Dallas 1374 Watchdog"
If you want to keep the backward compatibility, this probably needs to
be defaulting to RTC_DRV_DS1374 when RTC_DRV_DS1374_WDT is selected.
Also, maybe we need to make RTC_DRV_DS1374_WDT not user selectable.
If you don't think backward compatibility can be achieved, then we can
drop everything.
Else, the RTC part seems fine to me but the driver may be cleaned up a
bit further afterwards.
> + depends on MFD_DS1374
> + depends on REGMAP_I2C
> + select WATCHDOG_CORE
> + help
> + Support for the watchdog in the Maxim/Dallas DS1374 MFD.
> +
> + This driver can be built as a module. The module name is ds1374-wdt.
> +
> config GPIO_WATCHDOG
> tristate "Watchdog device controlled through GPIO-line"
> depends on OF_GPIO
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH] rtc: ds1374: Fix decimal permission coding issue
From: SZ Lin @ 2017-07-29 11:56 UTC (permalink / raw)
To: a.zummo; +Cc: alexandre.belloni, linux-rtc, linux-kernel, SZ Lin
Fixed ERROR: Use 4 digit octal (0777) not decimal permissions to fulfill
the current coding-style
This error was detected by checkpatch.pl
Signed-off-by: SZ Lin <sz.lin@moxa.com>
---
drivers/rtc/rtc-ds1374.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-ds1374.c b/drivers/rtc/rtc-ds1374.c
index 38a2e9e684df..7d55ad297f36 100644
--- a/drivers/rtc/rtc-ds1374.c
+++ b/drivers/rtc/rtc-ds1374.c
@@ -378,7 +378,7 @@ static struct i2c_client *save_client;
static int wdt_margin = WD_TIMO;
static unsigned long wdt_is_open;
-module_param(wdt_margin, int, 0);
+module_param(wdt_margin, int, 0000);
MODULE_PARM_DESC(wdt_margin, "Watchdog timeout in seconds (default 32s)");
static const struct watchdog_info ds1374_wdt_info = {
--
2.13.3
^ permalink raw reply related
* Re: [PATCH 2/2] mfd: ds1374: Add Dallas/Maxim DS1374 Multi Function Device
From: Alexandre Belloni @ 2017-07-29 11:07 UTC (permalink / raw)
To: Lee Jones
Cc: Moritz Fischer, robh+dt, mark.rutland, a.zummo, wim, linux,
devicetree, linux-kernel, linux-watchdog, linux-rtc,
Moritz Fischer
In-Reply-To: <20170717075117.btnhbmgpofwn7zdk@dell>
On 17/07/2017 at 08:51:17 +0100, Lee Jones wrote:
> > +static int ds1374_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct ds1374 *ds1374;
> > + u32 mode;
> > + int err;
> > +
> > + ds1374 = devm_kzalloc(&client->dev, sizeof(struct ds1374), GFP_KERNEL);
> > + if (!ds1374)
> > + return -ENOMEM;
> > +
> > + ds1374->regmap = devm_regmap_init_i2c(client, &ds1374_regmap_config);
> > + if (IS_ERR(ds1374->regmap))
> > + return PTR_ERR(ds1374->regmap);
> > +
> > + if (IS_ENABLED(CONFIG_OF) && client->dev.of_node) {
> > + err = of_property_read_u32(client->dev.of_node,
> > + "dallas,mode", &mode);
> > + if (err < 0) {
> > + dev_err(&client->dev, "missing dallas,mode property\n");
> > + return -EINVAL;
> > + }
> > +
> > + ds1374->remapped_reset
> > + = of_property_read_bool(client->dev.of_node,
> > + "dallas,remap-reset");
> > +
> > + ds1374->mode = (enum ds1374_mode)mode;
> > + } else if (IS_ENABLED(CONFIG_RTC_DRV_DS1374_WDT)) {
> > + ds1374->mode = DS1374_MODE_RTC_WDT;
> > + } else {
> > + ds1374->mode = DS1374_MODE_RTC_ALM;
> > + }
>
> This is non-standard. So if OF is enabled, you're taking the 'mode'
> from platform data (DT) and if it's not, you're relying on Kconfig.
> May I suggest that you pick platform data OR Kconfig, but not mix the
> two.
>
I've explicitly asked for that. CONFIG_RTC_DRV_DS1374_WDT has been in
the kernel since 2014. Requiring the use of dallas,mode now would break
the functionality for people not updating their DT.
However, my suggestion was that if OF is enabled and dallas,mode is not
present, then rely on CONFIG_RTC_DRV_DS1374_WDT to set the mode.
I personally don't care too much about breaking the DT ABI but this one
can be a bit tricky to discover and I still think it can be avoided.
At least, we should silently ignore CONFIG_RTC_DRV_DS1374_WDT.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH] rtc: ds1307: fix regmap config
From: Heiner Kallweit @ 2017-07-25 16:44 UTC (permalink / raw)
To: Alexandre Belloni, linux-rtc
Current max_register setting breaks reading nvram on certain chips and
also reading the standard registers on RX8130 where register map starts
at 0x10.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Fixes: 11e5890b5342 "rtc: ds1307: convert driver to regmap"
---
This fix should make it to 4.13.
---
drivers/rtc/rtc-ds1307.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 4fac49e5..4b43aa62 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1301,7 +1301,6 @@ static void ds1307_clks_register(struct ds1307 *ds1307)
static const struct regmap_config regmap_config = {
.reg_bits = 8,
.val_bits = 8,
- .max_register = 0x12,
};
static int ds1307_probe(struct i2c_client *client,
--
2.13.2
^ permalink raw reply related
* Re: Duplicated drivers for Epson RX8025 RTC support
From: Heiner Kallweit @ 2017-07-24 20:15 UTC (permalink / raw)
To: Alexandre Belloni, Akinobu Mita
Cc: Matthias Fuchs, linux-rtc, Wolfgang Grandegger, Alessandro Zummo
In-Reply-To: <20170724091601.sjz5shnv3ozq6the@piout.net>
Am 24.07.2017 um 11:16 schrieb Alexandre Belloni:
> Hi,
>
> On 23/07/2017 at 22:15:24 +0200, Heiner Kallweit wrote:
>> When working on refactoring parts of the ds1307 I stumbled across the fact
>> that there are two rtc drivers claiming to support the Epson RX8025 chip.
>>
>> 1. ds1307 claims to support also this chip. Support was added in 2009,
>> however I have doubts that it actually works.
>> RX8025 needs a special addressing mode (register address has to be
>> left-shifted by 4 bits) which is implemented for both control
>> registers but not for the standard registers.
>>
After checking again it could work due to the fact that mainly bulk ops
starting with address 0 are used. Therefore the missing shift doesn't hurt.
>> Also support for this chip misses the fix from patch 2e10e74df72
>> ("rtc: rx8025: fix transfer mode")
>>
>> I'm curious whether support for this chip was ever tested, commit message
>> of a216685818a5 "rtc: add EPSON RX8025 support to DS1307 RTC driver"
>> does not mention that this piece of code was tested with any device
>> with this chip.
>>
>> 2. There's a separate driver for this chip (rtc-rx8025), also added in 2009.
>> This drivers is actively maintained.
>>
>> The separate driver mentions that it supports SA/NB variants of the chip.
>> There's no info regarding supported chip variants in the ds1307-integrated
>> driver. So there is a small chance that both drivers support different
>> chip variants (in this case however the ds1307-integrated driver should
>> clearly mention this, also in the Kconfig help text).
>>
>> For me it's more likely that both drivers try support the same chip.
>> Having said that I would propose to remove (rudimentary) RX8025 support
>> in ds1307. As a first step we could leave the code in but use a WARN_ON
>> to notify potential users that this code is deprecated and they should
>> use the separate driver instead.
>>
>
> The remaining issue is described here:
> https://groups.google.com/d/msg/rtc-linux/M_uv9YkRbC8/MaKpMa3LGgAJ
>
Thanks a lot for sharing the link. So I involve the people from the
previous discussion.
> It is even more apparent after your regmap rework.
>
Most likely best would be to switch rtc-rx8025 to regmap-i2c as well
as it automatically selects proper low-level access methods based on
the supported i2c functions.
Shouldn't be a big deal using 11e5890b5342 "rtc: ds1307: convert driver
to regmap" as blueprint.
Afterwards support for rx8025 in ds1307 could be dropped.
Rgds, Heiner
^ permalink raw reply
* Re: Duplicated drivers for Epson RX8025 RTC support
From: Alexandre Belloni @ 2017-07-24 9:16 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Matthias Fuchs, linux-rtc
In-Reply-To: <a82c9f50-31d2-d750-39fc-05501f3724ad@gmail.com>
Hi,
On 23/07/2017 at 22:15:24 +0200, Heiner Kallweit wrote:
> When working on refactoring parts of the ds1307 I stumbled across the fact
> that there are two rtc drivers claiming to support the Epson RX8025 chip.
>
> 1. ds1307 claims to support also this chip. Support was added in 2009,
> however I have doubts that it actually works.
> RX8025 needs a special addressing mode (register address has to be
> left-shifted by 4 bits) which is implemented for both control
> registers but not for the standard registers.
>
> Also support for this chip misses the fix from patch 2e10e74df72
> ("rtc: rx8025: fix transfer mode")
>
> I'm curious whether support for this chip was ever tested, commit message
> of a216685818a5 "rtc: add EPSON RX8025 support to DS1307 RTC driver"
> does not mention that this piece of code was tested with any device
> with this chip.
>
> 2. There's a separate driver for this chip (rtc-rx8025), also added in 2009.
> This drivers is actively maintained.
>
> The separate driver mentions that it supports SA/NB variants of the chip.
> There's no info regarding supported chip variants in the ds1307-integrated
> driver. So there is a small chance that both drivers support different
> chip variants (in this case however the ds1307-integrated driver should
> clearly mention this, also in the Kconfig help text).
>
> For me it's more likely that both drivers try support the same chip.
> Having said that I would propose to remove (rudimentary) RX8025 support
> in ds1307. As a first step we could leave the code in but use a WARN_ON
> to notify potential users that this code is deprecated and they should
> use the separate driver instead.
>
The remaining issue is described here:
https://groups.google.com/d/msg/rtc-linux/M_uv9YkRbC8/MaKpMa3LGgAJ
It is even more apparent after your regmap rework.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v3 2/8] MIPS: ranchu: Add Goldfish RTC driver (fwd)
From: Julia Lawall @ 2017-07-24 4:54 UTC (permalink / raw)
To: Aleksandar Markovic
Cc: Miodrag Dinic, Goran Ferenc, Aleksandar Markovic,
Alessandro Zummo, Alexandre Belloni, Bo Hu, David S. Miller,
Douglas Leung, Greg Kroah-Hartman, James Hogan, Jin Qian,
linux-kernel, linux-rtc, Mauro Carvalho Chehab, Paul Burton,
Petar Jovanovic, Raghu Gandham, linux-mips, kbuild-all
Please check line 203; it seems that the tested value is unsigned.
julia
---------- Forwarded message ----------
Date: Mon, 24 Jul 2017 11:40:38 +0800
From: kbuild test robot <fengguang.wu@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH v3 2/8] MIPS: ranchu: Add Goldfish RTC driver
Hi Miodrag,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.13-rc2 next-20170721]
[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/Aleksandar-Markovic/MIPS-Add-virtual-Ranchu-board-as-a-generic-based-board/20170724-062318
:::::: branch date: 5 hours ago
:::::: commit date: 5 hours ago
>> drivers/rtc/rtc-goldfish.c:203:5-16: WARNING: Unsigned expression compared with zero: rtcdrv -> irq < 0
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 3b43c4b417a02749f734942456b41eb397e389ae
vim +203 drivers/rtc/rtc-goldfish.c
3b43c4b4 Miodrag Dinic 2017-07-21 181
3b43c4b4 Miodrag Dinic 2017-07-21 182 static int goldfish_rtc_probe(struct platform_device *pdev)
3b43c4b4 Miodrag Dinic 2017-07-21 183 {
3b43c4b4 Miodrag Dinic 2017-07-21 184 struct resource *r;
3b43c4b4 Miodrag Dinic 2017-07-21 185 struct goldfish_rtc *rtcdrv;
3b43c4b4 Miodrag Dinic 2017-07-21 186 int err;
3b43c4b4 Miodrag Dinic 2017-07-21 187
3b43c4b4 Miodrag Dinic 2017-07-21 188 rtcdrv = devm_kzalloc(&pdev->dev, sizeof(*rtcdrv), GFP_KERNEL);
3b43c4b4 Miodrag Dinic 2017-07-21 189 if (rtcdrv == NULL)
3b43c4b4 Miodrag Dinic 2017-07-21 190 return -ENOMEM;
3b43c4b4 Miodrag Dinic 2017-07-21 191
3b43c4b4 Miodrag Dinic 2017-07-21 192 platform_set_drvdata(pdev, rtcdrv);
3b43c4b4 Miodrag Dinic 2017-07-21 193
3b43c4b4 Miodrag Dinic 2017-07-21 194 r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
3b43c4b4 Miodrag Dinic 2017-07-21 195 if (r == NULL)
3b43c4b4 Miodrag Dinic 2017-07-21 196 return -ENODEV;
3b43c4b4 Miodrag Dinic 2017-07-21 197
3b43c4b4 Miodrag Dinic 2017-07-21 198 rtcdrv->base = devm_ioremap_resource(&pdev->dev, r);
3b43c4b4 Miodrag Dinic 2017-07-21 199 if (IS_ERR(rtcdrv->base))
3b43c4b4 Miodrag Dinic 2017-07-21 200 return -ENODEV;
3b43c4b4 Miodrag Dinic 2017-07-21 201
3b43c4b4 Miodrag Dinic 2017-07-21 202 rtcdrv->irq = platform_get_irq(pdev, 0);
3b43c4b4 Miodrag Dinic 2017-07-21 @203 if (rtcdrv->irq < 0)
3b43c4b4 Miodrag Dinic 2017-07-21 204 return -ENODEV;
3b43c4b4 Miodrag Dinic 2017-07-21 205
3b43c4b4 Miodrag Dinic 2017-07-21 206 rtcdrv->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
3b43c4b4 Miodrag Dinic 2017-07-21 207 &goldfish_rtc_ops, THIS_MODULE);
3b43c4b4 Miodrag Dinic 2017-07-21 208 if (IS_ERR(rtcdrv->rtc))
3b43c4b4 Miodrag Dinic 2017-07-21 209 return PTR_ERR(rtcdrv->rtc);
3b43c4b4 Miodrag Dinic 2017-07-21 210
3b43c4b4 Miodrag Dinic 2017-07-21 211 err = devm_request_irq(&pdev->dev, rtcdrv->irq, goldfish_rtc_interrupt,
3b43c4b4 Miodrag Dinic 2017-07-21 212 0, pdev->name, rtcdrv);
3b43c4b4 Miodrag Dinic 2017-07-21 213 if (err)
3b43c4b4 Miodrag Dinic 2017-07-21 214 return err;
3b43c4b4 Miodrag Dinic 2017-07-21 215
3b43c4b4 Miodrag Dinic 2017-07-21 216 return 0;
3b43c4b4 Miodrag Dinic 2017-07-21 217 }
3b43c4b4 Miodrag Dinic 2017-07-21 218
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH 1/3] rtc: sun6i: Remove double init of spinlock in sun6i_rtc_clk_init()
From: Chen-Yu Tsai @ 2017-07-24 3:24 UTC (permalink / raw)
To: Alexey Klimov
Cc: linux-rtc, Alessandro Zummo, linux-kernel, Alexandre Belloni,
Maxime Ripard, Chen-Yu Tsai, Rob Herring
In-Reply-To: <1499857190-30849-1-git-send-email-alexey.klimov@arm.com>
On Wed, Jul 12, 2017 at 6:59 PM, Alexey Klimov <alexey.klimov@arm.com> wrote:
A bit more description would be nice. Maybe a kernel message that
prompted this fix?
> Fixes: 847b8bf62eb4 ("rtc: sun6i: Expose the 32kHz oscillator")
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Alexey Klimov <alexey.klimov@arm.com>
Otherwise,
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
^ permalink raw reply
* Re: [PATCH 3/3] rtc: sun6i: Remove unneeded initalization of ret in sun6i_rtc_setalarm()
From: Chen-Yu Tsai @ 2017-07-24 3:23 UTC (permalink / raw)
To: Alexey Klimov
Cc: linux-rtc, Alessandro Zummo, linux-kernel, Alexandre Belloni,
Maxime Ripard, Chen-Yu Tsai, Rob Herring
In-Reply-To: <1499857190-30849-3-git-send-email-alexey.klimov@arm.com>
On Wed, Jul 12, 2017 at 6:59 PM, Alexey Klimov <alexey.klimov@arm.com> wrote:
A bit more description would be nice.
> Signed-off-by: Alexey Klimov <alexey.klimov@arm.com>
Otherwise,
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
^ permalink raw reply
* Re: [PATCH 2/3] rtc: sun6i: fix memleaks and add error-path in sun6i_rtc_clk_init()
From: Chen-Yu Tsai @ 2017-07-24 3:22 UTC (permalink / raw)
To: Alexey Klimov
Cc: linux-rtc, Alessandro Zummo, linux-kernel, Alexandre Belloni,
Maxime Ripard, Chen-Yu Tsai, Rob Herring
In-Reply-To: <1499857190-30849-2-git-send-email-alexey.klimov@arm.com>
On Wed, Jul 12, 2017 at 6:59 PM, Alexey Klimov <alexey.klimov@arm.com> wrote:
> The memory allocated for rtc and clk_data will never be freed in
> sun6i_rtc_clk_init() in case of error and return. This patch adds
> required error path with memory freeing.
>
> Fixes: 847b8bf62eb4 ("rtc: sun6i: Expose the 32kHz oscillator")
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Alexey Klimov <alexey.klimov@arm.com>
> ---
> drivers/rtc/rtc-sun6i.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> index 7e7da60..77bc4d3 100644
> --- a/drivers/rtc/rtc-sun6i.c
> +++ b/drivers/rtc/rtc-sun6i.c
> @@ -197,14 +197,14 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
> clk_data = kzalloc(sizeof(*clk_data) + sizeof(*clk_data->hws),
> GFP_KERNEL);
> if (!clk_data)
> - return;
> + goto out_rtc_free;
>
> spin_lock_init(&rtc->lock);
>
> rtc->base = of_io_request_and_map(node, 0, of_node_full_name(node));
> if (IS_ERR(rtc->base)) {
> pr_crit("Can't map RTC registers");
> - return;
> + goto out_clk_data_free;
> }
>
> /* Switch to the external, more precise, oscillator */
> @@ -216,7 +216,7 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
>
> /* Deal with old DTs */
> if (!of_get_property(node, "clocks", NULL))
> - return;
> + goto out_clk_data_free;
>
> rtc->int_osc = clk_hw_register_fixed_rate_with_accuracy(NULL,
> "rtc-int-osc",
> @@ -225,7 +225,7 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
> 300000000);
> if (IS_ERR(rtc->int_osc)) {
> pr_crit("Couldn't register the internal oscillator\n");
> - return;
> + goto out_clk_data_free;
> }
>
> parents[0] = clk_hw_get_name(rtc->int_osc);
> @@ -240,12 +240,19 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
> rtc->losc = clk_register(NULL, &rtc->hw);
> if (IS_ERR(rtc->losc)) {
> pr_crit("Couldn't register the LOSC clock\n");
> - return;
> + goto out_clk_data_free;
You should also unregister the fixed rate clk above.
> }
>
> clk_data->num = 1;
> clk_data->hws[0] = &rtc->hw;
> of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> +
> + return;
> +
> +out_clk_data_free:
> + kfree(clk_data);
> +out_rtc_free:
> + kfree(rtc);
rtc can not be freed. It has already been assigned to sun6i_rtc, a static
variable within this module. It then gets used by the actual RTC driver
later in this file to get the register base pointer. This was done to
avoid issues with trying to map the same I/O addresses twice.
Regards
ChenYu
> }
> CLK_OF_DECLARE_DRIVER(sun6i_rtc_clk, "allwinner,sun6i-a31-rtc",
> sun6i_rtc_clk_init);
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH v3 2/8] MIPS: ranchu: Add Goldfish RTC driver
From: kbuild test robot @ 2017-07-24 2:14 UTC (permalink / raw)
To: Aleksandar Markovic
Cc: kbuild-all, linux-mips, Miodrag Dinic, Goran Ferenc,
Aleksandar Markovic, Alessandro Zummo, Alexandre Belloni, Bo Hu,
David S. Miller, Douglas Leung, Greg Kroah-Hartman, James Hogan,
Jin Qian, linux-kernel, linux-rtc, Mauro Carvalho Chehab,
Paul Burton, Petar Jovanovic, Raghu Gandham
In-Reply-To: <1500656111-9520-3-git-send-email-aleksandar.markovic@rt-rk.com>
[-- Attachment #1: Type: text/plain, Size: 3371 bytes --]
Hi Miodrag,
[auto build test ERROR on linus/master]
[also build test ERROR on next-20170721]
[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/Aleksandar-Markovic/MIPS-Add-virtual-Ranchu-board-as-a-generic-based-board/20170724-062318
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.3.0-18) 6.3.0 20170516
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64
All errors (new ones prefixed by >>):
drivers/rtc/rtc-goldfish.c: In function 'goldfish_rtc_read_alarm':
>> drivers/rtc/rtc-goldfish.c:51:18: error: implicit declaration of function 'readl' [-Werror=implicit-function-declaration]
rtc_alarm_low = readl(base + TIMER_ALARM_LOW);
^~~~~
drivers/rtc/rtc-goldfish.c: In function 'goldfish_rtc_set_alarm':
>> drivers/rtc/rtc-goldfish.c:86:3: error: implicit declaration of function 'writel' [-Werror=implicit-function-declaration]
writel((rtc_alarm >> 32), base + TIMER_ALARM_HIGH);
^~~~~~
cc1: some warnings being treated as errors
vim +/readl +51 drivers/rtc/rtc-goldfish.c
38
39 static int goldfish_rtc_read_alarm(struct device *dev,
40 struct rtc_wkalrm *alrm)
41 {
42 u64 rtc_alarm;
43 u64 rtc_alarm_low;
44 u64 rtc_alarm_high;
45 void __iomem *base;
46 struct goldfish_rtc *rtcdrv;
47
48 rtcdrv = dev_get_drvdata(dev);
49 base = rtcdrv->base;
50
> 51 rtc_alarm_low = readl(base + TIMER_ALARM_LOW);
52 rtc_alarm_high = readl(base + TIMER_ALARM_HIGH);
53 rtc_alarm = (rtc_alarm_high << 32) | rtc_alarm_low;
54
55 do_div(rtc_alarm, NSEC_PER_SEC);
56 memset(alrm, 0, sizeof(struct rtc_wkalrm));
57
58 rtc_time_to_tm(rtc_alarm, &(alrm->time));
59
60 if (readl(base + TIMER_ALARM_STATUS))
61 alrm->enabled = 1;
62 else
63 alrm->enabled = 0;
64
65 return 0;
66 }
67
68 static int goldfish_rtc_set_alarm(struct device *dev,
69 struct rtc_wkalrm *alrm)
70 {
71 struct goldfish_rtc *rtcdrv;
72 unsigned long rtc_alarm;
73 u64 rtc_status_reg;
74 void __iomem *base;
75 int ret = 0;
76
77 rtcdrv = dev_get_drvdata(dev);
78 base = rtcdrv->base;
79
80 if (alrm->enabled) {
81 ret = rtc_tm_to_time(&(alrm->time), &rtc_alarm);
82 if (ret != 0)
83 return ret;
84
85 rtc_alarm *= NSEC_PER_SEC;
> 86 writel((rtc_alarm >> 32), base + TIMER_ALARM_HIGH);
87 writel(rtc_alarm, base + TIMER_ALARM_LOW);
88 } else {
89 /*
90 * if this function was called with enabled=0
91 * then it could mean that the application is
92 * trying to cancel an ongoing alarm
93 */
94 rtc_status_reg = readl(base + TIMER_ALARM_STATUS);
95 if (rtc_status_reg)
96 writel(1, base + TIMER_CLEAR_ALARM);
97 }
98
99 return ret;
100 }
101
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51323 bytes --]
^ permalink raw reply
* Re: [PATCH v3 2/8] MIPS: ranchu: Add Goldfish RTC driver
From: kbuild test robot @ 2017-07-23 23:50 UTC (permalink / raw)
To: Aleksandar Markovic
Cc: kbuild-all, linux-mips, Miodrag Dinic, Goran Ferenc,
Aleksandar Markovic, Alessandro Zummo, Alexandre Belloni, Bo Hu,
David S. Miller, Douglas Leung, Greg Kroah-Hartman, James Hogan,
Jin Qian, linux-kernel, linux-rtc, Mauro Carvalho Chehab,
Paul Burton, Petar Jovanovic, Raghu Gandham
In-Reply-To: <1500656111-9520-3-git-send-email-aleksandar.markovic@rt-rk.com>
[-- Attachment #1: Type: text/plain, Size: 2448 bytes --]
Hi Miodrag,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.13-rc1 next-20170721]
[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/Aleksandar-Markovic/MIPS-Add-virtual-Ranchu-board-as-a-generic-based-board/20170724-062318
config: blackfin-allyesconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin
All warnings (new ones prefixed by >>):
drivers/rtc/rtc-goldfish.c: In function 'goldfish_rtc_set_alarm':
>> drivers/rtc/rtc-goldfish.c:86:21: warning: right shift count >= width of type [-Wshift-count-overflow]
writel((rtc_alarm >> 32), base + TIMER_ALARM_HIGH);
^~
drivers/rtc/rtc-goldfish.c: In function 'goldfish_rtc_set_time':
drivers/rtc/rtc-goldfish.c:167:15: warning: right shift count >= width of type [-Wshift-count-overflow]
writel((now >> 32), base + TIMER_TIME_HIGH);
^~
vim +86 drivers/rtc/rtc-goldfish.c
67
68 static int goldfish_rtc_set_alarm(struct device *dev,
69 struct rtc_wkalrm *alrm)
70 {
71 struct goldfish_rtc *rtcdrv;
72 unsigned long rtc_alarm;
73 u64 rtc_status_reg;
74 void __iomem *base;
75 int ret = 0;
76
77 rtcdrv = dev_get_drvdata(dev);
78 base = rtcdrv->base;
79
80 if (alrm->enabled) {
81 ret = rtc_tm_to_time(&(alrm->time), &rtc_alarm);
82 if (ret != 0)
83 return ret;
84
85 rtc_alarm *= NSEC_PER_SEC;
> 86 writel((rtc_alarm >> 32), base + TIMER_ALARM_HIGH);
87 writel(rtc_alarm, base + TIMER_ALARM_LOW);
88 } else {
89 /*
90 * if this function was called with enabled=0
91 * then it could mean that the application is
92 * trying to cancel an ongoing alarm
93 */
94 rtc_status_reg = readl(base + TIMER_ALARM_STATUS);
95 if (rtc_status_reg)
96 writel(1, base + TIMER_CLEAR_ALARM);
97 }
98
99 return ret;
100 }
101
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45669 bytes --]
^ permalink raw reply
* Duplicated drivers for Epson RX8025 RTC support
From: Heiner Kallweit @ 2017-07-23 20:15 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: Matthias Fuchs, linux-rtc
When working on refactoring parts of the ds1307 I stumbled across the fact
that there are two rtc drivers claiming to support the Epson RX8025 chip.
1. ds1307 claims to support also this chip. Support was added in 2009,
however I have doubts that it actually works.
RX8025 needs a special addressing mode (register address has to be
left-shifted by 4 bits) which is implemented for both control
registers but not for the standard registers.
Also support for this chip misses the fix from patch 2e10e74df72
("rtc: rx8025: fix transfer mode")
I'm curious whether support for this chip was ever tested, commit message
of a216685818a5 "rtc: add EPSON RX8025 support to DS1307 RTC driver"
does not mention that this piece of code was tested with any device
with this chip.
2. There's a separate driver for this chip (rtc-rx8025), also added in 2009.
This drivers is actively maintained.
The separate driver mentions that it supports SA/NB variants of the chip.
There's no info regarding supported chip variants in the ds1307-integrated
driver. So there is a small chance that both drivers support different
chip variants (in this case however the ds1307-integrated driver should
clearly mention this, also in the Kconfig help text).
For me it's more likely that both drivers try support the same chip.
Having said that I would propose to remove (rudimentary) RX8025 support
in ds1307. As a first step we could leave the code in but use a WARN_ON
to notify potential users that this code is deprecated and they should
use the separate driver instead.
Rgds, Heiner
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox