-------------------------------------------- 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.