dt-bindings: clk: versaclock5: Add optional load capacitance property

PATCH Applied Adam Ford 4 participants 9 messages Feb 7, 2021 – Feb 11, 2021

Subsystems (3)

Device tree bindings Common clk Idt VersaClock 5 clock driver

lore

Share & tools

lore

Adam Ford

author

2021-02-07 18:52 UTC #0 lore raw

PATCH v3 1/2 dt-bindings: clk: versaclock5: Add optional load capacitance property

There are two registers which can set the load capacitance for XTAL1 and XTAL2. These are optional registers when using an external crystal. Since XTAL1 and XTAL2 will set to the same value, update the binding to support a single property called xtal-load-femtofarads.

Signed-off-by Adam Ford <[email protected]>

---

V3:  No Change
V2:  No Change

A couple people suggested that I not use the $ref, but without it, the bindings check failed with errors.

Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +6 -0

index 2ac1131fd922..c268debe5b8d 100644
@@ -59,6 +59,12 @@ properties:
     minItems: 1
     maxItems: 2
 
+  idt,xtal-load-femtofarads:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 9000
+    maximum: 22760
+    description: Optional load capacitor for XTAL1 and XTAL2
+
 patternProperties:
   "^OUT[1-4]$":
     type: object

Signature · 1 line

2.25.1

Adam Ford

author

2021-02-07 18:52 UTC #1 lore raw

PATCH v3 2/2 clk: vc5: Add support for optional load capacitance

There are two registers which can set the load capacitance for XTAL1 and XTAL2. These are optional registers when using an external crystal. Parse the device tree and set the corresponding registers accordingly.

Signed-off-by Adam Ford <[email protected]>

---
V3: Fix whitespace. Use regmap_update_bits instead of performing
a manual read-modify-write.
V2: Make the math subtract 9000 since we have a DIV_ROUND_CLOSEST
This also allows us to remove the check for 9430 since values between 9000 and 9430 will round up and down.
Make write VC5_XTAL_X1_LOAD_CAP and VC5_XTAL_X2_LOAD_CAP a read-modify-write to not worry about the contents of bits[1:0].

drivers/clk/clk-versaclock5.c +64 -0

index 43db67337bc0..344cd6c61188 100644
@@ -759,6 +759,63 @@ static int vc5_update_power(struct device_node *np_output,
 	return 0;
 }
 
+static int vc5_map_cap_value(u32 femtofarads)
+{
+	int mapped_value;
+
+	/*
+	 * The datasheet explicitly states 9000 - 25000 with 0.5pF
+	 * steps, but the Programmer's guide shows the steps are 0.430pF.
+	 * After getting feedback from Renesas, the .5pF steps were the
+	 * goal, but 430nF was the actual values.
+	 * Because of this, the actual range goes to 22760 instead of 25000
+	 */
+	if (femtofarads < 9000 || femtofarads > 22760)
+		return -EINVAL;
+
+	/*
+	 * The Programmer's guide shows XTAL[5:0] but in reality,
+	 * XTAL[0] and XTAL[1] are both LSB which makes the math
+	 * strange.  With clarfication from Renesas, setting the
+	 * values should be simpler by ignoring XTAL[0]
+	 */
+	mapped_value = DIV_ROUND_CLOSEST(femtofarads - 9000, 430);
+
+	/*
+	 * Since the calculation ignores XTAL[0], there is one
+	 * special case where mapped_value = 32.  In reality, this means
+	 * the real mapped value should be 111111b.  In other cases,
+	 * the mapped_value needs to be shifted 1 to the left.
+	 */
+	if (mapped_value > 31)
+		mapped_value = 0x3f;
+	else
+		mapped_value <<= 1;
+
+	return mapped_value;
+}
+static int vc5_update_cap_load(struct device_node *node, struct vc5_driver_data *vc5)
+{
+	u32 value;
+	int mapped_value;
+
+	if (!of_property_read_u32(node, "idt,xtal-load-femtofarads", &value)) {
+		mapped_value = vc5_map_cap_value(value);
+		if (mapped_value < 0)
+			return mapped_value;
+
+		/*
+		 * The mapped_value is really the high 6 bits of
+		 * VC5_XTAL_X1_LOAD_CAP and VC5_XTAL_X2_LOAD_CAP, so
+		 * shift the value 2 places.
+		 */
+		regmap_update_bits(vc5->regmap, VC5_XTAL_X1_LOAD_CAP, ~0x03, mapped_value << 2);
+		regmap_update_bits(vc5->regmap, VC5_XTAL_X2_LOAD_CAP, ~0x03, mapped_value << 2);
+	}
+
+	return 0;
+}
+
 static int vc5_update_slew(struct device_node *np_output,
 			   struct vc5_out_data *clk_out)
 {

@@ -884,6 +941,13 @@ static int vc5_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return -EINVAL;
 	}
 
