[PATCH v2 1/1] i2c: aspeed: add proper support fo 24xx clock params
Rick Altherr
raltherr at google.com
Sat Jul 29 06:57:39 AEST 2017
Is clk_fractional_divider from include/linux/clk-provider.h appropriate
here?
On Fri, Jul 28, 2017 at 1:45 PM, Brendan Higgins <brendanhiggins at google.com>
wrote:
> 24xx BMCs have larger clock divider granularity which can cause problems
> when trying to set them as 25xx clock dividers; this adds clock setting
> code specific to 24xx.
>
> This also fixes a potential issue where clock dividers were rounded down
> instead of up.
>
> Signed-off-by: Brendan Higgins <brendanhiggins at google.com>
> ---
> Changes for v2:
> - Fixed off by one error to check for divisors with base_clk == 0
> - Simplified some of the divisor math
> ---
> drivers/i2c/busses/i2c-aspeed.c | 74 ++++++++++++++++++++++++++++++
> +----------
> 1 file changed, 56 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-
> aspeed.c
> index f19348328a71..60afab866494 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -132,6 +132,7 @@ struct aspeed_i2c_bus {
> /* Synchronizes I/O mem access to base. */
> spinlock_t lock;
> struct completion cmd_complete;
> + u32 (*get_clk_reg_val)(u32 divisor);
> unsigned long parent_clk_frequency;
> u32 bus_frequency;
> /* Transaction state. */
> @@ -674,7 +675,7 @@ static const struct i2c_algorithm aspeed_i2c_algo = {
> #endif /* CONFIG_I2C_SLAVE */
> };
>
> -static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
> +static u32 aspeed_i2c_get_clk_reg_val(u32 clk_high_low_max, u32 divisor)
> {
> u32 base_clk, clk_high, clk_low, tmp;
>
> @@ -694,16 +695,22 @@ static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
> * Thus,
> * SCL_freq = APB_freq /
> * ((1 << base_clk) * (clk_high + 1 + clk_low + 1))
> - * The documentation recommends clk_high >= 8 and clk_low >= 7 when
> - * possible; this last constraint gives us the following solution:
> + * The documentation recommends clk_high >= clk_high_max / 2 and
> + * clk_low >= clk_low_max / 2 - 1 when possible; this last
> constraint
> + * gives us the following solution:
> */
> - base_clk = divisor > 33 ? ilog2((divisor - 1) / 32) + 1 : 0;
> - tmp = divisor / (1 << base_clk);
> - clk_high = tmp / 2 + tmp % 2;
> - clk_low = tmp - clk_high;
> + base_clk = divisor > clk_high_low_max ?
> + ilog2((divisor - 1) / clk_high_low_max) + 1 : 0;
> + tmp = (divisor + (1 << base_clk) - 1) >> base_clk;
> + clk_low = tmp / 2;
> + clk_high = tmp - clk_low;
> +
> + if (clk_high)
> + clk_high--;
> +
> + if (clk_low)
> + clk_low--;
>
> - clk_high -= 1;
> - clk_low -= 1;
>
> return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
> & ASPEED_I2CD_TIME_SCL_HIGH_MASK)
> @@ -712,13 +719,31 @@ static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
> | (base_clk & ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
> }
>
> +static u32 aspeed_i2c_24xx_get_clk_reg_val(u32 divisor)
> +{
> + /*
> + * 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(16, divisor);
> +}
> +
> +static u32 aspeed_i2c_25xx_get_clk_reg_val(u32 divisor)
> +{
> + /*
> + * 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(32, divisor);
> +}
> +
> /* precondition: bus.lock has been acquired. */
> static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus)
> {
> u32 divisor, clk_reg_val;
>
> - divisor = bus->parent_clk_frequency / bus->bus_frequency;
> - clk_reg_val = aspeed_i2c_get_clk_reg_val(divisor);
> + divisor = DIV_ROUND_UP(bus->parent_clk_frequency,
> bus->bus_frequency);
> + clk_reg_val = bus->get_clk_reg_val(divisor);
> writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1);
> writel(ASPEED_NO_TIMEOUT_CTRL, bus->base +
> ASPEED_I2C_AC_TIMING_REG2);
>
> @@ -777,8 +802,22 @@ static int aspeed_i2c_reset(struct aspeed_i2c_bus
> *bus)
> return ret;
> }
>
> +static const struct of_device_id aspeed_i2c_bus_of_table[] = {
> + {
> + .compatible = "aspeed,ast2400-i2c-bus",
> + .data = aspeed_i2c_24xx_get_clk_reg_val,
> + },
> + {
> + .compatible = "aspeed,ast2500-i2c-bus",
> + .data = aspeed_i2c_25xx_get_clk_reg_val,
> + },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
> +
> static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> {
> + const struct of_device_id *match;
> struct aspeed_i2c_bus *bus;
> struct clk *parent_clk;
> struct resource *res;
> @@ -808,6 +847,12 @@ static int aspeed_i2c_probe_bus(struct
> platform_device *pdev)
> bus->bus_frequency = 100000;
> }
>
> + 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 = match->data;
> +
> /* Initialize the I2C adapter */
> spin_lock_init(&bus->lock);
> init_completion(&bus->cmd_complete);
> @@ -869,13 +914,6 @@ static int aspeed_i2c_remove_bus(struct
> platform_device *pdev)
> return 0;
> }
>
> -static const struct of_device_id aspeed_i2c_bus_of_table[] = {
> - { .compatible = "aspeed,ast2400-i2c-bus", },
> - { .compatible = "aspeed,ast2500-i2c-bus", },
> - { },
> -};
> -MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
> -
> static struct platform_driver aspeed_i2c_bus_driver = {
> .probe = aspeed_i2c_probe_bus,
> .remove = aspeed_i2c_remove_bus,
> --
> 2.14.0.rc0.400.g1c36432dff-goog
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20170728/10195fe9/attachment.html>
More information about the openbmc
mailing list