* [rtc-linux] [PATCH] rtc-da9063: avoid writing undefined data to rtc
@ 2015-11-27 12:02 Enrico Scholz
2015-11-27 15:01 ` [rtc-linux] " Alexandre Belloni
2015-11-30 19:07 ` Alexandre Belloni
0 siblings, 2 replies; 7+ messages in thread
From: Enrico Scholz @ 2015-11-27 12:02 UTC (permalink / raw)
To: rtc-linux
Cc: Alessandro Zummo, Support Opensource, Alexandre Belloni, Enrico Scholz
driver did
| static void da9063_tm_to_data(struct rtc_time *tm, u8 *data,
| {
| const struct da9063_compatible_rtc_regmap *config = rtc->config;
|
| data[RTC_SEC] &= ~config->rtc_count_sec_mask;
| data[RTC_SEC] |= tm->tm_sec & config->rtc_count_sec_mask;
| ...
| }
| ...
| static int da9063_rtc_set_time(struct device *dev, struct rtc_time *tm)
| {
| ...
| u8 data[RTC_DATA_LEN];
| int ret;
|
| da9063_tm_to_data(tm, data, rtc);
which means that some bits of stack content (in 'data[]') was masked out
and written to the RTC.
Because da9063_tm_to_data() is used only by da9063_rtc_set_time() and
da9063_rtc_set_alarm(), we can write fields directly.
Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
---
drivers/rtc/rtc-da9063.c | 23 ++++++-----------------
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c
index 00a8f7f..8d5b9be 100644
--- a/drivers/rtc/rtc-da9063.c
+++ b/drivers/rtc/rtc-da9063.c
@@ -191,24 +191,13 @@ static void da9063_tm_to_data(struct rtc_time *tm, u8 *data,
{
const struct da9063_compatible_rtc_regmap *config = rtc->config;
- data[RTC_SEC] &= ~config->rtc_count_sec_mask;
- data[RTC_SEC] |= tm->tm_sec & config->rtc_count_sec_mask;
-
- data[RTC_MIN] &= ~config->rtc_count_min_mask;
- data[RTC_MIN] |= tm->tm_min & config->rtc_count_min_mask;
-
- data[RTC_HOUR] &= ~config->rtc_count_hour_mask;
- data[RTC_HOUR] |= tm->tm_hour & config->rtc_count_hour_mask;
-
- data[RTC_DAY] &= ~config->rtc_count_day_mask;
- data[RTC_DAY] |= tm->tm_mday & config->rtc_count_day_mask;
-
- data[RTC_MONTH] &= ~config->rtc_count_month_mask;
- data[RTC_MONTH] |= MONTHS_TO_DA9063(tm->tm_mon) &
+ data[RTC_SEC] = tm->tm_sec & config->rtc_count_sec_mask;
+ data[RTC_MIN] = tm->tm_min & config->rtc_count_min_mask;
+ data[RTC_HOUR] = tm->tm_hour & config->rtc_count_hour_mask;
+ data[RTC_DAY] = tm->tm_mday & config->rtc_count_day_mask;
+ data[RTC_MONTH] = MONTHS_TO_DA9063(tm->tm_mon) &
config->rtc_count_month_mask;
-
- data[RTC_YEAR] &= ~config->rtc_count_year_mask;
- data[RTC_YEAR] |= YEARS_TO_DA9063(tm->tm_year) &
+ data[RTC_YEAR] = YEARS_TO_DA9063(tm->tm_year) &
config->rtc_count_year_mask;
}
--
2.4.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 [flat|nested] 7+ messages in thread
* [rtc-linux] Re: [PATCH] rtc-da9063: avoid writing undefined data to rtc
2015-11-27 12:02 [rtc-linux] [PATCH] rtc-da9063: avoid writing undefined data to rtc Enrico Scholz
@ 2015-11-27 15:01 ` Alexandre Belloni
2015-11-27 15:27 ` Enrico Scholz
2015-11-30 19:07 ` Alexandre Belloni
1 sibling, 1 reply; 7+ messages in thread
From: Alexandre Belloni @ 2015-11-27 15:01 UTC (permalink / raw)
To: Enrico Scholz; +Cc: rtc-linux, Alessandro Zummo, Support Opensource
Hi,
On 27/11/2015 at 13:02:55 +0100, Enrico Scholz wrote :
> driver did
>
> | static void da9063_tm_to_data(struct rtc_time *tm, u8 *data,
> | {
> | const struct da9063_compatible_rtc_regmap *config = rtc->config;
> |
> | data[RTC_SEC] &= ~config->rtc_count_sec_mask;
> | data[RTC_SEC] |= tm->tm_sec & config->rtc_count_sec_mask;
> | ...
> | }
> | ...
> | static int da9063_rtc_set_time(struct device *dev, struct rtc_time *tm)
> | {
> | ...
> | u8 data[RTC_DATA_LEN];
> | int ret;
> |
> | da9063_tm_to_data(tm, data, rtc);
>
> which means that some bits of stack content (in 'data[]') was masked out
> and written to the RTC.
>
> Because da9063_tm_to_data() is used only by da9063_rtc_set_time() and
> da9063_rtc_set_alarm(), we can write fields directly.
>
Unfortunately, the datasheets are behind a register wall and I can't
check. Is there any meaningful bits mixed in the date/time registers?
I think this was intended to keep those bits to their previous value but
this is not yet implemented.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android 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 [flat|nested] 7+ messages in thread
* [rtc-linux] Re: [PATCH] rtc-da9063: avoid writing undefined data to rtc
2015-11-27 15:01 ` [rtc-linux] " Alexandre Belloni
@ 2015-11-27 15:27 ` Enrico Scholz
2015-11-27 15:57 ` Alexandre Belloni
0 siblings, 1 reply; 7+ messages in thread
From: Enrico Scholz @ 2015-11-27 15:27 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: rtc-linux, Alessandro Zummo, Support Opensource
Alexandre Belloni <alexandre.belloni@free-electrons.com> writes:
>> | static void da9063_tm_to_data(struct rtc_time *tm, u8 *data,
>> | {
>> | const struct da9063_compatible_rtc_regmap *config = rtc->config;
>> |
>> | data[RTC_SEC] &= ~config->rtc_count_sec_mask;
>> | data[RTC_SEC] |= tm->tm_sec & config->rtc_count_sec_mask;
>> | ...
>> | }
>> ...
>> which means that some bits of stack content (in 'data[]') was masked out
>> and written to the RTC.
>>
>> Because da9063_tm_to_data() is used only by da9063_rtc_set_time() and
>> da9063_rtc_set_alarm(), we can write fields directly.
>
> Unfortunately, the datasheets are behind a register wall and I can't
> check. Is there any meaningful bits mixed in the date/time registers?
We can set "RTC_EN and CRYSTAL writing disabled" (COUNT_Y[6]) randomly;
this bit has a different meaning for read ("RTC clock okay") and write
("disable writing"; not clearable).
ALARM_MO[4] "Period of the Tick (0: every second, 1: every minute)" or
ALARM_Y[7+6] (enable tick/alarm) should not be set randomly either.
I think, it is always a bad idea to write random data from stack to a
device.
> I think this was intended to keep those bits to their previous value but
> this is not yet implemented.
Datasheet does not say how to deal with "reserved" bits; but it would be
imo overkill to apply a regmap_update_bits() like access to the RTC
registers.
Enrico
--
--
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 [flat|nested] 7+ messages in thread
* Re: [rtc-linux] Re: [PATCH] rtc-da9063: avoid writing undefined data to rtc
2015-11-27 15:27 ` Enrico Scholz
@ 2015-11-27 15:57 ` Alexandre Belloni
2015-11-30 2:47 ` Opensource [Steve Twiss]
2015-11-30 14:32 ` Opensource [Steve Twiss]
0 siblings, 2 replies; 7+ messages in thread
From: Alexandre Belloni @ 2015-11-27 15:57 UTC (permalink / raw)
To: Enrico Scholz, Steve Twiss
Cc: rtc-linux, Alessandro Zummo, Support Opensource
On 27/11/2015 at 16:27:25 +0100, Enrico Scholz wrote :
> > Unfortunately, the datasheets are behind a register wall and I can't
> > check. Is there any meaningful bits mixed in the date/time registers?
>
> We can set "RTC_EN and CRYSTAL writing disabled" (COUNT_Y[6]) randomly;
> this bit has a different meaning for read ("RTC clock okay") and write
> ("disable writing"; not clearable).
>
> ALARM_MO[4] "Period of the Tick (0: every second, 1: every minute)" or
> ALARM_Y[7+6] (enable tick/alarm) should not be set randomly either.
>
>
> I think, it is always a bad idea to write random data from stack to a
> device.
>
I agree.
>
> > I think this was intended to keep those bits to their previous value but
> > this is not yet implemented.
>
> Datasheet does not say how to deal with "reserved" bits; but it would be
> imo overkill to apply a regmap_update_bits() like access to the RTC
> registers.
>
It depends on how much you care about those bits across reboots.
I'll take your patch anyway, unless Steve has another opinion on that.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android 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 [flat|nested] 7+ messages in thread
* RE: [rtc-linux] Re: [PATCH] rtc-da9063: avoid writing undefined data to rtc
2015-11-27 15:57 ` Alexandre Belloni
@ 2015-11-30 2:47 ` Opensource [Steve Twiss]
2015-11-30 14:32 ` Opensource [Steve Twiss]
1 sibling, 0 replies; 7+ messages in thread
From: Opensource [Steve Twiss] @ 2015-11-30 2:47 UTC (permalink / raw)
To: Alexandre Belloni, Enrico Scholz, Opensource [Steve Twiss]
Cc: rtc-linux, Alessandro Zummo, Support Opensource
On 27 November 2015 15:57 Alexandre Belloni wrote:
> To: Enrico Scholz; Opensource [Steve Twiss]
> Cc: rtc-linux@googlegroups.com; Alessandro Zummo; Support Opensource
> Subject: Re: [rtc-linux] Re: [PATCH] rtc-da9063: avoid writing undefined data
> to rtc
>
> On 27/11/2015 at 16:27:25 +0100, Enrico Scholz wrote :
> > > Unfortunately, the datasheets are behind a register wall and I can't
> > > check. Is there any meaningful bits mixed in the date/time registers?
> >
> > We can set "RTC_EN and CRYSTAL writing disabled" (COUNT_Y[6]) randomly;
> > this bit has a different meaning for read ("RTC clock okay") and write
> > ("disable writing"; not clearable).
> >
> > ALARM_MO[4] "Period of the Tick (0: every second, 1: every minute)" or
> > ALARM_Y[7+6] (enable tick/alarm) should not be set randomly either.
> >
> > I think, it is always a bad idea to write random data from stack to a
> > device.
>
> I agree.
>
> > > I think this was intended to keep those bits to their previous value but
> > > this is not yet implemented.
> >
> > Datasheet does not say how to deal with "reserved" bits; but it would be
> > imo overkill to apply a regmap_update_bits() like access to the RTC
> > registers.
> >
>
> It depends on how much you care about those bits across reboots.
>
> I'll take your patch anyway, unless Steve has another opinion on that.
>
I'll take a look at this later today.
Regards,
Steve
--
--
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 [flat|nested] 7+ messages in thread
* RE: [rtc-linux] Re: [PATCH] rtc-da9063: avoid writing undefined data to rtc
2015-11-27 15:57 ` Alexandre Belloni
2015-11-30 2:47 ` Opensource [Steve Twiss]
@ 2015-11-30 14:32 ` Opensource [Steve Twiss]
1 sibling, 0 replies; 7+ messages in thread
From: Opensource [Steve Twiss] @ 2015-11-30 14:32 UTC (permalink / raw)
To: Alexandre Belloni, Enrico Scholz
Cc: rtc-linux, Alessandro Zummo, Support Opensource
On Sent: 27 November 2015 15:57 Alexandre Belloni wrote:
> Subject: Re: [rtc-linux] Re: [PATCH] rtc-da9063: avoid writing undefined data to rtc
> On 27/11/2015 at 16:27:25 +0100, Enrico Scholz wrote :
> >
> > I think, it is always a bad idea to write random data from stack to a
> > device.
>
> I agree.
>
Hi Alexandre and Enrico,
I have double checked this morning and in these cases it is not important whether it is a 1 or a 0
written to these specific reserved bits.
But, I did intended to write a zero to the reserved bits.
Enrico's patch will make the recommendation clear.
>
> I'll take your patch anyway, unless Steve has another opinion on that.
>
Acked-by: Steve Twiss <stwiss.opensource@diasemi.com>
Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
Regards,
Steve
--
--
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 [flat|nested] 7+ messages in thread
* [rtc-linux] Re: [PATCH] rtc-da9063: avoid writing undefined data to rtc
2015-11-27 12:02 [rtc-linux] [PATCH] rtc-da9063: avoid writing undefined data to rtc Enrico Scholz
2015-11-27 15:01 ` [rtc-linux] " Alexandre Belloni
@ 2015-11-30 19:07 ` Alexandre Belloni
1 sibling, 0 replies; 7+ messages in thread
From: Alexandre Belloni @ 2015-11-30 19:07 UTC (permalink / raw)
To: Enrico Scholz; +Cc: rtc-linux, Alessandro Zummo, Support Opensource
On 27/11/2015 at 13:02:55 +0100, Enrico Scholz wrote :
> driver did
>
> | static void da9063_tm_to_data(struct rtc_time *tm, u8 *data,
> | {
> | const struct da9063_compatible_rtc_regmap *config = rtc->config;
> |
> | data[RTC_SEC] &= ~config->rtc_count_sec_mask;
> | data[RTC_SEC] |= tm->tm_sec & config->rtc_count_sec_mask;
> | ...
> | }
> | ...
> | static int da9063_rtc_set_time(struct device *dev, struct rtc_time *tm)
> | {
> | ...
> | u8 data[RTC_DATA_LEN];
> | int ret;
> |
> | da9063_tm_to_data(tm, data, rtc);
>
> which means that some bits of stack content (in 'data[]') was masked out
> and written to the RTC.
>
> Because da9063_tm_to_data() is used only by da9063_rtc_set_time() and
> da9063_rtc_set_alarm(), we can write fields directly.
>
> Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
> ---
> drivers/rtc/rtc-da9063.c | 23 ++++++-----------------
> 1 file changed, 6 insertions(+), 17 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android 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 [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-11-30 19:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-27 12:02 [rtc-linux] [PATCH] rtc-da9063: avoid writing undefined data to rtc Enrico Scholz
2015-11-27 15:01 ` [rtc-linux] " Alexandre Belloni
2015-11-27 15:27 ` Enrico Scholz
2015-11-27 15:57 ` Alexandre Belloni
2015-11-30 2:47 ` Opensource [Steve Twiss]
2015-11-30 14:32 ` Opensource [Steve Twiss]
2015-11-30 19:07 ` Alexandre Belloni
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.