[PATCH] mx6: peripheral clock from oscillator

[PATCH] mx6: peripheral clock from oscillator

[Jorge Ramirez-Ortiz]

In order to be able to run the I2C bus at 400Khz, the chip errata[1]
recommends that the peripheral clock runs out of the 24MHz oscillator.

[1] Rev 2, 10/2019, ERR007805

Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
---
 arch/arm/mach-imx/mx6/soc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
index e129286065..1da250bbcd 100644
--- a/arch/arm/mach-imx/mx6/soc.c
+++ b/arch/arm/mach-imx/mx6/soc.c
@@ -469,7 +469,7 @@ int arch_cpu_init(void)
 	}
 
 	/* Set perclk to source from OSC 24MHz */
-	if (is_mx6sl())
+	if (is_mx6sl() || is_mx6ull())
 		setbits_le32(&ccm->cscmr1, MXC_CCM_CSCMR1_PER_CLK_SEL_MASK);
 
 	imx_wdog_disable_powerdown(); /* Disable PDE bit of WMCR register */
-- 
2.17.1

[PATCH] mx6: peripheral clock from oscillator

[Jorge]

any thoughts?

On 14/10/20, Jorge Ramirez-Ortiz wrote:
> In order to be able to run the I2C bus at 400Khz, the chip errata[1]
> recommends that the peripheral clock runs out of the 24MHz oscillator.
> 
> [1] Rev 2, 10/2019, ERR007805
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> ---
>  arch/arm/mach-imx/mx6/soc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
> index e129286065..1da250bbcd 100644
> --- a/arch/arm/mach-imx/mx6/soc.c
> +++ b/arch/arm/mach-imx/mx6/soc.c
> @@ -469,7 +469,7 @@ int arch_cpu_init(void)
>  	}
>  
>  	/* Set perclk to source from OSC 24MHz */
> -	if (is_mx6sl())
> +	if (is_mx6sl() || is_mx6ull())
>  		setbits_le32(&ccm->cscmr1, MXC_CCM_CSCMR1_PER_CLK_SEL_MASK);
>  
>  	imx_wdog_disable_powerdown(); /* Disable PDE bit of WMCR register */
> -- 
> 2.17.1
> 

[PATCH] mx6: peripheral clock from oscillator

[Peng Fan]

> Subject: [PATCH] mx6: peripheral clock from oscillator
> 
> In order to be able to run the I2C bus at 400Khz, the chip errata[1]
> recommends that the peripheral clock runs out of the 24MHz oscillator.
> 
> [1] Rev 2, 10/2019, ERR007805
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> ---
>  arch/arm/mach-imx/mx6/soc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
> index e129286065..1da250bbcd 100644
> --- a/arch/arm/mach-imx/mx6/soc.c
> +++ b/arch/arm/mach-imx/mx6/soc.c
> @@ -469,7 +469,7 @@ int arch_cpu_init(void)
>  	}
> 
>  	/* Set perclk to source from OSC 24MHz */
> -	if (is_mx6sl())
> +	if (is_mx6sl() || is_mx6ull())
>  		setbits_le32(&ccm->cscmr1,
> MXC_CCM_CSCMR1_PER_CLK_SEL_MASK);
> 
>  	imx_wdog_disable_powerdown(); /* Disable PDE bit of WMCR register
> */
> --

Reviewed-by: Peng Fan <peng.fan@nxp.com>

[PATCH] mx6: peripheral clock from oscillator

[Fabio Estevam]

Hi Jorge,

On Wed, Oct 14, 2020 at 10:07 AM Jorge Ramirez-Ortiz <jorge@foundries.io> wrote:
>
> In order to be able to run the I2C bus at 400Khz, the chip errata[1]
> recommends that the peripheral clock runs out of the 24MHz oscillator.
>
> [1] Rev 2, 10/2019, ERR007805
>
> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> ---
>  arch/arm/mach-imx/mx6/soc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
> index e129286065..1da250bbcd 100644
> --- a/arch/arm/mach-imx/mx6/soc.c
> +++ b/arch/arm/mach-imx/mx6/soc.c
> @@ -469,7 +469,7 @@ int arch_cpu_init(void)
>         }
>
>         /* Set perclk to source from OSC 24MHz */
> -       if (is_mx6sl())
> +       if (is_mx6sl() || is_mx6ull())

According to https://www.nxp.com/docs/en/errata/IMX6SDLCE.pdf the
mx6solo and mx6dl are also affected by ERR007805.

These variants should be handled too.

[PATCH] mx6: peripheral clock from oscillator

[Jorge]

On 19/10/20, Fabio Estevam wrote:
> Hi Jorge,
> 
> On Wed, Oct 14, 2020 at 10:07 AM Jorge Ramirez-Ortiz <jorge@foundries.io> wrote:
> >
> > In order to be able to run the I2C bus at 400Khz, the chip errata[1]
> > recommends that the peripheral clock runs out of the 24MHz oscillator.
> >
> > [1] Rev 2, 10/2019, ERR007805
> >
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> > ---
> >  arch/arm/mach-imx/mx6/soc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
> > index e129286065..1da250bbcd 100644
> > --- a/arch/arm/mach-imx/mx6/soc.c
> > +++ b/arch/arm/mach-imx/mx6/soc.c
> > @@ -469,7 +469,7 @@ int arch_cpu_init(void)
> >         }
> >
> >         /* Set perclk to source from OSC 24MHz */
> > -       if (is_mx6sl())
> > +       if (is_mx6sl() || is_mx6ull())
> 
> According to https://www.nxp.com/docs/en/errata/IMX6SDLCE.pdf the
> mx6solo and mx6dl are also affected by ERR007805.

