Re: [PATCH v3 0/2] rtc: pcf2127: make battery switch-over configurable

[Philipp Rosenberger <p.rosenberger@kunbus.com>]

Hi,

On 14.11.24 09:51, Alexandre Belloni wrote:
> Hello,
> 
> This has been discussed multiple times in the past, we can't have a DT
> property for this as we need to be able to change it at runtime. There
> is already a userspace interface to do this.
> 
> Below is my current patch for this that has been tested on pcf2127. I
> didn't send it yet because we are losing information when switching from
> standard or direct mode to disabled because when BSM is disabled, there
> is no configuration where battery low detection function is enabled so
> going from disabled to standard or direct will keep BLD disabled.

Sorry for my late response. I've tested the patch on our hardware. and 
this would solve our problem. I understand, that it is not ideal, that 
information is lost when switching modes. But I can't figure a way how 
to avoid this.

Thank you and best regards,
Philipp

> 
> 8<--------------------------------------------------------------------
> 
>  From 7db70b33c3939a0ebe147c32f406b34a2f5f1be8 Mon Sep 17 00:00:00 2001
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Date: Sat, 24 Feb 2024 19:58:20 +0100
> Subject: [PATCH] rtc: pcf2127: add BSM support
> 
> The pcf2127 encodes BSM, BLD and power fail detection in the same set of
> bits so it is necessary to do some calculation when changing BSM to keep
> the rest of the configuration as-is. However, when BSM is disabled, there
> is no configuration with BLD enabled so this will be lost when coming back
> to a mode with BSM enabled.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>   drivers/rtc/rtc-pcf2127.c | 81 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 81 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
> index 9c04c4e1a49c..a7f73192d53d 100644
> --- a/drivers/rtc/rtc-pcf2127.c
> +++ b/drivers/rtc/rtc-pcf2127.c
> @@ -48,6 +48,7 @@
>   #define PCF2127_BIT_CTRL3_BLF                  BIT(2)
>   #define PCF2127_BIT_CTRL3_BF                   BIT(3)
>   #define PCF2127_BIT_CTRL3_BTSE                 BIT(4)
> +#define PCF2127_CTRL3_PM                       GENMASK(7, 5)
>   /* Time and date registers */
>   #define PCF2127_REG_TIME_BASE          0x03
>   #define PCF2127_BIT_SC_OSF                     BIT(7)
> @@ -331,6 +332,84 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm)
>          return 0;
>   }
> 
> +static int pcf2127_param_get(struct device *dev, struct rtc_param *param)
> +{
> +       struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
> +       u32 value;
> +       int ret;
> +
> +       switch (param->param) {
> +       case RTC_PARAM_BACKUP_SWITCH_MODE:
> +               ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL3, &value);
> +               if (ret < 0)
> +                       return ret;
> +
> +               value = FIELD_GET(PCF2127_CTRL3_PM, value);
> +
> +               if (value < 0x3)
> +                       param->uvalue = RTC_BSM_LEVEL;
> +               else if (value < 0x6)
> +                       param->uvalue = RTC_BSM_DIRECT;
> +               else
> +                       param->uvalue = RTC_BSM_DISABLED;
> +
> +               break;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static int pcf2127_param_set(struct device *dev, struct rtc_param *param)
> +{
> +       struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
> +       u8 mode = 0;
> +       u32 value;
> +       int ret;
> +
> +       switch (param->param) {
> +       case RTC_PARAM_BACKUP_SWITCH_MODE:
> +               ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL3, &value);
> +               if (ret < 0)
> +                       return ret;
> +
> +               value = FIELD_GET(PCF2127_CTRL3_PM, value);
> +
> +               if (value > 5)
> +                       value -= 5;
> +               else if (value > 2)
> +                       value -= 3;
> +
> +               switch (param->uvalue) {
> +               case RTC_BSM_LEVEL:
> +                       break;
> +               case RTC_BSM_DIRECT:
> +                       mode = 3;
> +                       break;
> +               case RTC_BSM_DISABLED:
> +                       if (value == 0)
> +                               value = 1;
> +                       mode = 5;
> +                       break;
> +               default:
> +                       return -EINVAL;
> +               }
> +
> +               return regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL3,
> +                                         PCF2127_CTRL3_PM,
> +                                         FIELD_PREP(PCF2127_CTRL3_PM, mode + value));
> +
> +               break;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
>   static int pcf2127_rtc_ioctl(struct device *dev,
>                                  unsigned int cmd, unsigned long arg)
>   {
> @@ -741,6 +820,8 @@ static const struct rtc_class_ops pcf2127_rtc_ops = {
>          .read_alarm       = pcf2127_rtc_read_alarm,
>          .set_alarm        = pcf2127_rtc_set_alarm,
>          .alarm_irq_enable = pcf2127_rtc_alarm_irq_enable,
> +       .param_get        = pcf2127_param_get,
> +       .param_set        = pcf2127_param_set,
>   };
> 
>   /* sysfs interface */
> --
> 2.47.0
> 
> 
> 
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com