<div dir="ltr">Is clk_fractional_divider from include/linux/clk-provider.h appropriate here?</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jul 28, 2017 at 1:45 PM, Brendan Higgins <span dir="ltr"><<a href="mailto:brendanhiggins@google.com" target="_blank">brendanhiggins@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">24xx BMCs have larger clock divider granularity which can cause problems<br>
when trying to set them as 25xx clock dividers; this adds clock setting<br>
code specific to 24xx.<br>
<br>
This also fixes a potential issue where clock dividers were rounded down<br>
instead of up.<br>
<br>
Signed-off-by: Brendan Higgins <<a href="mailto:brendanhiggins@google.com">brendanhiggins@google.com</a>><br>
---<br>
Changes for v2:<br>
- Fixed off by one error to check for divisors with base_clk == 0<br>
- Simplified some of the divisor math<br>
---<br>
drivers/i2c/busses/i2c-aspeed.<wbr>c | 74 ++++++++++++++++++++++++++++++<wbr>+----------<br>
1 file changed, 56 insertions(+), 18 deletions(-)<br>
<br>
diff --git a/drivers/i2c/busses/i2c-<wbr>aspeed.c b/drivers/i2c/busses/i2c-<wbr>aspeed.c<br>
index f19348328a71..60afab866494 100644<br>
--- a/drivers/i2c/busses/i2c-<wbr>aspeed.c<br>
+++ b/drivers/i2c/busses/i2c-<wbr>aspeed.c<br>
@@ -132,6 +132,7 @@ struct aspeed_i2c_bus {<br>
/* Synchronizes I/O mem access to base. */<br>
spinlock_t lock;<br>
struct completion cmd_complete;<br>
+ u32 (*get_clk_reg_val)(u32 divisor);<br>
unsigned long parent_clk_frequency;<br>
u32 bus_frequency;<br>
/* Transaction state. */<br>
@@ -674,7 +675,7 @@ static const struct i2c_algorithm aspeed_i2c_algo = {<br>
#endif /* CONFIG_I2C_SLAVE */<br>
};<br>
<br>
-static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)<br>
+static u32 aspeed_i2c_get_clk_reg_val(u32 clk_high_low_max, u32 divisor)<br>
{<br>
u32 base_clk, clk_high, clk_low, tmp;<br>
<br>
@@ -694,16 +695,22 @@ static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)<br>
* Thus,<br>
* SCL_freq = APB_freq /<br>
* ((1 << base_clk) * (clk_high + 1 + clk_low + 1))<br>
- * The documentation recommends clk_high >= 8 and clk_low >= 7 when<br>
- * possible; this last constraint gives us the following solution:<br>
+ * The documentation recommends clk_high >= clk_high_max / 2 and<br>
+ * clk_low >= clk_low_max / 2 - 1 when possible; this last constraint<br>
+ * gives us the following solution:<br>
*/<br>
- base_clk = divisor > 33 ? ilog2((divisor - 1) / 32) + 1 : 0;<br>
- tmp = divisor / (1 << base_clk);<br>
- clk_high = tmp / 2 + tmp % 2;<br>
- clk_low = tmp - clk_high;<br>
+ base_clk = divisor > clk_high_low_max ?<br>
+ ilog2((divisor - 1) / clk_high_low_max) + 1 : 0;<br>
+ tmp = (divisor + (1 << base_clk) - 1) >> base_clk;<br>
+ clk_low = tmp / 2;<br>
+ clk_high = tmp - clk_low;<br>
+<br>
+ if (clk_high)<br>
+ clk_high--;<br>
+<br>
+ if (clk_low)<br>
+ clk_low--;<br>
<br>
- clk_high -= 1;<br>
- clk_low -= 1;<br>
<br>
return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_<wbr>SHIFT)<br>
& ASPEED_I2CD_TIME_SCL_HIGH_<wbr>MASK)<br>
@@ -712,13 +719,31 @@ static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)<br>
| (base_clk & ASPEED_I2CD_TIME_BASE_DIVISOR_<wbr>MASK);<br>
}<br>
<br>
+static u32 aspeed_i2c_24xx_get_clk_reg_<wbr>val(u32 divisor)<br>
+{<br>
+ /*<br>
+ * clk_high and clk_low are each 3 bits wide, so each can hold a max<br>
+ * value of 8 giving a clk_high_low_max of 16.<br>
+ */<br>
+ return aspeed_i2c_get_clk_reg_val(16, divisor);<br>
+}<br>
+<br>
+static u32 aspeed_i2c_25xx_get_clk_reg_<wbr>val(u32 divisor)<br>
+{<br>
+ /*<br>
+ * clk_high and clk_low are each 4 bits wide, so each can hold a max<br>
+ * value of 16 giving a clk_high_low_max of 32.<br>
+ */<br>
+ return aspeed_i2c_get_clk_reg_val(32, divisor);<br>
+}<br>
+<br>
/* precondition: bus.lock has been acquired. */<br>
static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus)<br>
{<br>
u32 divisor, clk_reg_val;<br>
<br>
- divisor = bus->parent_clk_frequency / bus->bus_frequency;<br>
- clk_reg_val = aspeed_i2c_get_clk_reg_val(<wbr>divisor);<br>
+ divisor = DIV_ROUND_UP(bus->parent_clk_<wbr>frequency, bus->bus_frequency);<br>
+ clk_reg_val = bus->get_clk_reg_val(divisor);<br>
writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1);<br>
writel(ASPEED_NO_TIMEOUT_CTRL, bus->base + ASPEED_I2C_AC_TIMING_REG2);<br>
<br>
@@ -777,8 +802,22 @@ static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus)<br>
return ret;<br>
}<br>
<br>
+static const struct of_device_id aspeed_i2c_bus_of_table[] = {<br>
+ {<br>
+ .compatible = "aspeed,ast2400-i2c-bus",<br>
+ .data = aspeed_i2c_24xx_get_clk_reg_<wbr>val,<br>
+ },<br>
+ {<br>
+ .compatible = "aspeed,ast2500-i2c-bus",<br>
+ .data = aspeed_i2c_25xx_get_clk_reg_<wbr>val,<br>
+ },<br>
+ { },<br>
+};<br>
+MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);<br>
+<br>
static int aspeed_i2c_probe_bus(struct platform_device *pdev)<br>
{<br>
+ const struct of_device_id *match;<br>
struct aspeed_i2c_bus *bus;<br>
struct clk *parent_clk;<br>
struct resource *res;<br>
@@ -808,6 +847,12 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)<br>
bus->bus_frequency = 100000;<br>
}<br>
<br>
+ match = of_match_node(aspeed_i2c_bus_<wbr>of_table, pdev->dev.of_node);<br>
+ if (!match)<br>
+ bus->get_clk_reg_val = aspeed_i2c_24xx_get_clk_reg_<wbr>val;<br>
+ else<br>
+ bus->get_clk_reg_val = match->data;<br>
+<br>
/* Initialize the I2C adapter */<br>
spin_lock_init(&bus->lock);<br>
init_completion(&bus->cmd_<wbr>complete);<br>
@@ -869,13 +914,6 @@ static int aspeed_i2c_remove_bus(struct platform_device *pdev)<br>
return 0;<br>
}<br>
<br>
-static const struct of_device_id aspeed_i2c_bus_of_table[] = {<br>
- { .compatible = "aspeed,ast2400-i2c-bus", },<br>
- { .compatible = "aspeed,ast2500-i2c-bus", },<br>
- { },<br>
-};<br>
-MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);<br>
-<br>
static struct platform_driver aspeed_i2c_bus_driver = {<br>
.probe = aspeed_i2c_probe_bus,<br>
.remove = aspeed_i2c_remove_bus,<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.14.0.rc0.400.g1c36432dff-<wbr>goog<br>
<br>
</font></span></blockquote></div><br></div>