diff mbox series

[1/2] rtc: rx8025: implement RX-8035 support

Message ID 20210707071616.28976-2-matt@traverse.com.au
State New
Headers show
Series Implement EPSON RX-8035 support | expand

Commit Message

Mathew McBride July 7, 2021, 7:16 a.m. UTC
The RX-8035 is a newer RTC from EPSON that is very
similar to the RX-8025.

The key difference is in the oscillation stop (XSTP)
bit which is inverted on the RX-8035.

Signed-off-by: Mathew McBride <matt@traverse.com.au>

---
 drivers/rtc/rtc-rx8025.c | 59 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 53 insertions(+), 6 deletions(-)

-- 
2.30.1

Comments

Nobuhiro Iwamatsu July 7, 2021, 9:38 p.m. UTC | #1
Hi,

2021年7月7日(水) 16:17 Mathew McBride <matt@traverse.com.au>:
>

> The RX-8035 is a newer RTC from EPSON that is very

> similar to the RX-8025.

>

> The key difference is in the oscillation stop (XSTP)

> bit which is inverted on the RX-8035.

>

> Signed-off-by: Mathew McBride <matt@traverse.com.au>

> ---

>  drivers/rtc/rtc-rx8025.c | 59 ++++++++++++++++++++++++++++++++++++----

>  1 file changed, 53 insertions(+), 6 deletions(-)

>

> diff --git a/drivers/rtc/rtc-rx8025.c b/drivers/rtc/rtc-rx8025.c

> index c914091819ba..1a33ec402f4a 100644

> --- a/drivers/rtc/rtc-rx8025.c

> +++ b/drivers/rtc/rtc-rx8025.c

> @@ -60,14 +60,24 @@

>  #define RX8025_ADJ_DATA_MAX    62

>  #define RX8025_ADJ_DATA_MIN    -62

>

> +enum rx_model {

> +       model_rx_unknown,

> +       model_rx_8025,

> +       model_rx_8035,

> +       model_last

> +};

> +

>  static const struct d rx8025_id[] = {

> -       { "rx8025", 0 },

> +       { "rx8025", model_rx_8025 },

> +       { "rx8035", model_rx_8035 },

>         { }

>  };

> +

>  MODULE_DEVICE_TABLE(i2c, rx8025_id);

>

>  struct rx8025_data {

>         struct rtc_device *rtc;

> +       enum rx_model type;


I think 'model' is easier to understand than 'type'.

>         u8 ctrl1;

>  };

>

> @@ -100,10 +110,26 @@ static s32 rx8025_write_regs(const struct i2c_client *client,

>                                               length, values);

>  }

>

> +static int rx8025_is_osc_stopped(enum rx_model model, int ctrl2)

> +{

> +       int xstp = ctrl2 & RX8025_BIT_CTRL2_XST;

> +       /* XSTP bit has different polarity on RX-8025 vs RX-8035.

> +        * RX-8025: 0 == oscillator stopped

> +        * RX-8035: 1 == oscillator stopped

> +        */

> +

> +       if (model == model_rx_8025)

> +               xstp = !xstp;

> +

> +       return xstp;

> +}

> +

>  static int rx8025_check_validity(struct device *dev)

>  {

>         struct i2c_client *client = to_i2c_client(dev);

> +       struct rx8025_data *drvdata = dev_get_drvdata(dev);

>         int ctrl2;

> +       int xstp;

>

>         ctrl2 = rx8025_read_reg(client, RX8025_REG_CTRL2);

>         if (ctrl2 < 0)

> @@ -117,7 +143,8 @@ static int rx8025_check_validity(struct device *dev)

>                 return -EINVAL;

>         }

>

> -       if (!(ctrl2 & RX8025_BIT_CTRL2_XST)) {

> +       xstp = rx8025_is_osc_stopped(drvdata->type, ctrl2);

> +       if (xstp) {

>                 dev_warn(dev, "crystal stopped, date is invalid\n");

>                 return -EINVAL;

>         }

> @@ -125,7 +152,7 @@ static int rx8025_check_validity(struct device *dev)

>         return 0;

>  }

