* [PATCH v2 0/2] Add i2c clock manual tuning feature for aspeed-i2c driver @ 2022-06-01 4:15 Potin Lai 2022-06-01 4:15 ` [PATCH v2 1/2] aspeed: i2c: add manual clock setting feature Potin Lai 2022-06-01 4:15 ` [PATCH v2 2/2] dt-bindings: aspeed-i2c: add properties for manual clock setting Potin Lai 0 siblings, 2 replies; 5+ messages in thread From: Potin Lai @ 2022-06-01 4:15 UTC (permalink / raw) To: Brendan Higgins, Benjamin Herrenschmidt, Joel Stanley, Andrew Jeffery, Rob Herring, Rayn Chen Cc: Patrick Williams, Potin Lai, linux-i2c, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, Potin Lai 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/20220530114056.8722-1-potin.lai.pt@gmail.com/ 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] aspeed: i2c: add manual clock setting feature 2022-06-01 4:15 [PATCH v2 0/2] Add i2c clock manual tuning feature for aspeed-i2c driver Potin Lai @ 2022-06-01 4:15 ` Potin Lai 2022-06-01 4:15 ` [PATCH v2 2/2] dt-bindings: aspeed-i2c: add properties for manual clock setting Potin Lai 1 sibling, 0 replies; 5+ messages in thread From: Potin Lai @ 2022-06-01 4:15 UTC (permalink / raw) To: Brendan Higgins, Benjamin Herrenschmidt, Joel Stanley, Andrew Jeffery, Rob Herring, Rayn Chen Cc: Patrick Williams, Potin Lai, linux-i2c, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, Potin Lai Add manual tuning i2c clock timing register support by reading following properties. * 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 <potin.lai.pt@gmail.com> --- drivers/i2c/busses/i2c-aspeed.c | 57 ++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c index 67e8b97c0c95..64424f377f27 100644 --- a/drivers/i2c/busses/i2c-aspeed.c +++ b/drivers/i2c/busses/i2c-aspeed.c @@ -60,6 +60,7 @@ #define ASPEED_I2CD_TIME_SCL_LOW_MASK GENMASK(15, 12) #define ASPEED_I2CD_TIME_BASE_DIVISOR_MASK GENMASK(3, 0) #define ASPEED_I2CD_TIME_SCL_REG_MAX GENMASK(3, 0) +#define ASPEED_I2CD_TIME_BASE_DIVISOR_MAX 32768 /* 0x08 : I2CD Clock and AC Timing Control Register #2 */ #define ASPEED_NO_TIMEOUT_CTRL 0 @@ -898,6 +899,57 @@ static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus) return 0; } +/* precondition: bus.lock has been acquired. */ +static int aspeed_i2c_manual_clk_setup(struct aspeed_i2c_bus *bus) +{ + u32 divisor, clk_high, clk_low, clk_reg_val; + + if (device_property_read_u32(bus->dev, "aspeed,i2c-base-clk-div", + &divisor) != 0) { + dev_err(bus->dev, "Could not read aspeed,i2c-base-clk-div\n"); + return -EINVAL; + } else if (!divisor || divisor > ASPEED_I2CD_TIME_BASE_DIVISOR_MAX || + BIT(__fls(divisor)) != divisor) { + dev_err(bus->dev, "Invalid aspeed,i2c-base-clk-div: %u\n", + divisor); + return -EINVAL; + } + + if (device_property_read_u32(bus->dev, "aspeed,i2c-clk-high-cycle", + &clk_high) != 0) { + dev_err(bus->dev, "Could not read aspeed,i2c-clk-high-cycle\n"); + return -EINVAL; + } else if ((clk_high-1) > ASPEED_I2CD_TIME_SCL_REG_MAX) { + dev_err(bus->dev, "Invalid aspeed,i2c-clk-high-cycle: %u\n", + clk_high); + return -EINVAL; + } + + if (device_property_read_u32(bus->dev, "aspeed,i2c-clk-low-cycle", + &clk_low) != 0) { + dev_err(bus->dev, "Could not read aspeed,i2c-clk-low-cycle\n"); + return -EINVAL; + } else if ((clk_low-1) > ASPEED_I2CD_TIME_SCL_REG_MAX) { + dev_err(bus->dev, "Invalid aspeed,i2c-clk-low-cycle: %u\n", + clk_low); + return -EINVAL; + } + + clk_reg_val = readl(bus->base + ASPEED_I2C_AC_TIMING_REG1); + clk_reg_val &= (ASPEED_I2CD_TIME_TBUF_MASK | + ASPEED_I2CD_TIME_THDSTA_MASK | + ASPEED_I2CD_TIME_TACST_MASK); + clk_reg_val |= (ilog2(divisor) & ASPEED_I2CD_TIME_BASE_DIVISOR_MASK) + | (((clk_high-1) << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT) + & ASPEED_I2CD_TIME_SCL_HIGH_MASK) + | (((clk_low-1) << ASPEED_I2CD_TIME_SCL_LOW_SHIFT) + & ASPEED_I2CD_TIME_SCL_LOW_MASK); + writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1); + writel(ASPEED_NO_TIMEOUT_CTRL, bus->base + ASPEED_I2C_AC_TIMING_REG2); + + return 0; +} + /* precondition: bus.lock has been acquired. */ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus, struct platform_device *pdev) @@ -908,7 +960,10 @@ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus, /* Disable everything. */ writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG); - ret = aspeed_i2c_init_clk(bus); + if (of_property_read_bool(pdev->dev.of_node, "aspeed,i2c-manual-clk")) + ret = aspeed_i2c_manual_clk_setup(bus); + else + ret = aspeed_i2c_init_clk(bus); if (ret < 0) return ret; -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] dt-bindings: aspeed-i2c: add properties for manual clock setting 2022-06-01 4:15 [PATCH v2 0/2] Add i2c clock manual tuning feature for aspeed-i2c driver Potin Lai 2022-06-01 4:15 ` [PATCH v2 1/2] aspeed: i2c: add manual clock setting feature Potin Lai @ 2022-06-01 4:15 ` Potin Lai 2022-06-05 21:47 ` Rob Herring 1 sibling, 1 reply; 5+ messages in thread From: Potin Lai @ 2022-06-01 4:15 UTC (permalink / raw) To: Brendan Higgins, Benjamin Herrenschmidt, Joel Stanley, Andrew Jeffery, Rob Herring, Rayn Chen Cc: Patrick Williams, Potin Lai, linux-i2c, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, Potin Lai 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 <potin.lai.pt@gmail.com> --- .../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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: aspeed-i2c: add properties for manual clock setting 2022-06-01 4:15 ` [PATCH v2 2/2] dt-bindings: aspeed-i2c: add properties for manual clock setting Potin Lai @ 2022-06-05 21:47 ` Rob Herring 2022-06-06 8:39 ` Potin Lai 0 siblings, 1 reply; 5+ messages in thread From: Rob Herring @ 2022-06-05 21:47 UTC (permalink / raw) To: Potin Lai Cc: Brendan Higgins, Benjamin Herrenschmidt, Joel Stanley, Andrew Jeffery, Rayn Chen, Patrick Williams, Potin Lai, linux-i2c, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel 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 <potin.lai.pt@gmail.com> > --- > .../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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: aspeed-i2c: add properties for manual clock setting 2022-06-05 21:47 ` Rob Herring @ 2022-06-06 8:39 ` Potin Lai 0 siblings, 0 replies; 5+ messages in thread From: Potin Lai @ 2022-06-06 8:39 UTC (permalink / raw) To: Rob Herring Cc: Brendan Higgins, Benjamin Herrenschmidt, Joel Stanley, Andrew Jeffery, Rayn Chen, Patrick Williams, Potin Lai, linux-i2c, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel 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 <potin.lai.pt@gmail.com> >> --- >> .../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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-06-06 8:39 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-06-01 4:15 [PATCH v2 0/2] Add i2c clock manual tuning feature for aspeed-i2c driver Potin Lai 2022-06-01 4:15 ` [PATCH v2 1/2] aspeed: i2c: add manual clock setting feature Potin Lai 2022-06-01 4:15 ` [PATCH v2 2/2] dt-bindings: aspeed-i2c: add properties for manual clock setting Potin Lai 2022-06-05 21:47 ` Rob Herring 2022-06-06 8:39 ` Potin Lai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).