+	/* Configure Optional Loading Capacitance for external XTAL */
+	if (!(vc5->chip_info->flags & VC5_HAS_INTERNAL_XTAL)) {
+		ret = vc5_update_cap_load(client->dev.of_node, vc5);
+		if (ret)
+			goto err_clk_register;
+	}
+
 	init.name = kasprintf(GFP_KERNEL, "%pOFn.mux", client->dev.of_node);
 	init.ops = &vc5_mux_ops;
 	init.flags = 0;

Signature · 1 line

2.25.1

Luca Ceresoli

maintainer Reviewed-by on v3 2/2

2021-02-08 12:04 UTC #2 lore raw

Re: [PATCH V3 2/2] clk: vc5: Add support for optional load capacitance

Hi Adam,

Quoted 6 lines · Adam Ford

There are two registers which can set the load capacitance for XTAL1 and XTAL2. These are optional registers when using an external crystal. Parse the device tree and set the corresponding registers accordingly.
Signed-off-by Adam Ford <[email protected]>

Reviewed-by Luca Ceresoli <[email protected]>

Signature · 1 line

Luca

Stephen Boyd

maintainer Applied

2021-02-11 20:11 UTC #3 lore raw

Re: [PATCH V3 2/2] clk: vc5: Add support for optional load capacitance

In reply to Adam Ford

Quoting Adam Ford (2021-02-07 10:51:39)

Quoted 7 lines

There are two registers which can set the load capacitance for XTAL1 and XTAL2. These are optional registers when using an external crystal. Parse the device tree and set the corresponding registers accordingly.
Signed-off-by Adam Ford <[email protected]>
---

Applied to clk-next

Rob Herring

maintainer

2021-02-10 20:20 UTC #4 lore raw

In reply to Adam Ford

Quoted 19 lines · Adam Ford

There are two registers which can set the load capacitance for XTAL1 and XTAL2. These are optional registers when using an external crystal. Since XTAL1 and XTAL2 will set to the same value, update the binding to support a single property called xtal-load-femtofarads.
Signed-off-by Adam Ford <[email protected]>
---
V3:  No Change
V2:  No Change
A couple people suggested that I not use the $ref, but without it, the bindings check failed with errors.
Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +0 -0
index 2ac1131fd922..c268debe5b8d 100644

Documentation/devicetree/bindings/clock/idt,versaclock5.yaml properties: · line 63

Quoted 5 lines

Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +2 -0

@@ -59,6 +59,12 @@ properties:
     minItems: 1
     maxItems: 2
 
+  idt,xtal-load-femtofarads:
+    $ref: /schemas/types.yaml#/definitions/uint32

Don't need a type with standard unit suffix.

Quoted 10 lines

Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +4 -0

+    minimum: 9000
+    maximum: 22760
+    description: Optional load capacitor for XTAL1 and XTAL2
+
 patternProperties:
   "^OUT[1-4]$":
     type: object

Signature · 1 line

2.25.1

Adam Ford

author

2021-02-10 20:41 UTC #5 lore raw

Quoted 24 lines · Rob Herring

