Message ID | 20220826091122.2344503-1-daniel@zonque.org |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | [v2,1/2] dt-bindings: clock: cs2000-cp: Document cirrus,pll-lock-timeout-ms | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success | |
robh/patch-applied | success | |
robh/dtbs-check | warning | build log |
robh/dt-meta-schema | success |
Quoting Daniel Mack (2022-08-26 02:11:22) > 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". 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? > > 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. >
On Mon, Aug 29, 2022 at 06:49:06PM -0700, Stephen Boyd wrote: > Quoting Daniel Mack (2022-08-26 02:11:22) > > 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". > > 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
On 8/30/22 22:25, Rob Herring wrote: > On Mon, Aug 29, 2022 at 06:49:06PM -0700, Stephen Boyd wrote: >> Quoting Daniel Mack (2022-08-26 02:11:22) >>> 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". >> >> 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
diff --git a/Documentation/devicetree/bindings/clock/cirrus,cs2000-cp.yaml b/Documentation/devicetree/bindings/clock/cirrus,cs2000-cp.yaml index 0abd6ba82dfd..8e68e1746d1c 100644 --- a/Documentation/devicetree/bindings/clock/cirrus,cs2000-cp.yaml +++ b/Documentation/devicetree/bindings/clock/cirrus,cs2000-cp.yaml @@ -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
This property can be used to set the maximum time to wait for the PLL to lock. Signed-off-by: Daniel Mack <daniel@zonque.org> --- 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(+)