[1/2] dt-bindings: rtc: pcf85263/pcf85363: add some properties

[1/2] dt-bindings: rtc: pcf85263/pcf85363: add some properties

[Biwen Li]

Add some properties for pcf85263/pcf85363 as follows:
  - interrupt-output-pin: string type
  - quartz-load-capacitance: integer type
  - quartz-drive-strength: integer type
  - quartz-low-jitter: bool type
  - wakeup-source: bool type

Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
 .../devicetree/bindings/rtc/pcf85363.txt      | 31 +++++++++++++++++++
 include/dt-bindings/rtc/pcf85363.h            | 15 +++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 include/dt-bindings/rtc/pcf85363.h

diff --git a/Documentation/devicetree/bindings/rtc/pcf85363.txt b/Documentation/devicetree/bindings/rtc/pcf85363.txt
index 94adc1cf93d9..d83359990bd7 100644
--- a/Documentation/devicetree/bindings/rtc/pcf85363.txt
+++ b/Documentation/devicetree/bindings/rtc/pcf85363.txt
@@ -8,10 +8,41 @@ Required properties:
 Optional properties:
 - interrupts: IRQ line for the RTC (not implemented).
 
+- interrupt-output-pin: The interrupt output pin must be
+  "NONE", "INTA" or "INTB", default value is "NONE"
+
+- quartz-load-capacitance: The internal capacitor to select for the quartz:
+	PCF85263_QUARTZCAP_7pF		[0]
+	PCF85263_QUARTZCAP_6pF		[1]
+	PCF85263_QUARTZCAP_12p5pF	[2] DEFAULT
+
+- quartz-drive-strength: Drive strength for the quartz:
+	PCF85263_QUARTZDRIVE_NORMAL	[0] DEFAULT
+	PCF85263_QUARTZDRIVE_LOW	[1]
+	PCF85263_QUARTZDRIVE_HIGH	[2]
+
+- quartz-low-jitter: Boolean property, if present enables low jitter mode
+  which reduces jitter at the cost of increased power consumption.
+
+- wakeup-source: Boolean property, mark the chip as a wakeup source,
+  independently of the availability of an IRQ line connected to the SoC.
+  This is useful if the IRQ line is connected to a PMIC or other circuit
+  that can power up the device rather than to a normal SOC interrupt.
+
 Example:
 
 pcf85363: pcf85363@51 {
 	compatible = "nxp,pcf85363";
 	reg = <0x51>;
+
+	interrupt-parent = <&gpio1>;
+	interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
+
+	#include <dt-bindings/rtc/pcf85363.h>
+	wakeup-source;
+	interrupt-output-pin = "INTA";
+	quartz-load-capacitance = <PCF85363_QUARTZCAP_12p5pF>;
+	quartz-drive-strength = <PCF85363_QUARTZDRIVE_LOW>;
+	quartz-low-jitter;
 };
 
diff --git a/include/dt-bindings/rtc/pcf85363.h b/include/dt-bindings/rtc/pcf85363.h
new file mode 100644
index 000000000000..2c06c28eb5ff
--- /dev/null
+++ b/include/dt-bindings/rtc/pcf85363.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _DT_BINDINGS_RTC_PCF85363_H
+#define _DT_BINDINGS_RTC_PCF85363_H
+
+/* Quartz capacitance */
+#define PCF85363_QUARTZCAP_7pF		0
+#define PCF85363_QUARTZCAP_6pF		1
+#define PCF85363_QUARTZCAP_12p5pF	2
+
+/* Quartz drive strength */
+#define PCF85363_QUARTZDRIVE_NORMAL	0
+#define PCF85363_QUARTZDRIVE_LOW	1
+#define PCF85363_QUARTZDRIVE_HIGH	2
+
+#endif /* _DT_BINDINGS_RTC_PCF85363_H */
-- 
2.17.1

[2/2] rtc: pcf85263/pcf85363: support PM, wakeup device, improve performance

[Biwen Li]

