dt-bindings: clock: cs2000-cp: Document cirrus,pll-lock-timeout-ms
Subsystems (2)
PATCH v2 1/2 dt-bindings: clock: cs2000-cp: Document cirrus,pll-lock-timeout-ms
This property can be used to set the maximum time to wait for the PLL to lock.
---
v1 -> v2: rename property to standard unit suffix, drop $ref, amend default value
.../devicetree/bindings/clock/cirrus,cs2000-cp.yaml | 5 +++++
1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/clock/cirrus,cs2000-cp.yaml +5 -0
@@ -62,6 +62,11 @@ properties: output signal directly from the REF_CLK input. $ref: /schemas/types.yaml#/definitions/flag + cirrus,pll-lock-timeout-ms: + description: + Specifies the maximum time to wait for the PLL to lock, in milliseconds. + default: 100 + required: - compatible - reg
Signature · 1 line
2.37.2
PATCH v2 2/2 clk: cs2000-cp: make PLL lock timeout configurable
The driver currently does 256 iterations of reads from the DEVICE_CTRL register to wait for the PLL_LOCK bit to clear, and sleeps one microsecond after each attempt.
This isn't ideal because
a) the total time this allows for the device to settle depends on the I2C bus speed, and
b) the device might need more time, depending on the application.
This patch allows users to configure this timeout through a new device-tree property "cirrus,pll-lock-timeout-ms".
In order to not break existing applications, a default value of 100 ms is
assumed: For each read cycle, 8 bits are sent for the register address, and 8 bits are read with the values. 16 bits take about 160 us on a 100 kHz bus and 40 us on a 400 kHz bus. Hence 256 iterations would take a maximum of around 44 ms. Round up and double that value to be on the safe side.
--- drivers/clk/clk-cs2000-cp.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)
drivers/clk/clk-cs2000-cp.c +13 -12
@@ -110,6 +110,8 @@ struct cs2000_priv {bool lf_ratio;bool clk_skip; +unsigned int pll_lock_timeout_ms; +/* suspend/resume */ unsigned long saved_rate;unsigned long saved_parent_rate;
@@ -171,21 +173,16 @@ static int cs2000_ref_clk_bound_rate(struct cs2000_priv *priv,static int cs2000_wait_pll_lock (struct cs2000_priv *priv) {struct device *dev =priv_to_dev (priv); -unsigned int i, val; +unsigned int val;int ret; -for (i =0 ; i <256 ; i++) { - ret =regmap_read (priv->regmap,DEVICE_CTRL , &val); -if (ret <0 ) -return ret; -if (!(val &PLL_UNLOCK )) -return 0 ; -udelay (1 ); - } - -dev_err (dev,"pll lock failed\n" ); + ret =regmap_read_poll_timeout (priv->regmap,DEVICE_CTRL , val, + !(val &PLL_UNLOCK ),USEC_PER_MSEC , + priv->pll_lock_timeout_ms *USEC_PER_MSEC ); +if (ret <0 ) +dev_err (dev,"pll lock failed\n" ); -return -ETIMEDOUT ; +return ret; }static int cs2000_clk_out_enable (struct cs2000_priv *priv,bool enable)
@@ -481,6 +478,10 @@ static int cs2000_clk_register(struct cs2000_priv *priv)if (ret <0 )return ret; + priv->pll_lock_timeout_ms =100 ; +of_property_read_u32 (np,"cirrus,pll-lock-timeout-ms" , + &priv->pll_lock_timeout_ms); + priv->clk_skip =of_property_read_bool (np,"cirrus,clock-skip" ); ref_clk_rate =clk_get_rate (priv->ref_clk);
Signature · 1 line
2.37.2
Stephen Boyd
maintainerRe: [PATCH v2 2/2] clk: cs2000-cp: make PLL lock timeout configurable
Quoting Daniel Mack (2022-08-26 02:11:22)
Quoted 4 lines
The driver currently does 256 iterations of reads from the
DEVICE_CTRLregister to wait for thePLL_LOCKbit to clear, and sleeps one microsecond after each attempt.
Quoted 8 lines
This isn't ideal because
a) the total time this allows for the device to settle depends on the I2C bus speed, and
b) the device might need more time, depending on the application.This patch allows users to configure this timeout through a new device-tree property "cirrus,pll-lock-timeout-ms".
It's a timeout, so why not just increase the timeout regardless of everything else? Or can we parse the bus speed (100kHz or 400kHz) instead of adding a new property?
Quoted 7 lines
In order to not break existing applications, a default value of 100 ms is
assumed: For each read cycle, 8 bits are sent for the register address, and 8 bits are read with the values. 16 bits take about 160 us on a 100 kHz bus and 40 us on a 400 kHz bus. Hence 256 iterations would take a maximum of around 44 ms. Round up and double that value to be on the safe side.
Rob Herring
maintainerRe: [PATCH v2 2/2] clk: cs2000-cp: make PLL lock timeout configurable
Quoted 11 lines · Stephen Boyd
Quoting Daniel Mack (2022-08-26 02:11:22)
Quoted 10 lines
The driver currently does 256 iterations of reads from the
DEVICE_CTRLregister to wait for thePLL_LOCKbit to clear, and sleeps one microsecond after each attempt.This isn't ideal because
a) the total time this allows for the device to settle depends on the I2C bus speed, and
b) the device might need more time, depending on the application.
Quoted 6 lines
Quoted 2 lines
This patch allows users to configure this timeout through a new device-tree property "cirrus,pll-lock-timeout-ms".
It's a timeout, so why not just increase the timeout regardless of everything else? Or can we parse the bus speed (100kHz or 400kHz) instead of adding a new property?
My thought too. Usually PLLs have a spec for max/typ lock times. Given it's a should never happen type of thing, it doesn't seem like we need a super precise time.
Rob
Re: [PATCH v2 2/2] clk: cs2000-cp: make PLL lock timeout configurable
Quoted 15 lines · Rob Herring
Quoted 14 lines · Stephen Boyd
Quoting Daniel Mack (2022-08-26 02:11:22)
Quoted 12 lines
The driver currently does 256 iterations of reads from the
DEVICE_CTRLregister to wait for thePLL_LOCKbit to clear, and sleeps one microsecond after each attempt.This isn't ideal because
a) the total time this allows for the device to settle depends on the I2C bus speed, and
b) the device might need more time, depending on the application.This patch allows users to configure this timeout through a new device-tree property "cirrus,pll-lock-timeout-ms".
Quoted 7 lines
Quoted 3 lines
It's a timeout, so why not just increase the timeout regardless of everything else? Or can we parse the bus speed (100kHz or 400kHz) instead of adding a new property?
My thought too. Usually PLLs have a spec for max/typ lock times. Given it's a should never happen type of thing, it doesn't seem like we need a super precise time.
Alright. I guess the real problem here is with the generation of the input clock signals, so I need to address that side of the design instead.
Thanks for the feedback!
Daniel