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

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

[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 <aford173@gmail.com>

diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
index 2ac1131fd922..c268debe5b8d 100644
--- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
+++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
@@ -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
-- 
2.25.1

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

[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 <aford173@gmail.com>

diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
index 43db67337bc0..224118ca08fd 100644
--- a/drivers/clk/clk-versaclock5.c
+++ b/drivers/clk/clk-versaclock5.c
@@ -759,6 +759,72 @@ 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 lowest target we can hit is 9430, so exit if it's less */
+	if (femtofarads < 9430)
+		return 0;
+
+	/*
+	 * 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 - 9430, 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 clases,
+	 * 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;
+
+		/*
+		 * According to Renesas, bits [1:0] of VC5_XTAL_X1_LOAD_CAP
+		 * and VC5_XTAL_X2_LOAD_CAP should always be 01b.
+		 * Since the mapped_value is really the high 6 bits of 8,
+		 * shift the value 2 places and or in the 0x01;
+		 */
+
+		mapped_value = (mapped_value << 2) | 0x01;
+		regmap_write(vc5->regmap, VC5_XTAL_X1_LOAD_CAP, mapped_value);
+		regmap_write(vc5->regmap, VC5_XTAL_X2_LOAD_CAP, mapped_value);
+	}
+
+	return 0;
+}
+
 static int vc5_update_slew(struct device_node *np_output,
 			   struct vc5_out_data *clk_out)
 {
@@ -884,6 +950,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;
-- 
2.25.1

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

[Adam Ford]

On Sat, Jan 16, 2021 at 3:55 PM Adam Ford <aford173@gmail.com> wrote:
>
> 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 <aford173@gmail.com>
>
> diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
> index 43db67337bc0..224118ca08fd 100644
> --- a/drivers/clk/clk-versaclock5.c
> +++ b/drivers/clk/clk-versaclock5.c
> @@ -759,6 +759,72 @@ 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 lowest target we can hit is 9430, so exit if it's less */
> +       if (femtofarads < 9430)
> +               return 0;
> +
> +       /*
> +        * 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 - 9430, 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 clases,
> +        * 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;
> +
> +               /*
> +                * According to Renesas, bits [1:0] of VC5_XTAL_X1_LOAD_CAP
> +                * and VC5_XTAL_X2_LOAD_CAP should always be 01b.
> +                * Since the mapped_value is really the high 6 bits of 8,
> +                * shift the value 2 places and or in the 0x01;
> +                */
> +
> +               mapped_value = (mapped_value << 2) | 0x01;
> +               regmap_write(vc5->regmap, VC5_XTAL_X1_LOAD_CAP, mapped_value);
> +               regmap_write(vc5->regmap, VC5_XTAL_X2_LOAD_CAP, mapped_value);

On second thought I'm going to change register write to a
read-modify-write since the low two bits are unclear and X1 and X2 low
bits are not exactly the same.   Since the info is confusing, I can
cache VC5_XTAL_X1_LOAD_CAP, clear the upper 6 bits and then logic-or
the value.  This way we don't have to guess about what the 0x01.  It
also appears the 0x01 is only for one of the registers and not both.


> +       }
> +
> +       return 0;
> +}
> +
>  static int vc5_update_slew(struct device_node *np_output,
>                            struct vc5_out_data *clk_out)
>  {
> @@ -884,6 +950,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;
> --
> 2.25.1
>