>

> -static int rx8025_reset_validity(struct i2c_client *client)

> +static int rx8025_reset_validity(enum rx_model model, struct i2c_client *client)


We can get the struct rx8025_data by using i2c_get_clientdata().
Therefore, I think that it can be updated without increasing the arguments.

```
struct rx8025_data *rx8025 = i2c_get_clientdata(client);

if (rx8025->type == model_rx_8025)
```

>  {

>         int ctrl2 = rx8025_read_reg(client, RX8025_REG_CTRL2);

>

> @@ -134,8 +161,13 @@ static int rx8025_reset_validity(struct i2c_client *client)

>

>         ctrl2 &= ~(RX8025_BIT_CTRL2_PON | RX8025_BIT_CTRL2_VDET);

>

> +       if (model == model_rx_8025)

> +               ctrl2 |= RX8025_BIT_CTRL2_XST;

> +       else

> +               ctrl2 &= ~(RX8025_BIT_CTRL2_XST);

> +

>         return rx8025_write_reg(client, RX8025_REG_CTRL2,

> -                               ctrl2 | RX8025_BIT_CTRL2_XST);

> +                               ctrl2);

>  }

>

>  static irqreturn_t rx8025_handle_irq(int irq, void *dev_id)

> @@ -149,7 +181,7 @@ static irqreturn_t (int irq, void *dev_id)

>         if (status < 0)

>                 goto out;

>

> -       if (!(status & RX8025_BIT_CTRL2_XST))

> +       if (rx8025_is_osc_stopped(rx8025->type, status))


In rx8025_check_validity(), the return value is put in xstp and confirmed.
I thought it would be better to unify to either one.

>                 dev_warn(&client->dev, "Oscillation stop was detected,"

>                          "you may have to readjust the clock\n");

>

> @@ -241,7 +273,7 @@ static int rx8025_set_time(struct device *dev, struct rtc_time *dt)

>         if (ret < 0)

>                 return ret;

>

> -       return rx8025_reset_validity(client);

> +       return rx8025_reset_validity(rx8025->type, client);

>  }

>

>  static int rx8025_init_client(struct i2c_client *client)

> @@ -519,6 +551,21 @@ static int rx8025_probe(struct i2c_client *client,

>

>         i2c_set_clientdata(client, rx8025);

>

> +       if (id) {

> +               rx8025->type = id->driver_data;

> +               switch (rx8025->type) {

> +               case model_rx_8025:

> +                       dev_info(&client->dev, "Type RX-8025");

> +               break;


Please fix indent.

> +               case model_rx_8035:

> +                       dev_info(&client->dev, "Type RX-8035");

> +                       break;

> +               default:

> +                       dev_warn(&client->dev, "Unknown type: %d\n", rx8025->type);

> +               break;


ditto.

> +               }

> +       }

> +

>         err = rx8025_init_client(client);

>         if (err)

>                 return err;

> --

> 2.30.1

>


Best regards,
  Nobuhiro

-- 
Nobuhiro Iwamatsu
   iwamatsu at {nigauri.org / debian.org / kernel.org}
   GPG ID: 40AD1FA6
diff mbox series

Patch

diff --git a/drivers/rtc/rtc-rx8025.c b/drivers/rtc/rtc-rx8025.c
index c914091819ba..1a33ec402f4a 100644
--- a/drivers/rtc/rtc-rx8025.c
+++ b/drivers/rtc/rtc-rx8025.c
@@ -60,14 +60,24 @@ 
 #define RX8025_ADJ_DATA_MAX	62
 #define RX8025_ADJ_DATA_MIN	-62
 
+enum rx_model {
+	model_rx_unknown,
+	model_rx_8025,
+	model_rx_8035,
+	model_last
+};
+
 static const struct i2c_device_id rx8025_id[] = {
-	{ "rx8025", 0 },
+	{ "rx8025", model_rx_8025 },
+	{ "rx8035", model_rx_8035 },
 	{ }
 };
+
 MODULE_DEVICE_TABLE(i2c, rx8025_id);
 
 struct rx8025_data {
 	struct rtc_device *rtc;
+	enum rx_model type;
 	u8 ctrl1;
 };
 
@@ -100,10 +110,26 @@  static s32 rx8025_write_regs(const struct i2c_client *client,
 					      length, values);
 }
 
