RE: [EXT] Re: [PATCH v3 1/3] drivers: rtc: add pcf2131 rtc driver

2024-03-26 Thread Joy Zou
> -Original Message-
> From: Fabio Estevam 
> Sent: 2024年3月27日 6:10
> To: Joy Zou 
> Cc: Peng Fan ; Ye Li ; Jacky Bai
> ; sba...@denx.de; s...@chromium.org;
> sap...@gmail.com; judge.pack...@gmail.com; dl-uboot-imx
> ; u-boot@lists.denx.de
> Subject: [EXT] Re: [PATCH v3 1/3] drivers: rtc: add pcf2131 rtc driver
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> On Tue, Mar 26, 2024 at 12:30 AM Joy Zou  wrote:
> 
> > +bool is_pcf2131_type(struct udevice *dev)
> 
> static bool
Will add static key word!
> 
> >  static int pcf2127_rtc_read(struct udevice *dev, uint offset, u8
> > *buffer, uint len)  {
> > struct dm_i2c_chip *chip = dev_get_parent_plat(dev); @@
> -43,10
> > +75,64 @@ static int pcf2127_rtc_read(struct udevice *dev, uint offset, u8
> *buffer, uint l
> > return dm_i2c_xfer(dev, , 1);  }
> >
> > +static int pcf2131_rtc_lock(struct udevice *dev) {
> > +   int ret = 0;
> 
> No need to initialize ret with 0.
Will remove initialization 0.
> 
> > +static int pcf2131_rtc_unlock(struct udevice *dev) {
> > +   int ret = 0;
> 
> Ditto.
> 
> >  static int pcf2127_rtc_write(struct udevice *dev, uint offset,
> >  const u8 *buffer, uint len)  {
> > -   return dm_i2c_write(dev, offset, buffer, len);
> > +   int ret = 0;
> 
> Ditto.
Will remove initialization 0.
Thanks for your comments!
BR
Joy Zou


Re: [PATCH v3 1/3] drivers: rtc: add pcf2131 rtc driver

2024-03-26 Thread Fabio Estevam
On Tue, Mar 26, 2024 at 12:30 AM Joy Zou  wrote:

> +bool is_pcf2131_type(struct udevice *dev)

static bool

>  static int pcf2127_rtc_read(struct udevice *dev, uint offset, u8 *buffer, 
> uint len)
>  {
> struct dm_i2c_chip *chip = dev_get_parent_plat(dev);
> @@ -43,10 +75,64 @@ static int pcf2127_rtc_read(struct udevice *dev, uint 
> offset, u8 *buffer, uint l
> return dm_i2c_xfer(dev, , 1);
>  }
>
> +static int pcf2131_rtc_lock(struct udevice *dev)
> +{
> +   int ret = 0;

No need to initialize ret with 0.

> +static int pcf2131_rtc_unlock(struct udevice *dev)
> +{
> +   int ret = 0;

Ditto.

>  static int pcf2127_rtc_write(struct udevice *dev, uint offset,
>  const u8 *buffer, uint len)
>  {
> -   return dm_i2c_write(dev, offset, buffer, len);
> +   int ret = 0;

Ditto.


[PATCH v3 1/3] drivers: rtc: add pcf2131 rtc driver

2024-03-26 Thread Joy Zou
Adding support for pcf2131 RTC chip.

The pcf2131 is similar to the pcf2127. The driver support rtc register
read/write by using rtc cmd and rtc date set/get by using date cmd.

The pcf2131 is special when write access to time registers. it requires
setting the STOP and CPR bits. STOP bit needs to be cleared after time
registers are updated.

Signed-off-by: Joy Zou 
---
Changes in v3:
1.merge pcf2131 into pcf2127 in order to keep same with kernel.

Changes in v2:
1. delete the unnecessary initialization.
2. retrun directly function insteand of redundancy return ret.
3. delete the unnecessary comment line.
---
 drivers/rtc/pcf2127.c | 144 ++
 1 file changed, 131 insertions(+), 13 deletions(-)

diff --git a/drivers/rtc/pcf2127.c b/drivers/rtc/pcf2127.c
index 2f3fafb496..58ab5a8601 100644
--- a/drivers/rtc/pcf2127.c
+++ b/drivers/rtc/pcf2127.c
@@ -23,6 +23,38 @@
 #define PCF2127_REG_MO 0x08
 #define PCF2127_REG_YR 0x09
 
+#define PCF2131_REG_CTRL1   0x00
+#define PCF2131_BIT_CTRL1_STOP  BIT(5)
+#define PCF2131_REG_SR_RESET0x05
+#define PCF2131_SR_VAL_Clr_Pres 0xa4
+#define PCF2131_REG_SC  0x07
+#define PCF2131_REG_MN  0x08
+#define PCF2131_REG_HR  0x09
+#define PCF2131_REG_DM  0x0a
+#define PCF2131_REG_DW  0x0b
+#define PCF2131_REG_MO  0x0c
+#define PCF2131_REG_YR  0x0d
+
+enum {
+   NXP_CHIP_TYPE_PCF2127 = 0,
+   NXP_CHIP_TYPE_PCF2129,
+   NXP_CHIP_TYPE_PCA2129,
+   NXP_CHIP_TYPE_PCF2131,
+   NXP_CHIP_TYPE_AMOUNT
+};
+
+bool is_pcf2131_type(struct udevice *dev)
+{
+   int type;
+
+   type = dev_get_driver_data(dev);
+
+   if (type == NXP_CHIP_TYPE_PCF2131)
+   return true;
+   else
+   return false;
+}
+
 static int pcf2127_rtc_read(struct udevice *dev, uint offset, u8 *buffer, uint 
len)
 {
struct dm_i2c_chip *chip = dev_get_parent_plat(dev);
@@ -43,10 +75,64 @@ static int pcf2127_rtc_read(struct udevice *dev, uint 
offset, u8 *buffer, uint l
return dm_i2c_xfer(dev, , 1);
 }
 
+static int pcf2131_rtc_lock(struct udevice *dev)
+{
+   int ret = 0;
+   uchar buf[6] = { PCF2131_REG_CTRL1 };
+
+   ret = pcf2127_rtc_read(dev, PCF2131_REG_CTRL1, buf, sizeof(buf));
+   if (ret < 0)
+   return ret;
+
+   buf[PCF2131_REG_CTRL1] |= PCF2131_BIT_CTRL1_STOP;
+   ret = dm_i2c_write(dev, PCF2131_REG_CTRL1, [PCF2131_REG_CTRL1], 1);
+   if (ret < 0)
+   return ret;
+
+   buf[PCF2131_REG_SR_RESET] = PCF2131_SR_VAL_Clr_Pres;
+
+   return dm_i2c_write(dev, PCF2131_REG_SR_RESET, 
[PCF2131_REG_SR_RESET], 1);
+}
+
+static int pcf2131_rtc_unlock(struct udevice *dev)
+{
+   int ret = 0;
+   uchar buf[6] = { PCF2131_REG_CTRL1 };
+
+   ret = pcf2127_rtc_read(dev, PCF2131_REG_CTRL1, buf, sizeof(buf));
+   if (ret < 0)
+   return ret;
+
+   buf[PCF2131_REG_CTRL1] &= ~PCF2131_BIT_CTRL1_STOP;
+   return dm_i2c_write(dev, PCF2131_REG_CTRL1, [PCF2131_REG_CTRL1], 1);
+}
+
 static int pcf2127_rtc_write(struct udevice *dev, uint offset,
 const u8 *buffer, uint len)
 {
-   return dm_i2c_write(dev, offset, buffer, len);
+   int ret = 0;
+   bool flag;
+
+   flag = is_pcf2131_type(dev);
+   if (flag) {
+   ret = pcf2131_rtc_lock(dev);
+   if (ret < 0)
+   return ret;
+   }
+
+   ret = dm_i2c_write(dev, offset, buffer, len);
+   if (ret < 0) {
+   if (flag)
+   pcf2131_rtc_unlock(dev);
+   return ret;
+   }
+
+   if (flag) {
+   ret = pcf2131_rtc_unlock(dev);
+   if (ret < 0)
+   return ret;
+   }
+   return ret;
 }
 
 static int pcf2127_rtc_set(struct udevice *dev, const struct rtc_time *tm)
@@ -68,7 +154,10 @@ static int pcf2127_rtc_set(struct udevice *dev, const 
struct rtc_time *tm)
buf[i++] = bin2bcd(tm->tm_year % 100);
 
/* write register's data */
-   ret = dm_i2c_write(dev, PCF2127_REG_SC, buf, i);
+   if (is_pcf2131_type(dev))
+   ret = pcf2127_rtc_write(dev, PCF2131_REG_SC, buf, i);
+   else
+   ret = pcf2127_rtc_write(dev, PCF2127_REG_SC, buf, i);
 
return ret;
 }
@@ -76,7 +165,8 @@ static int pcf2127_rtc_set(struct udevice *dev, const struct 
rtc_time *tm)
 static int pcf2127_rtc_get(struct udevice *dev, struct rtc_time *tm)
 {
int ret = 0;
-   uchar buf[10] = { PCF2127_REG_CTRL1 };
+   bool flag;
+   uchar buf[14] = { PCF2127_REG_CTRL1 };
 
ret = pcf2127_rtc_read(dev, PCF2127_REG_CTRL1, buf, sizeof(buf));
if (ret < 0)
@@ -85,15 +175,28 @@ static int pcf2127_rtc_get(struct udevice *dev, struct 
rtc_time *tm)
if (buf[PCF2127_REG_CTRL3] & 0x04)