Add some features as follow:
    - Set quartz oscillator load capacitance by DT
      (generate more accuracy frequency)
    - Set quartz oscillator drive control by DT
      (reduce/increase the current consumption)
    - Set low jitter mode by DT
      (improve jitter performance)
    - Set wakeup source by DT
      (wakeup device from suspend
    - Select interrupt output pin by DT
      (INTA/TS(INTB)/NONE)
    - Add power management
    - Add ioctl to check rtc status
      (check whether oscillator of pcf85263/pcf85363 is stopped)

Datasheet url:
    - https://www.nxp.com/docs/en/data-sheet/PCF85263A.pdf
    - https://www.nxp.com/docs/en/data-sheet/PCF85363A.pdf

Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
 drivers/rtc/rtc-pcf85363.c | 287 +++++++++++++++++++++++++++++++++++--
 1 file changed, 274 insertions(+), 13 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index a075e77617dc..f8916497efd4 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -18,6 +18,17 @@
 #include <linux/of_device.h>
 #include <linux/regmap.h>
 
+/* Quartz capacitance */
+#define PCF85363_QUARTZCAP_7pF		0
+#define PCF85363_QUARTZCAP_6pF		1
+#define PCF85363_QUARTZCAP_12p5pF	2
+
+/* Quartz drive strength */
+#define PCF85363_QUARTZDRIVE_NORMAL	0
+#define PCF85363_QUARTZDRIVE_LOW	1
+#define PCF85363_QUARTZDRIVE_HIGH	2
+
+
 /*
  * Date/Time registers
  */
@@ -96,10 +107,20 @@
 #define FLAGS_PIF	BIT(7)
 
 #define PIN_IO_INTAPM	GENMASK(1, 0)
-#define PIN_IO_INTA_CLK	0
-#define PIN_IO_INTA_BAT	1
-#define PIN_IO_INTA_OUT	2
-#define PIN_IO_INTA_HIZ	3
+#define PIN_IO_INTAPM_SHIFT	0
+#define PIN_IO_INTA_CLK	(0 << PIN_IO_INTAPM_SHIFT)
+#define PIN_IO_INTA_BAT	(1 << PIN_IO_INTAPM_SHIFT)
+#define PIN_IO_INTA_OUT	(2 << PIN_IO_INTAPM_SHIFT)
+#define PIN_IO_INTA_HIZ	(3 << PIN_IO_INTAPM_SHIFT)
+
+#define PIN_IO_TSPM	 GENMASK(3, 2)
+#define PIN_IO_TSPM_SHIFT	2
+#define PIN_IO_TS_DISABLE	(0x0 << PIN_IO_TSPM_SHIFT)
+#define PIN_IO_TS_INTB_OUT	(0x1 << PIN_IO_TSPM_SHIFT)
+#define PIN_IO_TS_CLK_OUT	(0x2 << PIN_IO_TSPM_SHIFT)
+#define PIN_IO_TS_IN	(0x3 << PIN_IO_TSPM_SHIFT)
+
+#define PIN_IO_CLKPM	BIT(7) /* 0 = enable CLK pin,1 = disable CLK pin */
 
 #define STOP_EN_STOP	BIT(0)
 
@@ -107,9 +128,35 @@
 
 #define NVRAM_SIZE	0x40
 
+#define DT_SECS_OS BIT(7)
+
+#define CTRL_OSCILLATOR_CL_MASK	GENMASK(1, 0)
+#define CTRL_OSCILLATOR_CL_SHIFT	0
+#define CTRL_OSCILLATOR_OSCD_MASK	GENMASK(3, 2)
+#define CTRL_OSCILLATOR_OSCD_SHIFT	2
+#define CTRL_OSCILLATOR_LOWJ		BIT(4)
+
+#define CTRL_FUNCTION_COF_OFF	0x7 /* No clock output */
+
+enum pcf85363_irqpin {
+	IRQPIN_NONE,
+	IRQPIN_INTA,
+	IRQPIN_INTB
+};
+
+static const char *const pcf85363_irqpin_names[] = {
+	[IRQPIN_NONE] = "NONE",
+	[IRQPIN_INTA] = "INTA",
+	[IRQPIN_INTB] = "INTB"
+};
+
+
 struct pcf85363 {
+	struct device *dev;
 	struct rtc_device	*rtc;
 	struct regmap		*regmap;
+	enum pcf85363_irqpin irq_pin;
+	int irq;
 };
 
 struct pcf85x63_config {
@@ -205,14 +252,29 @@ static int _pcf85363_rtc_alarm_irq_enable(struct pcf85363 *pcf85363, unsigned
 {
 	unsigned int alarm_flags = ALRM_SEC_A1E | ALRM_MIN_A1E | ALRM_HR_A1E |
 				   ALRM_DAY_A1E | ALRM_MON_A1E;
-	int ret;
+	int ret, reg;
 
 	ret = regmap_update_bits(pcf85363->regmap, DT_ALARM_EN, alarm_flags,
 				 enabled ? alarm_flags : 0);
 	if (ret)
 		return ret;
 
-	ret = regmap_update_bits(pcf85363->regmap, CTRL_INTA_EN,
+	switch (pcf85363->irq_pin) {
+	case IRQPIN_NONE:
+		return 0;
+
+	case IRQPIN_INTA:
+		reg = CTRL_INTA_EN;
+		break;
+
+	case IRQPIN_INTB:
+		reg = CTRL_INTB_EN;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+	ret = regmap_update_bits(pcf85363->regmap, reg,
 				 INT_A1IE, enabled ? INT_A1IE : 0);
 
 	if (ret || enabled)
@@ -277,12 +339,55 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
+static int pcf85363_osc_is_stopped(struct pcf85363 *pcf85363)
+{
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(pcf85363->regmap, DT_SECS, &regval);
+	if (ret)
+		return ret;
+
+	ret = regval & DT_SECS_OS ? 1 : 0;
+	if (ret)
+		dev_warn(pcf85363->dev, "Oscillator stop detected, date/time is not reliable.\n");
+
+	return ret;
+}
+
+static int pcf85363_ioctl(struct device *dev,
+			  unsigned int cmd, unsigned long arg)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+	int ret;
+
+	switch (cmd) {
+	case RTC_VL_READ:
+		ret = pcf85363_osc_is_stopped(pcf85363);
+		if (ret < 0)
+			return ret;
+
+		if (copy_to_user((void __user *)arg, &ret, sizeof(int)))
+			return -EFAULT;
+		return 0;
+
+	case RTC_VL_CLR:
+		return regmap_update_bits(pcf85363->regmap,
+					  DT_SECS,
+					  DT_SECS_OS, 0);
+	default:
+		return -ENOIOCTLCMD;
+	}
+}
+
 static const struct rtc_class_ops rtc_ops = {
+	.ioctl = pcf85363_ioctl,
 	.read_time	= pcf85363_rtc_read_time,
 	.set_time	= pcf85363_rtc_set_time,
 };
 
 static const struct rtc_class_ops rtc_ops_alarm = {
+	.ioctl = pcf85363_ioctl,
 	.read_time	= pcf85363_rtc_read_time,
 	.set_time	= pcf85363_rtc_set_time,
 	.read_alarm	= pcf85363_rtc_read_alarm,
@@ -350,6 +455,110 @@ static const struct pcf85x63_config pcf_85363_config = {
 	.num_nvram = 2
 };
 
+/*
+ * On some boards the interrupt line may not be wired to the CPU but only to
+ * a power supply circuit.
+ * In that case no interrupt will be specified in the device tree but the
+ * wakeup-source DT property may be used to enable wakeup programming in
+ * sysfs
+ */
+static bool pcf85363_can_wakeup_machine(struct pcf85363 *pcf85363)
+{
+	return pcf85363->irq ||
+		of_property_read_bool(pcf85363->dev->of_node, "wakeup-source");
+}
+
+static int pcf85363_init_hw(struct pcf85363 *pcf85363)
+{
+	struct device_node *np = pcf85363->dev->of_node;
+	unsigned int regval;
+	u32 propval;
+	int ret;
+
+	/* Determine if oscilator has been stopped (probably low power) */
+	ret = pcf85363_osc_is_stopped(pcf85363);
+	if (ret < 0) {
+		/* Log here since this is the first hw access on probe */
+		dev_err(pcf85363->dev, "Unable to read register\n");
+
+		return ret;
+	}
+
+	ret = regmap_read(pcf85363->regmap, CTRL_OSCILLATOR, &regval);
+	if (ret)
+		return ret;
+
+	/* Set oscilator register */
+	propval = PCF85363_QUARTZCAP_12p5pF;
+	of_property_read_u32(np, "quartz-load-capacitance", &propval);
+	regval |= ((propval << CTRL_OSCILLATOR_CL_SHIFT)
+		    & CTRL_OSCILLATOR_CL_MASK);
+
+	propval = PCF85363_QUARTZDRIVE_NORMAL;
+	of_property_read_u32(np, "quartz-drive-strength", &propval);
+	regval |= ((propval << CTRL_OSCILLATOR_OSCD_SHIFT)
+		    & CTRL_OSCILLATOR_OSCD_MASK);
+
+	if (of_property_read_bool(np, "quartz-low-jitter"))
+		regval |= CTRL_OSCILLATOR_LOWJ;
+
+	ret = regmap_write(pcf85363->regmap, CTRL_OSCILLATOR, regval);
+	if (ret)
+		return ret;
+
+	/* Set function register
+	 * (100th second disabled
+	 * no periodic interrupt
+	 * real-time clock mode
+	 * RTC stop is controlled by STOP bit only
+	 * no clock output)
+	 */
+	ret = regmap_write(pcf85363->regmap, CTRL_FUNCTION,
+			   CTRL_FUNCTION_COF_OFF);
+	if (ret)
+		return ret;
+
+	/* Set all interrupts to disabled, level mode */
+	ret = regmap_write(pcf85363->regmap, CTRL_INTA_EN,
+			   INT_ILP);
+	if (ret)
+		return ret;
+	ret = regmap_write(pcf85363->regmap, CTRL_INTB_EN,
+			   INT_ILP);
+	if (ret)
+		return ret;
+
+	/* Determine which interrupt pin the board uses */
+	if (pcf85363_can_wakeup_machine(pcf85363)) {
+		if (of_property_match_string(pcf85363->dev->of_node,
+					     "interrupt-output-pin",
+					     "INTB") >= 0)
+			pcf85363->irq_pin = IRQPIN_INTB;
+		else
+			pcf85363->irq_pin = IRQPIN_INTA;
+	} else {
+		pcf85363->irq_pin = IRQPIN_NONE;
+	}
+
+	/* Setup IO pin config register */
+	regval = PIN_IO_CLKPM; /* disable CLK pin*/
+	switch (pcf85363->irq_pin) {
+	case IRQPIN_INTA:
+		regval |= (PIN_IO_INTA_OUT | PIN_IO_TS_DISABLE);
+		break;
+	case IRQPIN_INTB:
+		regval |= (PIN_IO_INTA_HIZ | PIN_IO_TS_INTB_OUT);
+		break;
+	case IRQPIN_NONE:
+		regval |= (PIN_IO_INTA_HIZ | PIN_IO_TS_DISABLE);
+		break;
+	}
+	ret = regmap_write(pcf85363->regmap, CTRL_PIN_IO, regval);
+
+	return ret;
+}
+
+
 static int pcf85363_probe(struct i2c_client *client,
 			  const struct i2c_device_id *id)
 {
@@ -389,9 +598,11 @@ static int pcf85363_probe(struct i2c_client *client,
 		return PTR_ERR(pcf85363->regmap);
 	}
 
+	pcf85363->irq = client->irq;
+	pcf85363->dev = &client->dev;
 	i2c_set_clientdata(client, pcf85363);
 
-	pcf85363->rtc = devm_rtc_allocate_device(&client->dev);
+	pcf85363->rtc = devm_rtc_allocate_device(pcf85363->dev);
 	if (IS_ERR(pcf85363->rtc))
 		return PTR_ERR(pcf85363->rtc);
 
@@ -399,20 +610,28 @@ static int pcf85363_probe(struct i2c_client *client,
 	pcf85363->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
 	pcf85363->rtc->range_max = RTC_TIMESTAMP_END_2099;
 
-	if (client->irq > 0) {
+	ret = pcf85363_init_hw(pcf85363);
+	if (ret)
+		return ret;
+
+	if (pcf85363->irq > 0) {
 		regmap_write(pcf85363->regmap, CTRL_FLAGS, 0);
-		regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
-				   PIN_IO_INTA_OUT, PIN_IO_INTAPM);
-		ret = devm_request_threaded_irq(&client->dev, client->irq,
+		ret = devm_request_threaded_irq(pcf85363->dev, pcf85363->irq,
 						NULL, pcf85363_rtc_handle_irq,
-						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT,
 						"pcf85363", client);
-		if (ret)
+		if (ret) {
 			dev_warn(&client->dev, "unable to request IRQ, alarms disabled\n");
+			pcf85363->irq = 0;
+		}
 		else
 			pcf85363->rtc->ops = &rtc_ops_alarm;
 	}
 
+	if (pcf85363_can_wakeup_machine(pcf85363))
+		device_init_wakeup(pcf85363->dev, true);
+
 	ret = rtc_register_device(pcf85363->rtc);
 
 	for (i = 0; i < config->num_nvram; i++) {
@@ -420,6 +639,10 @@ static int pcf85363_probe(struct i2c_client *client,
 		rtc_nvmem_register(pcf85363->rtc, &nvmem_cfg[i]);
 	}
 
+	/* We cannot support UIE mode if we do not have an IRQ line */
+	if (!pcf85363->irq)
+		pcf85363->rtc->uie_unsupported = 1;
+
 	return ret;
 }
 
@@ -430,12 +653,50 @@ static const struct of_device_id dev_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, dev_ids);
 
+static int pcf85363_remove(struct i2c_client *client)
+{
+	struct pcf85363 *pcf85363 = i2c_get_clientdata(client);
+
+	if (pcf85363_can_wakeup_machine(pcf85363))
+		device_init_wakeup(pcf85363->dev, false);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int pcf85363_suspend(struct device *dev)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+	int ret = 0;
+
+	if (device_may_wakeup(dev))
+		ret = enable_irq_wake(pcf85363->irq);
+
+	return ret;
+}
+
+static int pcf85363_resume(struct device *dev)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+	int ret = 0;
+
+	if (device_may_wakeup(dev))
+		ret = disable_irq_wake(pcf85363->irq);
+
+	return ret;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(pcf85363_pm_ops, pcf85363_suspend,  pcf85363_resume);
+
 static struct i2c_driver pcf85363_driver = {
 	.driver	= {
 		.name	= "pcf85363",
 		.of_match_table = of_match_ptr(dev_ids),
+		.pm = &pcf85363_pm_ops,
 	},
 	.probe	= pcf85363_probe,
+	.remove	= pcf85363_remove,
 };
 
 module_i2c_driver(pcf85363_driver);
-- 
2.17.1

Re: [1/2] dt-bindings: rtc: pcf85263/pcf85363: add some properties

[Alexandre Belloni]

On 30/08/2019 17:17:19+0800, Biwen Li wrote:
> Add some properties for pcf85263/pcf85363 as follows:
>   - interrupt-output-pin: string type
>   - quartz-load-capacitance: integer type
>   - quartz-drive-strength: integer type
>   - quartz-low-jitter: bool type
>   - wakeup-source: bool type
> 
> Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
> Signed-off-by: Biwen Li <biwen.li@nxp.com>
> ---
>  .../devicetree/bindings/rtc/pcf85363.txt      | 31 +++++++++++++++++++
>  include/dt-bindings/rtc/pcf85363.h            | 15 +++++++++
>  2 files changed, 46 insertions(+)
>  create mode 100644 include/dt-bindings/rtc/pcf85363.h
> 
> diff --git a/Documentation/devicetree/bindings/rtc/pcf85363.txt b/Documentation/devicetree/bindings/rtc/pcf85363.txt
> index 94adc1cf93d9..d83359990bd7 100644
> --- a/Documentation/devicetree/bindings/rtc/pcf85363.txt
> +++ b/Documentation/devicetree/bindings/rtc/pcf85363.txt
> @@ -8,10 +8,41 @@ Required properties:
>  Optional properties:
>  - interrupts: IRQ line for the RTC (not implemented).
>  
> +- interrupt-output-pin: The interrupt output pin must be
> +  "NONE", "INTA" or "INTB", default value is "NONE"
> +

default value can't be none if there is an interrupts property. Also,
both pins can be enabled at the same time and this binding would prevent
that.
Finally, it may also be desirable to have some interrupts on one pin and
other interrupts on another pin e.g. alarms and timestamping on INTA
going to the SoC and only alarms on INTB going to a PMIC.

> +- quartz-load-capacitance: The internal capacitor to select for the quartz:
> +	PCF85263_QUARTZCAP_7pF		[0]
> +	PCF85263_QUARTZCAP_6pF		[1]
> +	PCF85263_QUARTZCAP_12p5pF	[2] DEFAULT
> +

The correct generic property is quartz-load-femtofarads.

> +- quartz-drive-strength: Drive strength for the quartz:
> +	PCF85263_QUARTZDRIVE_NORMAL	[0] DEFAULT
> +	PCF85263_QUARTZDRIVE_LOW	[1]
> +	PCF85263_QUARTZDRIVE_HIGH	[2]
> +

This has to take a value in ohm to be generic and then you don't need
the include file.

> +- quartz-low-jitter: Boolean property, if present enables low jitter mode
> +  which reduces jitter at the cost of increased power consumption.
> +

I think that property needs to be nxp specific.

> +- wakeup-source: Boolean property, mark the chip as a wakeup source,
> +  independently of the availability of an IRQ line connected to the SoC.
> +  This is useful if the IRQ line is connected to a PMIC or other circuit
> +  that can power up the device rather than to a normal SOC interrupt.
> +

This is already defined in bindings/power/wakeup-source.txt I guess you
can simply refer to it.

>  Example:
>  
>  pcf85363: pcf85363@51 {
>  	compatible = "nxp,pcf85363";
>  	reg = <0x51>;
> +
> +	interrupt-parent = <&gpio1>;
> +	interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +
> +	#include <dt-bindings/rtc/pcf85363.h>
> +	wakeup-source;
> +	interrupt-output-pin = "INTA";
> +	quartz-load-capacitance = <PCF85363_QUARTZCAP_12p5pF>;
> +	quartz-drive-strength = <PCF85363_QUARTZDRIVE_LOW>;
> +	quartz-low-jitter;
>  };
>  
> diff --git a/include/dt-bindings/rtc/pcf85363.h b/include/dt-bindings/rtc/pcf85363.h
> new file mode 100644
> index 000000000000..2c06c28eb5ff
> --- /dev/null
> +++ b/include/dt-bindings/rtc/pcf85363.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _DT_BINDINGS_RTC_PCF85363_H
> +#define _DT_BINDINGS_RTC_PCF85363_H
> +
> +/* Quartz capacitance */
> +#define PCF85363_QUARTZCAP_7pF		0
> +#define PCF85363_QUARTZCAP_6pF		1
> +#define PCF85363_QUARTZCAP_12p5pF	2
> +
> +/* Quartz drive strength */
> +#define PCF85363_QUARTZDRIVE_NORMAL	0
> +#define PCF85363_QUARTZDRIVE_LOW	1
> +#define PCF85363_QUARTZDRIVE_HIGH	2
> +
> +#endif /* _DT_BINDINGS_RTC_PCF85363_H */
> -- 
> 2.17.1
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [EXT] Re: [1/2] dt-bindings: rtc: pcf85263/pcf85363: add some properties

[Biwen Li]

> 
> On 30/08/2019 17:17:19+0800, Biwen Li wrote:
> > Add some properties for pcf85263/pcf85363 as follows:
> >   - interrupt-output-pin: string type
> >   - quartz-load-capacitance: integer type
> >   - quartz-drive-strength: integer type
> >   - quartz-low-jitter: bool type
> >   - wakeup-source: bool type
> >
> > Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
> > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > ---
> >  .../devicetree/bindings/rtc/pcf85363.txt      | 31
> +++++++++++++++++++
> >  include/dt-bindings/rtc/pcf85363.h            | 15 +++++++++
> >  2 files changed, 46 insertions(+)
> >  create mode 100644 include/dt-bindings/rtc/pcf85363.h
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/pcf85363.txt
> > b/Documentation/devicetree/bindings/rtc/pcf85363.txt
> > index 94adc1cf93d9..d83359990bd7 100644
> > --- a/Documentation/devicetree/bindings/rtc/pcf85363.txt
> > +++ b/Documentation/devicetree/bindings/rtc/pcf85363.txt
> > @@ -8,10 +8,41 @@ Required properties:
> >  Optional properties:
> >  - interrupts: IRQ line for the RTC (not implemented).
> >
> > +- interrupt-output-pin: The interrupt output pin must be
> > +  "NONE", "INTA" or "INTB", default value is "NONE"
> > +
> 
> default value can't be none if there is an interrupts property. Also, both pins
> can be enabled at the same time and this binding would prevent that.
> Finally, it may also be desirable to have some interrupts on one pin and
> other interrupts on another pin e.g. alarms and timestamping on INTA going
> to the SoC and only alarms on INTB going to a PMIC.
Ok, got it, I will correct it on v2.
> 
> > +- quartz-load-capacitance: The internal capacitor to select for the quartz:
> > +     PCF85263_QUARTZCAP_7pF          [0]
> > +     PCF85263_QUARTZCAP_6pF          [1]
> > +     PCF85263_QUARTZCAP_12p5pF       [2] DEFAULT
> > +
> 
> The correct generic property is quartz-load-femtofarads.
I will replace it on v2.
> 
> > +- quartz-drive-strength: Drive strength for the quartz:
> > +     PCF85263_QUARTZDRIVE_NORMAL     [0] DEFAULT
> > +     PCF85263_QUARTZDRIVE_LOW        [1]
> > +     PCF85263_QUARTZDRIVE_HIGH       [2]
> > +
> 
> This has to take a value in ohm to be generic and then you don't need the
> include file.
I will adjust it on v2.
> 
> > +- quartz-low-jitter: Boolean property, if present enables low jitter
> > +mode
> > +  which reduces jitter at the cost of increased power consumption.
> > +
> 
> I think that property needs to be nxp specific.
I will replace it with nxp,quartz-low-jitter on v2.
> 
> > +- wakeup-source: Boolean property, mark the chip as a wakeup source,
> > +  independently of the availability of an IRQ line connected to the SoC.
> > +  This is useful if the IRQ line is connected to a PMIC or other
> > +circuit
> > +  that can power up the device rather than to a normal SOC interrupt.
> > +
> 
> This is already defined in bindings/power/wakeup-source.txt I guess you can
> simply refer to it.
I will correct it on v2.
> 
> >  Example:
> >
> >  pcf85363: pcf85363@51 {
> >       compatible = "nxp,pcf85363";
> >       reg = <0x51>;
> > +
> > +     interrupt-parent = <&gpio1>;
> > +     interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> > +
> > +     #include <dt-bindings/rtc/pcf85363.h>
> > +     wakeup-source;
> > +     interrupt-output-pin = "INTA";
> > +     quartz-load-capacitance = <PCF85363_QUARTZCAP_12p5pF>;
> > +     quartz-drive-strength = <PCF85363_QUARTZDRIVE_LOW>;
> > +     quartz-low-jitter;
> >  };
> >
> > diff --git a/include/dt-bindings/rtc/pcf85363.h
> > b/include/dt-bindings/rtc/pcf85363.h
> > new file mode 100644
> > index 000000000000..2c06c28eb5ff
> > --- /dev/null
> > +++ b/include/dt-bindings/rtc/pcf85363.h
> > @@ -0,0 +1,15 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */ #ifndef
> > +_DT_BINDINGS_RTC_PCF85363_H #define
> _DT_BINDINGS_RTC_PCF85363_H
> > +
> > +/* Quartz capacitance */
> > +#define PCF85363_QUARTZCAP_7pF               0
> > +#define PCF85363_QUARTZCAP_6pF               1
> > +#define PCF85363_QUARTZCAP_12p5pF    2
> > +
> > +/* Quartz drive strength */
> > +#define PCF85363_QUARTZDRIVE_NORMAL  0
> > +#define PCF85363_QUARTZDRIVE_LOW     1
> > +#define PCF85363_QUARTZDRIVE_HIGH    2
> > +
> > +#endif /* _DT_BINDINGS_RTC_PCF85363_H */
> > --
> > 2.17.1
> >
> 
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootl
> in.com&amp;data=02%7C01%7Cbiwen.li%40nxp.com%7C0a2e5b50f8fc45a
> ef6a208d72d2ebe2e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C637027551094795780&amp;sdata=PMMS6PMBPkuuIYgMJFmtOaoD%
> 2B7fCO3eZvOtlYhTEL5w%3D&amp;reserved=0

Re: [1/2] dt-bindings: rtc: pcf85263/pcf85363: add some properties

[Rob Herring]

On Fri, Aug 30, 2019 at 05:17:19PM +0800, Biwen Li wrote:
> Add some properties for pcf85263/pcf85363 as follows:
>   - interrupt-output-pin: string type
>   - quartz-load-capacitance: integer type
>   - quartz-drive-strength: integer type
>   - quartz-low-jitter: bool type
>   - wakeup-source: bool type
> 
> Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
> Signed-off-by: Biwen Li <biwen.li@nxp.com>
> ---
>  .../devicetree/bindings/rtc/pcf85363.txt      | 31 +++++++++++++++++++
>  include/dt-bindings/rtc/pcf85363.h            | 15 +++++++++
>  2 files changed, 46 insertions(+)
>  create mode 100644 include/dt-bindings/rtc/pcf85363.h
> 
> diff --git a/Documentation/devicetree/bindings/rtc/pcf85363.txt b/Documentation/devicetree/bindings/rtc/pcf85363.txt
> index 94adc1cf93d9..d83359990bd7 100644
> --- a/Documentation/devicetree/bindings/rtc/pcf85363.txt
> +++ b/Documentation/devicetree/bindings/rtc/pcf85363.txt
> @@ -8,10 +8,41 @@ Required properties:
>  Optional properties:
>  - interrupts: IRQ line for the RTC (not implemented).
>  
> +- interrupt-output-pin: The interrupt output pin must be
> +  "NONE", "INTA" or "INTB", default value is "NONE"
> +
> +- quartz-load-capacitance: The internal capacitor to select for the quartz:
> +	PCF85263_QUARTZCAP_7pF		[0]
> +	PCF85263_QUARTZCAP_6pF		[1]
> +	PCF85263_QUARTZCAP_12p5pF	[2] DEFAULT

We have a common property for this. Use it.

> +
> +- quartz-drive-strength: Drive strength for the quartz:
> +	PCF85263_QUARTZDRIVE_NORMAL	[0] DEFAULT
> +	PCF85263_QUARTZDRIVE_LOW	[1]
> +	PCF85263_QUARTZDRIVE_HIGH	[2]
> +
> +- quartz-low-jitter: Boolean property, if present enables low jitter mode
> +  which reduces jitter at the cost of increased power consumption.

These 2  need vendor prefixes.

> +
> +- wakeup-source: Boolean property, mark the chip as a wakeup source,
> +  independently of the availability of an IRQ line connected to the SoC.
> +  This is useful if the IRQ line is connected to a PMIC or other circuit
> +  that can power up the device rather than to a normal SOC interrupt.
> +
>  Example:
>  
>  pcf85363: pcf85363@51 {
>  	compatible = "nxp,pcf85363";
>  	reg = <0x51>;
> +
> +	interrupt-parent = <&gpio1>;
> +	interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +
> +	#include <dt-bindings/rtc/pcf85363.h>
> +	wakeup-source;
> +	interrupt-output-pin = "INTA";
> +	quartz-load-capacitance = <PCF85363_QUARTZCAP_12p5pF>;
> +	quartz-drive-strength = <PCF85363_QUARTZDRIVE_LOW>;
> +	quartz-low-jitter;
>  };
>  
> diff --git a/include/dt-bindings/rtc/pcf85363.h b/include/dt-bindings/rtc/pcf85363.h
> new file mode 100644
> index 000000000000..2c06c28eb5ff
> --- /dev/null
> +++ b/include/dt-bindings/rtc/pcf85363.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _DT_BINDINGS_RTC_PCF85363_H
> +#define _DT_BINDINGS_RTC_PCF85363_H
> +
> +/* Quartz capacitance */
> +#define PCF85363_QUARTZCAP_7pF		0
> +#define PCF85363_QUARTZCAP_6pF		1
> +#define PCF85363_QUARTZCAP_12p5pF	2
> +
> +/* Quartz drive strength */
> +#define PCF85363_QUARTZDRIVE_NORMAL	0
> +#define PCF85363_QUARTZDRIVE_LOW	1
> +#define PCF85363_QUARTZDRIVE_HIGH	2
> +
> +#endif /* _DT_BINDINGS_RTC_PCF85363_H */
> -- 
> 2.17.1
> 

RE: [EXT] Re: [1/2] dt-bindings: rtc: pcf85263/pcf85363: add some properties

[Biwen Li]

> 
> Caution: EXT Email
> 
> On Fri, Aug 30, 2019 at 05:17:19PM +0800, Biwen Li wrote:
> > Add some properties for pcf85263/pcf85363 as follows:
> >   - interrupt-output-pin: string type
> >   - quartz-load-capacitance: integer type
> >   - quartz-drive-strength: integer type
> >   - quartz-low-jitter: bool type
> >   - wakeup-source: bool type
> >
> > Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
> > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > ---
> >  .../devicetree/bindings/rtc/pcf85363.txt      | 31
> +++++++++++++++++++
> >  include/dt-bindings/rtc/pcf85363.h            | 15 +++++++++
> >  2 files changed, 46 insertions(+)
> >  create mode 100644 include/dt-bindings/rtc/pcf85363.h
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/pcf85363.txt
> > b/Documentation/devicetree/bindings/rtc/pcf85363.txt
> > index 94adc1cf93d9..d83359990bd7 100644
> > --- a/Documentation/devicetree/bindings/rtc/pcf85363.txt
> > +++ b/Documentation/devicetree/bindings/rtc/pcf85363.txt
> > @@ -8,10 +8,41 @@ Required properties:
> >  Optional properties:
> >  - interrupts: IRQ line for the RTC (not implemented).
> >
> > +- interrupt-output-pin: The interrupt output pin must be
> > +  "NONE", "INTA" or "INTB", default value is "NONE"
> > +
> > +- quartz-load-capacitance: The internal capacitor to select for the quartz:
> > +     PCF85263_QUARTZCAP_7pF          [0]
> > +     PCF85263_QUARTZCAP_6pF          [1]
> > +     PCF85263_QUARTZCAP_12p5pF       [2] DEFAULT
> 
> We have a common property for this. Use it.
Ok, I will replace it in v2.
> 
> > +
> > +- quartz-drive-strength: Drive strength for the quartz:
> > +     PCF85263_QUARTZDRIVE_NORMAL     [0] DEFAULT
> > +     PCF85263_QUARTZDRIVE_LOW        [1]
> > +     PCF85263_QUARTZDRIVE_HIGH       [2]
> > +
> > +- quartz-low-jitter: Boolean property, if present enables low jitter
> > +mode
> > +  which reduces jitter at the cost of increased power consumption.
> 
> These 2  need vendor prefixes.
Okay, I will add vendor prefixes in v2.
> 
> > +
> > +- wakeup-source: Boolean property, mark the chip as a wakeup source,
> > +  independently of the availability of an IRQ line connected to the SoC.
> > +  This is useful if the IRQ line is connected to a PMIC or other
> > +circuit
> > +  that can power up the device rather than to a normal SOC interrupt.
> > +
> >  Example:
> >
> >  pcf85363: pcf85363@51 {
> >       compatible = "nxp,pcf85363";
> >       reg = <0x51>;
> > +
> > +     interrupt-parent = <&gpio1>;
> > +     interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> > +
> > +     #include <dt-bindings/rtc/pcf85363.h>
> > +     wakeup-source;
> > +     interrupt-output-pin = "INTA";
> > +     quartz-load-capacitance = <PCF85363_QUARTZCAP_12p5pF>;
> > +     quartz-drive-strength = <PCF85363_QUARTZDRIVE_LOW>;
> > +     quartz-low-jitter;
> >  };
> >
> > diff --git a/include/dt-bindings/rtc/pcf85363.h
> > b/include/dt-bindings/rtc/pcf85363.h
> > new file mode 100644
> > index 000000000000..2c06c28eb5ff
> > --- /dev/null
> > +++ b/include/dt-bindings/rtc/pcf85363.h
> > @@ -0,0 +1,15 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */ #ifndef
> > +_DT_BINDINGS_RTC_PCF85363_H #define
> _DT_BINDINGS_RTC_PCF85363_H
> > +
> > +/* Quartz capacitance */
> > +#define PCF85363_QUARTZCAP_7pF               0
> > +#define PCF85363_QUARTZCAP_6pF               1
> > +#define PCF85363_QUARTZCAP_12p5pF    2
> > +
> > +/* Quartz drive strength */
> > +#define PCF85363_QUARTZDRIVE_NORMAL  0
> > +#define PCF85363_QUARTZDRIVE_LOW     1
> > +#define PCF85363_QUARTZDRIVE_HIGH    2
> > +
> > +#endif /* _DT_BINDINGS_RTC_PCF85363_H */
> > --
> > 2.17.1
> >