+static int rx8025_is_osc_stopped(enum rx_model model, int ctrl2)
+{
+	int xstp = ctrl2 & RX8025_BIT_CTRL2_XST;
+	/* XSTP bit has different polarity on RX-8025 vs RX-8035.
+	 * RX-8025: 0 == oscillator stopped
+	 * RX-8035: 1 == oscillator stopped
+	 */
+
+	if (model == model_rx_8025)
+		xstp = !xstp;
+
+	return xstp;
+}
+
 static int rx8025_check_validity(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct rx8025_data *drvdata = dev_get_drvdata(dev);
 	int ctrl2;
+	int xstp;
 
 	ctrl2 = rx8025_read_reg(client, RX8025_REG_CTRL2);
 	if (ctrl2 < 0)
@@ -117,7 +143,8 @@  static int rx8025_check_validity(struct device *dev)
 		return -EINVAL;
 	}
 
-	if (!(ctrl2 & RX8025_BIT_CTRL2_XST)) {
+	xstp = rx8025_is_osc_stopped(drvdata->type, ctrl2);
+	if (xstp) {
 		dev_warn(dev, "crystal stopped, date is invalid\n");
 		return -EINVAL;
 	}
@@ -125,7 +152,7 @@  static int rx8025_check_validity(struct device *dev)
 	return 0;
 }
 
-static int rx8025_reset_validity(struct i2c_client *client)
+static int rx8025_reset_validity(enum rx_model model, struct i2c_client *client)
 {
 	int ctrl2 = rx8025_read_reg(client, RX8025_REG_CTRL2);
 
@@ -134,8 +161,13 @@  static int rx8025_reset_validity(struct i2c_client *client)
 
 	ctrl2 &= ~(RX8025_BIT_CTRL2_PON | RX8025_BIT_CTRL2_VDET);
 
+	if (model == model_rx_8025)
+		ctrl2 |= RX8025_BIT_CTRL2_XST;
+	else
+		ctrl2 &= ~(RX8025_BIT_CTRL2_XST);
+
 	return rx8025_write_reg(client, RX8025_REG_CTRL2,
-				ctrl2 | RX8025_BIT_CTRL2_XST);
+				ctrl2);
 }
 
 static irqreturn_t rx8025_handle_irq(int irq, void *dev_id)
@@ -149,7 +181,7 @@  static irqreturn_t rx8025_handle_irq(int irq, void *dev_id)
 	if (status < 0)
 		goto out;
 
-	if (!(status & RX8025_BIT_CTRL2_XST))
+	if (rx8025_is_osc_stopped(rx8025->type, status))
 		dev_warn(&client->dev, "Oscillation stop was detected,"
 			 "you may have to readjust the clock\n");
 
@@ -241,7 +273,7 @@  static int rx8025_set_time(struct device *dev, struct rtc_time *dt)
 	if (ret < 0)
 		return ret;
 
-	return rx8025_reset_validity(client);
+	return rx8025_reset_validity(rx8025->type, client);
 }
 
 static int rx8025_init_client(struct i2c_client *client)
@@ -519,6 +551,21 @@  static int rx8025_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, rx8025);
 
+	if (id) {
+		rx8025->type = id->driver_data;
+		switch (rx8025->type) {
+		case model_rx_8025:
+			dev_info(&client->dev, "Type RX-8025");
+		break;
+		case model_rx_8035:
+			dev_info(&client->dev, "Type RX-8035");
+			break;
+		default:
+			dev_warn(&client->dev, "Unknown type: %d\n", rx8025->type);
+		break;
+		}
+	}
+
 	err = rx8025_init_client(client);
 	if (err)
 		return err;