This series cleans up I2C clock selection for Freescale/NXP MPC SoCs during
the controller initialization for cases when clock settings are not to be
preserved from the bootloader.
Patch 1/4 fixes division by zero which happens during controller
initialization when (1) clock frequency is not specified in the Device
Tree, (2) preservation of clock settings from the bootloader is not
requested, and (3) the clock prescaler (which may actually depend
on the POR configuration) is not explicitly specified. It simply moves
obtaining the prescaler value before the clock computation.
Patch 2/4 unifies obtaining the prescaler value for MPC8544 with other
SoCs. It moves the relevant code to the helper function introduced
in commit 8ce795cb0c6b ("i2c: mpc: assign the correct prescaler from SVR")
and also adds handling of MPC8533 is similar to MPC8544 in this regard.
Patch 3/4 fixes checking the relevant bit in a controller's register used
for selecting the prescaler value for MPC8533 and MPC8544.
Patch 4/4 removes the facility for setting the clock prescaler value
at compile time. This facility is not used in the majority of cases. Getting
the prescaler value at run time currently covers more SoCs. Hardcoding it
is also wrong for some SoCs as it can be configured on board during POR.
This series is an exact copy of [1] which has received no feedback so far.
[1] https://marc.info/?l=linux-i2c&m=151030076114573&w=2
Arseny Solokha (4):
i2c: mpc: get MPC8xxx I2C clock prescaler before using it in
calculations
i2c: mpc: unify obtaining the MPC8533/44 I2C clock prescaler w/
MPC8xxx
i2c: mpc: fix PORDEVSR2 mask for MPC8533/44
i2c: mpc: always determine I2C clock prescaler at runtime
drivers/i2c/busses/i2c-mpc.c | 72 ++++++++++++++++++--------------------------
1 file changed, 30 insertions(+), 42 deletions(-)
--
2.15.1
Commit 8ce795cb0c6b ("i2c: mpc: assign the correct prescaler from SVR")
introduced the common helper function for obtaining the actual clock
prescaler value for MPC85xx. However, getting the prescaler for MPC8544
which depends on the SEC frequency ratio on this platform, has been always
performed separately based on the corresponding Device Tree configuration.
Move special handling of MPC8544 into that common helper. Make it dependent
on the SoC version and not on Device Tree compatible node, as is the case
with all other SoCs. Handle MPC8533 the same way which is similar
to MPC8544 in this regard, according to AN2919 "Determining the I2C
Frequency Divider Ratio for SCL".
Signed-off-by: Arseny Solokha <[email protected]>
---
drivers/i2c/busses/i2c-mpc.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 6ad87555f71e..648a5afded64 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -350,7 +350,11 @@ static u32 mpc_i2c_get_sec_cfg_8xxx(void)
static u32 mpc_i2c_get_prescaler_8xxx(void)
{
- /* mpc83xx and mpc82xx all have prescaler 1 */
+ /*
+ * According to the AN2919 all MPC824x have prescaler 1, while MPC83xx
+ * may have prescaler 1, 2, or 3, depending on the power-on
+ * configuration.
+ */
u32 prescaler = 1;
/* mpc85xx */
@@ -367,6 +371,10 @@ static u32 mpc_i2c_get_prescaler_8xxx(void)
|| (SVR_SOC_VER(svr) == SVR_8610))
/* the above 85xx SoCs have prescaler 1 */
prescaler = 1;
+ else if ((SVR_SOC_VER(svr) == SVR_8533)
+ || (SVR_SOC_VER(svr) == SVR_8544))
+ /* the above 85xx SoCs have prescaler 3 or 2 */
+ prescaler = mpc_i2c_get_sec_cfg_8xxx() ? 3 : 2;
else
/* all the other 85xx have prescaler 2 */
prescaler = 2;
@@ -383,8 +391,6 @@ static int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
int i;
/* Determine proper divider value */
- if (of_device_is_compatible(node, "fsl,mpc8544-i2c"))
- prescaler = mpc_i2c_get_sec_cfg_8xxx() ? 3 : 2;
if (!prescaler)
prescaler = mpc_i2c_get_prescaler_8xxx();
--
2.15.1
Obtaining the actual I2C clock prescaler value in mpc_i2c_setup_8xxx() only
happens when the clock parameter is set to something other than
MPC_I2C_CLOCK_LEGACY. When the clock parameter is exactly
MPC_I2C_CLOCK_LEGACY, the prescaler parameter is used in arithmetic
division as provided by the caller, resulting in a division by zero
for the majority of processors supported by the module.
Avoid division by zero by obtaining the actual I2C clock prescaler
in mpc_i2c_setup_8xxx() unconditionally regardless of the passed clock
value.
Signed-off-by: Arseny Solokha <[email protected]>
---
drivers/i2c/busses/i2c-mpc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 950a9d74f54d..6ad87555f71e 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -382,18 +382,18 @@ static int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
u32 divider;
int i;
- if (clock == MPC_I2C_CLOCK_LEGACY) {
- /* see below - default fdr = 0x1031 -> div = 16 * 3072 */
- *real_clk = fsl_get_sys_freq() / prescaler / (16 * 3072);
- return -EINVAL;
- }
-
/* Determine proper divider value */
if (of_device_is_compatible(node, "fsl,mpc8544-i2c"))
prescaler = mpc_i2c_get_sec_cfg_8xxx() ? 3 : 2;
if (!prescaler)
prescaler = mpc_i2c_get_prescaler_8xxx();
+ if (clock == MPC_I2C_CLOCK_LEGACY) {
+ /* see below - default fdr = 0x1031 -> div = 16 * 3072 */
+ *real_clk = fsl_get_sys_freq() / prescaler / (16 * 3072);
+ return -EINVAL;
+ }
+
divider = fsl_get_sys_freq() / clock / prescaler;
pr_debug("I2C: src_clock=%d clock=%d divider=%d\n",
--
2.15.1
According to the reference manuals for the corresponding SoCs, SEC
frequency ratio configuration is indicated by bit 26 of the POR Device
Status Register 2. Consequently, SEC_CFG bit should be tested by mask 0x20,
not 0x80. Testing the wrong bit leads to selection of wrong I2C clock
prescaler on those SoCs.
Signed-off-by: Arseny Solokha <[email protected]>
---
drivers/i2c/busses/i2c-mpc.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 648a5afded64..aac0ec6dc5fc 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -332,14 +332,18 @@ static u32 mpc_i2c_get_sec_cfg_8xxx(void)
if (prop) {
/*
* Map and check POR Device Status Register 2
- * (PORDEVSR2) at 0xE0014
+ * (PORDEVSR2) at 0xE0014. Note than while MPC8533
+ * and MPC8544 indicate SEC frequency ratio
+ * configuration as bit 26 in PORDEVSR2, other MPC8xxx
+ * parts may store it differently or may not have it
+ * at all.
*/
reg = ioremap(get_immrbase() + *prop + 0x14, 0x4);
if (!reg)
printk(KERN_ERR
"Error: couldn't map PORDEVSR2\n");
else
- val = in_be32(reg) & 0x00000080; /* sec-cfg */
+ val = in_be32(reg) & 0x00000020; /* sec-cfg */
iounmap(reg);
}
}
--
2.15.1
Remove the facility for setting the prescaler value at compile time
entirely. It was only used for two SoCs, duplicating the actual value
for one of them and setting sometimes bogus value for another. Make all
MPC8xxx SoCs obtain their actual I2C clock prescaler from a single place
in the code.
Signed-off-by: Arseny Solokha <[email protected]>
---
drivers/i2c/busses/i2c-mpc.c | 52 +++++++++++++-------------------------------
1 file changed, 15 insertions(+), 37 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index aac0ec6dc5fc..ad9af3ca35aa 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -78,9 +78,7 @@ struct mpc_i2c_divider {
};
struct mpc_i2c_data {
- void (*setup)(struct device_node *node, struct mpc_i2c *i2c,
- u32 clock, u32 prescaler);
- u32 prescaler;
+ void (*setup)(struct device_node *node, struct mpc_i2c *i2c, u32 clock);
};
static inline void writeccr(struct mpc_i2c *i2c, u32 x)
@@ -201,7 +199,7 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
};
static int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock,
- int prescaler, u32 *real_clk)
+ u32 *real_clk)
{
const struct mpc_i2c_divider *div = NULL;
unsigned int pvr = mfspr(SPRN_PVR);
@@ -236,7 +234,7 @@ static int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock,
static void mpc_i2c_setup_52xx(struct device_node *node,
struct mpc_i2c *i2c,
- u32 clock, u32 prescaler)
+ u32 clock)
{
int ret, fdr;
@@ -246,7 +244,7 @@ static void mpc_i2c_setup_52xx(struct device_node *node,
return;
}
- ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler, &i2c->real_clk);
+ ret = mpc_i2c_get_fdr_52xx(node, clock, &i2c->real_clk);
fdr = (ret >= 0) ? ret : 0x3f; /* backward compatibility */
writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR);
@@ -258,7 +256,7 @@ static void mpc_i2c_setup_52xx(struct device_node *node,
#else /* !(CONFIG_PPC_MPC52xx || CONFIG_PPC_MPC512x) */
static void mpc_i2c_setup_52xx(struct device_node *node,
struct mpc_i2c *i2c,
- u32 clock, u32 prescaler)
+ u32 clock)
{
}
#endif /* CONFIG_PPC_MPC52xx || CONFIG_PPC_MPC512x */
@@ -266,7 +264,7 @@ static void mpc_i2c_setup_52xx(struct device_node *node,
#ifdef CONFIG_PPC_MPC512x
static void mpc_i2c_setup_512x(struct device_node *node,
struct mpc_i2c *i2c,
- u32 clock, u32 prescaler)
+ u32 clock)
{
struct device_node *node_ctrl;
void __iomem *ctrl;
@@ -289,12 +287,12 @@ static void mpc_i2c_setup_512x(struct device_node *node,
}
/* The clock setup for the 52xx works also fine for the 512x */
- mpc_i2c_setup_52xx(node, i2c, clock, prescaler);
+ mpc_i2c_setup_52xx(node, i2c, clock);
}
#else /* CONFIG_PPC_MPC512x */
static void mpc_i2c_setup_512x(struct device_node *node,
struct mpc_i2c *i2c,
- u32 clock, u32 prescaler)
+ u32 clock)
{
}
#endif /* CONFIG_PPC_MPC512x */
@@ -388,16 +386,13 @@ static u32 mpc_i2c_get_prescaler_8xxx(void)
}
static int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
- u32 prescaler, u32 *real_clk)
+ u32 *real_clk)
{
const struct mpc_i2c_divider *div = NULL;
+ u32 prescaler = mpc_i2c_get_prescaler_8xxx();
u32 divider;
int i;
- /* Determine proper divider value */
- if (!prescaler)
- prescaler = mpc_i2c_get_prescaler_8xxx();
-
if (clock == MPC_I2C_CLOCK_LEGACY) {
/* see below - default fdr = 0x1031 -> div = 16 * 3072 */
*real_clk = fsl_get_sys_freq() / prescaler / (16 * 3072);
@@ -425,7 +420,7 @@ static int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
static void mpc_i2c_setup_8xxx(struct device_node *node,
struct mpc_i2c *i2c,
- u32 clock, u32 prescaler)
+ u32 clock)
{
int ret, fdr;
@@ -436,7 +431,7 @@ static void mpc_i2c_setup_8xxx(struct device_node *node,
return;
}
- ret = mpc_i2c_get_fdr_8xxx(node, clock, prescaler, &i2c->real_clk);
+ ret = mpc_i2c_get_fdr_8xxx(node, clock, &i2c->real_clk);
fdr = (ret >= 0) ? ret : 0x1031; /* backward compatibility */
writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR);
@@ -450,7 +445,7 @@ static void mpc_i2c_setup_8xxx(struct device_node *node,
#else /* !CONFIG_FSL_SOC */
static void mpc_i2c_setup_8xxx(struct device_node *node,
struct mpc_i2c *i2c,
- u32 clock, u32 prescaler)
+ u32 clock)
{
}
#endif /* CONFIG_FSL_SOC */
@@ -721,11 +716,11 @@ static int fsl_i2c_probe(struct platform_device *op)
if (match->data) {
const struct mpc_i2c_data *data = match->data;
- data->setup(op->dev.of_node, i2c, clock, data->prescaler);
+ data->setup(op->dev.of_node, i2c, clock);
} else {
/* Backwards compatibility */
if (of_get_property(op->dev.of_node, "dfsrr", NULL))
- mpc_i2c_setup_8xxx(op->dev.of_node, i2c, clock, 0);
+ mpc_i2c_setup_8xxx(op->dev.of_node, i2c, clock);
}
prop = of_get_property(op->dev.of_node, "fsl,timeout", &plen);
@@ -817,28 +812,11 @@ static const struct mpc_i2c_data mpc_i2c_data_52xx = {
.setup = mpc_i2c_setup_52xx,
};
-static const struct mpc_i2c_data mpc_i2c_data_8313 = {
- .setup = mpc_i2c_setup_8xxx,
-};
-
-static const struct mpc_i2c_data mpc_i2c_data_8543 = {
- .setup = mpc_i2c_setup_8xxx,
- .prescaler = 2,
-};
-
-static const struct mpc_i2c_data mpc_i2c_data_8544 = {
- .setup = mpc_i2c_setup_8xxx,
- .prescaler = 3,
-};
-
static const struct of_device_id mpc_i2c_of_match[] = {
{.compatible = "mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
{.compatible = "fsl,mpc5200b-i2c", .data = &mpc_i2c_data_52xx, },
{.compatible = "fsl,mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
{.compatible = "fsl,mpc5121-i2c", .data = &mpc_i2c_data_512x, },
- {.compatible = "fsl,mpc8313-i2c", .data = &mpc_i2c_data_8313, },
- {.compatible = "fsl,mpc8543-i2c", .data = &mpc_i2c_data_8543, },
- {.compatible = "fsl,mpc8544-i2c", .data = &mpc_i2c_data_8544, },
/* Backward compatibility */
{.compatible = "fsl-i2c", },
{},
--
2.15.1
> /*
> * Map and check POR Device Status Register 2
> - * (PORDEVSR2) at 0xE0014
> + * (PORDEVSR2) at 0xE0014. Note than while MPC8533
> + * and MPC8544 indicate SEC frequency ratio
> + * configuration as bit 26 in PORDEVSR2, other MPC8xxx
> + * parts may store it differently or may not have it
> + * at all.
So given this comment which you added...
> */
> reg = ioremap(get_immrbase() + *prop + 0x14, 0x4);
> if (!reg)
> printk(KERN_ERR
> "Error: couldn't map PORDEVSR2\n");
> else
> - val = in_be32(reg) & 0x00000080; /* sec-cfg */
> + val = in_be32(reg) & 0x00000020; /* sec-cfg */
... are you really sure there is no ancient device which needs the
0x00000080?
> static const struct of_device_id mpc_i2c_of_match[] = {
> {.compatible = "mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
> {.compatible = "fsl,mpc5200b-i2c", .data = &mpc_i2c_data_52xx, },
> {.compatible = "fsl,mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
> {.compatible = "fsl,mpc5121-i2c", .data = &mpc_i2c_data_512x, },
> - {.compatible = "fsl,mpc8313-i2c", .data = &mpc_i2c_data_8313, },
> - {.compatible = "fsl,mpc8543-i2c", .data = &mpc_i2c_data_8543, },
> - {.compatible = "fsl,mpc8544-i2c", .data = &mpc_i2c_data_8544, },
We can't remove compatibles. It may break DTBs. We can change the data
pointer, though.
On Thu, Dec 07, 2017 at 05:20:00PM +0700, Arseny Solokha wrote:
> Obtaining the actual I2C clock prescaler value in mpc_i2c_setup_8xxx() only
> happens when the clock parameter is set to something other than
> MPC_I2C_CLOCK_LEGACY. When the clock parameter is exactly
> MPC_I2C_CLOCK_LEGACY, the prescaler parameter is used in arithmetic
> division as provided by the caller, resulting in a division by zero
> for the majority of processors supported by the module.
>
> Avoid division by zero by obtaining the actual I2C clock prescaler
> in mpc_i2c_setup_8xxx() unconditionally regardless of the passed clock
> value.
>
> Signed-off-by: Arseny Solokha <[email protected]>
Applied to for-current, thanks!
On Thu, Dec 07, 2017 at 05:20:01PM +0700, Arseny Solokha wrote:
> Commit 8ce795cb0c6b ("i2c: mpc: assign the correct prescaler from SVR")
> introduced the common helper function for obtaining the actual clock
> prescaler value for MPC85xx. However, getting the prescaler for MPC8544
> which depends on the SEC frequency ratio on this platform, has been always
> performed separately based on the corresponding Device Tree configuration.
>
> Move special handling of MPC8544 into that common helper. Make it dependent
> on the SoC version and not on Device Tree compatible node, as is the case
> with all other SoCs. Handle MPC8533 the same way which is similar
> to MPC8544 in this regard, according to AN2919 "Determining the I2C
> Frequency Divider Ratio for SCL".
>
> Signed-off-by: Arseny Solokha <[email protected]>
Looks good to me, but I have comments to patches 3+4.
>> /*
>> * Map and check POR Device Status Register 2
>> - * (PORDEVSR2) at 0xE0014
>> + * (PORDEVSR2) at 0xE0014. Note than while MPC8533
>> + * and MPC8544 indicate SEC frequency ratio
>> + * configuration as bit 26 in PORDEVSR2, other MPC8xxx
>> + * parts may store it differently or may not have it
>> + * at all.
>
> So given this comment which you added...
>
>> */
>> reg = ioremap(get_immrbase() + *prop + 0x14, 0x4);
>> if (!reg)
>> printk(KERN_ERR
>> "Error: couldn't map PORDEVSR2\n");
>> else
>> - val = in_be32(reg) & 0x00000080; /* sec-cfg */
>> + val = in_be32(reg) & 0x00000020; /* sec-cfg */
>
> ... are you really sure there is no ancient device which needs the
> 0x00000080?
Various MPC SoCs indeed have different bit layout of
PORDEVSR2. Obviously not all of them indicate SEC frequency ratio
configuration at all, either in PORDEVSR2 or in some other register, as
not all SoCs contain SEC engine. In this regard,
mpc_i2c_get_sec_cfg_8xxx() is poorly named as it in its current form is
only applicable to a few SoCs from the whole MPC8xxx family.
mpc_i2c_get_sec_cfg_8xxx() is only called from the following snippet:
else if ((SVR_SOC_VER(svr) == SVR_8533)
|| (SVR_SOC_VER(svr) == SVR_8544))
/* the above 85xx SoCs have prescaler 3 or 2 */
prescaler = mpc_i2c_get_sec_cfg_8xxx() ? 3 : 2;
Both MPC8533 and MPC8544 do in fact indicate SEC frequency ratio
configuration as bit 26 in PORDEVSR2 according to [1,2], so I've added
the comment as a precaution for future uses. I can also rename the
function to something like mpc_i2c_get_sec_cfg_8533_8544(), which looks
ugly and doesn't scale.
AFAICS, mask 0x00000080 in the code clearly contradicts to what I read
in the docs.
[1] https://www.nxp.com/docs/en/reference-manual/MPC8533ERM.pdf
[2] https://www.nxp.com/docs/en/reference-manual/MPC8544ERM.pdf
Arsény
Remove the facility for setting the prescaler value at compile time
entirely. It was only used for two SoCs, duplicating the actual value
for one of them and setting sometimes bogus value for another. Make all
MPC8xxx SoCs obtain their actual I2C clock prescaler from a single place
in the code.
Changes from v2:
- left Device Tree compatibles in place
Signed-off-by: Arseny Solokha <[email protected]>
---
drivers/i2c/busses/i2c-mpc.c | 37 +++++++++++++++----------------------
1 file changed, 15 insertions(+), 22 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index aac0ec6dc5fc..d94f05c8b8b7 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -78,9 +78,7 @@ struct mpc_i2c_divider {
};
struct mpc_i2c_data {
- void (*setup)(struct device_node *node, struct mpc_i2c *i2c,
- u32 clock, u32 prescaler);
- u32 prescaler;
+ void (*setup)(struct device_node *node, struct mpc_i2c *i2c, u32 clock);
};
static inline void writeccr(struct mpc_i2c *i2c, u32 x)
@@ -201,7 +199,7 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
};
static int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock,
- int prescaler, u32 *real_clk)
+ u32 *real_clk)
{
const struct mpc_i2c_divider *div = NULL;
unsigned int pvr = mfspr(SPRN_PVR);
@@ -236,7 +234,7 @@ static int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock,
static void mpc_i2c_setup_52xx(struct device_node *node,
struct mpc_i2c *i2c,
- u32 clock, u32 prescaler)
+ u32 clock)
{
int ret, fdr;
@@ -246,7 +244,7 @@ static void mpc_i2c_setup_52xx(struct device_node *node,
return;
}
- ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler, &i2c->real_clk);
+ ret = mpc_i2c_get_fdr_52xx(node, clock, &i2c->real_clk);
fdr = (ret >= 0) ? ret : 0x3f; /* backward compatibility */
writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR);
@@ -258,7 +256,7 @@ static void mpc_i2c_setup_52xx(struct device_node *node,
#else /* !(CONFIG_PPC_MPC52xx || CONFIG_PPC_MPC512x) */
static void mpc_i2c_setup_52xx(struct device_node *node,
struct mpc_i2c *i2c,
- u32 clock, u32 prescaler)
+ u32 clock)
{
}
#endif /* CONFIG_PPC_MPC52xx || CONFIG_PPC_MPC512x */
@@ -266,7 +264,7 @@ static void mpc_i2c_setup_52xx(struct device_node *node,
#ifdef CONFIG_PPC_MPC512x
static void mpc_i2c_setup_512x(struct device_node *node,
struct mpc_i2c *i2c,
- u32 clock, u32 prescaler)
+ u32 clock)
{
struct device_node *node_ctrl;
void __iomem *ctrl;
@@ -289,12 +287,12 @@ static void mpc_i2c_setup_512x(struct device_node *node,
}
/* The clock setup for the 52xx works also fine for the 512x */
- mpc_i2c_setup_52xx(node, i2c, clock, prescaler);
+ mpc_i2c_setup_52xx(node, i2c, clock);
}
#else /* CONFIG_PPC_MPC512x */
static void mpc_i2c_setup_512x(struct device_node *node,
struct mpc_i2c *i2c,
- u32 clock, u32 prescaler)
+ u32 clock)
{
}
#endif /* CONFIG_PPC_MPC512x */
@@ -388,16 +386,13 @@ static u32 mpc_i2c_get_prescaler_8xxx(void)
}
static int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
- u32 prescaler, u32 *real_clk)
+ u32 *real_clk)
{
const struct mpc_i2c_divider *div = NULL;
+ u32 prescaler = mpc_i2c_get_prescaler_8xxx();
u32 divider;
int i;
- /* Determine proper divider value */
- if (!prescaler)
- prescaler = mpc_i2c_get_prescaler_8xxx();
-
if (clock == MPC_I2C_CLOCK_LEGACY) {
/* see below - default fdr = 0x1031 -> div = 16 * 3072 */
*real_clk = fsl_get_sys_freq() / prescaler / (16 * 3072);
@@ -425,7 +420,7 @@ static int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
static void mpc_i2c_setup_8xxx(struct device_node *node,
struct mpc_i2c *i2c,
- u32 clock, u32 prescaler)
+ u32 clock)
{
int ret, fdr;
@@ -436,7 +431,7 @@ static void mpc_i2c_setup_8xxx(struct device_node *node,
return;
}
- ret = mpc_i2c_get_fdr_8xxx(node, clock, prescaler, &i2c->real_clk);
+ ret = mpc_i2c_get_fdr_8xxx(node, clock, &i2c->real_clk);
fdr = (ret >= 0) ? ret : 0x1031; /* backward compatibility */
writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR);
@@ -450,7 +445,7 @@ static void mpc_i2c_setup_8xxx(struct device_node *node,
#else /* !CONFIG_FSL_SOC */
static void mpc_i2c_setup_8xxx(struct device_node *node,
struct mpc_i2c *i2c,
- u32 clock, u32 prescaler)
+ u32 clock)
{
}
#endif /* CONFIG_FSL_SOC */
@@ -721,11 +716,11 @@ static int fsl_i2c_probe(struct platform_device *op)
if (match->data) {
const struct mpc_i2c_data *data = match->data;
- data->setup(op->dev.of_node, i2c, clock, data->prescaler);
+ data->setup(op->dev.of_node, i2c, clock);
} else {
/* Backwards compatibility */
if (of_get_property(op->dev.of_node, "dfsrr", NULL))
- mpc_i2c_setup_8xxx(op->dev.of_node, i2c, clock, 0);
+ mpc_i2c_setup_8xxx(op->dev.of_node, i2c, clock);
}
prop = of_get_property(op->dev.of_node, "fsl,timeout", &plen);
@@ -823,12 +818,10 @@ static const struct mpc_i2c_data mpc_i2c_data_8313 = {
static const struct mpc_i2c_data mpc_i2c_data_8543 = {
.setup = mpc_i2c_setup_8xxx,
- .prescaler = 2,
};
static const struct mpc_i2c_data mpc_i2c_data_8544 = {
.setup = mpc_i2c_setup_8xxx,
- .prescaler = 3,
};
static const struct of_device_id mpc_i2c_of_match[] = {
--
2.15.1
On Sat, Dec 30, 2017 at 07:18:14PM +0100, Wolfram Sang wrote:
> On Thu, Dec 07, 2017 at 05:20:00PM +0700, Arseny Solokha wrote:
> > Obtaining the actual I2C clock prescaler value in mpc_i2c_setup_8xxx() only
> > happens when the clock parameter is set to something other than
> > MPC_I2C_CLOCK_LEGACY. When the clock parameter is exactly
> > MPC_I2C_CLOCK_LEGACY, the prescaler parameter is used in arithmetic
> > division as provided by the caller, resulting in a division by zero
> > for the majority of processors supported by the module.
> >
> > Avoid division by zero by obtaining the actual I2C clock prescaler
> > in mpc_i2c_setup_8xxx() unconditionally regardless of the passed clock
> > value.
> >
> > Signed-off-by: Arseny Solokha <[email protected]>
>
> Applied to for-current, thanks!
Reconsidered and moved to for-next.
On Thu, Dec 07, 2017 at 05:20:01PM +0700, Arseny Solokha wrote:
> Commit 8ce795cb0c6b ("i2c: mpc: assign the correct prescaler from SVR")
> introduced the common helper function for obtaining the actual clock
> prescaler value for MPC85xx. However, getting the prescaler for MPC8544
> which depends on the SEC frequency ratio on this platform, has been always
> performed separately based on the corresponding Device Tree configuration.
>
> Move special handling of MPC8544 into that common helper. Make it dependent
> on the SoC version and not on Device Tree compatible node, as is the case
> with all other SoCs. Handle MPC8533 the same way which is similar
> to MPC8544 in this regard, according to AN2919 "Determining the I2C
> Frequency Divider Ratio for SCL".
>
> Signed-off-by: Arseny Solokha <[email protected]>
Applied to for-next, thanks!
On Thu, Dec 07, 2017 at 05:20:02PM +0700, Arseny Solokha wrote:
> According to the reference manuals for the corresponding SoCs, SEC
> frequency ratio configuration is indicated by bit 26 of the POR Device
> Status Register 2. Consequently, SEC_CFG bit should be tested by mask 0x20,
> not 0x80. Testing the wrong bit leads to selection of wrong I2C clock
> prescaler on those SoCs.
>
> Signed-off-by: Arseny Solokha <[email protected]>
Applied to for-next, thanks!
On Wed, Jan 10, 2018 at 06:36:07PM +0700, Arseny Solokha wrote:
> Remove the facility for setting the prescaler value at compile time
> entirely. It was only used for two SoCs, duplicating the actual value
> for one of them and setting sometimes bogus value for another. Make all
> MPC8xxx SoCs obtain their actual I2C clock prescaler from a single place
> in the code.
>
> Changes from v2:
> - left Device Tree compatibles in place
>
> Signed-off-by: Arseny Solokha <[email protected]>
Applied to for-next, thanks!