Quoted 22 lines · Adam Ford
There are two registers which can set the load capacitance for XTAL1 and XTAL2. These are optional registers when using an external crystal. Since XTAL1 and XTAL2 will set to the same value, update the binding to support a single property called xtal-load-femtofarads.
Signed-off-by Adam Ford <[email protected]>
---
V3:  No Change
V2:  No Change
A couple people suggested that I not use the $ref, but without it, the bindings check failed with errors.
Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +0 -0
index 2ac1131fd922..c268debe5b8d 100644
@@ -59,6 +59,12 @@ properties:
     minItems: 1
     maxItems: 2
 

Quoted 4 lines

Documentation/devicetree/bindings/clock/idt,versaclock5.yaml properties: · line 63
Quoted 2 lines
Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +2 -0
+  idt,xtal-load-femtofarads:
+    $ref: /schemas/types.yaml#/definitions/uint32
Don't need a type with standard unit suffix.

If I remove that line, the binding check fails.

adam

Quoted 11 lines

Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +4 -1

+    minimum: 9000
+    maximum: 22760
+    description: Optional load capacitor for XTAL1 and XTAL2
+
 patternProperties:
   "^OUT[1-4]$":
     type: object
--

2.25.1

Stephen Boyd

maintainer

2021-02-11 02:36 UTC #6 lore raw

Quoting Adam Ford (2021-02-10 12:40:38)

Quoted 25 lines · Rob Herring

Quoted 24 lines · Adam Ford
Quoted 22 lines
There are two registers which can set the load capacitance for XTAL1 and XTAL2. These are optional registers when using an external crystal. Since XTAL1 and XTAL2 will set to the same value, update the binding to support a single property called xtal-load-femtofarads.
Signed-off-by Adam Ford <[email protected]>
---
V3:  No Change
V2:  No Change
A couple people suggested that I not use the $ref, but without it, the bindings check failed with errors.
Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +0 -0
index 2ac1131fd922..c268debe5b8d 100644
@@ -59,6 +59,12 @@ properties:
     minItems: 1
     maxItems: 2
 

Quoted 7 lines

Quoted 4 lines
Documentation/devicetree/bindings/clock/idt,versaclock5.yaml properties: · line 63
Quoted 2 lines
Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +2 -0
+  idt,xtal-load-femtofarads:
+    $ref: /schemas/types.yaml#/definitions/uint32
Don't need a type with standard unit suffix.
If I remove that line, the binding check fails.

Is your dt-schema up to date?

Rob Herring

maintainer Reviewed-by

2021-02-11 14:59 UTC #7 lore raw

In reply to Adam Ford

Quoted 26 lines · Adam Ford

Quoted 24 lines · Rob Herring
Quoted 22 lines · Adam Ford
There are two registers which can set the load capacitance for XTAL1 and XTAL2. These are optional registers when using an external crystal. Since XTAL1 and XTAL2 will set to the same value, update the binding to support a single property called xtal-load-femtofarads.
Signed-off-by Adam Ford <[email protected]>
---
V3:  No Change
V2:  No Change
A couple people suggested that I not use the $ref, but without it, the bindings check failed with errors.
Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +0 -0
index 2ac1131fd922..c268debe5b8d 100644
@@ -59,6 +59,12 @@ properties:
     minItems: 1
     maxItems: 2
 

Quoted 6 lines

Quoted 4 lines
Documentation/devicetree/bindings/clock/idt,versaclock5.yaml properties: · line 63
Quoted 2 lines
Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +2 -0
+  idt,xtal-load-femtofarads:
+    $ref: /schemas/types.yaml#/definitions/uint32
Don't need a type with standard unit suffix.
If I remove that line, the binding check fails.

Ah, looks like femtofarads got added to property-units.txt but not the schemas. I'll add it, but fine to leave this as-is for now.

Reviewed-by Rob Herring <[email protected]>

Rob

Stephen Boyd

maintainer Applied

2021-02-11 20:10 UTC #8 lore raw

In reply to Adam Ford

Quoting Adam Ford (2021-02-07 10:51:38)

Quoted 8 lines

There are two registers which can set the load capacitance for XTAL1 and XTAL2. These are optional registers when using an external crystal. Since XTAL1 and XTAL2 will set to the same value, update the binding to support a single property called xtal-load-femtofarads.
Signed-off-by Adam Ford <[email protected]>
---

Applied to clk-next