diff mbox

[v4,08/10] ARM: mxs: add ocotp read function

Message ID 1294297998-26930-9-git-send-email-shawn.guo@freescale.com
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Shawn Guo Jan. 6, 2011, 7:13 a.m. UTC
Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
Changes for v4:
 - Call cpu_relax() during polling

Changes for v2:
 - Add mutex locking for mxs_read_ocotp()
 - Use type size_t for count and i
 - Add comment for clk_enable/disable skipping
 - Add ERROR bit clearing and polling step

 arch/arm/mach-mxs/Makefile              |    2 +-
 arch/arm/mach-mxs/include/mach/common.h |    1 +
 arch/arm/mach-mxs/ocotp.c               |   79 +++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-mxs/ocotp.c

Comments

Sascha Hauer Jan. 11, 2011, 1:31 p.m. UTC | #1
On Thu, Jan 06, 2011 at 03:13:16PM +0800, Shawn Guo wrote:
> Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> ---
> Changes for v4:
>  - Call cpu_relax() during polling
> 
> Changes for v2:
>  - Add mutex locking for mxs_read_ocotp()
>  - Use type size_t for count and i
>  - Add comment for clk_enable/disable skipping
>  - Add ERROR bit clearing and polling step
> 
>  arch/arm/mach-mxs/Makefile              |    2 +-
>  arch/arm/mach-mxs/include/mach/common.h |    1 +
>  arch/arm/mach-mxs/ocotp.c               |   79 +++++++++++++++++++++++++++++++
>  3 files changed, 81 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arm/mach-mxs/ocotp.c
> 
> diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
> index 39d3f9c..f23ebbd 100644
> --- a/arch/arm/mach-mxs/Makefile
> +++ b/arch/arm/mach-mxs/Makefile
> @@ -1,5 +1,5 @@
>  # Common support
> -obj-y := clock.o devices.o gpio.o icoll.o iomux.o system.o timer.o
> +obj-y := clock.o devices.o gpio.o icoll.o iomux.o ocotp.o system.o timer.o
>  
>  obj-$(CONFIG_SOC_IMX23) += clock-mx23.o mm-mx23.o
>  obj-$(CONFIG_SOC_IMX28) += clock-mx28.o mm-mx28.o
> diff --git a/arch/arm/mach-mxs/include/mach/common.h b/arch/arm/mach-mxs/include/mach/common.h
> index 59133eb..cf02552 100644
> --- a/arch/arm/mach-mxs/include/mach/common.h
> +++ b/arch/arm/mach-mxs/include/mach/common.h
> @@ -13,6 +13,7 @@
>  
>  struct clk;
>  
> +extern int mxs_read_ocotp(int offset, int count, u32 *values);
>  extern int mxs_reset_block(void __iomem *);
>  extern void mxs_timer_init(struct clk *, int);
>  
> diff --git a/arch/arm/mach-mxs/ocotp.c b/arch/arm/mach-mxs/ocotp.c
> new file mode 100644
> index 0000000..e2d39aa
> --- /dev/null
> +++ b/arch/arm/mach-mxs/ocotp.c
> @@ -0,0 +1,79 @@
> +/*
> + * Copyright 2010 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +
> +#include <mach/mxs.h>
> +
> +#define BM_OCOTP_CTRL_BUSY		(1 << 8)
> +#define BM_OCOTP_CTRL_ERROR		(1 << 9)
> +#define BM_OCOTP_CTRL_RD_BANK_OPEN	(1 << 12)
> +
> +static DEFINE_MUTEX(ocotp_mutex);
> +
> +int mxs_read_ocotp(unsigned offset, size_t count, u32 *values)
> +{
> +	void __iomem *ocotp_base = MXS_IO_ADDRESS(MXS_OCOTP_BASE_ADDR);
> +	int timeout = 0x400;
> +	size_t i;
> +
> +	mutex_lock(&ocotp_mutex);
> +
> +	/*
> +	 * clk_enable(hbus_clk) for ocotp can be skipped
> +	 * as it must be on when system is running.
> +	 */
> +
> +	/* try to clear ERROR bit */
> +	__mxs_clrl(BM_OCOTP_CTRL_ERROR, ocotp_base);

This operation does not try to clear the error bit but actually clears
it...

> +
> +	/* check both BUSY and ERROR cleared */
> +	while ((__raw_readl(ocotp_base) &
> +		(BM_OCOTP_CTRL_BUSY | BM_OCOTP_CTRL_ERROR)) && --timeout)
> +		cpu_relax();

...which means you do not have to poll the error bit here...

> +
> +	if (unlikely(!timeout))
> +		goto error_unlock;
> +
> +	/* open OCOTP banks for read */
> +	__mxs_setl(BM_OCOTP_CTRL_RD_BANK_OPEN, ocotp_base);
> +
> +	/* approximately wait 32 hclk cycles */
> +	udelay(1);
> +
> +	/* poll BUSY bit becoming cleared */
> +	timeout = 0x400;
> +	while ((__raw_readl(ocotp_base) & BM_OCOTP_CTRL_BUSY) && --timeout)
> +		cpu_relax();

...which means you can factor out a ocotp_wait_busy function and let the
code speak instead of the comments.

> +
> +	if (unlikely(!timeout))
> +		goto error_unlock;
> +
> +	for (i = 0; i < count; i++, offset += 4)
> +		*values++ = __raw_readl(ocotp_base + offset);

The registers in the ocotp are 16 byte aligned. Does it really make
sense to provide a function allowing to read the gaps between the
registers?

Sascha
Uwe Kleine-König Jan. 11, 2011, 2:05 p.m. UTC | #2
Hello Sascha,

