2022-06-10 06:34:32

by Potin Lai

[permalink] [raw]
Subject: [PATCH v4 1/2] aspeed: i2c: add clock duty cycle property

Introduce i2c-clk-high-min-percent property for setting a minimum clock
high percentage.

This driver calculate clk_high and clk_low with giving duty cycle. If it
could not find a suit clk_high and clk_low, it apply default duty cycle
50%.

Signed-off-by: Potin Lai <[email protected]>
---
drivers/i2c/busses/i2c-aspeed.c | 56 ++++++++++++++++++++++++++-------
1 file changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 67e8b97c0c95..9715dc4f933f 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -19,6 +19,7 @@
#include <linux/irqchip/chained_irq.h>
#include <linux/irqdomain.h>
#include <linux/kernel.h>
+#include <linux/math.h>
#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
@@ -27,6 +28,8 @@
#include <linux/reset.h>
#include <linux/slab.h>

+#define DEFAULT_I2C_CLK_DUTY_CYCLE 50
+
/* I2C Register */
#define ASPEED_I2C_FUN_CTRL_REG 0x00
#define ASPEED_I2C_AC_TIMING_REG1 0x04
@@ -149,9 +152,11 @@ struct aspeed_i2c_bus {
spinlock_t lock;
struct completion cmd_complete;
u32 (*get_clk_reg_val)(struct device *dev,
- u32 divisor);
+ u32 divisor,
+ u32 duty_cycle);
unsigned long parent_clk_frequency;
u32 bus_frequency;
+ u32 duty_cycle;
/* Transaction state. */
enum aspeed_i2c_master_state master_state;
struct i2c_msg *msgs;
@@ -798,9 +803,11 @@ static const struct i2c_algorithm aspeed_i2c_algo = {

static u32 aspeed_i2c_get_clk_reg_val(struct device *dev,
u32 clk_high_low_mask,
- u32 divisor)
+ u32 divisor,
+ u32 duty_cycle)
{
u32 base_clk_divisor, clk_high_low_max, clk_high, clk_low, tmp;
+ u32 tmp_base_clk_divisor;

/*
* SCL_high and SCL_low represent a value 1 greater than what is stored
@@ -842,10 +849,32 @@ static u32 aspeed_i2c_get_clk_reg_val(struct device *dev,
"clamping clock divider: divider requested, %u, is greater than largest possible divider, %u.\n",
divisor, (1 << base_clk_divisor) * clk_high_low_max);
} else {
- tmp = (divisor + (1 << base_clk_divisor) - 1)
+ for (tmp_base_clk_divisor = base_clk_divisor;
+ tmp_base_clk_divisor <= ASPEED_I2CD_TIME_BASE_DIVISOR_MASK;
+ tmp_base_clk_divisor++) {
+ /* calculate clk_high and clk_low with duty cycle */
+ tmp = (divisor + (1 << tmp_base_clk_divisor) - 1)
+ >> tmp_base_clk_divisor;
+
+ clk_high = DIV_ROUND_UP(tmp * duty_cycle, 100);
+ clk_low = tmp - clk_high;
+
+ if (max(clk_high, clk_low) <= (clk_high_low_mask + 1))
+ break;
+ }
+
+ if (tmp_base_clk_divisor <= ASPEED_I2CD_TIME_BASE_DIVISOR_MASK)
+ base_clk_divisor = tmp_base_clk_divisor;
+ else {
+ dev_err(dev,
+ "could not find clk_high and clk_low with duty cycle %u%%\n, recalculate with base_clk_divisor %u and duty_cycle 50%%",
+ duty_cycle, base_clk_divisor);
+ duty_cycle = 50;
+ tmp = (divisor + (1 << base_clk_divisor) - 1)
>> base_clk_divisor;
- clk_low = tmp / 2;
- clk_high = tmp - clk_low;
+ clk_high = DIV_ROUND_UP(tmp * duty_cycle, 100);
+ clk_low = tmp - clk_high;
+ }

if (clk_high)
clk_high--;
@@ -863,22 +892,22 @@ static u32 aspeed_i2c_get_clk_reg_val(struct device *dev,
& ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
}

-static u32 aspeed_i2c_24xx_get_clk_reg_val(struct device *dev, u32 divisor)
+static u32 aspeed_i2c_24xx_get_clk_reg_val(struct device *dev, u32 divisor, u32 duty_cycle)
{
/*
* clk_high and clk_low are each 3 bits wide, so each can hold a max
* value of 8 giving a clk_high_low_max of 16.
*/
- return aspeed_i2c_get_clk_reg_val(dev, GENMASK(2, 0), divisor);
+ return aspeed_i2c_get_clk_reg_val(dev, GENMASK(2, 0), divisor, duty_cycle);
}