ah ok, I'll repost.

just as background info, this is needed in order for OP-TEE to access
the EdgeLock SE050 secure element at a decent speed. I couldnt merge
the support for the imx6ul i2c driver during the last OP-TEE release
but will try again when I find some time (surely before their next
release)


> 
> These variants should be handled too.

[PATCH] mx6: peripheral clock from oscillator

[Jorge]

On 19/10/20, Jorge Ramirez-Ortiz, Foundries wrote:
> On 19/10/20, Fabio Estevam wrote:
> > Hi Jorge,
> > 
> > On Wed, Oct 14, 2020 at 10:07 AM Jorge Ramirez-Ortiz <jorge@foundries.io> wrote:
> > >
> > > In order to be able to run the I2C bus at 400Khz, the chip errata[1]
> > > recommends that the peripheral clock runs out of the 24MHz oscillator.
> > >
> > > [1] Rev 2, 10/2019, ERR007805
> > >
> > > Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> > > ---
> > >  arch/arm/mach-imx/mx6/soc.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
> > > index e129286065..1da250bbcd 100644
> > > --- a/arch/arm/mach-imx/mx6/soc.c
> > > +++ b/arch/arm/mach-imx/mx6/soc.c
> > > @@ -469,7 +469,7 @@ int arch_cpu_init(void)
> > >         }
> > >
> > >         /* Set perclk to source from OSC 24MHz */
> > > -       if (is_mx6sl())
> > > +       if (is_mx6sl() || is_mx6ull())
> > 
> > According to https://www.nxp.com/docs/en/errata/IMX6SDLCE.pdf the
> > mx6solo and mx6dl are also affected by ERR007805.
> 
> ah ok, I'll repost.
> 
> just as background info, this is needed in order for OP-TEE to access
> the EdgeLock SE050 secure element at a decent speed. I couldnt merge
> the support for the imx6ul i2c driver during the last OP-TEE release
> but will try again when I find some time (surely before their next
> release)
> 
> 
> > 
> > These variants should be handled too.

I just reposted but realized I forgot to ammend the patch version. let
me know if you would like me to repost with that corrected or if that
is ok.

[PATCH] mx6: peripheral clock from oscillator

[Jorge Ramirez-Ortiz]

In order to be able to run the I2C bus at 400Khz, the chip errata[1]
recommends that the peripheral clock runs out of the 24MHz oscillator.

[1] Rev 2, 10/2019, ERR007805

Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
---
 arch/arm/mach-imx/mx6/soc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
index e129286065..f498c93b00 100644
--- a/arch/arm/mach-imx/mx6/soc.c
+++ b/arch/arm/mach-imx/mx6/soc.c
@@ -26,6 +26,8 @@
 #include <imx_thermal.h>
 #include <mmc.h>
 
+#define ERRATA_ERR007805 (is_mx6dl() || is_mx6solo() || is_mx6ull())
+
 struct scu_regs {
 	u32	ctrl;
 	u32	config;
@@ -469,7 +471,7 @@ int arch_cpu_init(void)
 	}
 
 	/* Set perclk to source from OSC 24MHz */
-	if (is_mx6sl())
+	if (is_mx6sl() || ERRATA_ERR007805)
 		setbits_le32(&ccm->cscmr1, MXC_CCM_CSCMR1_PER_CLK_SEL_MASK);
 
 	imx_wdog_disable_powerdown(); /* Disable PDE bit of WMCR register */
-- 
2.17.1

[PATCH] mx6: peripheral clock from oscillator

[Jorge]

On 19/10/20, Jorge Ramirez-Ortiz wrote:
> In order to be able to run the I2C bus at 400Khz, the chip errata[1]
> recommends that the peripheral clock runs out of the 24MHz oscillator.
> 
> [1] Rev 2, 10/2019, ERR007805
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> ---
>  arch/arm/mach-imx/mx6/soc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
> index e129286065..f498c93b00 100644
> --- a/arch/arm/mach-imx/mx6/soc.c
> +++ b/arch/arm/mach-imx/mx6/soc.c
> @@ -26,6 +26,8 @@
>  #include <imx_thermal.h>
>  #include <mmc.h>
>  
> +#define ERRATA_ERR007805 (is_mx6dl() || is_mx6solo() || is_mx6ull())
> +
>  struct scu_regs {
>  	u32	ctrl;
>  	u32	config;
> @@ -469,7 +471,7 @@ int arch_cpu_init(void)
>  	}
>  
>  	/* Set perclk to source from OSC 24MHz */
> -	if (is_mx6sl())
> +	if (is_mx6sl() || ERRATA_ERR007805)
>  		setbits_le32(&ccm->cscmr1, MXC_CCM_CSCMR1_PER_CLK_SEL_MASK);
>  
>  	imx_wdog_disable_powerdown(); /* Disable PDE bit of WMCR register */
> --