On Tue, Jan 11, 2011 at 02:31:37PM +0100, Sascha Hauer wrote:
> On Thu, Jan 06, 2011 at 03:13:16PM +0800, Shawn Guo wrote:
> > diff --git a/arch/arm/mach-mxs/ocotp.c b/arch/arm/mach-mxs/ocotp.c
> > new file mode 100644
> > index 0000000..e2d39aa
> > --- /dev/null
> > +++ b/arch/arm/mach-mxs/ocotp.c
> > @@ -0,0 +1,79 @@
> > +/*
> > + * Copyright 2010 Freescale Semiconductor, Inc. All Rights Reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/mutex.h>
> > +
> > +#include <mach/mxs.h>
> > +
> > +#define BM_OCOTP_CTRL_BUSY		(1 << 8)
> > +#define BM_OCOTP_CTRL_ERROR		(1 << 9)
> > +#define BM_OCOTP_CTRL_RD_BANK_OPEN	(1 << 12)
> > +
> > +static DEFINE_MUTEX(ocotp_mutex);
> > +
> > +int mxs_read_ocotp(unsigned offset, size_t count, u32 *values)
> > +{
> > +	void __iomem *ocotp_base = MXS_IO_ADDRESS(MXS_OCOTP_BASE_ADDR);
> > +	int timeout = 0x400;
> > +	size_t i;
> > +
> > +	mutex_lock(&ocotp_mutex);
> > +
> > +	/*
> > +	 * clk_enable(hbus_clk) for ocotp can be skipped
> > +	 * as it must be on when system is running.
> > +	 */
> > +
> > +	/* try to clear ERROR bit */
> > +	__mxs_clrl(BM_OCOTP_CTRL_ERROR, ocotp_base);
> 
> This operation does not try to clear the error bit but actually clears
> it...
> 
> > +
> > +	/* check both BUSY and ERROR cleared */
> > +	while ((__raw_readl(ocotp_base) &
> > +		(BM_OCOTP_CTRL_BUSY | BM_OCOTP_CTRL_ERROR)) && --timeout)
> > +		cpu_relax();
> 
> ...which means you do not have to poll the error bit here...
well, I don't know how the hardware works here, but in general the
argument is broken.  Registers are not memory, so just because you set a
bit in register space it doesn't mean it is really set when you read
from the same address.

If there is something wrong with the ocotp I'd even expect that clearing
the error bit doesn't work because it doesn't change the general
condition.

Best regards
Uwe
Shawn Guo Jan. 12, 2011, 6:47 a.m. UTC | #3
Hi Sascha,

On Tue, Jan 11, 2011 at 02:31:37PM +0100, Sascha Hauer wrote:
> On Thu, Jan 06, 2011 at 03:13:16PM +0800, Shawn Guo wrote:
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> > Changes for v4:
> >  - Call cpu_relax() during polling
> > 
> > Changes for v2:
> >  - Add mutex locking for mxs_read_ocotp()
> >  - Use type size_t for count and i
> >  - Add comment for clk_enable/disable skipping
> >  - Add ERROR bit clearing and polling step
> > 
> >  arch/arm/mach-mxs/Makefile              |    2 +-
> >  arch/arm/mach-mxs/include/mach/common.h |    1 +
> >  arch/arm/mach-mxs/ocotp.c               |   79 +++++++++++++++++++++++++++++++
> >  3 files changed, 81 insertions(+), 1 deletions(-)
> >  create mode 100644 arch/arm/mach-mxs/ocotp.c
> > 
> > diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
> > index 39d3f9c..f23ebbd 100644
> > --- a/arch/arm/mach-mxs/Makefile
> > +++ b/arch/arm/mach-mxs/Makefile
> > @@ -1,5 +1,5 @@
> >  # Common support
> > -obj-y := clock.o devices.o gpio.o icoll.o iomux.o system.o timer.o
> > +obj-y := clock.o devices.o gpio.o icoll.o iomux.o ocotp.o system.o timer.o
> >  
> >  obj-$(CONFIG_SOC_IMX23) += clock-mx23.o mm-mx23.o
> >  obj-$(CONFIG_SOC_IMX28) += clock-mx28.o mm-mx28.o
> > diff --git a/arch/arm/mach-mxs/include/mach/common.h b/arch/arm/mach-mxs/include/mach/common.h
> > index 59133eb..cf02552 100644
> > --- a/arch/arm/mach-mxs/include/mach/common.h
> > +++ b/arch/arm/mach-mxs/include/mach/common.h
> > @@ -13,6 +13,7 @@
> >  
> >  struct clk;
> >  
> > +extern int mxs_read_ocotp(int offset, int count, u32 *values);
> >  extern int mxs_reset_block(void __iomem *);
> >  extern void mxs_timer_init(struct clk *, int);
> >  
> > diff --git a/arch/arm/mach-mxs/ocotp.c b/arch/arm/mach-mxs/ocotp.c
> > new file mode 100644
> > index 0000000..e2d39aa
> > --- /dev/null
> > +++ b/arch/arm/mach-mxs/ocotp.c
> > @@ -0,0 +1,79 @@
> > +/*
> > + * Copyright 2010 Freescale Semiconductor, Inc. All Rights Reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/mutex.h>
> > +
> > +#include <mach/mxs.h>
> > +
> > +#define BM_OCOTP_CTRL_BUSY		(1 << 8)
> > +#define BM_OCOTP_CTRL_ERROR		(1 << 9)
> > +#define BM_OCOTP_CTRL_RD_BANK_OPEN	(1 << 12)
> > +
> > +static DEFINE_MUTEX(ocotp_mutex);
> > +
> > +int mxs_read_ocotp(unsigned offset, size_t count, u32 *values)
> > +{
> > +	void __iomem *ocotp_base = MXS_IO_ADDRESS(MXS_OCOTP_BASE_ADDR);
> > +	int timeout = 0x400;
> > +	size_t i;
> > +
> > +	mutex_lock(&ocotp_mutex);
> > +
> > +	/*
> > +	 * clk_enable(hbus_clk) for ocotp can be skipped
> > +	 * as it must be on when system is running.
> > +	 */
> > +
> > +	/* try to clear ERROR bit */
> > +	__mxs_clrl(BM_OCOTP_CTRL_ERROR, ocotp_base);
> 
> This operation does not try to clear the error bit but actually clears
> it...
> 
> > +
> > +	/* check both BUSY and ERROR cleared */
> > +	while ((__raw_readl(ocotp_base) &
> > +		(BM_OCOTP_CTRL_BUSY | BM_OCOTP_CTRL_ERROR)) && --timeout)
> > +		cpu_relax();
> 
> ...which means you do not have to poll the error bit here...
> 
> > +
> > +	if (unlikely(!timeout))
> > +		goto error_unlock;
> > +
> > +	/* open OCOTP banks for read */
> > +	__mxs_setl(BM_OCOTP_CTRL_RD_BANK_OPEN, ocotp_base);
> > +
> > +	/* approximately wait 32 hclk cycles */
> > +	udelay(1);
> > +
> > +	/* poll BUSY bit becoming cleared */
> > +	timeout = 0x400;
> > +	while ((__raw_readl(ocotp_base) & BM_OCOTP_CTRL_BUSY) && --timeout)
> > +		cpu_relax();
> 
> ...which means you can factor out a ocotp_wait_busy function and let the
> code speak instead of the comments.
> 
> > +
> > +	if (unlikely(!timeout))
> > +		goto error_unlock;
> > +
> > +	for (i = 0; i < count; i++, offset += 4)
> > +		*values++ = __raw_readl(ocotp_base + offset);
> 
> The registers in the ocotp are 16 byte aligned. Does it really make
> sense to provide a function allowing to read the gaps between the
> registers?
> 
Good catch.  The count was added to ease the consecutive otp word
reading, as there is bank open/close cost for otp read.  What about
the following changes?

