[PATCH 0/4] i2c: mpc: Clean up clock selection

[PATCH 0/4] i2c: mpc: Clean up clock selection

[Arseny Solokha]

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.

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.0

[PATCH 1/4] i2c: mpc: get MPC8xxx I2C clock prescaler before using it in calculations

[Arseny Solokha]

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 <asolokha@kb.kras.ru>
---
 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 96caf378b1dc..bf0c86d41f1a 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.0

[PATCH 2/4] i2c: mpc: unify obtaining the MPC8533/44 I2C clock prescaler w/ MPC8xxx

[Arseny Solokha]

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 <asolokha@kb.kras.ru>
---
 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 bf0c86d41f1a..f47916466b82 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.0

[PATCH 3/4] i2c: mpc: fix PORDEVSR2 mask for MPC8533/44

[Arseny Solokha]

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 <asolokha@kb.kras.ru>
---
 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 f47916466b82..8d60db0080f6 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.0

[PATCH 4/4] i2c: mpc: always determine I2C clock prescaler at runtime

[Arseny Solokha]

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 <asolokha@kb.kras.ru>
---
 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 8d60db0080f6..e0f059687c2d 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.0

[PATCH RESEND 0/4] i2c: mpc: Clean up clock selection

[Arseny Solokha]

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

[PATCH RESEND 1/4] i2c: mpc: get MPC8xxx I2C clock prescaler before using it in calculations

[Arseny Solokha]

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 <asolokha@kb.kras.ru>
---
 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

Re: [RESEND, 1/4] i2c: mpc: get MPC8xxx I2C clock prescaler before using it in calculations

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 727 bytes --]

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 <asolokha@kb.kras.ru>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RESEND, 1/4] i2c: mpc: get MPC8xxx I2C clock prescaler before using it in calculations

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 861 bytes --]

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 <asolokha@kb.kras.ru>
> 
> Applied to for-current, thanks!

Reconsidered and moved to for-next.



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH RESEND 2/4] i2c: mpc: unify obtaining the MPC8533/44 I2C clock prescaler w/ MPC8xxx

[Arseny Solokha]

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 <asolokha@kb.kras.ru>
---
 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

Re: [RESEND, 2/4] i2c: mpc: unify obtaining the MPC8533/44 I2C clock prescaler w/ MPC8xxx

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 898 bytes --]

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 <asolokha@kb.kras.ru>

Looks good to me, but I have comments to patches 3+4.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RESEND, 2/4] i2c: mpc: unify obtaining the MPC8533/44 I2C clock prescaler w/ MPC8xxx

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 873 bytes --]

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 <asolokha@kb.kras.ru>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH RESEND 3/4] i2c: mpc: fix PORDEVSR2 mask for MPC8533/44

[Arseny Solokha]

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 <asolokha@kb.kras.ru>
---
 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

Re: [RESEND,3/4] i2c: mpc: fix PORDEVSR2 mask for MPC8533/44

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]


>  			/*
>  			 * 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?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RESEND,3/4] i2c: mpc: fix PORDEVSR2 mask for MPC8533/44

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 482 bytes --]

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 <asolokha@kb.kras.ru>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH RESEND 4/4] i2c: mpc: always determine I2C clock prescaler at runtime

[Arseny Solokha]

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 <asolokha@kb.kras.ru>
---
 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

Re: [RESEND, 4/4] i2c: mpc: always determine I2C clock prescaler at runtime

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]


>  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.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RESEND, 4/4] i2c: mpc: always determine I2C clock prescaler at runtime

[Arseny Solokha]

>>  			/*
>>  			 * 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

[PATCH v2 4/4] i2c: mpc: always determine I2C clock prescaler at runtime

[Arseny Solokha]

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 <asolokha@kb.kras.ru>
---
 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

Re: [PATCH v2 4/4] i2c: mpc: always determine I2C clock prescaler at runtime

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

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 <asolokha@kb.kras.ru>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]