-static u32 aspeed_i2c_25xx_get_clk_reg_val(struct device *dev, u32 divisor)
+static u32 aspeed_i2c_25xx_get_clk_reg_val(struct device *dev, u32 divisor, u32 duty_cycle)
{
/*
* clk_high and clk_low are each 4 bits wide, so each can hold a max
* value of 16 giving a clk_high_low_max of 32.
*/
- return aspeed_i2c_get_clk_reg_val(dev, GENMASK(3, 0), divisor);
+ return aspeed_i2c_get_clk_reg_val(dev, GENMASK(3, 0), divisor, duty_cycle);
}

/* precondition: bus.lock has been acquired. */
@@ -891,7 +920,7 @@ static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus)
clk_reg_val &= (ASPEED_I2CD_TIME_TBUF_MASK |
ASPEED_I2CD_TIME_THDSTA_MASK |
ASPEED_I2CD_TIME_TACST_MASK);
- clk_reg_val |= bus->get_clk_reg_val(bus->dev, divisor);
+ clk_reg_val |= bus->get_clk_reg_val(bus->dev, divisor, bus->duty_cycle);
writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1);
writel(ASPEED_NO_TIMEOUT_CTRL, bus->base + ASPEED_I2C_AC_TIMING_REG2);

@@ -1009,11 +1038,16 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
}

+ ret = of_property_read_u32(pdev->dev.of_node,
+ "i2c-clk-high-min-percent", &bus->duty_cycle);
+ if (ret < 0 || !bus->duty_cycle || bus->duty_cycle > 100)
+ bus->duty_cycle = DEFAULT_I2C_CLK_DUTY_CYCLE;
+
match = of_match_node(aspeed_i2c_bus_of_table, pdev->dev.of_node);
if (!match)
bus->get_clk_reg_val = aspeed_i2c_24xx_get_clk_reg_val;
else
- bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
+ bus->get_clk_reg_val = (u32 (*)(struct device *, u32, u32))
match->data;

/* Initialize the I2C adapter */
--
2.17.1


2022-06-22 15:28:32

by Potin Lai

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] aspeed: i2c: add clock duty cycle property