int mxs_read_ocotp(unsigned offset, size_t otp_word_cnt, u32 *values)
{
	......

	for (i = 0; i < otp_word_cnt; i++, offset += 0x10)
		*values++ = __raw_readl(ocotp_base + offset);

	......
}
Sascha Hauer Jan. 12, 2011, 2:50 p.m. UTC | #4
On Wed, Jan 12, 2011 at 02:47:12PM +0800, Shawn Guo wrote:
> Hi Sascha,
> 
> On Tue, Jan 11, 2011 at 02:31:37PM +0100, Sascha Hauer wrote:
> > On Thu, Jan 06, 2011 at 03:13:16PM +0800, Shawn Guo wrote:
> > > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > > ---
> > > Changes for v4:
> > >  - Call cpu_relax() during polling
> > > 
> > > Changes for v2:
> > >  - Add mutex locking for mxs_read_ocotp()
> > >  - Use type size_t for count and i
> > >  - Add comment for clk_enable/disable skipping
> > >  - Add ERROR bit clearing and polling step
> > > 
> > >  arch/arm/mach-mxs/Makefile              |    2 +-
> > >  arch/arm/mach-mxs/include/mach/common.h |    1 +
> > >  arch/arm/mach-mxs/ocotp.c               |   79 +++++++++++++++++++++++++++++++
> > >  3 files changed, 81 insertions(+), 1 deletions(-)
> > >  create mode 100644 arch/arm/mach-mxs/ocotp.c
> > > 
> > > diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
> > > index 39d3f9c..f23ebbd 100644
> > > --- a/arch/arm/mach-mxs/Makefile
> > > +++ b/arch/arm/mach-mxs/Makefile
> > > @@ -1,5 +1,5 @@
> > >  # Common support
> > > -obj-y := clock.o devices.o gpio.o icoll.o iomux.o system.o timer.o
> > > +obj-y := clock.o devices.o gpio.o icoll.o iomux.o ocotp.o system.o timer.o
> > >  
> > >  obj-$(CONFIG_SOC_IMX23) += clock-mx23.o mm-mx23.o
> > >  obj-$(CONFIG_SOC_IMX28) += clock-mx28.o mm-mx28.o
> > > diff --git a/arch/arm/mach-mxs/include/mach/common.h b/arch/arm/mach-mxs/include/mach/common.h
> > > index 59133eb..cf02552 100644
> > > --- a/arch/arm/mach-mxs/include/mach/common.h
> > > +++ b/arch/arm/mach-mxs/include/mach/common.h
> > > @@ -13,6 +13,7 @@
> > >  
> > >  struct clk;
> > >  
> > > +extern int mxs_read_ocotp(int offset, int count, u32 *values);
> > >  extern int mxs_reset_block(void __iomem *);
> > >  extern void mxs_timer_init(struct clk *, int);
> > >  
> > > diff --git a/arch/arm/mach-mxs/ocotp.c b/arch/arm/mach-mxs/ocotp.c
> > > new file mode 100644
> > > index 0000000..e2d39aa
> > > --- /dev/null
> > > +++ b/arch/arm/mach-mxs/ocotp.c
> > > @@ -0,0 +1,79 @@
> > > +/*
> > > + * Copyright 2010 Freescale Semiconductor, Inc. All Rights Reserved.
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > > + */
> > > +
> > > +#include <linux/delay.h>
> > > +#include <linux/err.h>
> > > +#include <linux/mutex.h>
> > > +
> > > +#include <mach/mxs.h>
> > > +
> > > +#define BM_OCOTP_CTRL_BUSY		(1 << 8)
> > > +#define BM_OCOTP_CTRL_ERROR		(1 << 9)
> > > +#define BM_OCOTP_CTRL_RD_BANK_OPEN	(1 << 12)
> > > +
> > > +static DEFINE_MUTEX(ocotp_mutex);
> > > +
> > > +int mxs_read_ocotp(unsigned offset, size_t count, u32 *values)
> > > +{
> > > +	void __iomem *ocotp_base = MXS_IO_ADDRESS(MXS_OCOTP_BASE_ADDR);
> > > +	int timeout = 0x400;
> > > +	size_t i;
> > > +
> > > +	mutex_lock(&ocotp_mutex);
> > > +
> > > +	/*
> > > +	 * clk_enable(hbus_clk) for ocotp can be skipped
> > > +	 * as it must be on when system is running.
> > > +	 */
> > > +
> > > +	/* try to clear ERROR bit */
> > > +	__mxs_clrl(BM_OCOTP_CTRL_ERROR, ocotp_base);
> > 
> > This operation does not try to clear the error bit but actually clears
> > it...
> > 
> > > +
> > > +	/* check both BUSY and ERROR cleared */
> > > +	while ((__raw_readl(ocotp_base) &
> > > +		(BM_OCOTP_CTRL_BUSY | BM_OCOTP_CTRL_ERROR)) && --timeout)
> > > +		cpu_relax();
> > 
> > ...which means you do not have to poll the error bit here...
> > 
> > > +
> > > +	if (unlikely(!timeout))
> > > +		goto error_unlock;
> > > +
> > > +	/* open OCOTP banks for read */
> > > +	__mxs_setl(BM_OCOTP_CTRL_RD_BANK_OPEN, ocotp_base);
> > > +
> > > +	/* approximately wait 32 hclk cycles */
> > > +	udelay(1);
> > > +
> > > +	/* poll BUSY bit becoming cleared */
> > > +	timeout = 0x400;
> > > +	while ((__raw_readl(ocotp_base) & BM_OCOTP_CTRL_BUSY) && --timeout)
> > > +		cpu_relax();
> > 
> > ...which means you can factor out a ocotp_wait_busy function and let the
> > code speak instead of the comments.
> > 
> > > +
> > > +	if (unlikely(!timeout))
> > > +		goto error_unlock;
> > > +
> > > +	for (i = 0; i < count; i++, offset += 4)
> > > +		*values++ = __raw_readl(ocotp_base + offset);
> > 
> > The registers in the ocotp are 16 byte aligned. Does it really make
> > sense to provide a function allowing to read the gaps between the
> > registers?
> > 
> Good catch.  The count was added to ease the consecutive otp word
> reading, as there is bank open/close cost for otp read.  What about
> the following changes?
> 
> int mxs_read_ocotp(unsigned offset, size_t otp_word_cnt, u32 *values)
> {
> 	......
> 
> 	for (i = 0; i < otp_word_cnt; i++, offset += 0x10)
> 		*values++ = __raw_readl(ocotp_base + offset);
> 
> 	......
> }

I would rather make a function like this:

static u32 ocotp[0x27];

const u32 *mxs_get_ocotp(void)
{
	static int once = 0;

	if (once)
		return ocotp

	/* bank open */

	for (i = 0; i < 0x27; i++)
		ocotp[i] = readl(ocotp_base + 0x20 + i * 0x10)

	/* bank_close */

	once = 1;

	return ocotp;
}
Uwe Kleine-König Jan. 12, 2011, 4:01 p.m. UTC | #5
Hello Sascha,

On Wed, Jan 12, 2011 at 03:50:36PM +0100, Sascha Hauer wrote:
> On Wed, Jan 12, 2011 at 02:47:12PM +0800, Shawn Guo wrote:
> > On Tue, Jan 11, 2011 at 02:31:37PM +0100, Sascha Hauer wrote:
> > > On Thu, Jan 06, 2011 at 03:13:16PM +0800, Shawn Guo wrote:
> > > > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > > > ---
> > > > Changes for v4:
> > > >  - Call cpu_relax() during polling
> > > > 
> > > > Changes for v2:
> > > >  - Add mutex locking for mxs_read_ocotp()
> > > >  - Use type size_t for count and i
> > > >  - Add comment for clk_enable/disable skipping
> > > >  - Add ERROR bit clearing and polling step
> > > > 
> > > >  arch/arm/mach-mxs/Makefile              |    2 +-
> > > >  arch/arm/mach-mxs/include/mach/common.h |    1 +
> > > >  arch/arm/mach-mxs/ocotp.c               |   79 +++++++++++++++++++++++++++++++
> > > >  3 files changed, 81 insertions(+), 1 deletions(-)
> > > >  create mode 100644 arch/arm/mach-mxs/ocotp.c
> > > > 
> > > > diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
> > > > index 39d3f9c..f23ebbd 100644
> > > > --- a/arch/arm/mach-mxs/Makefile
> > > > +++ b/arch/arm/mach-mxs/Makefile
> > > > @@ -1,5 +1,5 @@
> > > >  # Common support
> > > > -obj-y := clock.o devices.o gpio.o icoll.o iomux.o system.o timer.o
> > > > +obj-y := clock.o devices.o gpio.o icoll.o iomux.o ocotp.o system.o timer.o
> > > >  
> > > >  obj-$(CONFIG_SOC_IMX23) += clock-mx23.o mm-mx23.o
> > > >  obj-$(CONFIG_SOC_IMX28) += clock-mx28.o mm-mx28.o
> > > > diff --git a/arch/arm/mach-mxs/include/mach/common.h b/arch/arm/mach-mxs/include/mach/common.h
> > > > index 59133eb..cf02552 100644
> > > > --- a/arch/arm/mach-mxs/include/mach/common.h
> > > > +++ b/arch/arm/mach-mxs/include/mach/common.h
> > > > @@ -13,6 +13,7 @@
> > > >  
> > > >  struct clk;
> > > >  
> > > > +extern int mxs_read_ocotp(int offset, int count, u32 *values);
> > > >  extern int mxs_reset_block(void __iomem *);
> > > >  extern void mxs_timer_init(struct clk *, int);
> > > >  
> > > > diff --git a/arch/arm/mach-mxs/ocotp.c b/arch/arm/mach-mxs/ocotp.c
> > > > new file mode 100644
> > > > index 0000000..e2d39aa
> > > > --- /dev/null
> > > > +++ b/arch/arm/mach-mxs/ocotp.c
> > > > @@ -0,0 +1,79 @@
> > > > +/*
> > > > + * Copyright 2010 Freescale Semiconductor, Inc. All Rights Reserved.
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License as published by
> > > > + * the Free Software Foundation; either version 2 of the License, or
> > > > + * (at your option) any later version.
> > > > + *
> > > > + * This program is distributed in the hope that it will be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > > + * GNU General Public License for more details.
> > > > + */
> > > > +
> > > > +#include <linux/delay.h>
> > > > +#include <linux/err.h>
> > > > +#include <linux/mutex.h>
> > > > +
> > > > +#include <mach/mxs.h>
> > > > +
> > > > +#define BM_OCOTP_CTRL_BUSY		(1 << 8)
> > > > +#define BM_OCOTP_CTRL_ERROR		(1 << 9)
> > > > +#define BM_OCOTP_CTRL_RD_BANK_OPEN	(1 << 12)
> > > > +
> > > > +static DEFINE_MUTEX(ocotp_mutex);
> > > > +
> > > > +int mxs_read_ocotp(unsigned offset, size_t count, u32 *values)
> > > > +{
> > > > +	void __iomem *ocotp_base = MXS_IO_ADDRESS(MXS_OCOTP_BASE_ADDR);
> > > > +	int timeout = 0x400;
> > > > +	size_t i;
> > > > +
> > > > +	mutex_lock(&ocotp_mutex);
> > > > +
> > > > +	/*
> > > > +	 * clk_enable(hbus_clk) for ocotp can be skipped
> > > > +	 * as it must be on when system is running.
> > > > +	 */
> > > > +
> > > > +	/* try to clear ERROR bit */
> > > > +	__mxs_clrl(BM_OCOTP_CTRL_ERROR, ocotp_base);
> > > 
> > > This operation does not try to clear the error bit but actually clears
> > > it...
> > > 
> > > > +
> > > > +	/* check both BUSY and ERROR cleared */
> > > > +	while ((__raw_readl(ocotp_base) &
> > > > +		(BM_OCOTP_CTRL_BUSY | BM_OCOTP_CTRL_ERROR)) && --timeout)
> > > > +		cpu_relax();
> > > 
> > > ...which means you do not have to poll the error bit here...
> > > 
> > > > +
> > > > +	if (unlikely(!timeout))
> > > > +		goto error_unlock;
> > > > +
> > > > +	/* open OCOTP banks for read */
> > > > +	__mxs_setl(BM_OCOTP_CTRL_RD_BANK_OPEN, ocotp_base);
> > > > +
> > > > +	/* approximately wait 32 hclk cycles */
> > > > +	udelay(1);
> > > > +
> > > > +	/* poll BUSY bit becoming cleared */
> > > > +	timeout = 0x400;
> > > > +	while ((__raw_readl(ocotp_base) & BM_OCOTP_CTRL_BUSY) && --timeout)
> > > > +		cpu_relax();
> > > 
> > > ...which means you can factor out a ocotp_wait_busy function and let the
> > > code speak instead of the comments.
> > > 
> > > > +
> > > > +	if (unlikely(!timeout))
> > > > +		goto error_unlock;
> > > > +
> > > > +	for (i = 0; i < count; i++, offset += 4)
> > > > +		*values++ = __raw_readl(ocotp_base + offset);
> > > 
> > > The registers in the ocotp are 16 byte aligned. Does it really make
> > > sense to provide a function allowing to read the gaps between the
> > > registers?
> > > 
> > Good catch.  The count was added to ease the consecutive otp word
> > reading, as there is bank open/close cost for otp read.  What about
> > the following changes?
> > 
> > int mxs_read_ocotp(unsigned offset, size_t otp_word_cnt, u32 *values)
> > {
> > 	......
> > 
> > 	for (i = 0; i < otp_word_cnt; i++, offset += 0x10)
> > 		*values++ = __raw_readl(ocotp_base + offset);
> > 
> > 	......
> > }
> 
> I would rather make a function like this:
> 
> static u32 ocotp[0x27];
> 
> const u32 *mxs_get_ocotp(void)
> {
> 	static int once = 0;
> 
> 	if (once)
> 		return ocotp
> 
> 	/* bank open */
> 
> 	for (i = 0; i < 0x27; i++)
> 		ocotp[i] = readl(ocotp_base + 0x20 + i * 0x10)
> 
> 	/* bank_close */
> 
> 	once = 1;
> 
> 	return ocotp;
which is save on UP when it's not called from irq context.

Additionally I suggest a #define for 0x27 and 0x20.

Uwe
Shawn Guo Jan. 13, 2011, 1:55 a.m. UTC | #6
On Wed, Jan 12, 2011 at 05:01:06PM +0100, Uwe Kleine-König wrote:
> Hello Sascha,
> 
> On Wed, Jan 12, 2011 at 03:50:36PM +0100, Sascha Hauer wrote:
> > On Wed, Jan 12, 2011 at 02:47:12PM +0800, Shawn Guo wrote:
> > > On Tue, Jan 11, 2011 at 02:31:37PM +0100, Sascha Hauer wrote:
> > > > On Thu, Jan 06, 2011 at 03:13:16PM +0800, Shawn Guo wrote:
> > > > > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > > > > ---
> > > > > Changes for v4:
> > > > >  - Call cpu_relax() during polling
> > > > > 
> > > > > Changes for v2:
> > > > >  - Add mutex locking for mxs_read_ocotp()
> > > > >  - Use type size_t for count and i
> > > > >  - Add comment for clk_enable/disable skipping
> > > > >  - Add ERROR bit clearing and polling step
> > > > > 
> > > > >  arch/arm/mach-mxs/Makefile              |    2 +-
> > > > >  arch/arm/mach-mxs/include/mach/common.h |    1 +
> > > > >  arch/arm/mach-mxs/ocotp.c               |   79 +++++++++++++++++++++++++++++++
> > > > >  3 files changed, 81 insertions(+), 1 deletions(-)
> > > > >  create mode 100644 arch/arm/mach-mxs/ocotp.c
> > > > > 
> > > > > diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
> > > > > index 39d3f9c..f23ebbd 100644
> > > > > --- a/arch/arm/mach-mxs/Makefile
> > > > > +++ b/arch/arm/mach-mxs/Makefile
> > > > > @@ -1,5 +1,5 @@
> > > > >  # Common support
> > > > > -obj-y := clock.o devices.o gpio.o icoll.o iomux.o system.o timer.o
> > > > > +obj-y := clock.o devices.o gpio.o icoll.o iomux.o ocotp.o system.o timer.o
> > > > >  
> > > > >  obj-$(CONFIG_SOC_IMX23) += clock-mx23.o mm-mx23.o
> > > > >  obj-$(CONFIG_SOC_IMX28) += clock-mx28.o mm-mx28.o
> > > > > diff --git a/arch/arm/mach-mxs/include/mach/common.h b/arch/arm/mach-mxs/include/mach/common.h
> > > > > index 59133eb..cf02552 100644
> > > > > --- a/arch/arm/mach-mxs/include/mach/common.h
> > > > > +++ b/arch/arm/mach-mxs/include/mach/common.h
> > > > > @@ -13,6 +13,7 @@
> > > > >  
> > > > >  struct clk;
> > > > >  
> > > > > +extern int mxs_read_ocotp(int offset, int count, u32 *values);
> > > > >  extern int mxs_reset_block(void __iomem *);
> > > > >  extern void mxs_timer_init(struct clk *, int);
> > > > >  
> > > > > diff --git a/arch/arm/mach-mxs/ocotp.c b/arch/arm/mach-mxs/ocotp.c
> > > > > new file mode 100644
> > > > > index 0000000..e2d39aa
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/mach-mxs/ocotp.c
> > > > > @@ -0,0 +1,79 @@
> > > > > +/*
> > > > > + * Copyright 2010 Freescale Semiconductor, Inc. All Rights Reserved.
> > > > > + *
> > > > > + * This program is free software; you can redistribute it and/or modify
> > > > > + * it under the terms of the GNU General Public License as published by
> > > > > + * the Free Software Foundation; either version 2 of the License, or
> > > > > + * (at your option) any later version.
> > > > > + *
> > > > > + * This program is distributed in the hope that it will be useful,
> > > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > > > + * GNU General Public License for more details.
> > > > > + */
> > > > > +
> > > > > +#include <linux/delay.h>
> > > > > +#include <linux/err.h>
> > > > > +#include <linux/mutex.h>
> > > > > +
> > > > > +#include <mach/mxs.h>
> > > > > +
> > > > > +#define BM_OCOTP_CTRL_BUSY		(1 << 8)
> > > > > +#define BM_OCOTP_CTRL_ERROR		(1 << 9)
> > > > > +#define BM_OCOTP_CTRL_RD_BANK_OPEN	(1 << 12)
> > > > > +
> > > > > +static DEFINE_MUTEX(ocotp_mutex);
> > > > > +
> > > > > +int mxs_read_ocotp(unsigned offset, size_t count, u32 *values)
> > > > > +{
> > > > > +	void __iomem *ocotp_base = MXS_IO_ADDRESS(MXS_OCOTP_BASE_ADDR);
> > > > > +	int timeout = 0x400;
> > > > > +	size_t i;
> > > > > +
> > > > > +	mutex_lock(&ocotp_mutex);
> > > > > +
> > > > > +	/*
> > > > > +	 * clk_enable(hbus_clk) for ocotp can be skipped
> > > > > +	 * as it must be on when system is running.
> > > > > +	 */
> > > > > +
> > > > > +	/* try to clear ERROR bit */
> > > > > +	__mxs_clrl(BM_OCOTP_CTRL_ERROR, ocotp_base);
> > > > 
> > > > This operation does not try to clear the error bit but actually clears
> > > > it...
> > > > 
> > > > > +
> > > > > +	/* check both BUSY and ERROR cleared */
> > > > > +	while ((__raw_readl(ocotp_base) &
> > > > > +		(BM_OCOTP_CTRL_BUSY | BM_OCOTP_CTRL_ERROR)) && --timeout)
> > > > > +		cpu_relax();
> > > > 
> > > > ...which means you do not have to poll the error bit here...
> > > > 
> > > > > +
> > > > > +	if (unlikely(!timeout))
> > > > > +		goto error_unlock;
> > > > > +
> > > > > +	/* open OCOTP banks for read */
> > > > > +	__mxs_setl(BM_OCOTP_CTRL_RD_BANK_OPEN, ocotp_base);
> > > > > +
> > > > > +	/* approximately wait 32 hclk cycles */
> > > > > +	udelay(1);
> > > > > +
> > > > > +	/* poll BUSY bit becoming cleared */
> > > > > +	timeout = 0x400;
> > > > > +	while ((__raw_readl(ocotp_base) & BM_OCOTP_CTRL_BUSY) && --timeout)
> > > > > +		cpu_relax();
> > > > 
> > > > ...which means you can factor out a ocotp_wait_busy function and let the
> > > > code speak instead of the comments.
> > > > 
> > > > > +
> > > > > +	if (unlikely(!timeout))
> > > > > +		goto error_unlock;
> > > > > +
> > > > > +	for (i = 0; i < count; i++, offset += 4)
> > > > > +		*values++ = __raw_readl(ocotp_base + offset);
> > > > 
> > > > The registers in the ocotp are 16 byte aligned. Does it really make
> > > > sense to provide a function allowing to read the gaps between the
> > > > registers?
> > > > 
> > > Good catch.  The count was added to ease the consecutive otp word
> > > reading, as there is bank open/close cost for otp read.  What about
> > > the following changes?
> > > 
> > > int mxs_read_ocotp(unsigned offset, size_t otp_word_cnt, u32 *values)
> > > {
> > > 	......
> > > 
> > > 	for (i = 0; i < otp_word_cnt; i++, offset += 0x10)
> > > 		*values++ = __raw_readl(ocotp_base + offset);
> > > 
> > > 	......
> > > }
> > 
> > I would rather make a function like this:
> > 
> > static u32 ocotp[0x27];
> > 
> > const u32 *mxs_get_ocotp(void)
> > {
> > 	static int once = 0;
> > 
> > 	if (once)
> > 		return ocotp
> > 
> > 	/* bank open */
> > 
> > 	for (i = 0; i < 0x27; i++)
> > 		ocotp[i] = readl(ocotp_base + 0x20 + i * 0x10)
> > 
> > 	/* bank_close */
> > 
> > 	once = 1;
> > 
> > 	return ocotp;
> which is save on UP when it's not called from irq context.
> 
> Additionally I suggest a #define for 0x27 and 0x20.
> 

So I will keep the mutex and not read SRK bits.

Thanks for the comments.
Uwe Kleine-König Jan. 13, 2011, 3:19 p.m. UTC | #7
On Thu, Jan 06, 2011 at 03:13:16PM +0800, Shawn Guo wrote:
> Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> ---
> Changes for v4:
>  - Call cpu_relax() during polling
> 
> Changes for v2:
>  - Add mutex locking for mxs_read_ocotp()
>  - Use type size_t for count and i
>  - Add comment for clk_enable/disable skipping
>  - Add ERROR bit clearing and polling step
> 
>  arch/arm/mach-mxs/Makefile              |    2 +-
>  arch/arm/mach-mxs/include/mach/common.h |    1 +
>  arch/arm/mach-mxs/ocotp.c               |   79 +++++++++++++++++++++++++++++++
>  3 files changed, 81 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arm/mach-mxs/ocotp.c
> 
> diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
> index 39d3f9c..f23ebbd 100644
> --- a/arch/arm/mach-mxs/Makefile
> +++ b/arch/arm/mach-mxs/Makefile
> @@ -1,5 +1,5 @@
>  # Common support
> -obj-y := clock.o devices.o gpio.o icoll.o iomux.o system.o timer.o
> +obj-y := clock.o devices.o gpio.o icoll.o iomux.o ocotp.o system.o timer.o
is it worth to make ocotp optional?  (and let evk select
CONFIG_MXS_OCOTP)

Best regards
Uwe
>  
>  obj-$(CONFIG_SOC_IMX23) += clock-mx23.o mm-mx23.o
>  obj-$(CONFIG_SOC_IMX28) += clock-mx28.o mm-mx28.o
> diff --git a/arch/arm/mach-mxs/include/mach/common.h b/arch/arm/mach-mxs/include/mach/common.h
> index 59133eb..cf02552 100644
> --- a/arch/arm/mach-mxs/include/mach/common.h
> +++ b/arch/arm/mach-mxs/include/mach/common.h
> @@ -13,6 +13,7 @@
>  
>  struct clk;
>  
> +extern int mxs_read_ocotp(int offset, int count, u32 *values);
>  extern int mxs_reset_block(void __iomem *);
>  extern void mxs_timer_init(struct clk *, int);
>  
> diff --git a/arch/arm/mach-mxs/ocotp.c b/arch/arm/mach-mxs/ocotp.c
> new file mode 100644
> index 0000000..e2d39aa
> --- /dev/null
> +++ b/arch/arm/mach-mxs/ocotp.c
> @@ -0,0 +1,79 @@
> +/*
> + * Copyright 2010 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +
> +#include <mach/mxs.h>
> +
> +#define BM_OCOTP_CTRL_BUSY		(1 << 8)
> +#define BM_OCOTP_CTRL_ERROR		(1 << 9)
> +#define BM_OCOTP_CTRL_RD_BANK_OPEN	(1 << 12)
> +
> +static DEFINE_MUTEX(ocotp_mutex);
> +
> +int mxs_read_ocotp(unsigned offset, size_t count, u32 *values)
> +{
> +	void __iomem *ocotp_base = MXS_IO_ADDRESS(MXS_OCOTP_BASE_ADDR);
> +	int timeout = 0x400;
> +	size_t i;
> +
> +	mutex_lock(&ocotp_mutex);
> +
> +	/*
> +	 * clk_enable(hbus_clk) for ocotp can be skipped
> +	 * as it must be on when system is running.
> +	 */
> +
> +	/* try to clear ERROR bit */
> +	__mxs_clrl(BM_OCOTP_CTRL_ERROR, ocotp_base);
> +
> +	/* check both BUSY and ERROR cleared */
> +	while ((__raw_readl(ocotp_base) &
> +		(BM_OCOTP_CTRL_BUSY | BM_OCOTP_CTRL_ERROR)) && --timeout)
> +		cpu_relax();
> +
> +	if (unlikely(!timeout))
> +		goto error_unlock;
> +
> +	/* open OCOTP banks for read */
> +	__mxs_setl(BM_OCOTP_CTRL_RD_BANK_OPEN, ocotp_base);
> +
> +	/* approximately wait 32 hclk cycles */
> +	udelay(1);
> +
> +	/* poll BUSY bit becoming cleared */
> +	timeout = 0x400;
> +	while ((__raw_readl(ocotp_base) & BM_OCOTP_CTRL_BUSY) && --timeout)
> +		cpu_relax();
> +
> +	if (unlikely(!timeout))
> +		goto error_unlock;
> +
> +	for (i = 0; i < count; i++, offset += 4)
> +		*values++ = __raw_readl(ocotp_base + offset);
> +
> +	/* close banks for power saving */
> +	__mxs_clrl(BM_OCOTP_CTRL_RD_BANK_OPEN, ocotp_base);
> +
> +	mutex_unlock(&ocotp_mutex);
> +
> +	return 0;
> +
> +error_unlock:
> +	mutex_unlock(&ocotp_mutex);
> +	pr_err("%s: timeout in reading OCOTP\n", __func__);
> +	return -ETIMEDOUT;
> +}
> -- 
> 1.7.1
> 
> 
>
Sascha Hauer Jan. 14, 2011, 8:33 a.m. UTC | #8
On Thu, Jan 13, 2011 at 04:19:39PM +0100, Uwe Kleine-König wrote:
> On Thu, Jan 06, 2011 at 03:13:16PM +0800, Shawn Guo wrote:
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> > Changes for v4:
> >  - Call cpu_relax() during polling
> > 
> > Changes for v2:
> >  - Add mutex locking for mxs_read_ocotp()
> >  - Use type size_t for count and i
> >  - Add comment for clk_enable/disable skipping
> >  - Add ERROR bit clearing and polling step
> > 
> >  arch/arm/mach-mxs/Makefile              |    2 +-
> >  arch/arm/mach-mxs/include/mach/common.h |    1 +
> >  arch/arm/mach-mxs/ocotp.c               |   79 +++++++++++++++++++++++++++++++
> >  3 files changed, 81 insertions(+), 1 deletions(-)
> >  create mode 100644 arch/arm/mach-mxs/ocotp.c
> > 
> > diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
> > index 39d3f9c..f23ebbd 100644
> > --- a/arch/arm/mach-mxs/Makefile
> > +++ b/arch/arm/mach-mxs/Makefile
> > @@ -1,5 +1,5 @@
> >  # Common support
> > -obj-y := clock.o devices.o gpio.o icoll.o iomux.o system.o timer.o
> > +obj-y := clock.o devices.o gpio.o icoll.o iomux.o ocotp.o system.o timer.o
> is it worth to make ocotp optional?  (and let evk select
> CONFIG_MXS_OCOTP)

I think not.

Sascha
Shawn Guo Jan. 14, 2011, 1:16 p.m. UTC | #9
On Fri, Jan 14, 2011 at 09:33:20AM +0100, Sascha Hauer wrote:
> On Thu, Jan 13, 2011 at 04:19:39PM +0100, Uwe Kleine-König wrote:
> > On Thu, Jan 06, 2011 at 03:13:16PM +0800, Shawn Guo wrote:
> > > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > > ---
> > > Changes for v4:
> > >  - Call cpu_relax() during polling
> > > 
> > > Changes for v2:
> > >  - Add mutex locking for mxs_read_ocotp()
> > >  - Use type size_t for count and i
> > >  - Add comment for clk_enable/disable skipping
> > >  - Add ERROR bit clearing and polling step
> > > 
> > >  arch/arm/mach-mxs/Makefile              |    2 +-
> > >  arch/arm/mach-mxs/include/mach/common.h |    1 +
> > >  arch/arm/mach-mxs/ocotp.c               |   79 +++++++++++++++++++++++++++++++
> > >  3 files changed, 81 insertions(+), 1 deletions(-)
> > >  create mode 100644 arch/arm/mach-mxs/ocotp.c
> > > 
> > > diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
> > > index 39d3f9c..f23ebbd 100644
> > > --- a/arch/arm/mach-mxs/Makefile
> > > +++ b/arch/arm/mach-mxs/Makefile
> > > @@ -1,5 +1,5 @@
> > >  # Common support
> > > -obj-y := clock.o devices.o gpio.o icoll.o iomux.o system.o timer.o
> > > +obj-y := clock.o devices.o gpio.o icoll.o iomux.o ocotp.o system.o timer.o
> > is it worth to make ocotp optional?  (and let evk select
> > CONFIG_MXS_OCOTP)
> 
> I think not.
> 
I thought it depends on how we think about it.  If we think that
ocotp is a block of soc and should be on every system based on
the soc, we should build it unconditional.  However, if we think
ocotp is a function that could either be used on a machine or not,
it should be built conditional, so that the machine does not use
ocotp function could save the ocotp build and the memory of
u32 ocotp_words[0x20].
Shawn Guo Jan. 19, 2011, 10:48 a.m. UTC | #10
Hi Sascha,

On Fri, Jan 14, 2011 at 09:16:56PM +0800, Shawn Guo wrote:
> On Fri, Jan 14, 2011 at 09:33:20AM +0100, Sascha Hauer wrote:
> > On Thu, Jan 13, 2011 at 04:19:39PM +0100, Uwe Kleine-König wrote:
> > > On Thu, Jan 06, 2011 at 03:13:16PM +0800, Shawn Guo wrote:
> > > > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > > > ---
> > > > Changes for v4:
> > > >  - Call cpu_relax() during polling
> > > > 
> > > > Changes for v2:
> > > >  - Add mutex locking for mxs_read_ocotp()
> > > >  - Use type size_t for count and i
> > > >  - Add comment for clk_enable/disable skipping
> > > >  - Add ERROR bit clearing and polling step
> > > > 
> > > >  arch/arm/mach-mxs/Makefile              |    2 +-
> > > >  arch/arm/mach-mxs/include/mach/common.h |    1 +
> > > >  arch/arm/mach-mxs/ocotp.c               |   79 +++++++++++++++++++++++++++++++
> > > >  3 files changed, 81 insertions(+), 1 deletions(-)
> > > >  create mode 100644 arch/arm/mach-mxs/ocotp.c
> > > > 
> > > > diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
> > > > index 39d3f9c..f23ebbd 100644
> > > > --- a/arch/arm/mach-mxs/Makefile
> > > > +++ b/arch/arm/mach-mxs/Makefile
> > > > @@ -1,5 +1,5 @@
> > > >  # Common support
> > > > -obj-y := clock.o devices.o gpio.o icoll.o iomux.o system.o timer.o
> > > > +obj-y := clock.o devices.o gpio.o icoll.o iomux.o ocotp.o system.o timer.o
> > > is it worth to make ocotp optional?  (and let evk select
> > > CONFIG_MXS_OCOTP)
> > 
> > I think not.
> > 
> I thought it depends on how we think about it.  If we think that
> ocotp is a block of soc and should be on every system based on
> the soc, we should build it unconditional.  However, if we think
> ocotp is a function that could either be used on a machine or not,
> it should be built conditional, so that the machine does not use
> ocotp function could save the ocotp build and the memory of
> u32 ocotp_words[0x20].
> 
You've not merged patch #8, #9, #10, so you still think
CONFIG_MXS_OCOTP should not be added?
diff mbox

Patch

diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
index 39d3f9c..f23ebbd 100644
--- a/arch/arm/mach-mxs/Makefile
+++ b/arch/arm/mach-mxs/Makefile
@@ -1,5 +1,5 @@ 
 # Common support
-obj-y := clock.o devices.o gpio.o icoll.o iomux.o system.o timer.o
+obj-y := clock.o devices.o gpio.o icoll.o iomux.o ocotp.o system.o timer.o
 
 obj-$(CONFIG_SOC_IMX23) += clock-mx23.o mm-mx23.o
 obj-$(CONFIG_SOC_IMX28) += clock-mx28.o mm-mx28.o
diff --git a/arch/arm/mach-mxs/include/mach/common.h b/arch/arm/mach-mxs/include/mach/common.h
index 59133eb..cf02552 100644
--- a/arch/arm/mach-mxs/include/mach/common.h
+++ b/arch/arm/mach-mxs/include/mach/common.h
@@ -13,6 +13,7 @@ 
 
 struct clk;
 
+extern int mxs_read_ocotp(int offset, int count, u32 *values);
 extern int mxs_reset_block(void __iomem *);
 extern void mxs_timer_init(struct clk *, int);
 
diff --git a/arch/arm/mach-mxs/ocotp.c b/arch/arm/mach-mxs/ocotp.c
new file mode 100644
index 0000000..e2d39aa
--- /dev/null
+++ b/arch/arm/mach-mxs/ocotp.c
@@ -0,0 +1,79 @@ 
+/*
+ * Copyright 2010 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+
+#include <mach/mxs.h>
+
+#define BM_OCOTP_CTRL_BUSY		(1 << 8)
+#define BM_OCOTP_CTRL_ERROR		(1 << 9)
+#define BM_OCOTP_CTRL_RD_BANK_OPEN	(1 << 12)
+
+static DEFINE_MUTEX(ocotp_mutex);
+
+int mxs_read_ocotp(unsigned offset, size_t count, u32 *values)
+{
+	void __iomem *ocotp_base = MXS_IO_ADDRESS(MXS_OCOTP_BASE_ADDR);
+	int timeout = 0x400;
+	size_t i;
+
+	mutex_lock(&ocotp_mutex);
+
+	/*
+	 * clk_enable(hbus_clk) for ocotp can be skipped
+	 * as it must be on when system is running.
+	 */
+
+	/* try to clear ERROR bit */
+	__mxs_clrl(BM_OCOTP_CTRL_ERROR, ocotp_base);
+
+	/* check both BUSY and ERROR cleared */
+	while ((__raw_readl(ocotp_base) &
+		(BM_OCOTP_CTRL_BUSY | BM_OCOTP_CTRL_ERROR)) && --timeout)
+		cpu_relax();
+
+	if (unlikely(!timeout))
+		goto error_unlock;
+
+	/* open OCOTP banks for read */
+	__mxs_setl(BM_OCOTP_CTRL_RD_BANK_OPEN, ocotp_base);
+
+	/* approximately wait 32 hclk cycles */
+	udelay(1);
+
+	/* poll BUSY bit becoming cleared */
+	timeout = 0x400;
+	while ((__raw_readl(ocotp_base) & BM_OCOTP_CTRL_BUSY) && --timeout)
+		cpu_relax();
+
+	if (unlikely(!timeout))
+		goto error_unlock;
+
+	for (i = 0; i < count; i++, offset += 4)
+		*values++ = __raw_readl(ocotp_base + offset);
+
+	/* close banks for power saving */
+	__mxs_clrl(BM_OCOTP_CTRL_RD_BANK_OPEN, ocotp_base);
+
+	mutex_unlock(&ocotp_mutex);
+
+	return 0;
+
+error_unlock:
+	mutex_unlock(&ocotp_mutex);
+	pr_err("%s: timeout in reading OCOTP\n", __func__);
+	return -ETIMEDOUT;
+}