Current aspeed-i2c driver could calculate a suited clock divisor and
high/low cycles automatically, and it try to assign high/low periods with
close number of cycles.
Because of board schematic design, sometimes we need to manual tune i2c
clock timing register to get longer high clock cycle to match hardware
requirement, which is not supportted in current driver.
In this patch series, we add new properties for manually assigning clock
divisor, high clock cycles and low clock cycles.
LINK: [v1] https://lore.kernel.org/all/[email protected]/
changes v1 --> v2:
* update bt-bindings documentation
* use meaningful values for properties instead of acture value in register
Potin Lai (1):
aspeed: i2c: add manual clock setting feature
Potin Lai (1):
dt-bindings: aspeed-i2c: add properties for manual clock setting
.../devicetree/bindings/i2c/aspeed,i2c.yaml | 44 ++++++++++++++
drivers/i2c/busses/i2c-aspeed.c | 57 ++++++++++++++++++-
2 files changed, 100 insertions(+), 1 deletion(-)
--
2.17.1
Add following properties for manual tuning clock divisor and cycle of
hign/low pulse witdh.
* aspeed,i2c-manual-clk: Enable aspeed i2c clock manual setting
* aspeed,i2c-base-clk-div: Base Clock divisor (tBaseClk)
* aspeed,i2c-clk-high-cycle: Cycles of clock-high pulse (tClkHigh)
* aspeed,i2c-clk-low-cycle: Cycles of clock-low pulse (tClkLow)
Signed-off-by: Potin Lai <[email protected]>
---
.../devicetree/bindings/i2c/aspeed,i2c.yaml | 44 +++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
index ea643e6c3ef5..e2f67fe2aa0c 100644
--- a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
@@ -12,6 +12,28 @@ maintainers:
allOf:
- $ref: /schemas/i2c/i2c-controller.yaml#
+ - if:
+ properties:
+ compatible:
+ const: st,stm32-uart
+
+ then:
+ properties:
+ aspeed,i2c-clk-high-cycle:
+ maximum: 8
+ aspeed,i2c-clk-low-cycle:
+ maximum: 8
+
+ - if:
+ required:
+ - aspeed,i2c-manual-clk
+
+ then:
+ required:
+ - aspeed,i2c-base-clk-div
+ - aspeed,i2c-clk-high-cycle
+ - aspeed,i2c-clk-low-cycle
+
properties:
compatible:
enum:
@@ -49,6 +71,28 @@ properties:
description:
states that there is another master active on this bus
+ aspeed,i2c-manual-clk:
+ type: boolean
+ description: enable manual clock setting
+
+ aspeed,i2c-base-clk-div:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192,
+ 16384, 32768]
+ description: base clock divisor
+
+ aspeed,i2c-clk-high-cycle:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 1
+ maximum: 16
+ description: cycles of master clock-high pulse width
+
+ aspeed,i2c-clk-low-cycle:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 1
+ maximum: 16
+ description: cycles of master clock-low pulse width
+
required:
- reg
- compatible
--
2.17.1
On Wed, Jun 01, 2022 at 12:15:12PM +0800, Potin Lai wrote:
> Add following properties for manual tuning clock divisor and cycle of
> hign/low pulse witdh.
>
> * aspeed,i2c-manual-clk: Enable aspeed i2c clock manual setting
> * aspeed,i2c-base-clk-div: Base Clock divisor (tBaseClk)
> * aspeed,i2c-clk-high-cycle: Cycles of clock-high pulse (tClkHigh)
> * aspeed,i2c-clk-low-cycle: Cycles of clock-low pulse (tClkLow)
>
> Signed-off-by: Potin Lai <[email protected]>
> ---
> .../devicetree/bindings/i2c/aspeed,i2c.yaml | 44 +++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> index ea643e6c3ef5..e2f67fe2aa0c 100644
> --- a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> @@ -12,6 +12,28 @@ maintainers:
> allOf:
> - $ref: /schemas/i2c/i2c-controller.yaml#
>
> + - if:
> + properties:
> + compatible:
> + const: st,stm32-uart
stm32 uart?
> +
> + then:
> + properties:
> + aspeed,i2c-clk-high-cycle:
> + maximum: 8
> + aspeed,i2c-clk-low-cycle:
> + maximum: 8
> +
> + - if:
> + required:
> + - aspeed,i2c-manual-clk
> +
> + then:
> + required:
> + - aspeed,i2c-base-clk-div
> + - aspeed,i2c-clk-high-cycle
> + - aspeed,i2c-clk-low-cycle
'dependencies' can better express this than an if/then.
However, I think this should all be done in a common way.
> +
> properties:
> compatible:
> enum:
> @@ -49,6 +71,28 @@ properties:
> description:
> states that there is another master active on this bus
>
> + aspeed,i2c-manual-clk:
> + type: boolean
> + description: enable manual clock setting
No need for this as presence of the other properties can determine this.
> +
> + aspeed,i2c-base-clk-div:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192,
> + 16384, 32768]
> + description: base clock divisor
Specify the i2c bus frequency and calculate the divider.
> +
> + aspeed,i2c-clk-high-cycle:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 1
> + maximum: 16
> + description: cycles of master clock-high pulse width
> +
> + aspeed,i2c-clk-low-cycle:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 1
> + maximum: 16
> + description: cycles of master clock-low pulse width
These 2 should be common. I think you just need a single property
expressing duty cycle.
Rob
Rob Herring 於 2022/6/6 上午 05:47 寫道:
> On Wed, Jun 01, 2022 at 12:15:12PM +0800, Potin Lai wrote:
>> Add following properties for manual tuning clock divisor and cycle of
>> hign/low pulse witdh.
>>
>> * aspeed,i2c-manual-clk: Enable aspeed i2c clock manual setting
>> * aspeed,i2c-base-clk-div: Base Clock divisor (tBaseClk)
>> * aspeed,i2c-clk-high-cycle: Cycles of clock-high pulse (tClkHigh)
>> * aspeed,i2c-clk-low-cycle: Cycles of clock-low pulse (tClkLow)
>>
>> Signed-off-by: Potin Lai <[email protected]>
>> ---
>> .../devicetree/bindings/i2c/aspeed,i2c.yaml | 44 +++++++++++++++++++
>> 1 file changed, 44 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>> index ea643e6c3ef5..e2f67fe2aa0c 100644
>> --- a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>> +++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>> @@ -12,6 +12,28 @@ maintainers:
>> allOf:
>> - $ref: /schemas/i2c/i2c-controller.yaml#
>>
>> + - if:
>> + properties:
>> + compatible:
>> + const: st,stm32-uart
> stm32 uart?
Sorry, I will remove it.
>> +
>> + then:
>> + properties:
>> + aspeed,i2c-clk-high-cycle:
>> + maximum: 8
>> + aspeed,i2c-clk-low-cycle:
>> + maximum: 8
>> +
>> + - if:
>> + required:
>> + - aspeed,i2c-manual-clk
>> +
>> + then:
>> + required:
>> + - aspeed,i2c-base-clk-div
>> + - aspeed,i2c-clk-high-cycle
>> + - aspeed,i2c-clk-low-cycle
> 'dependencies' can better express this than an if/then.
>
> However, I think this should all be done in a common way.
>
>> +
>> properties:
>> compatible:
>> enum:
>> @@ -49,6 +71,28 @@ properties:
>> description:
>> states that there is another master active on this bus
>>
>> + aspeed,i2c-manual-clk:
>> + type: boolean
>> + description: enable manual clock setting
> No need for this as presence of the other properties can determine this.
>
>> +
>> + aspeed,i2c-base-clk-div:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192,
>> + 16384, 32768]
>> + description: base clock divisor
> Specify the i2c bus frequency and calculate the divider.
>
>> +
>> + aspeed,i2c-clk-high-cycle:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minimum: 1
>> + maximum: 16
>> + description: cycles of master clock-high pulse width
>> +
>> + aspeed,i2c-clk-low-cycle:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minimum: 1
>> + maximum: 16
>> + description: cycles of master clock-low pulse width
> These 2 should be common. I think you just need a single property
> expressing duty cycle.
>
> Rob
Got it, thank you for the suggestion of duty cycle.
I will use duty cycle for next version.
Potin