On Fri, 20 Jan 2023 19:57:07 +0100 Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx> wrote: > On 15/12/2022 10:02:08-0500, Hugo Villeneuve wrote: > > PCF2127 has an additional feature of 512 bytes battery backed > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > > index 3265878edc48..4148e135f935 100644 > > --- a/drivers/rtc/rtc-pcf2127.c > > +++ b/drivers/rtc/rtc-pcf2127.c > > @@ -1,16 +1,26 @@ > > // SPDX-License-Identifier: GPL-2.0-only > > /* > > - * An I2C and SPI driver for the NXP PCF2127/29 RTC > > + * An I2C and SPI driver for the NXP PCF2127/29/31 RTC > > * Copyright 2013 Til-Technologies > > + * Copyright 2021 DimOnOff > > For the record, I don't really like that because git will be the > authoritative source for the copyright. This will only end up being > outdated info (as it is already) Removed. > > > * > > * Author: Renaud Cerrato <r.cerrato@xxxxxxxxxxxxxxxxxxx> > > * > > * Watchdog and tamper functions > > * Author: Bruno Thomsen <bruno.thomsen@xxxxxxxxx> > > * > > + * PCF2131 support > > + * Author: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx> > > + * > > * based on the other drivers in this same directory. > > * > > - * Datasheet: https://www.nxp.com/docs/en/data-sheet/PCF2127.pdf > > + * Datasheets: https://www.nxp.com/docs/en/data-sheet/PCF2127.pdf > > + * https://www.nxp.com/docs/en/data-sheet/PCF2131DS.pdf > > + */ > > + > > +/* > > + * The following features are not yet implemented for the PCF2131: > > + * - support for 1/100th seconds > > This will never be added so I would remove that comment Done. > > > */ > > > > #include <linux/i2c.h> > > @@ -43,8 +53,30 @@ > > #define PCF2127_BIT_CTRL3_BLF BIT(2) > > #define PCF2127_BIT_CTRL3_BF BIT(3) > > #define PCF2127_BIT_CTRL3_BTSE BIT(4) > > +/* Control register 4 */ > > +#define PCF2131_REG_CTRL4 0x03 > > +#define PCF2131_BIT_CTRL4_TSF4 BIT(4) > > +#define PCF2131_BIT_CTRL4_TSF3 BIT(5) > > +#define PCF2131_BIT_CTRL4_TSF2 BIT(6) > > +#define PCF2131_BIT_CTRL4_TSF1 BIT(7) > > +/* Control register 5 */ > > +#define PCF2131_REG_CTRL5 0x04 > > +#define PCF2131_BIT_CTRL5_TSIE4 BIT(4) > > +#define PCF2131_BIT_CTRL5_TSIE3 BIT(5) > > +#define PCF2131_BIT_CTRL5_TSIE2 BIT(6) > > +#define PCF2131_BIT_CTRL5_TSIE1 BIT(7) > > +/* Software reset register */ > > +#define PCF2131_REG_SR_RESET 0x05 > > +#define PCF2131_SR_RESET_READ_PATTERN 0b00100100 /* Fixed pattern. */ > > +#define PCF2131_SR_RESET_RESET_CMD 0x2C /* SR is bit 3. */ > > /* Time and date registers */ > > #define PCF2127_REG_TIME_DATE_BASE 0x03 > > +#define PCF2131_REG_TIME_DATE_BASE 0x07 /* Register 0x06 is 100th seconds, > > + * but we do not support it. By > > + * using offset 0x07, we can be > > + * compatible with existing > > + * time/date functions. > > + */ > > Because we will never support 100th of seconds, this comment is not > useful Removed. > > > /* Time and date registers offsets (starting from base register) */ > > #define PCF2127_OFFSET_TD_SC 0 > > #define PCF2127_OFFSET_TD_MN 1 > > @@ -57,6 +89,7 @@ > > #define PCF2127_BIT_SC_OSF BIT(7) > > /* Alarm registers */ > > #define PCF2127_REG_ALARM_BASE 0x0A > > +#define PCF2131_REG_ALARM_BASE 0x0E > > I'd keep the defines ordered by address, so you could move all the > PCF2131 defines after the PCF2127 ones Done. > > > /* Alarm registers offsets (starting from base register) */ > > #define PCF2127_OFFSET_ALARM_SC 0 > > #define PCF2127_OFFSET_ALARM_MN 1 > > @@ -67,16 +100,26 @@ > > #define PCF2127_BIT_ALARM_AE BIT(7) > > /* CLKOUT control register */ > > #define PCF2127_REG_CLKOUT 0x0f > > +#define PCF2131_REG_CLKOUT 0x13 > > #define PCF2127_BIT_CLKOUT_OTPR BIT(5) > > /* Watchdog registers */ > > #define PCF2127_REG_WD_CTL 0x10 > > +#define PCF2131_REG_WD_CTL 0x35 > > #define PCF2127_BIT_WD_CTL_TF0 BIT(0) > > #define PCF2127_BIT_WD_CTL_TF1 BIT(1) > > #define PCF2127_BIT_WD_CTL_CD0 BIT(6) > > #define PCF2127_BIT_WD_CTL_CD1 BIT(7) > > #define PCF2127_REG_WD_VAL 0x11 > > +#define PCF2131_REG_WD_VAL 0x36 > > /* Tamper timestamp1 registers */ > > #define PCF2127_REG_TS1_BASE 0x12 > > +#define PCF2131_REG_TS1_BASE 0x14 > > +/* Tamper timestamp2 registers */ > > +#define PCF2131_REG_TS2_BASE 0x1B > > +/* Tamper timestamp3 registers */ > > +#define PCF2131_REG_TS3_BASE 0x22 > > +/* Tamper timestamp4 registers */ > > +#define PCF2131_REG_TS4_BASE 0x29 > > /* Tamper timestamp registers common offsets (starting from base register) */ > > #define PCF2127_OFFSET_TS_CTL 0 > > #define PCF2127_OFFSET_TS_SC 1 > > @@ -92,11 +135,22 @@ > > * RAM registers > > * PCF2127 has 512 bytes general-purpose static RAM (SRAM) that is > > * battery backed and can survive a power outage. > > - * PCF2129 doesn't have this feature. > > + * PCF2129/31 doesn't have this feature. > > */ > > #define PCF2127_REG_RAM_ADDR_MSB 0x1A > > #define PCF2127_REG_RAM_WRT_CMD 0x1C > > #define PCF2127_REG_RAM_RD_CMD 0x1D > > +/* Interrupt mask registers */ > > +#define PCF2131_REG_INT_A_MASK1 0x31 > > +#define PCF2131_REG_INT_A_MASK2 0x32 > > +#define PCF2131_REG_INT_B_MASK1 0x33 > > +#define PCF2131_REG_INT_B_MASK2 0x34 > > +#define PCF2131_BIT_INT_BLIE BIT(0) > > +#define PCF2131_BIT_INT_BIE BIT(1) > > +#define PCF2131_BIT_INT_AIE BIT(2) > > +#define PCF2131_BIT_INT_WD_CD BIT(3) > > +#define PCF2131_BIT_INT_SI BIT(4) > > +#define PCF2131_BIT_INT_MI BIT(5) > > > > /* Watchdog timer value constants */ > > #define PCF2127_WD_VAL_STOP 0 > > @@ -110,6 +164,14 @@ > > PCF2127_BIT_CTRL2_AF | \ > > PCF2127_BIT_CTRL2_WDTF | \ > > PCF2127_BIT_CTRL2_TSF2) > > +#define PCF2131_CTRL2_IRQ_MASK ( \ > > + PCF2127_BIT_CTRL2_AF | \ > > + PCF2127_BIT_CTRL2_WDTF) > > +#define PCF2131_CTRL4_IRQ_MASK ( \ > > + PCF2131_BIT_CTRL4_TSF4 | \ > > + PCF2131_BIT_CTRL4_TSF3 | \ > > + PCF2131_BIT_CTRL4_TSF2 | \ > > + PCF2131_BIT_CTRL4_TSF1) > > > > struct pcf21xx_ts_config { > > u8 regs_base; /* Base register to read timestamp values. */ > > @@ -370,7 +432,7 @@ static int pcf2127_wdt_set_timeout(struct watchdog_device *wdd, > > } > > > > static const struct watchdog_info pcf2127_wdt_info = { > > - .identity = "NXP PCF2127/PCF2129 Watchdog", > > + .identity = "NXP PCF2127/29/31 Watchdog", > > This change may break userspace tools as this is exposed to userspace Ok, removed. > Alexandre Belloni, co-owner and COO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
