Message ID | 20250419-pcf8563-fix-alarm-v1-1-b893a5de55b8@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | rtc: pcf8563: fix wrong alarm register | expand |
On 19/04/2025 22:37:10+0800, Troy Mitchell wrote: > Fix wrong register and align `pcf8563_get_alarm_mode` > with the naming convention used in ops. > > Signed-off-by: Troy Mitchell <troymitchell988@gmail.com> > --- > Since this patch[1], the set_alarm function has been setting > an wrong register. > > Link: > https://lore.kernel.org/all/20241010084949.3351182-3-iwamatsu@nigauri.org/ [1] > --- > drivers/rtc/rtc-pcf8563.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c > index 5a084d426e58d09cfedf0809695a96a27627c420..61e2f9757de9f24407f9262657da0d1586ce124e 100644 > --- a/drivers/rtc/rtc-pcf8563.c > +++ b/drivers/rtc/rtc-pcf8563.c > @@ -103,7 +103,7 @@ static int pcf8563_set_alarm_mode(struct pcf8563 *pcf8563, bool on) > return regmap_write(pcf8563->regmap, PCF8563_REG_ST2, buf); > } > > -static int pcf8563_get_alarm_mode(struct pcf8563 *pcf8563, unsigned char *en, > +static int pcf8563_read_alarm_mode(struct pcf8563 *pcf8563, unsigned char *en, I was going to apply the patch but this is an unrelated change, please submit just the fix so it can be backported. > unsigned char *pen) > { > u32 buf; > @@ -127,7 +127,7 @@ static irqreturn_t pcf8563_irq(int irq, void *dev_id) > char pending; > int err; > > - err = pcf8563_get_alarm_mode(pcf8563, NULL, &pending); > + err = pcf8563_read_alarm_mode(pcf8563, NULL, &pending); > if (err) > return IRQ_NONE; > > @@ -262,7 +262,7 @@ static int pcf8563_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *tm) > tm->time.tm_mday = bcd2bin(buf[2] & 0x3F); > tm->time.tm_wday = bcd2bin(buf[3] & 0x7); > > - err = pcf8563_get_alarm_mode(pcf8563, &tm->enabled, &tm->pending); > + err = pcf8563_read_alarm_mode(pcf8563, &tm->enabled, &tm->pending); > if (err < 0) > return err; > > @@ -285,7 +285,7 @@ static int pcf8563_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm) > buf[2] = bin2bcd(tm->time.tm_mday); > buf[3] = tm->time.tm_wday & 0x07; > > - err = regmap_bulk_write(pcf8563->regmap, PCF8563_REG_SC, buf, > + err = regmap_bulk_write(pcf8563->regmap, PCF8563_REG_AMN, buf, > sizeof(buf)); > if (err) > return err; > > --- > base-commit: 8560697b23dc2f405cb463af2b17256a9888129d > change-id: 20250419-pcf8563-fix-alarm-5e787f095861 > > Best regards, > -- > Troy Mitchell <troymitchell988@gmail.com> >
On 2025/5/25 05:36, Alexandre Belloni wrote: > On 19/04/2025 22:37:10+0800, Troy Mitchell wrote: >> Fix wrong register and align `pcf8563_get_alarm_mode` >> with the naming convention used in ops. >> >> Signed-off-by: Troy Mitchell <troymitchell988@gmail.com> >> --- >> Since this patch[1], the set_alarm function has been setting >> an wrong register. >> >> Link: >> https://lore.kernel.org/all/20241010084949.3351182-3-iwamatsu@nigauri.org/ [1] >> --- >> drivers/rtc/rtc-pcf8563.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c >> index 5a084d426e58d09cfedf0809695a96a27627c420..61e2f9757de9f24407f9262657da0d1586ce124e 100644 >> --- a/drivers/rtc/rtc-pcf8563.c >> +++ b/drivers/rtc/rtc-pcf8563.c >> @@ -103,7 +103,7 @@ static int pcf8563_set_alarm_mode(struct pcf8563 *pcf8563, bool on) >> return regmap_write(pcf8563->regmap, PCF8563_REG_ST2, buf); >> } >> >> -static int pcf8563_get_alarm_mode(struct pcf8563 *pcf8563, unsigned char *en, >> +static int pcf8563_read_alarm_mode(struct pcf8563 *pcf8563, unsigned char *en, > > I was going to apply the patch but this is an unrelated change, please submit > just the fix so it can be backported. Hi, Could you please clarify if this renaming change would be acceptable? If it is acceptable, I will split the original patch into two. If not, I will remove the renaming change. - Troy > >> unsigned char *pen) >> { >> u32 buf; >> @@ -127,7 +127,7 @@ static irqreturn_t pcf8563_irq(int irq, void *dev_id) >> char pending; >> int err; >> >> - err = pcf8563_get_alarm_mode(pcf8563, NULL, &pending); >> + err = pcf8563_read_alarm_mode(pcf8563, NULL, &pending); >> if (err) >> return IRQ_NONE; >> >> @@ -262,7 +262,7 @@ static int pcf8563_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *tm) >> tm->time.tm_mday = bcd2bin(buf[2] & 0x3F); >> tm->time.tm_wday = bcd2bin(buf[3] & 0x7); >> >> - err = pcf8563_get_alarm_mode(pcf8563, &tm->enabled, &tm->pending); >> + err = pcf8563_read_alarm_mode(pcf8563, &tm->enabled, &tm->pending); >> if (err < 0) >> return err; >> >> @@ -285,7 +285,7 @@ static int pcf8563_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm) >> buf[2] = bin2bcd(tm->time.tm_mday); >> buf[3] = tm->time.tm_wday & 0x07; >> >> - err = regmap_bulk_write(pcf8563->regmap, PCF8563_REG_SC, buf, >> + err = regmap_bulk_write(pcf8563->regmap, PCF8563_REG_AMN, buf, >> sizeof(buf)); >> if (err) >> return err; >> >> --- >> base-commit: 8560697b23dc2f405cb463af2b17256a9888129d >> change-id: 20250419-pcf8563-fix-alarm-5e787f095861 >> >> Best regards, >> -- >> Troy Mitchell <troymitchell988@gmail.com> >> >
Hello Troy, On 25/05/2025 10:36:55+0800, Troy Mitchell wrote: > On 2025/5/25 05:36, Alexandre Belloni wrote: > > On 19/04/2025 22:37:10+0800, Troy Mitchell wrote: > >> Fix wrong register and align `pcf8563_get_alarm_mode` > >> with the naming convention used in ops. > >> > >> Signed-off-by: Troy Mitchell <troymitchell988@gmail.com> > >> --- > >> Since this patch[1], the set_alarm function has been setting > >> an wrong register. > >> > >> Link: > >> https://lore.kernel.org/all/20241010084949.3351182-3-iwamatsu@nigauri.org/ [1] > >> --- > >> drivers/rtc/rtc-pcf8563.c | 8 ++++---- > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c > >> index 5a084d426e58d09cfedf0809695a96a27627c420..61e2f9757de9f24407f9262657da0d1586ce124e 100644 > >> --- a/drivers/rtc/rtc-pcf8563.c > >> +++ b/drivers/rtc/rtc-pcf8563.c > >> @@ -103,7 +103,7 @@ static int pcf8563_set_alarm_mode(struct pcf8563 *pcf8563, bool on) > >> return regmap_write(pcf8563->regmap, PCF8563_REG_ST2, buf); > >> } > >> > >> -static int pcf8563_get_alarm_mode(struct pcf8563 *pcf8563, unsigned char *en, > >> +static int pcf8563_read_alarm_mode(struct pcf8563 *pcf8563, unsigned char *en, > > > > I was going to apply the patch but this is an unrelated change, please submit > > just the fix so it can be backported. > Hi, Could you please clarify if this renaming change would be acceptable? > If it is acceptable, I will split the original patch into two. > If not, I will remove the renaming change. Thanks for v2, I don't think the renaming is actually worth it, unless there is more rework on the driver. > > - Troy > > > >> unsigned char *pen) > >> { > >> u32 buf; > >> @@ -127,7 +127,7 @@ static irqreturn_t pcf8563_irq(int irq, void *dev_id) > >> char pending; > >> int err; > >> > >> - err = pcf8563_get_alarm_mode(pcf8563, NULL, &pending); > >> + err = pcf8563_read_alarm_mode(pcf8563, NULL, &pending); > >> if (err) > >> return IRQ_NONE; > >> > >> @@ -262,7 +262,7 @@ static int pcf8563_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *tm) > >> tm->time.tm_mday = bcd2bin(buf[2] & 0x3F); > >> tm->time.tm_wday = bcd2bin(buf[3] & 0x7); > >> > >> - err = pcf8563_get_alarm_mode(pcf8563, &tm->enabled, &tm->pending); > >> + err = pcf8563_read_alarm_mode(pcf8563, &tm->enabled, &tm->pending); > >> if (err < 0) > >> return err; > >> > >> @@ -285,7 +285,7 @@ static int pcf8563_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm) > >> buf[2] = bin2bcd(tm->time.tm_mday); > >> buf[3] = tm->time.tm_wday & 0x07; > >> > >> - err = regmap_bulk_write(pcf8563->regmap, PCF8563_REG_SC, buf, > >> + err = regmap_bulk_write(pcf8563->regmap, PCF8563_REG_AMN, buf, > >> sizeof(buf)); > >> if (err) > >> return err; > >> > >> --- > >> base-commit: 8560697b23dc2f405cb463af2b17256a9888129d > >> change-id: 20250419-pcf8563-fix-alarm-5e787f095861 > >> > >> Best regards, > >> -- > >> Troy Mitchell <troymitchell988@gmail.com> > >> > > > > -- > Troy Mitchell >
diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c index 5a084d426e58d09cfedf0809695a96a27627c420..61e2f9757de9f24407f9262657da0d1586ce124e 100644 --- a/drivers/rtc/rtc-pcf8563.c +++ b/drivers/rtc/rtc-pcf8563.c @@ -103,7 +103,7 @@ static int pcf8563_set_alarm_mode(struct pcf8563 *pcf8563, bool on) return regmap_write(pcf8563->regmap, PCF8563_REG_ST2, buf); } -static int pcf8563_get_alarm_mode(struct pcf8563 *pcf8563, unsigned char *en, +static int pcf8563_read_alarm_mode(struct pcf8563 *pcf8563, unsigned char *en, unsigned char *pen) { u32 buf; @@ -127,7 +127,7 @@ static irqreturn_t pcf8563_irq(int irq, void *dev_id) char pending; int err; - err = pcf8563_get_alarm_mode(pcf8563, NULL, &pending); + err = pcf8563_read_alarm_mode(pcf8563, NULL, &pending); if (err) return IRQ_NONE; @@ -262,7 +262,7 @@ static int pcf8563_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *tm) tm->time.tm_mday = bcd2bin(buf[2] & 0x3F); tm->time.tm_wday = bcd2bin(buf[3] & 0x7); - err = pcf8563_get_alarm_mode(pcf8563, &tm->enabled, &tm->pending); + err = pcf8563_read_alarm_mode(pcf8563, &tm->enabled, &tm->pending); if (err < 0) return err; @@ -285,7 +285,7 @@ static int pcf8563_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm) buf[2] = bin2bcd(tm->time.tm_mday); buf[3] = tm->time.tm_wday & 0x07; - err = regmap_bulk_write(pcf8563->regmap, PCF8563_REG_SC, buf, + err = regmap_bulk_write(pcf8563->regmap, PCF8563_REG_AMN, buf, sizeof(buf)); if (err) return err;
Fix wrong register and align `pcf8563_get_alarm_mode` with the naming convention used in ops. Signed-off-by: Troy Mitchell <troymitchell988@gmail.com> --- Since this patch[1], the set_alarm function has been setting an wrong register. Link: https://lore.kernel.org/all/20241010084949.3351182-3-iwamatsu@nigauri.org/ [1] --- drivers/rtc/rtc-pcf8563.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) --- base-commit: 8560697b23dc2f405cb463af2b17256a9888129d change-id: 20250419-pcf8563-fix-alarm-5e787f095861 Best regards,