Potin Lai 於 6/10/2022 1:47 PM 寫道:
> Introduce i2c-clk-high-min-percent property for setting a minimum clock
> high percentage.
>
> This driver calculate clk_high and clk_low with giving duty cycle. If it
> could not find a suit clk_high and clk_low, it apply default duty cycle
> 50%.
>
> Signed-off-by: Potin Lai <[email protected]>
> ---
> drivers/i2c/busses/i2c-aspeed.c | 56 ++++++++++++++++++++++++++-------
> 1 file changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index 67e8b97c0c95..9715dc4f933f 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -19,6 +19,7 @@
> #include <linux/irqchip/chained_irq.h>
> #include <linux/irqdomain.h>
> #include <linux/kernel.h>
> +#include <linux/math.h>
> #include <linux/module.h>
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
> @@ -27,6 +28,8 @@
> #include <linux/reset.h>
> #include <linux/slab.h>
>
> +#define DEFAULT_I2C_CLK_DUTY_CYCLE 50
> +
> /* I2C Register */
> #define ASPEED_I2C_FUN_CTRL_REG 0x00
> #define ASPEED_I2C_AC_TIMING_REG1 0x04
> @@ -149,9 +152,11 @@ struct aspeed_i2c_bus {
> spinlock_t lock;
> struct completion cmd_complete;
> u32 (*get_clk_reg_val)(struct device *dev,
> - u32 divisor);
> + u32 divisor,
> + u32 duty_cycle);
> unsigned long parent_clk_frequency;
> u32 bus_frequency;
> + u32 duty_cycle;
> /* Transaction state. */
> enum aspeed_i2c_master_state master_state;
> struct i2c_msg *msgs;
> @@ -798,9 +803,11 @@ static const struct i2c_algorithm aspeed_i2c_algo = {
>
> static u32 aspeed_i2c_get_clk_reg_val(struct device *dev,
> u32 clk_high_low_mask,
> - u32 divisor)
> + u32 divisor,
> + u32 duty_cycle)
> {
> u32 base_clk_divisor, clk_high_low_max, clk_high, clk_low, tmp;
> + u32 tmp_base_clk_divisor;
>
> /*
> * SCL_high and SCL_low represent a value 1 greater than what is stored
> @@ -842,10 +849,32 @@ static u32 aspeed_i2c_get_clk_reg_val(struct device *dev,
> "clamping clock divider: divider requested, %u, is greater than largest possible divider, %u.\n",
> divisor, (1 << base_clk_divisor) * clk_high_low_max);
> } else {
> - tmp = (divisor + (1 << base_clk_divisor) - 1)
> + for (tmp_base_clk_divisor = base_clk_divisor;
> + tmp_base_clk_divisor <= ASPEED_I2CD_TIME_BASE_DIVISOR_MASK;
> + tmp_base_clk_divisor++) {
> + /* calculate clk_high and clk_low with duty cycle */
> + tmp = (divisor + (1 << tmp_base_clk_divisor) - 1)
> + >> tmp_base_clk_divisor;
> +
> + clk_high = DIV_ROUND_UP(tmp * duty_cycle, 100);
> + clk_low = tmp - clk_high;
> +
> + if (max(clk_high, clk_low) <= (clk_high_low_mask + 1))
> + break;
> + }
> +
> + if (tmp_base_clk_divisor <= ASPEED_I2CD_TIME_BASE_DIVISOR_MASK)
> + base_clk_divisor = tmp_base_clk_divisor;
> + else {
> + dev_err(dev,
> + "could not find clk_high and clk_low with duty cycle %u%%\n, recalculate with base_clk_divisor %u and duty_cycle 50%%",
> + duty_cycle, base_clk_divisor);
> + duty_cycle = 50;
> + tmp = (divisor + (1 << base_clk_divisor) - 1)
> >> base_clk_divisor;
> - clk_low = tmp / 2;
> - clk_high = tmp - clk_low;
> + clk_high = DIV_ROUND_UP(tmp * duty_cycle, 100);
> + clk_low = tmp - clk_high;
> + }
>
> if (clk_high)
> clk_high--;
> @@ -863,22 +892,22 @@ static u32 aspeed_i2c_get_clk_reg_val(struct device *dev,
> & ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
> }
>
> -static u32 aspeed_i2c_24xx_get_clk_reg_val(struct device *dev, u32 divisor)
> +static u32 aspeed_i2c_24xx_get_clk_reg_val(struct device *dev, u32 divisor, u32 duty_cycle)
> {
> /*
> * clk_high and clk_low are each 3 bits wide, so each can hold a max
> * value of 8 giving a clk_high_low_max of 16.
> */
> - return aspeed_i2c_get_clk_reg_val(dev, GENMASK(2, 0), divisor);
> + return aspeed_i2c_get_clk_reg_val(dev, GENMASK(2, 0), divisor, duty_cycle);
> }
>
> -static u32 aspeed_i2c_25xx_get_clk_reg_val(struct device *dev, u32 divisor)
> +static u32 aspeed_i2c_25xx_get_clk_reg_val(struct device *dev, u32 divisor, u32 duty_cycle)
> {
> /*
> * clk_high and clk_low are each 4 bits wide, so each can hold a max
> * value of 16 giving a clk_high_low_max of 32.
> */
> - return aspeed_i2c_get_clk_reg_val(dev, GENMASK(3, 0), divisor);
> + return aspeed_i2c_get_clk_reg_val(dev, GENMASK(3, 0), divisor, duty_cycle);
> }
>
> /* precondition: bus.lock has been acquired. */
> @@ -891,7 +920,7 @@ static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus)
> clk_reg_val &= (ASPEED_I2CD_TIME_TBUF_MASK |
> ASPEED_I2CD_TIME_THDSTA_MASK |
> ASPEED_I2CD_TIME_TACST_MASK);
> - clk_reg_val |= bus->get_clk_reg_val(bus->dev, divisor);
> + clk_reg_val |= bus->get_clk_reg_val(bus->dev, divisor, bus->duty_cycle);
> writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1);
> writel(ASPEED_NO_TIMEOUT_CTRL, bus->base + ASPEED_I2C_AC_TIMING_REG2);
>
> @@ -1009,11 +1038,16 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
> }
>
> + ret = of_property_read_u32(pdev->dev.of_node,
> + "i2c-clk-high-min-percent", &bus->duty_cycle);
> + if (ret < 0 || !bus->duty_cycle || bus->duty_cycle > 100)
> + bus->duty_cycle = DEFAULT_I2C_CLK_DUTY_CYCLE;
> +
> match = of_match_node(aspeed_i2c_bus_of_table, pdev->dev.of_node);
> if (!match)
> bus->get_clk_reg_val = aspeed_i2c_24xx_get_clk_reg_val;
> else
> - bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
> + bus->get_clk_reg_val = (u32 (*)(struct device *, u32, u32))
> match->data;
>
> /* Initialize the I2C adapter */
Hi Maintainers,
Could someone please help me review this patch and give me some advice?
Thanks!

Potin