all ok with this ?

> 2.17.1
> 

[PATCH] mx6: peripheral clock from oscillator

[Fabio Estevam]

On Mon, Oct 19, 2020 at 11:23 AM Jorge Ramirez-Ortiz <jorge@foundries.io> wrote:
>
> In order to be able to run the I2C bus at 400Khz, the chip errata[1]
> recommends that the peripheral clock runs out of the 24MHz oscillator.

I would suggest adding the optee related motivation that you explained
earlier here in the commit log too.

> [1] Rev 2, 10/2019, ERR007805

Not clear what document this refers to.

>         /* Set perclk to source from OSC 24MHz */
> -       if (is_mx6sl())
> +       if (is_mx6sl() || ERRATA_ERR007805)

This gives the impression that imx6sl is not affected by the erratum,
but in fact it is:
https://www.nxp.com/docs/en/errata/IMX6SLCE.pdf

You could define it like this:

#define has_err007805() (is_mx6sl() || is_mx6dl() || is_mx6solo() ||
is_mx6ull())

[PATCH] mx6: peripheral clock from oscillator

[Jorge]

On 22/10/20, Fabio Estevam wrote:
> On Mon, Oct 19, 2020 at 11:23 AM Jorge Ramirez-Ortiz <jorge@foundries.io> wrote:
> >
> > In order to be able to run the I2C bus at 400Khz, the chip errata[1]
> > recommends that the peripheral clock runs out of the 24MHz oscillator.
> 
> I would suggest adding the optee related motivation that you explained
> earlier here in the commit log too.

sure, will do.

> 
> > [1] Rev 2, 10/2019, ERR007805
> 
> Not clear what document this refers to.

um, weird, I just googled "imx Rev 2, 10/2019, ERR007805" and it showed
the document and a link to the errata.

how do you tipically do this otherwise? I didnt want to depend on
links that migh break in the future.

> 
> >         /* Set perclk to source from OSC 24MHz */
> > -       if (is_mx6sl())
> > +       if (is_mx6sl() || ERRATA_ERR007805)
> 
> This gives the impression that imx6sl is not affected by the erratum,
> but in fact it is:
> https://www.nxp.com/docs/en/errata/IMX6SLCE.pdf
> 
> You could define it like this:
> 
> #define has_err007805() (is_mx6sl() || is_mx6dl() || is_mx6solo() ||
> is_mx6ull())


sounds good. will do (I didnt know the reason for the mx6sl configuring the
clock was that same errata)

[PATCH] mx6: peripheral clock from oscillator

[Fabio Estevam]

On Thu, Oct 22, 2020 at 6:24 PM Jorge Ramirez-Ortiz, Foundries
<jorge@foundries.io> wrote:

> um, weird, I just googled "imx Rev 2, 10/2019, ERR007805" and it showed
> the document and a link to the errata.

The errata document is specific for a SoC.

It does not help to add "Rev 2, 10/2019, ERR007805" in the commit log
as we cannot know which SoC it refers to.

Just mention the SoC errata document. For example: IMX6SLCE Rev. 5, 02/2019

[PATCH] mx6: peripheral clock from oscillator

[Jorge]

On 22/10/20, Fabio Estevam wrote:
> On Thu, Oct 22, 2020 at 6:24 PM Jorge Ramirez-Ortiz, Foundries
> <jorge@foundries.io> wrote:
> 
> > um, weird, I just googled "imx Rev 2, 10/2019, ERR007805" and it showed
> > the document and a link to the errata.
> 
> The errata document is specific for a SoC.

perfect. now you understand my point and why I was being generic.

> 
> It does not help to add "Rev 2, 10/2019, ERR007805" in the commit log
> as we cannot know which SoC it refers to.

um, weird....the patch indicates imx6 in the name. this should be enough no?

> 
> Just mention the SoC errata document. For example: IMX6SLCE Rev. 5, 02/2019

so you need the names and versions of the errata document for each SoC being affected?
that being the case, should it be a patch per family or a patch per SoC?

[PATCH] mx6: peripheral clock from oscillator

[Fabio Estevam]

On Fri, Oct 23, 2020 at 3:35 AM Jorge Ramirez-Ortiz, Foundries
<jorge@foundries.io> wrote:

> um, weird....the patch indicates imx6 in the name. this should be enough no?

imx6 is too generic. Which imx6? imx6sx, imx6sl, imx6solo, imx6dl,
imx6qp, imx6q, imx6ul, imx6ull, imx6ulz?

It is OK to keep mx6 in the Subject though.

> so you need the names and versions of the errata document for each SoC being affected?
> that being the case, should it be a patch per family or a patch per SoC?

No, my point was just to replace:

[1] Rev 2, 10/2019, ERR007805 (this is not a doc reference)

by a real errata document reference. It can be imx6ull, imx6sl or any
other SoC errata document like, for example:

IMX6SLCE Rev. 5, 02/2019