--------------------------------------------
On Thu, 1/5/17, stevegreen472 via rtc-linux <rtc-linux@googlegroups.com> wrote:
Subject: Re: [rtc-linux] [PATCH] rtc: Add support for RX4035
To: rtc-linux@googlegroups.com
Date: Thursday, January 5, 2017, 2:52 AM
--------------------------------------------
On Thu, 1/5/17, denniseburnett via rtc-linux <rtc-linux@googlegroups.com>
wrote:
Subject: Re: [rtc-linux] [PATCH] rtc: Add support for
RX4035
To: rtc-linux@googlegroups.com
Date: Thursday, January 5, 2017, 2:06 AM
--------------------------------------------
On Thu, 1/5/17, bret.ng via rtc-linux <rtc-linux@googlegroups.com>
wrote:
Subject: Re: [rtc-linux] [PATCH] rtc: Add support
for
RX4035
To: rtc-linux@googlegroups.com
Date: Thursday, January 5, 2017, 12:40 AM
--------------------------------------------
On Wed, 1/4/17, Alexandre Belloni <alexandre.bell...@free-electrons.com>
wrote:
Subject: Re: [rtc-linux] [PATCH] rtc: Add support
for
RX4035
To: "Ryan Jones" <r...@rjones.io>
Cc: rtc-linux@googlegroups.com
Date: Wednesday, January 4, 2017, 11:57 PM
Hi,
Sorry for the late review!
On 05/08/2016 at 11:54:36 -0600, Ryan Jones wrote :
> + * Based on:
> + * drivers/rtc/rtc-rx4581.c
I'd argue that it doesn't really matter ;)
> +#define RX4035_REG_SC
0x00 /* Second in BCD */
> +#define RX4035_REG_MN
0x01 /* Minute in BCD */
> +#define RX4035_REG_HR
0x02 /* Hour in BCD */
> +#define RX4035_REG_DW
0x03 /* Day of Week */
> +#define RX4035_REG_DM
0x04 /* Day of Month in BCD */
> +#define RX4035_REG_MO
0x05 /* Month in BCD */
> +#define RX4035_REG_YR
0x06 /* Year in BCD */
> +#define RX4035_REG_RAM
0x0D /* RAM */
> +#define RX4035_REG_CTL1
0x0E /* Control Register 1*/
> +#define RX4035_REG_CTL2
0x0F /* Control Register 2*/
> +
> +/* Flag Register bit definitions */
> +#define RX4035_FLAG_VDET 0x40 /* Low
Voltage Detection */
> +#define RX4035_FLAG_PON
0x10 /* Indicates a Power On condition
*/
Please use BIT().
> +
> +static int rx4035_set_reg(struct spi_device
*spi,
unsigned char address,
> + unsigned char data)
> +{
> + unsigned char buf[2];
> +
> + /*Set MSB to indicate a write
transfer*/
> + buf[0] = (address<<4) |
0x08;
checkpatch --strict is complaining about spaces here
and
also a few
alignements later. Can you fix those?
> + buf[1] = data;
> +
> + return spi_write_then_read(spi,
buf, 2, NULL, 0);
> +}
> +
> +static int rx4035_get_reg(struct spi_device
*spi,
unsigned char address,
> + unsigned char *data)
> +{
> + /*Set MSB to indicate a write
transfer*/
This comment is probably wrong...
> + *data = (address<<4) | 0x0c;
> +
> + return spi_write_then_read(spi,
data, 1, data, 1);
> +}
> +
> +/*
> + * In the routines that deal directly with the
rx4035
hardware, we use
> + * rtc_time -- month 0-11, hour 0-23, yr =
calendar
year-epoch.
> + */
This comment is not useful
> +static int rx4035_get_datetime(struct device
*dev,
struct rtc_time *tm)
> +{
> + struct spi_device *spi =
to_spi_device(dev);
> + unsigned char date[7];
> + unsigned char data;
> + int err;
> +
> + /*Check that data is reliable*/
> + err = rx4035_get_reg(spi,
RX4035_REG_CTL1, &data);
> + if (err != 0) {
> + dev_err(dev,
"Unable to read control register\n");
> + return -EIO;
> + }
> + if (data & (RX4035_FLAG_VDET |
RX4035_FLAG_PON))
> + dev_err(dev,
"RTC DATA UNRELIABLE - battery voltage fell below
minimum
value\n");
It prints an error but the function continues as if
nothing
happend so
userspace doesn't actually know something is wrong.
You
probably need to
return -EINVAL here.
> +
> + /* Read time and date */
> + date[0] = 0x04;
This is a magic value. I understand what it does but
maybe
you could have
a define for each read and write mode and then remove
the
(inaccurate)
comments.
> + err = spi_write_then_read(spi,
date, 1, date, 7);
> + if (err < 0) {
> + dev_err(dev,
"Unable to read date\n");
> + return -EIO;
> + }
> +
> + dev_dbg(dev,
> + "%s: raw data is
sec=%02x, min=%02x, hr=%02x, wday=%02x, mday=%02x,
mon=%02x,
year=%02x\n",
> + __func__,
> + date[0],
date[1], date[2], date[3], date[4], date[5],
date[6]);
> +
> + tm->tm_sec =
bcd2bin(date[RX4035_REG_SC] & 0x7F);
> + tm->tm_min =
bcd2bin(date[RX4035_REG_MN] & 0x7F);
> + tm->tm_hour =
bcd2bin(date[RX4035_REG_HR] & 0x3F); /* rtc hr
0-23 */
> + tm->tm_wday =
date[RX4035_REG_DW] & 0x7F;
> + tm->tm_mday =
bcd2bin(date[RX4035_REG_DM] & 0x3F);
> + tm->tm_mon =
bcd2bin(date[RX4035_REG_MO] & 0x1F) - 1; /* rtc
mn
1-12
*/
> + tm->tm_year =
bcd2bin(date[RX4035_REG_YR]);
> + if (tm->tm_year < 70)
> + tm->tm_year
+= 100; /* assume we are in 1970...2069 */
This is bad and usually doesn't work properly.
The datasheet explicitly state:
"The auto calendar function updates all dates,
months, and
years from
January 1, 2001 to December 31, 2099."
It will not work outside this range. The leap year
handling
will
certainly fail at the edges.
--
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.
entre de presa politica germana Centre de presa
politica
maghiara | Aparitia unor organe de presa
muncitoresti
Asociatii stiintifice culturale si
academice0 W Periodice de cultura
stiintifice
--
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.ntre
timp in tara se inregistreaza primul semn de
sange al veacului. tarani rasculati la 1907 sunt ucisi
sau raniti de armata din ordinul autoritatilor presate
si de un factor extrem de periculos armatele
Austro-Ungariei ocupa pozitii de atac pe linia
Carpatilor iar cele ale Rusiei la Prut .
Dumnezeu sa-i ierte scrie N. lorga in
ziarul Neamul Romanesc indemnand la o
reconciliere din care rezulta prezenta eroica a fiilor
satelor in Razboiul pentru Reintregirea Nationala
1916-1919 . Mobilizati de un guvern condus de Ion I.C.
Bratianu si comandati intre alti generali de
Alexandru Averescu peste 800 000 de ostasi participa
la cele trei campanii de pe frontul romanesc 300 000
dintre ei jertfindu-si viata pentru supremul ideal
national.
--
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.iaza
caracterul preponderent romanesc al provinciei
numara 83 848 de
--
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.e-a lungul anilor fluxul demografic
romanesc s-a manifestat constant dinspre celelalte provincii istorice aportul
cel mai ridicat fund cel din Transilvania Nu fara temei s-a spus ca aici
ardelenii au facut una dintre cele mai trainice si mai semnificative
descalecari Prezenta pastorilor transhumanti - mocanii - cu deosebire a celor
din tara Barsei si din partea Sibiului in Dobrogea era apreciata de Ion
lonescu de la Brad la 6 000.de persoane anual Cel din urma autor se arata
impresionat si de contributia lor la pastrarea nationalitatii prin intermediul
lor cauza nationala poate sa progreseze pretutindeni unde ei merg
--
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.
