[rtc-linux] Re: [PATCH 2/2] time: alarmtimer: Add the trcepoints for alarmtimer
On Wed, 21 Sep 2016 09:26:20 +0200 (CEST) Thomas Gleixnerwrote: > > As suggested by Steven, change the type of RTC value to save trace buffer. The original code did everything just like the above but stored every value into ints, I just said that the value could be stored in chars to save space. > > A single u64 does not take more storage space than this and it's a single > store. So to use rtc_tm_to_time64()? Is the work to do the calculations of the conversion faster than a bunch of stores that are going to be in hot cache? -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[rtc-linux] Re: [PATCH 2/2] time: alarmtimer: Add the trcepoints for alarmtimer
On Wed, 21 Sep 2016 14:36:23 +0200 (CEST) Thomas Gleixnerwrote: > On Wed, 21 Sep 2016, Steven Rostedt wrote: > > On Wed, 21 Sep 2016 09:26:20 +0200 (CEST) > > Thomas Gleixner wrote: > > > A single u64 does not take more storage space than this and it's a single > > > store. > > > > So to use rtc_tm_to_time64()? Is the work to do the calculations of the > > conversion faster than a bunch of stores that are going to be in hot > > cache? > > Look at the call site. It has already the scalar nsec value and it does a > conversion to rtc time in order to trace it. OK. I haven't looked at the callsite. I just did a quick look at the patch as is, and noticed the wasted space in the buffer for storing a bunch of ints that will never be bigger than 256. > > Ditto for the other tracepoints where the conversion from scalar nsec is > done in the tracepoint itself. This is why I like to have the maintainers review the rest. -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[rtc-linux] Re: [PATCH 2/2] time: alarmtimer: Add the trcepoints for alarmtimer
On Wed, 21 Sep 2016, Steven Rostedt wrote: > On Wed, 21 Sep 2016 09:26:20 +0200 (CEST) > Thomas Gleixnerwrote: > > A single u64 does not take more storage space than this and it's a single > > store. > > So to use rtc_tm_to_time64()? Is the work to do the calculations of the > conversion faster than a bunch of stores that are going to be in hot > cache? Look at the call site. It has already the scalar nsec value and it does a conversion to rtc time in order to trace it. Ditto for the other tracepoints where the conversion from scalar nsec is done in the tracepoint itself. Thanks, tglx -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[rtc-linux] Re: [PATCH 1/3] dt-binding: rtc Add DT binding for NXP 85263 RTC
Rob, can you review those bindings? On 01/08/2016 at 17:50:32 +0200, Martin Fuzzey wrote : Please always include a commit message. > Signed-off-by: Martin Fuzzey> --- > .../devicetree/bindings/rtc/nxp,pcf85263.txt | 41 > > include/dt-bindings/rtc/nxp,pcf85263.h | 14 +++ > 2 files changed, 55 insertions(+) > create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf85263.txt > create mode 100644 include/dt-bindings/rtc/nxp,pcf85263.h > > diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85263.txt > b/Documentation/devicetree/bindings/rtc/nxp,pcf85263.txt > new file mode 100644 > index 000..03b9505 > --- /dev/null > +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85263.txt > @@ -0,0 +1,41 @@ > +NXP PCF85263 I2C Real Time Clock > + > +Required properties: > +- compatible: must be: "nxp,rtc-pcf85263" > +- reg: must be the I2C address > + > +Optional properties: > +- interrupt-names: Which interrupt signal is used must be "INTA" or "INTB" > +Defaults to "INTA" > + > +- quartz-load-capacitance: The internal capacitor to select for the quartz: > + PCF85263_QUARTZCAP_7pF [0] > + PCF85263_QUARTZCAP_6pF [1] > + PCF85263_QUARTZCAP_12p5pF [2] DEFAULT > + > +- quartz-drive-strength: Drive strength for the quartz: > + PCF85263_QUARTZDRIVE_NORMAL [0] DEFAULT > + PCF85263_QUARTZDRIVE_LOW[1] > + PCF85263_QUARTZDRIVE_HIGH [2] > + > +- quartz-low-jitter: Boolean property, if present enables low jitter mode > which > +reduces jitter at the cost of increased power consumption. > + Maybe that one should be handled using sysfs instead of DT as it is more policy than HW related. > +- wakeup-source: mark the chip as a wakeup source, independently of > +the availability of an IRQ line connected to the SoC. > +This is useful if the IRQ line is connected to a PMIC or other circuit > +that can power up the device rather than to a normal SOC interrupt. > + > +Example: > + > +rtc@51 { > + compatible = "nxp,pcf85263"; > + reg = <0x51>; > + > + interrupt-parent = <>; > + interrupts = <5 IRQ_TYPE_LEVEL_LOW>; > + interrupt-names = "INTB"; > + > + quartz-load-capacitance = ; > + quartz-drive-strength = ; > +}; > diff --git a/include/dt-bindings/rtc/nxp,pcf85263.h > b/include/dt-bindings/rtc/nxp,pcf85263.h > new file mode 100644 > index 000..ea87ae4 > --- /dev/null > +++ b/include/dt-bindings/rtc/nxp,pcf85263.h > @@ -0,0 +1,14 @@ > +#ifndef _DT_BINDINGS_RTC_NXP_PCF85263_H > +#define _DT_BINDINGS_RTC_NXP_PCF85263_H > + > +/* Quartz capacitance */ > +#define PCF85263_QUARTZCAP_7pF 0 > +#define PCF85263_QUARTZCAP_6pF 1 > +#define PCF85263_QUARTZCAP_12p5pF2 > + > +/* Quartz drive strength */ > +#define PCF85263_QUARTZDRIVE_NORMAL 0 > +#define PCF85263_QUARTZDRIVE_LOW 1 > +#define PCF85263_QUARTZDRIVE_HIGH2 > + > +#endif /* _DT_BINDINGS_RTC_NXP_PCF85263_H */ > -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[rtc-linux] Re: [PATCH v4 1/2] rtc-cmos: Clear ACPI-driven alarms upon resume
Hi, Thank you for your perseverance. On 20/09/2016 at 01:12:43 +0200, Gabriele Mazzotta wrote : > Currently ACPI-driven alarms are not cleared when they wake the > system. As consequence, expired alarms must be manually cleared to > program a new alarm. Fix this by correctly handling ACPI-driven > alarms. > > More specifically, the ACPI specification [1] provides for two > alternative implementations of the RTC. Depending on the > implementation, the driver either clear the alarm from the resume > callback or from ACPI interrupt handler: > > - The platform has the RTC wakeup status fixed in hardware >(ACPI_FADT_FIXED_RTC is 0). In this case the driver can determine >if the RTC was the reason of the wakeup from the resume callback >by reading the RTC status register. > > - The platform has no fixed hardware feature event bits. In this >case a GPE is used to wake the system and the driver clears the >alarm from its handler. > > [1] http://www.acpi.info/DOWNLOADS/ACPI_5_Errata%20A.pdf > > Signed-off-by: Gabriele Mazzotta> --- > drivers/rtc/rtc-cmos.c | 49 + > 1 file changed, 49 insertions(+) Applied, with the following cosmetic change: @@ -982,13 +982,11 @@ static u32 rtc_handler(void *context) { struct device *dev = context; struct cmos_rtc *cmos = dev_get_drvdata(dev); - unsigned char rtc_control; + unsigned char rtc_control = 0; unsigned char rtc_intr; spin_lock_irq(_lock); - if (!cmos_rtc.suspend_ctrl) - rtc_control = cmos_rtc.suspend_ctrl; - else + if (cmos_rtc.suspend_ctrl) rtc_control = CMOS_READ(RTC_CONTROL); if (rtc_control & RTC_AIE) { cmos_rtc.suspend_ctrl &= ~RTC_AIE; -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[rtc-linux] Re: [PATCH 2/2] time: alarmtimer: Add the trcepoints for alarmtimer
On Wed, 21 Sep 2016, Baolin Wang wrote: > On 21 September 2016 at 06:27, Thomas Gleixnerwrote: > >> + TP_fast_assign( > >> + __entry->second = rtc_time->tm_sec; > >> + __entry->minute = rtc_time->tm_min; > >> + __entry->hour = rtc_time->tm_hour; > >> + __entry->day = rtc_time->tm_mday; > >> + __entry->mon = rtc_time->tm_mon; > >> + __entry->year = rtc_time->tm_year; > >> + __entry->alarm_type = flag; > > > > What's the value of storing the alarm time in RTC format? > > As suggested by Steven, change the type of RTC value to save trace buffer. A single u64 does not take more storage space than this and it's a single store. > > 2) You store the expiry time again in RTC format. Store the information in > >a plain u64 and be done with it. > > But I still think the RTC format is more readable for debugging alarm timer. That's what post processing is for. > > What's the point of this conditional? Avoiding rtc_ktime_to_tm() ? Oh > > well... > > > >> + tm_set = rtc_ktime_to_tm(now); > >> + trace_alarmtimer_suspend(_set, type); > > > > "now" is CLOCK_REALTIME based. You store the type of the alarm timer which > > is the first to expire and therefor is the one setting the RTC value, but > > we don't know which timer it is. Useful - NOT! > > We can know the timer by comparing the expire time. Please make it similar to the timer/hrtimer tracing so people can reuse their postprocessing scripts with minimial tweaks. Thanks, tglx -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[rtc-linux] Re: [PATCH v2] rtc-cmos: Reject unsupported alarm values
On 02/09/2016 at 21:55:16 +0200, Gabriele Mazzotta wrote : > Some platforms allows to specify the month and day of the month in > which an alarm should go off, some others the day of the month and > some others just the time. > > Currently any given value is accepted by the driver and only the > supported fields are used to program the hardware. As consequence, > alarms are potentially programmed to go off in the wrong moment. > > Fix this by rejecting any value not supported by the hardware. > > Signed-off-by: Gabriele Mazzotta> --- > > I revisited the naive implementation of v1. I tested the new > algorithm using some dates that and verified that it behaved as > expected, but I might have missed some corner cases. > > I made some assumptions that maybe should be dropped, at least > two of them. They are commented in the code, but I didn't mention > that they are assumptions: > > - If the day can't be specified, the alarm can only be set to go >off 24 hours minus 1 second in the future. I'm worried things >would go wrong if the current time is used to set an alarm that >should go off the next day. > - If the mday can be specified and the next month has more days >than the current month, the alarm can be set to go off in the >extra days of the next month. Hum, you are actually not allowing them in the code below (which I think is the right thing to do). > - If the month can be specified, it's the 28th of February and the >next year is a leap year, the alarm can be set for the 29th of >February of the next year. Is that really true? I would expect the opposite. If it is currently 28/02, the max date you can actually go to is 27/02. If you allow 29/02, at some time the RTC will have 28/02 and the alarm will fire. > > Basically I'm assuming that the hardware decides when an alarm should > go off comparing the current date with the one programmed. If they > match, the alarm goes off. This seemed reasonable to me, but it's > not easy to verify. > > drivers/rtc/rtc-cmos.c | 104 > + > 1 file changed, 104 insertions(+) > > diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c > index 4cdb335..37cb7c1 100644 > --- a/drivers/rtc/rtc-cmos.c > +++ b/drivers/rtc/rtc-cmos.c > @@ -328,14 +328,118 @@ static void cmos_irq_disable(struct cmos_rtc *cmos, > unsigned char mask) > cmos_checkintr(cmos, rtc_control); > } > > +static int cmos_validate_alarm(struct device *dev, struct rtc_wkalrm *t) > +{ > + struct cmos_rtc *cmos = dev_get_drvdata(dev); > + struct rtc_time now; > + > + cmos_read_time(dev, ); > + > + if (!cmos->day_alrm) { > + time64_t t_max_date; > + time64_t t_alrm; > + > + t_alrm = rtc_tm_to_time64(>time); > + t_max_date = rtc_tm_to_time64(); > + /* > + * Subtract 1 second to ensure that the alarm time is > + * different from the current time. > + */ > + t_max_date += 24 * 60 * 60 - 1; > + if (t_alrm > t_max_date) { > + dev_err(dev, > + "Alarms can be up to one day in the future\n"); > + return -EINVAL; > + } > + } else if (!cmos->mon_alrm) { > + struct rtc_time max_date = now; > + time64_t t_max_date; > + time64_t t_alrm; > + int max_mday; > + bool is_max_mday = false; > + > + /* > + * If the next month has more days than the current month > + * and we are at the max mday of this month, we can program > + * the alarm to go off the max mday of the next month without > + * it going off sooner than expected. > + */ > + max_mday = rtc_month_days(now.tm_mon, now.tm_year); > + if (now.tm_mday == max_mday) > + is_max_mday = true; > + > + if (max_date.tm_mon == 11) { > + max_date.tm_mon = 0; > + max_date.tm_year += 1; > + } else { > + max_date.tm_mon += 1; > + } > + max_mday = rtc_month_days(max_date.tm_mon, max_date.tm_year); > + if (max_date.tm_mday > max_mday || is_max_mday) > + max_date.tm_mday = max_mday; > + > + max_date.tm_hour = 23; > + max_date.tm_min = 59; > + max_date.tm_sec = 59; > + Actually, this is wrong. If it is currently 1:23:45 on 22/09, you can go up to 1:23:44 on 22/10. trying to set a time after 1:23:45 will actually match on the same day instead of a month later. > + t_max_date = rtc_tm_to_time64(_date); > + t_alrm = rtc_tm_to_time64(>time); > + if (t_alrm > t_max_date) { > + dev_err(dev, > + "Alarms can be
[rtc-linux] Re: [PATCH] rtc: ds1347: changed raw spi calls to register map calls
On 31/08/2016 at 23:03:58 +0530, Raghavendra Chandra Ganiga wrote : > From 633c012ba6c8f9bc6cf475d2434ca450cd06b18e Mon Sep 17 00:00:00 2001 > From: Raghavendra Ganiga> Date: Wed, 31 Aug 2016 22:56:41 +0530 > Subject: [PATCH] rtc: ds1347: changed raw spi calls to register map calls > > This patch changes calls of spi read write calls to register map > read and write calls in rtc ds1347 > > Signed-off-by: Raghavendra Chandra Ganiga > --- > drivers/rtc/Kconfig | 1 + > drivers/rtc/rtc-ds1347.c | 96 > ++-- > 2 files changed, 54 insertions(+), 43 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[rtc-linux] Re: [PATCH] rtc: ac100: Add NULL checking for devm_kzalloc call
On 09/09/2016 at 18:03:54 +0800, Axel Lin wrote : > devm_kzalloc can return NULL, add NULL checking to prevent NULL pointer > dereference. > > Signed-off-by: Axel Lin> --- > drivers/rtc/rtc-ac100.c | 3 +++ > 1 file changed, 3 insertions(+) > 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.