[PATCH V2 0/3] introduce stmp-style devices

[PATCH V2 0/3] introduce stmp-style devices

[Wolfram Sang]

This series makes support for a certain type of devices mach independant. We
want that because such devices (having a special register layout) have been
found in mach-mxs and mach-mx6 meanwhile. Please refer to patch 1/3 for a more
detailed description.

Patch 1/3 adds the basic support and is of main interest to get upstream.
Patches 2/3 and 3/3 are usage examples and will be send to the relevant
subsystems once the basic support is accepted.

Main changes since the V1:

* moved stmp_device.c from drivers/base to lib/

Main changes since the RFC:

* renamed the macros to STMP_OFFSET_*
* rebased to 3.3-rc6

Tested with a TX28 board.

Thanks,

   Wolfram

Wolfram Sang (3):
  lib: add support for stmp-style devices
  i2c: mxs: use global reset function
  rtc: stmp3xxx: use global stmp_device functionality

 drivers/i2c/busses/Kconfig   |    1 +
 drivers/i2c/busses/i2c-mxs.c |    9 +----
 drivers/rtc/Kconfig          |    1 +
 drivers/rtc/rtc-stmp3xxx.c   |   29 +++++----------
 include/linux/stmp_device.h  |   20 ++++++++++
 lib/Kconfig                  |    3 ++
 lib/Makefile                 |    2 +
 lib/stmp_device.c            |   80 ++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 119 insertions(+), 26 deletions(-)
 create mode 100644 include/linux/stmp_device.h
 create mode 100644 lib/stmp_device.c

-- 
1.7.2.5

[PATCH V2 0/3] introduce stmp-style devices

[Wolfram Sang]

This series makes support for a certain type of devices mach independant. We
want that because such devices (having a special register layout) have been
found in mach-mxs and mach-mx6 meanwhile. Please refer to patch 1/3 for a more
detailed description.

Patch 1/3 adds the basic support and is of main interest to get upstream.
Patches 2/3 and 3/3 are usage examples and will be send to the relevant
subsystems once the basic support is accepted.

Main changes since the V1:

* moved stmp_device.c from drivers/base to lib/

Main changes since the RFC:

* renamed the macros to STMP_OFFSET_*
* rebased to 3.3-rc6

Tested with a TX28 board.

Thanks,

   Wolfram

Wolfram Sang (3):
  lib: add support for stmp-style devices
  i2c: mxs: use global reset function
  rtc: stmp3xxx: use global stmp_device functionality

 drivers/i2c/busses/Kconfig   |    1 +
 drivers/i2c/busses/i2c-mxs.c |    9 +----
 drivers/rtc/Kconfig          |    1 +
 drivers/rtc/rtc-stmp3xxx.c   |   29 +++++----------
 include/linux/stmp_device.h  |   20 ++++++++++
 lib/Kconfig                  |    3 ++
 lib/Makefile                 |    2 +
 lib/stmp_device.c            |   80 ++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 119 insertions(+), 26 deletions(-)
 create mode 100644 include/linux/stmp_device.h
 create mode 100644 lib/stmp_device.c

-- 
1.7.2.5

[PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

MX23/28 use IP cores which follow a register layout I have first seen on
STMP3xxx SoCs. In this layout, every register actually has four u32:

 1.) to store a value directly
 2.) a SET register where every 1-bit sets the corresponding bit,
     others are unaffected
 3.) same with a CLR register
 4.) same with a TOG (toggle) register

Also, the 2 MSBs in register 0 are always the same and can be used to reset
the IP core.

All this is strictly speaking not mach-specific (but IP core specific) and,
thus, doesn't need to be in mach-mxs/include. At least, mx6 and mx50 also
utilize IP cores following this stmp-style. So:

Introduce a stmp-style device, put the code and defines for that in a public
place (lib/), and let drivers for stmp-style devices select that code.
To avoid regressions and ease reviewing, the actual code is simply copied from
mach-mxs. It definately wants updates, but those need a seperate patch series.

Voila, mach dependency gone, reusable code introduced. Note that I didn't
remove the duplicated code from mach-mxs yet, first the drivers have to be
converted.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
---
 include/linux/stmp_device.h |   20 +++++++++++
 lib/Kconfig                 |    3 ++
 lib/Makefile                |    2 +
 lib/stmp_device.c           |   80 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 105 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/stmp_device.h
 create mode 100644 lib/stmp_device.c

diff --git a/include/linux/stmp_device.h b/include/linux/stmp_device.h
new file mode 100644
index 0000000..6cf7ec9
--- /dev/null
+++ b/include/linux/stmp_device.h
@@ -0,0 +1,20 @@
+/*
+ * basic functions for devices following the "stmp" style register layout
+ *
+ * Copyright (C) 2011 Wolfram Sang, Pengutronix e.K.
+ *
+ * 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.
+ */
+
+#ifndef __STMP_DEVICE_H__
+#define __STMP_DEVICE_H__
+
+#define STMP_OFFSET_REG_SET	0x4
+#define STMP_OFFSET_REG_CLR	0x8
+#define STMP_OFFSET_REG_TOG	0xc
+
+extern int stmp_reset_block(void __iomem *);
+#endif /* __STMP_DEVICE_H__ */
diff --git a/lib/Kconfig b/lib/Kconfig
index 028aba9..d4af46f 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -29,6 +29,9 @@ config GENERIC_IOMAP
 	bool
 	select GENERIC_PCI_IOMAP
 
+config STMP_DEVICE
+	bool
+
 config CRC_CCITT
 	tristate "CRC-CCITT functions"
 	help
diff --git a/lib/Makefile b/lib/Makefile
index 18515f0..f78dbcd 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -123,6 +123,8 @@ obj-$(CONFIG_SIGNATURE) += digsig.o
 
 obj-$(CONFIG_CLZ_TAB) += clz_tab.o
 
+obj-$(CONFIG_STMP_DEVICE) += stmp_device.o
+
 hostprogs-y	:= gen_crc32table
 clean-files	:= crc32table.h
 
diff --git a/lib/stmp_device.c b/lib/stmp_device.c
new file mode 100644
index 0000000..8ac9bcc
--- /dev/null
+++ b/lib/stmp_device.c
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 1999 ARM Limited
+ * Copyright (C) 2000 Deep Blue Solutions Ltd
+ * Copyright 2006-2007,2010 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright 2008 Juergen Beisert, kernel@pengutronix.de
+ * Copyright 2009 Ilya Yanok, Emcraft Systems Ltd, yanok@emcraft.com
+ * Copyright (C) 2011 Wolfram Sang, Pengutronix e.K.
+ *
+ * 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.
+ */
+
+#include <linux/io.h>
+#include <linux/errno.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/stmp_device.h>
+
+#define STMP_MODULE_CLKGATE	(1 << 30)
+#define STMP_MODULE_SFTRST	(1 << 31)
+
+/*
+ * Clear the bit and poll it cleared.  This is usually called with
+ * a reset address and mask being either SFTRST(bit 31) or CLKGATE
+ * (bit 30).
+ */
+static int stmp_clear_poll_bit(void __iomem *addr, u32 mask)
+{
+	int timeout = 0x400;
+
+	writel(mask, addr + STMP_OFFSET_REG_CLR);
+	udelay(1);
+	while ((readl(addr) & mask) && --timeout)
+		/* nothing */;
+
+	return !timeout;
+}
+
+int stmp_reset_block(void __iomem *reset_addr)
+{
+	int ret;
+	int timeout = 0x400;
+
+	/* clear and poll SFTRST */
+	ret = stmp_clear_poll_bit(reset_addr, STMP_MODULE_SFTRST);
+	if (unlikely(ret))
+		goto error;
+
+	/* clear CLKGATE */
+	writel(STMP_MODULE_CLKGATE, reset_addr + STMP_OFFSET_REG_CLR);
+
+	/* set SFTRST to reset the block */
+	writel(STMP_MODULE_SFTRST, reset_addr + STMP_OFFSET_REG_SET);
+	udelay(1);
+
+	/* poll CLKGATE becoming set */
+	while ((!(readl(reset_addr) & STMP_MODULE_CLKGATE)) && --timeout)
+		/* nothing */;
+	if (unlikely(!timeout))
+		goto error;
+
+	/* clear and poll SFTRST */
+	ret = stmp_clear_poll_bit(reset_addr, STMP_MODULE_SFTRST);
+	if (unlikely(ret))
+		goto error;
+
+	/* clear and poll CLKGATE */
+	ret = stmp_clear_poll_bit(reset_addr, STMP_MODULE_CLKGATE);
+	if (unlikely(ret))
+		goto error;
+
+	return 0;
+
+error:
+	pr_err("%s(%p): module reset timeout\n", __func__, reset_addr);
+	return -ETIMEDOUT;
+}
+EXPORT_SYMBOL(stmp_reset_block);
-- 
1.7.2.5

[PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

MX23/28 use IP cores which follow a register layout I have first seen on
STMP3xxx SoCs. In this layout, every register actually has four u32:

 1.) to store a value directly
 2.) a SET register where every 1-bit sets the corresponding bit,
     others are unaffected
 3.) same with a CLR register
 4.) same with a TOG (toggle) register

Also, the 2 MSBs in register 0 are always the same and can be used to reset
the IP core.

All this is strictly speaking not mach-specific (but IP core specific) and,
thus, doesn't need to be in mach-mxs/include. At least, mx6 and mx50 also
utilize IP cores following this stmp-style. So:

Introduce a stmp-style device, put the code and defines for that in a public
place (lib/), and let drivers for stmp-style devices select that code.
To avoid regressions and ease reviewing, the actual code is simply copied from
mach-mxs. It definately wants updates, but those need a seperate patch series.

Voila, mach dependency gone, reusable code introduced. Note that I didn't
remove the duplicated code from mach-mxs yet, first the drivers have to be
converted.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
---
 include/linux/stmp_device.h |   20 +++++++++++
 lib/Kconfig                 |    3 ++
 lib/Makefile                |    2 +
 lib/stmp_device.c           |   80 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 105 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/stmp_device.h
 create mode 100644 lib/stmp_device.c

diff --git a/include/linux/stmp_device.h b/include/linux/stmp_device.h
new file mode 100644
index 0000000..6cf7ec9
--- /dev/null
+++ b/include/linux/stmp_device.h
@@ -0,0 +1,20 @@
+/*
+ * basic functions for devices following the "stmp" style register layout
+ *
+ * Copyright (C) 2011 Wolfram Sang, Pengutronix e.K.
+ *
+ * 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.
+ */
+
+#ifndef __STMP_DEVICE_H__
+#define __STMP_DEVICE_H__
+
+#define STMP_OFFSET_REG_SET	0x4
+#define STMP_OFFSET_REG_CLR	0x8
+#define STMP_OFFSET_REG_TOG	0xc
+
+extern int stmp_reset_block(void __iomem *);
+#endif /* __STMP_DEVICE_H__ */
diff --git a/lib/Kconfig b/lib/Kconfig
index 028aba9..d4af46f 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -29,6 +29,9 @@ config GENERIC_IOMAP
 	bool
 	select GENERIC_PCI_IOMAP
 
+config STMP_DEVICE
+	bool
+
 config CRC_CCITT
 	tristate "CRC-CCITT functions"
 	help
diff --git a/lib/Makefile b/lib/Makefile
index 18515f0..f78dbcd 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -123,6 +123,8 @@ obj-$(CONFIG_SIGNATURE) += digsig.o
 
 obj-$(CONFIG_CLZ_TAB) += clz_tab.o
 
+obj-$(CONFIG_STMP_DEVICE) += stmp_device.o
+
 hostprogs-y	:= gen_crc32table
 clean-files	:= crc32table.h
 
diff --git a/lib/stmp_device.c b/lib/stmp_device.c
new file mode 100644
index 0000000..8ac9bcc
--- /dev/null
+++ b/lib/stmp_device.c
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 1999 ARM Limited
+ * Copyright (C) 2000 Deep Blue Solutions Ltd
+ * Copyright 2006-2007,2010 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright 2008 Juergen Beisert, kernel at pengutronix.de
+ * Copyright 2009 Ilya Yanok, Emcraft Systems Ltd, yanok at emcraft.com
+ * Copyright (C) 2011 Wolfram Sang, Pengutronix e.K.
+ *
+ * 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.
+ */
+
+#include <linux/io.h>
+#include <linux/errno.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/stmp_device.h>
+
+#define STMP_MODULE_CLKGATE	(1 << 30)
+#define STMP_MODULE_SFTRST	(1 << 31)
+
+/*
+ * Clear the bit and poll it cleared.  This is usually called with
+ * a reset address and mask being either SFTRST(bit 31) or CLKGATE
+ * (bit 30).
+ */
+static int stmp_clear_poll_bit(void __iomem *addr, u32 mask)
+{
+	int timeout = 0x400;
+
+	writel(mask, addr + STMP_OFFSET_REG_CLR);
+	udelay(1);
+	while ((readl(addr) & mask) && --timeout)
+		/* nothing */;
+
+	return !timeout;
+}
+
+int stmp_reset_block(void __iomem *reset_addr)
+{
+	int ret;
+	int timeout = 0x400;
+
+	/* clear and poll SFTRST */
+	ret = stmp_clear_poll_bit(reset_addr, STMP_MODULE_SFTRST);
+	if (unlikely(ret))
+		goto error;
+
+	/* clear CLKGATE */
+	writel(STMP_MODULE_CLKGATE, reset_addr + STMP_OFFSET_REG_CLR);
+
+	/* set SFTRST to reset the block */
+	writel(STMP_MODULE_SFTRST, reset_addr + STMP_OFFSET_REG_SET);
+	udelay(1);
+
+	/* poll CLKGATE becoming set */
+	while ((!(readl(reset_addr) & STMP_MODULE_CLKGATE)) && --timeout)
+		/* nothing */;
+	if (unlikely(!timeout))
+		goto error;
+
+	/* clear and poll SFTRST */
+	ret = stmp_clear_poll_bit(reset_addr, STMP_MODULE_SFTRST);
+	if (unlikely(ret))
+		goto error;
+
+	/* clear and poll CLKGATE */
+	ret = stmp_clear_poll_bit(reset_addr, STMP_MODULE_CLKGATE);
+	if (unlikely(ret))
+		goto error;
+
+	return 0;
+
+error:
+	pr_err("%s(%p): module reset timeout\n", __func__, reset_addr);
+	return -ETIMEDOUT;
+}
+EXPORT_SYMBOL(stmp_reset_block);
-- 
1.7.2.5

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Huang Shijie]

Hi Wolfram:
>  include/linux/stmp_device.h |   20 +++++++++++
IMHO, this header should be placed at include/linux/fsl/.
But the patch "mxs-dma : move the mxs dma.h to a more common place" is
still in the l2-mtd tree, not pushed
to upstream. :(

BR
Huang Shijie

[PATCH V2 1/3] lib: add support for stmp-style devices

[Huang Shijie]

Hi Wolfram:
>  include/linux/stmp_device.h |   20 +++++++++++
IMHO, this header should be placed at include/linux/fsl/.
But the patch "mxs-dma : move the mxs dma.h to a more common place" is
still in the l2-mtd tree, not pushed
to upstream. :(

BR
Huang Shijie

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

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

Hi,

> >  include/linux/stmp_device.h |   20 +++++++++++
> IMHO, this header should be placed at include/linux/fsl/.

Hmm, one never knows if somewhen in the future some other vendor decides to do
do a device with this register layout. It is just a layout, not an IP core per
se. So, I'd prefer to leave it as is.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

Hi,

> >  include/linux/stmp_device.h |   20 +++++++++++
> IMHO, this header should be placed at include/linux/fsl/.

Hmm, one never knows if somewhen in the future some other vendor decides to do
do a device with this register layout. It is just a layout, not an IP core per
se. So, I'd prefer to leave it as is.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120329/bac4e086/attachment.sig>

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Dong Aisheng]

Hi Wolfram,

On Thu, Mar 29, 2012 at 08:43:20AM +0200, Wolfram Sang wrote:
> Hi,
> 
> > >  include/linux/stmp_device.h |   20 +++++++++++
> > IMHO, this header should be placed at include/linux/fsl/.
> 
> Hmm, one never knows if somewhen in the future some other vendor decides to do
> do a device with this register layout. It is just a layout, not an IP core per
> se. So, I'd prefer to leave it as is.
> 

Acked-by: Dong Aisheng <dong.aisheng@linaro.org>

I'm converting mxs-dma driver to be mach-independent which also needs
this patch.

Do you know when this will be merged?

BTW, one small question:
Is it possible that stmp_reset_block also be used by other vendors?
If not, i guess include/linux/fsl which already exists may be the
right place.

Regards
Dong Aisheng

[PATCH V2 1/3] lib: add support for stmp-style devices

[Dong Aisheng]

Hi Wolfram,

On Thu, Mar 29, 2012 at 08:43:20AM +0200, Wolfram Sang wrote:
> Hi,
> 
> > >  include/linux/stmp_device.h |   20 +++++++++++
> > IMHO, this header should be placed at include/linux/fsl/.
> 
> Hmm, one never knows if somewhen in the future some other vendor decides to do
> do a device with this register layout. It is just a layout, not an IP core per
> se. So, I'd prefer to leave it as is.
> 

Acked-by: Dong Aisheng <dong.aisheng@linaro.org>

I'm converting mxs-dma driver to be mach-independent which also needs
this patch.

Do you know when this will be merged?

BTW, one small question:
Is it possible that stmp_reset_block also be used by other vendors?
If not, i guess include/linux/fsl which already exists may be the
right place.

Regards
Dong Aisheng

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

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

On Wed, Apr 18, 2012 at 05:05:52PM +0800, Dong Aisheng wrote:
> Hi Wolfram,
> 
> On Thu, Mar 29, 2012 at 08:43:20AM +0200, Wolfram Sang wrote:
> > Hi,
> > 
> > > >  include/linux/stmp_device.h |   20 +++++++++++
> > > IMHO, this header should be placed at include/linux/fsl/.
> > 
> > Hmm, one never knows if somewhen in the future some other vendor decides to do
> > do a device with this register layout. It is just a layout, not an IP core per
> > se. So, I'd prefer to leave it as is.
> > 
> 
> Acked-by: Dong Aisheng <dong.aisheng@linaro.org>

Thanks!

> 
> I'm converting mxs-dma driver to be mach-independent which also needs
> this patch.
> 
> Do you know when this will be merged?

Nope. I assumed it could go in via arm-soc, since it makes sense and
Arnd used to comment this series. He is probably busy, currently.

> BTW, one small question:
> Is it possible that stmp_reset_block also be used by other vendors?

Sure, why not? I could ask a friend to to do an FPGA for me with this
layout.

> If not, i guess include/linux/fsl which already exists may be the
> right place.

Which is a bad choice in my book. linux/dma would have been the proper
thing for mxs-dma.h, I'd think.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

On Wed, Apr 18, 2012 at 05:05:52PM +0800, Dong Aisheng wrote:
> Hi Wolfram,
> 
> On Thu, Mar 29, 2012 at 08:43:20AM +0200, Wolfram Sang wrote:
> > Hi,
> > 
> > > >  include/linux/stmp_device.h |   20 +++++++++++
> > > IMHO, this header should be placed at include/linux/fsl/.
> > 
> > Hmm, one never knows if somewhen in the future some other vendor decides to do
> > do a device with this register layout. It is just a layout, not an IP core per
> > se. So, I'd prefer to leave it as is.
> > 
> 
> Acked-by: Dong Aisheng <dong.aisheng@linaro.org>

Thanks!

> 
> I'm converting mxs-dma driver to be mach-independent which also needs
> this patch.
> 
> Do you know when this will be merged?

Nope. I assumed it could go in via arm-soc, since it makes sense and
Arnd used to comment this series. He is probably busy, currently.

> BTW, one small question:
> Is it possible that stmp_reset_block also be used by other vendors?

Sure, why not? I could ask a friend to to do an FPGA for me with this
layout.

> If not, i guess include/linux/fsl which already exists may be the
> right place.

Which is a bad choice in my book. linux/dma would have been the proper
thing for mxs-dma.h, I'd think.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120418/e219cda9/attachment.sig>

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Arnd Bergmann]

On Wednesday 18 April 2012, Wolfram Sang wrote:
> > 
> > I'm converting mxs-dma driver to be mach-independent which also needs
> > this patch.
> > 
> > Do you know when this will be merged?
> 
> Nope. I assumed it could go in via arm-soc, since it makes sense and
> Arnd used to comment this series. He is probably busy, currently.

I think that's fine to go in arm-soc, please send a pull request for this.

	Arnd

[PATCH V2 1/3] lib: add support for stmp-style devices

[Arnd Bergmann]

On Wednesday 18 April 2012, Wolfram Sang wrote:
> > 
> > I'm converting mxs-dma driver to be mach-independent which also needs
> > this patch.
> > 
> > Do you know when this will be merged?
> 
> Nope. I assumed it could go in via arm-soc, since it makes sense and
> Arnd used to comment this series. He is probably busy, currently.

I think that's fine to go in arm-soc, please send a pull request for this.

	Arnd

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Shawn Guo]

On Wed, Apr 18, 2012 at 11:20:19AM +0200, Wolfram Sang wrote:
> > If not, i guess include/linux/fsl which already exists may be the
> > right place.
> 
> Which is a bad choice in my book. linux/dma would have been the proper
> thing for mxs-dma.h, I'd think.
> 
If it already exists, it will be the proper place.  We do not want to
be too invasive to create folders in include/linux for every subsystem
for only the need of fsl drivers.

On the other hand, I have seen a clear need for include/linux/fsl.
The following files are all good candidates to be moved there.

  include/linux/fsl_devices.h
  include/linux/fsl-diu-fb.h
  include/linux/fsl_hypervisor.h

IMO, include/linux/stmp_device.h is just another good candidate.

Oh, today I was asked by Fabio where is the good place for ssp-regs.h
which will be shared by mxs-mmc and spi-mxs, as we are cleaning up
<mach/*> from drivers.  I told him include/linux/fsl.

-- 
Regards,
Shawn

[PATCH V2 1/3] lib: add support for stmp-style devices

[Shawn Guo]

On Wed, Apr 18, 2012 at 11:20:19AM +0200, Wolfram Sang wrote:
> > If not, i guess include/linux/fsl which already exists may be the
> > right place.
> 
> Which is a bad choice in my book. linux/dma would have been the proper
> thing for mxs-dma.h, I'd think.
> 
If it already exists, it will be the proper place.  We do not want to
be too invasive to create folders in include/linux for every subsystem
for only the need of fsl drivers.

On the other hand, I have seen a clear need for include/linux/fsl.
The following files are all good candidates to be moved there.

  include/linux/fsl_devices.h
  include/linux/fsl-diu-fb.h
  include/linux/fsl_hypervisor.h

IMO, include/linux/stmp_device.h is just another good candidate.

Oh, today I was asked by Fabio where is the good place for ssp-regs.h
which will be shared by mxs-mmc and spi-mxs, as we are cleaning up
<mach/*> from drivers.  I told him include/linux/fsl.

-- 
Regards,
Shawn

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

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

On Fri, Apr 20, 2012 at 01:30:17PM +0800, Shawn Guo wrote:
> On Wed, Apr 18, 2012 at 11:20:19AM +0200, Wolfram Sang wrote:
> > > If not, i guess include/linux/fsl which already exists may be the
> > > right place.
> > 
> > Which is a bad choice in my book. linux/dma would have been the proper
> > thing for mxs-dma.h, I'd think.
> > 
> If it already exists, it will be the proper place.  We do not want to
> be too invasive to create folders in include/linux for every subsystem
> for only the need of fsl drivers.

I think linux/dma would have made sense:

$:~/Kernel/linux/include/linux$ ll *dma*
-rw-r--r-- 1 ninja ninja  1809 Apr 15 23:32 dma-attrs.h
-rw-r--r-- 1 ninja ninja  8343 Apr 15 23:32 dma-buf.h
-rw-r--r-- 1 ninja ninja  4915 Aug 20  2011 dma-debug.h
-rw-r--r-- 1 ninja ninja   299 Nov  8 20:06 dma-direction.h
-rw-r--r-- 1 ninja ninja 33123 Apr 16 11:47 dmaengine.h
-rw-r--r-- 1 ninja ninja  7018 Apr 15 23:32 dma-mapping.h
-rw-r--r-- 1 ninja ninja   923 Aug 20  2011 dmapool.h
-rw-r--r-- 1 ninja ninja  1288 Mar 30 22:12 dma_remapping.h
-rw-r--r-- 1 ninja ninja  6933 Mar 30 22:12 dmar.h
-rw-r--r-- 1 ninja ninja  3203 Apr 15 23:32 dw_dmac.h
-rw-r--r-- 1 ninja ninja   157 Aug 20  2011 init_ohci1394_dma.h
-rw-r--r-- 1 ninja ninja  2463 Aug 20  2011 intel_mid_dma.h
-rw-r--r-- 1 ninja ninja  3305 Apr 15 23:32 nfs_idmap.h
-rw-r--r-- 1 ninja ninja   999 Aug 20  2011 pch_dma.h
-rw-r--r-- 1 ninja ninja   415 Aug 20  2011 pci-dma.h
-rw-r--r-- 1 ninja ninja   572 Apr 15 23:32 sa11x0-dma.h
-rw-r--r-- 1 ninja ninja  2631 Apr 15 23:32 sh_dma.h
-rw-r--r-- 1 ninja ninja   123 Mar 30 22:12 sirfsoc_dma.h
-rw-r--r-- 1 ninja ninja  1784 Aug 20  2011 timb_dma.h

> On the other hand, I have seen a clear need for include/linux/fsl.
> The following files are all good candidates to be moved there.
> 
>   include/linux/fsl_devices.h

I stumbled over this file some time ago. It is an arbitrary collection of stuff
which should better be seperate. See 'fsl_spi_platform_data'? That should be
cpm_spi_platform_data. It doesn't even fit to MPC5200 (which SPI driver
unnecessarily includes this file), let alone i.MX, so calling it 'fsl' doesn't
make sense at all. I fear similar confusion with a fsl-directory.

>   include/linux/fsl-diu-fb.h

Sadly, another bad choice. It should have gone to include/video like all other
fb-drivers.

>   include/linux/fsl_hypervisor.h

And that could simply stay in include/linux then, like kvm does.

> IMO, include/linux/stmp_device.h is just another good candidate.

Still not convinced, more the opposite.

> Oh, today I was asked by Fabio where is the good place for ssp-regs.h
> which will be shared by mxs-mmc and spi-mxs, as we are cleaning up
> <mach/*> from drivers.  I told him include/linux/fsl.

What's wrong with linux/spi? SPI devices capable of doing MMC as well shouldn't
be much of a surprise, they are common.

Also, what will happen if Freescale ever gets renamed?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

On Fri, Apr 20, 2012 at 01:30:17PM +0800, Shawn Guo wrote:
> On Wed, Apr 18, 2012 at 11:20:19AM +0200, Wolfram Sang wrote:
> > > If not, i guess include/linux/fsl which already exists may be the
> > > right place.
> > 
> > Which is a bad choice in my book. linux/dma would have been the proper
> > thing for mxs-dma.h, I'd think.
> > 
> If it already exists, it will be the proper place.  We do not want to
> be too invasive to create folders in include/linux for every subsystem
> for only the need of fsl drivers.

I think linux/dma would have made sense:

$:~/Kernel/linux/include/linux$ ll *dma*
-rw-r--r-- 1 ninja ninja  1809 Apr 15 23:32 dma-attrs.h
-rw-r--r-- 1 ninja ninja  8343 Apr 15 23:32 dma-buf.h
-rw-r--r-- 1 ninja ninja  4915 Aug 20  2011 dma-debug.h
-rw-r--r-- 1 ninja ninja   299 Nov  8 20:06 dma-direction.h
-rw-r--r-- 1 ninja ninja 33123 Apr 16 11:47 dmaengine.h
-rw-r--r-- 1 ninja ninja  7018 Apr 15 23:32 dma-mapping.h
-rw-r--r-- 1 ninja ninja   923 Aug 20  2011 dmapool.h
-rw-r--r-- 1 ninja ninja  1288 Mar 30 22:12 dma_remapping.h
-rw-r--r-- 1 ninja ninja  6933 Mar 30 22:12 dmar.h
-rw-r--r-- 1 ninja ninja  3203 Apr 15 23:32 dw_dmac.h
-rw-r--r-- 1 ninja ninja   157 Aug 20  2011 init_ohci1394_dma.h
-rw-r--r-- 1 ninja ninja  2463 Aug 20  2011 intel_mid_dma.h
-rw-r--r-- 1 ninja ninja  3305 Apr 15 23:32 nfs_idmap.h
-rw-r--r-- 1 ninja ninja   999 Aug 20  2011 pch_dma.h
-rw-r--r-- 1 ninja ninja   415 Aug 20  2011 pci-dma.h
-rw-r--r-- 1 ninja ninja   572 Apr 15 23:32 sa11x0-dma.h
-rw-r--r-- 1 ninja ninja  2631 Apr 15 23:32 sh_dma.h
-rw-r--r-- 1 ninja ninja   123 Mar 30 22:12 sirfsoc_dma.h
-rw-r--r-- 1 ninja ninja  1784 Aug 20  2011 timb_dma.h

> On the other hand, I have seen a clear need for include/linux/fsl.
> The following files are all good candidates to be moved there.
> 
>   include/linux/fsl_devices.h

I stumbled over this file some time ago. It is an arbitrary collection of stuff
which should better be seperate. See 'fsl_spi_platform_data'? That should be
cpm_spi_platform_data. It doesn't even fit to MPC5200 (which SPI driver
unnecessarily includes this file), let alone i.MX, so calling it 'fsl' doesn't
make sense at all. I fear similar confusion with a fsl-directory.

>   include/linux/fsl-diu-fb.h

Sadly, another bad choice. It should have gone to include/video like all other
fb-drivers.

>   include/linux/fsl_hypervisor.h

And that could simply stay in include/linux then, like kvm does.

> IMO, include/linux/stmp_device.h is just another good candidate.

Still not convinced, more the opposite.

> Oh, today I was asked by Fabio where is the good place for ssp-regs.h
> which will be shared by mxs-mmc and spi-mxs, as we are cleaning up
> <mach/*> from drivers.  I told him include/linux/fsl.

What's wrong with linux/spi? SPI devices capable of doing MMC as well shouldn't
be much of a surprise, they are common.

Also, what will happen if Freescale ever gets renamed?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120420/d3a18aca/attachment.sig>

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Arnd Bergmann]

On Friday 20 April 2012, Wolfram Sang wrote:
> On Fri, Apr 20, 2012 at 01:30:17PM +0800, Shawn Guo wrote:
> > On Wed, Apr 18, 2012 at 11:20:19AM +0200, Wolfram Sang wrote:
> > > > If not, i guess include/linux/fsl which already exists may be the
> > > > right place.
> > > 
> > > Which is a bad choice in my book. linux/dma would have been the proper
> > > thing for mxs-dma.h, I'd think.
> > > 
> > If it already exists, it will be the proper place.  We do not want to
> > be too invasive to create folders in include/linux for every subsystem
> > for only the need of fsl drivers.
> 
> I think linux/dma would have made sense:
> 
> $:~/Kernel/linux/include/linux$ ll *dma*
> -rw-r--r-- 1 ninja ninja  1809 Apr 15 23:32 dma-attrs.h
> ...
> -rw-r--r-- 1 ninja ninja  2631 Apr 15 23:32 sh_dma.h
> -rw-r--r-- 1 ninja ninja   123 Mar 30 22:12 sirfsoc_dma.h
> -rw-r--r-- 1 ninja ninja  1784 Aug 20  2011 timb_dma.h

+1

> > On the other hand, I have seen a clear need for include/linux/fsl.
> > The following files are all good candidates to be moved there.
> > 
> >   include/linux/fsl_devices.h
> 
> I stumbled over this file some time ago. It is an arbitrary collection of stuff
> which should better be seperate. See 'fsl_spi_platform_data'? That should be
> cpm_spi_platform_data. It doesn't even fit to MPC5200 (which SPI driver
> unnecessarily includes this file), let alone i.MX, so calling it 'fsl' doesn't
> make sense at all. I fear similar confusion with a fsl-directory.

Agreed. I would much prefer to avoid any such vendor specific silos for where to
dump random code. A file for a company that makes both powerpc and arm SoCs
that have a few components in common but also share other components with other
SoCs of the respective architectures does not seem very useful to me.

> >   include/linux/fsl-diu-fb.h
> 
> Sadly, another bad choice. It should have gone to include/video like all other
> fb-drivers.
> 
> >   include/linux/fsl_hypervisor.h
> 
> And that could simply stay in include/linux then, like kvm does.

Well, we could also have a linux/virt/ directory. Unfortunately in both these
cases we have no choice any more because they contain ioctl data structures
that are exported to user space, so we cannot move these files without
breaking user space programs and we don't do that.

	Arnd

[PATCH V2 1/3] lib: add support for stmp-style devices

[Arnd Bergmann]

On Friday 20 April 2012, Wolfram Sang wrote:
> On Fri, Apr 20, 2012 at 01:30:17PM +0800, Shawn Guo wrote:
> > On Wed, Apr 18, 2012 at 11:20:19AM +0200, Wolfram Sang wrote:
> > > > If not, i guess include/linux/fsl which already exists may be the
> > > > right place.
> > > 
> > > Which is a bad choice in my book. linux/dma would have been the proper
> > > thing for mxs-dma.h, I'd think.
> > > 
> > If it already exists, it will be the proper place.  We do not want to
> > be too invasive to create folders in include/linux for every subsystem
> > for only the need of fsl drivers.
> 
> I think linux/dma would have made sense:
> 
> $:~/Kernel/linux/include/linux$ ll *dma*
> -rw-r--r-- 1 ninja ninja  1809 Apr 15 23:32 dma-attrs.h
> ...
> -rw-r--r-- 1 ninja ninja  2631 Apr 15 23:32 sh_dma.h
> -rw-r--r-- 1 ninja ninja   123 Mar 30 22:12 sirfsoc_dma.h
> -rw-r--r-- 1 ninja ninja  1784 Aug 20  2011 timb_dma.h

+1

> > On the other hand, I have seen a clear need for include/linux/fsl.
> > The following files are all good candidates to be moved there.
> > 
> >   include/linux/fsl_devices.h
> 
> I stumbled over this file some time ago. It is an arbitrary collection of stuff
> which should better be seperate. See 'fsl_spi_platform_data'? That should be
> cpm_spi_platform_data. It doesn't even fit to MPC5200 (which SPI driver
> unnecessarily includes this file), let alone i.MX, so calling it 'fsl' doesn't
> make sense at all. I fear similar confusion with a fsl-directory.

Agreed. I would much prefer to avoid any such vendor specific silos for where to
dump random code. A file for a company that makes both powerpc and arm SoCs
that have a few components in common but also share other components with other
SoCs of the respective architectures does not seem very useful to me.

> >   include/linux/fsl-diu-fb.h
> 
> Sadly, another bad choice. It should have gone to include/video like all other
> fb-drivers.
> 
> >   include/linux/fsl_hypervisor.h
> 
> And that could simply stay in include/linux then, like kvm does.

Well, we could also have a linux/virt/ directory. Unfortunately in both these
cases we have no choice any more because they contain ioctl data structures
that are exported to user space, so we cannot move these files without
breaking user space programs and we don't do that.

	Arnd

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Huang Shijie]

HI Wolfram:

Could you provide an API such as gpmi_reset_block()?
It will much helpful to me.

BR
Huang Shijie

On Wed, Mar 21, 2012 at 6:21 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> MX23/28 use IP cores which follow a register layout I have first seen on
> STMP3xxx SoCs. In this layout, every register actually has four u32:
>
>  1.) to store a value directly
>  2.) a SET register where every 1-bit sets the corresponding bit,
>     others are unaffected
>  3.) same with a CLR register
>  4.) same with a TOG (toggle) register
>
> Also, the 2 MSBs in register 0 are always the same and can be used to reset
> the IP core.
>
> All this is strictly speaking not mach-specific (but IP core specific) and,
> thus, doesn't need to be in mach-mxs/include. At least, mx6 and mx50 also
> utilize IP cores following this stmp-style. So:
>
> Introduce a stmp-style device, put the code and defines for that in a public
> place (lib/), and let drivers for stmp-style devices select that code.
> To avoid regressions and ease reviewing, the actual code is simply copied from
> mach-mxs. It definately wants updates, but those need a seperate patch series.
>
> Voila, mach dependency gone, reusable code introduced. Note that I didn't
> remove the duplicated code from mach-mxs yet, first the drivers have to be
> converted.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  include/linux/stmp_device.h |   20 +++++++++++
>  lib/Kconfig                 |    3 ++
>  lib/Makefile                |    2 +
>  lib/stmp_device.c           |   80 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 105 insertions(+), 0 deletions(-)
>  create mode 100644 include/linux/stmp_device.h
>  create mode 100644 lib/stmp_device.c
>
> diff --git a/include/linux/stmp_device.h b/include/linux/stmp_device.h
> new file mode 100644
> index 0000000..6cf7ec9
> --- /dev/null
> +++ b/include/linux/stmp_device.h
> @@ -0,0 +1,20 @@
> +/*
> + * basic functions for devices following the "stmp" style register layout
> + *
> + * Copyright (C) 2011 Wolfram Sang, Pengutronix e.K.
> + *
> + * 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.
> + */
> +
> +#ifndef __STMP_DEVICE_H__
> +#define __STMP_DEVICE_H__
> +
> +#define STMP_OFFSET_REG_SET    0x4
> +#define STMP_OFFSET_REG_CLR    0x8
> +#define STMP_OFFSET_REG_TOG    0xc
> +
> +extern int stmp_reset_block(void __iomem *);
> +#endif /* __STMP_DEVICE_H__ */
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 028aba9..d4af46f 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -29,6 +29,9 @@ config GENERIC_IOMAP
>        bool
>        select GENERIC_PCI_IOMAP
>
> +config STMP_DEVICE
> +       bool
> +
>  config CRC_CCITT
>        tristate "CRC-CCITT functions"
>        help
> diff --git a/lib/Makefile b/lib/Makefile
> index 18515f0..f78dbcd 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -123,6 +123,8 @@ obj-$(CONFIG_SIGNATURE) += digsig.o
>
>  obj-$(CONFIG_CLZ_TAB) += clz_tab.o
>
> +obj-$(CONFIG_STMP_DEVICE) += stmp_device.o
> +
>  hostprogs-y    := gen_crc32table
>  clean-files    := crc32table.h
>
> diff --git a/lib/stmp_device.c b/lib/stmp_device.c
> new file mode 100644
> index 0000000..8ac9bcc
> --- /dev/null
> +++ b/lib/stmp_device.c
> @@ -0,0 +1,80 @@
> +/*
> + * Copyright (C) 1999 ARM Limited
> + * Copyright (C) 2000 Deep Blue Solutions Ltd
> + * Copyright 2006-2007,2010 Freescale Semiconductor, Inc. All Rights Reserved.
> + * Copyright 2008 Juergen Beisert, kernel@pengutronix.de
> + * Copyright 2009 Ilya Yanok, Emcraft Systems Ltd, yanok@emcraft.com
> + * Copyright (C) 2011 Wolfram Sang, Pengutronix e.K.
> + *
> + * 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.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/errno.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/stmp_device.h>
> +
> +#define STMP_MODULE_CLKGATE    (1 << 30)
> +#define STMP_MODULE_SFTRST     (1 << 31)
> +
> +/*
> + * Clear the bit and poll it cleared.  This is usually called with
> + * a reset address and mask being either SFTRST(bit 31) or CLKGATE
> + * (bit 30).
> + */
> +static int stmp_clear_poll_bit(void __iomem *addr, u32 mask)
> +{
> +       int timeout = 0x400;
> +
> +       writel(mask, addr + STMP_OFFSET_REG_CLR);
> +       udelay(1);
> +       while ((readl(addr) & mask) && --timeout)
> +               /* nothing */;
> +
> +       return !timeout;
> +}
> +
> +int stmp_reset_block(void __iomem *reset_addr)
> +{
> +       int ret;
> +       int timeout = 0x400;
> +
> +       /* clear and poll SFTRST */
> +       ret = stmp_clear_poll_bit(reset_addr, STMP_MODULE_SFTRST);
> +       if (unlikely(ret))
> +               goto error;
> +
> +       /* clear CLKGATE */
> +       writel(STMP_MODULE_CLKGATE, reset_addr + STMP_OFFSET_REG_CLR);
> +
> +       /* set SFTRST to reset the block */
> +       writel(STMP_MODULE_SFTRST, reset_addr + STMP_OFFSET_REG_SET);
> +       udelay(1);
> +
> +       /* poll CLKGATE becoming set */
> +       while ((!(readl(reset_addr) & STMP_MODULE_CLKGATE)) && --timeout)
> +               /* nothing */;
> +       if (unlikely(!timeout))
> +               goto error;
> +
> +       /* clear and poll SFTRST */
> +       ret = stmp_clear_poll_bit(reset_addr, STMP_MODULE_SFTRST);
> +       if (unlikely(ret))
> +               goto error;
> +
> +       /* clear and poll CLKGATE */
> +       ret = stmp_clear_poll_bit(reset_addr, STMP_MODULE_CLKGATE);
> +       if (unlikely(ret))
> +               goto error;
> +
> +       return 0;
> +
> +error:
> +       pr_err("%s(%p): module reset timeout\n", __func__, reset_addr);
> +       return -ETIMEDOUT;
> +}
> +EXPORT_SYMBOL(stmp_reset_block);
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V2 1/3] lib: add support for stmp-style devices

[Huang Shijie]

HI Wolfram:

Could you provide an API such as gpmi_reset_block()?
It will much helpful to me.

BR
Huang Shijie

On Wed, Mar 21, 2012 at 6:21 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> MX23/28 use IP cores which follow a register layout I have first seen on
> STMP3xxx SoCs. In this layout, every register actually has four u32:
>
> ?1.) to store a value directly
> ?2.) a SET register where every 1-bit sets the corresponding bit,
> ? ? others are unaffected
> ?3.) same with a CLR register
> ?4.) same with a TOG (toggle) register
>
> Also, the 2 MSBs in register 0 are always the same and can be used to reset
> the IP core.
>
> All this is strictly speaking not mach-specific (but IP core specific) and,
> thus, doesn't need to be in mach-mxs/include. At least, mx6 and mx50 also
> utilize IP cores following this stmp-style. So:
>
> Introduce a stmp-style device, put the code and defines for that in a public
> place (lib/), and let drivers for stmp-style devices select that code.
> To avoid regressions and ease reviewing, the actual code is simply copied from
> mach-mxs. It definately wants updates, but those need a seperate patch series.
>
> Voila, mach dependency gone, reusable code introduced. Note that I didn't
> remove the duplicated code from mach-mxs yet, first the drivers have to be
> converted.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> ?include/linux/stmp_device.h | ? 20 +++++++++++
> ?lib/Kconfig ? ? ? ? ? ? ? ? | ? ?3 ++
> ?lib/Makefile ? ? ? ? ? ? ? ?| ? ?2 +
> ?lib/stmp_device.c ? ? ? ? ? | ? 80 +++++++++++++++++++++++++++++++++++++++++++
> ?4 files changed, 105 insertions(+), 0 deletions(-)
> ?create mode 100644 include/linux/stmp_device.h
> ?create mode 100644 lib/stmp_device.c
>
> diff --git a/include/linux/stmp_device.h b/include/linux/stmp_device.h
> new file mode 100644
> index 0000000..6cf7ec9
> --- /dev/null
> +++ b/include/linux/stmp_device.h
> @@ -0,0 +1,20 @@
> +/*
> + * basic functions for devices following the "stmp" style register layout
> + *
> + * Copyright (C) 2011 Wolfram Sang, Pengutronix e.K.
> + *
> + * 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.
> + */
> +
> +#ifndef __STMP_DEVICE_H__
> +#define __STMP_DEVICE_H__
> +
> +#define STMP_OFFSET_REG_SET ? ?0x4
> +#define STMP_OFFSET_REG_CLR ? ?0x8
> +#define STMP_OFFSET_REG_TOG ? ?0xc
> +
> +extern int stmp_reset_block(void __iomem *);
> +#endif /* __STMP_DEVICE_H__ */
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 028aba9..d4af46f 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -29,6 +29,9 @@ config GENERIC_IOMAP
> ? ? ? ?bool
> ? ? ? ?select GENERIC_PCI_IOMAP
>
> +config STMP_DEVICE
> + ? ? ? bool
> +
> ?config CRC_CCITT
> ? ? ? ?tristate "CRC-CCITT functions"
> ? ? ? ?help
> diff --git a/lib/Makefile b/lib/Makefile
> index 18515f0..f78dbcd 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -123,6 +123,8 @@ obj-$(CONFIG_SIGNATURE) += digsig.o
>
> ?obj-$(CONFIG_CLZ_TAB) += clz_tab.o
>
> +obj-$(CONFIG_STMP_DEVICE) += stmp_device.o
> +
> ?hostprogs-y ? ?:= gen_crc32table
> ?clean-files ? ?:= crc32table.h
>
> diff --git a/lib/stmp_device.c b/lib/stmp_device.c
> new file mode 100644
> index 0000000..8ac9bcc
> --- /dev/null
> +++ b/lib/stmp_device.c
> @@ -0,0 +1,80 @@
> +/*
> + * Copyright (C) 1999 ARM Limited
> + * Copyright (C) 2000 Deep Blue Solutions Ltd
> + * Copyright 2006-2007,2010 Freescale Semiconductor, Inc. All Rights Reserved.
> + * Copyright 2008 Juergen Beisert, kernel at pengutronix.de
> + * Copyright 2009 Ilya Yanok, Emcraft Systems Ltd, yanok at emcraft.com
> + * Copyright (C) 2011 Wolfram Sang, Pengutronix e.K.
> + *
> + * 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.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/errno.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/stmp_device.h>
> +
> +#define STMP_MODULE_CLKGATE ? ?(1 << 30)
> +#define STMP_MODULE_SFTRST ? ? (1 << 31)
> +
> +/*
> + * Clear the bit and poll it cleared. ?This is usually called with
> + * a reset address and mask being either SFTRST(bit 31) or CLKGATE
> + * (bit 30).
> + */
> +static int stmp_clear_poll_bit(void __iomem *addr, u32 mask)
> +{
> + ? ? ? int timeout = 0x400;
> +
> + ? ? ? writel(mask, addr + STMP_OFFSET_REG_CLR);
> + ? ? ? udelay(1);
> + ? ? ? while ((readl(addr) & mask) && --timeout)
> + ? ? ? ? ? ? ? /* nothing */;
> +
> + ? ? ? return !timeout;
> +}
> +
> +int stmp_reset_block(void __iomem *reset_addr)
> +{
> + ? ? ? int ret;
> + ? ? ? int timeout = 0x400;
> +
> + ? ? ? /* clear and poll SFTRST */
> + ? ? ? ret = stmp_clear_poll_bit(reset_addr, STMP_MODULE_SFTRST);
> + ? ? ? if (unlikely(ret))
> + ? ? ? ? ? ? ? goto error;
> +
> + ? ? ? /* clear CLKGATE */
> + ? ? ? writel(STMP_MODULE_CLKGATE, reset_addr + STMP_OFFSET_REG_CLR);
> +
> + ? ? ? /* set SFTRST to reset the block */
> + ? ? ? writel(STMP_MODULE_SFTRST, reset_addr + STMP_OFFSET_REG_SET);
> + ? ? ? udelay(1);
> +
> + ? ? ? /* poll CLKGATE becoming set */
> + ? ? ? while ((!(readl(reset_addr) & STMP_MODULE_CLKGATE)) && --timeout)
> + ? ? ? ? ? ? ? /* nothing */;
> + ? ? ? if (unlikely(!timeout))
> + ? ? ? ? ? ? ? goto error;
> +
> + ? ? ? /* clear and poll SFTRST */
> + ? ? ? ret = stmp_clear_poll_bit(reset_addr, STMP_MODULE_SFTRST);
> + ? ? ? if (unlikely(ret))
> + ? ? ? ? ? ? ? goto error;
> +
> + ? ? ? /* clear and poll CLKGATE */
> + ? ? ? ret = stmp_clear_poll_bit(reset_addr, STMP_MODULE_CLKGATE);
> + ? ? ? if (unlikely(ret))
> + ? ? ? ? ? ? ? goto error;
> +
> + ? ? ? return 0;
> +
> +error:
> + ? ? ? pr_err("%s(%p): module reset timeout\n", __func__, reset_addr);
> + ? ? ? return -ETIMEDOUT;
> +}
> +EXPORT_SYMBOL(stmp_reset_block);
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

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

Hi,

> Could you provide an API such as gpmi_reset_block()?
> It will much helpful to me.

Ah, that's what you meant in the other thread. Well, this can be added
later. My suggestion

1) get this mainline to remove mach dependencies
2) convert the drivers
3) do proper timeout mechanisms
4) add features

where 2-4 can probably happen more or less simultaneously.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

Hi,

> Could you provide an API such as gpmi_reset_block()?
> It will much helpful to me.

Ah, that's what you meant in the other thread. Well, this can be added
later. My suggestion

1) get this mainline to remove mach dependencies
2) convert the drivers
3) do proper timeout mechanisms
4) add features

where 2-4 can probably happen more or less simultaneously.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120404/564fdad5/attachment.sig>

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Huang Shijie]

Hi Wolfram:

On Wed, Apr 4, 2012 at 8:23 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> Hi,
>
>> Could you provide an API such as gpmi_reset_block()?
>> It will much helpful to me.
>
> Ah, that's what you meant in the other thread. Well, this can be added
> later. My suggestion
>
> 1) get this mainline to remove mach dependencies
> 2) convert the drivers
> 3) do proper timeout mechanisms
> 4) add features
>
> where 2-4 can probably happen more or less simultaneously.
I wish you could send out the new patch as soon as possible. :)

Best Regards
Huang Shijie



>
> Regards,
>
>   Wolfram
>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAk98PUcACgkQD27XaX1/VRvkOQCdGqBQAgDeW5xpykbKe4LTlqXA
> twIAnA+IivcJwTQsnrPSjLJWTNlFAULt
> =01hN
> -----END PGP SIGNATURE-----
>

[PATCH V2 1/3] lib: add support for stmp-style devices

[Huang Shijie]

Hi Wolfram:

On Wed, Apr 4, 2012 at 8:23 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> Hi,
>
>> Could you provide an API such as gpmi_reset_block()?
>> It will much helpful to me.
>
> Ah, that's what you meant in the other thread. Well, this can be added
> later. My suggestion
>
> 1) get this mainline to remove mach dependencies
> 2) convert the drivers
> 3) do proper timeout mechanisms
> 4) add features
>
> where 2-4 can probably happen more or less simultaneously.
I wish you could send out the new patch as soon as possible. :)

Best Regards
Huang Shijie



>
> Regards,
>
> ? Wolfram
>
> --
> Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? | Wolfram Sang ? ? ? ? ? ? ? ?|
> Industrial Linux Solutions ? ? ? ? ? ? ? ? | http://www.pengutronix.de/ ?|
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAk98PUcACgkQD27XaX1/VRvkOQCdGqBQAgDeW5xpykbKe4LTlqXA
> twIAnA+IivcJwTQsnrPSjLJWTNlFAULt
> =01hN
> -----END PGP SIGNATURE-----
>

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

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


> > where 2-4 can probably happen more or less simultaneously.
> I wish you could send out the new patch as soon as possible. :)

Is a new version needed? I thought that this version could go in...

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

> > where 2-4 can probably happen more or less simultaneously.
> I wish you could send out the new patch as soon as possible. :)

Is a new version needed? I thought that this version could go in...

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120406/437d0124/attachment.sig>

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Huang Shijie]

Hi,

On Fri, Apr 6, 2012 at 2:21 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>
>> > where 2-4 can probably happen more or less simultaneously.
>> I wish you could send out the new patch as soon as possible. :)
>
> Is a new version needed? I thought that this version could go in...

emm..
If you think this version is ok, I will ask Artem to accept another patch:
"  mtd/gpmi : do not include the mxs.h".

My other patches depend on this patch. And they have been delayed for some time.

thanks
Huang Shijie

>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH V2 1/3] lib: add support for stmp-style devices

[Huang Shijie]

Hi,

On Fri, Apr 6, 2012 at 2:21 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>
>> > where 2-4 can probably happen more or less simultaneously.
>> I wish you could send out the new patch as soon as possible. :)
>
> Is a new version needed? I thought that this version could go in...

emm..
If you think this version is ok, I will ask Artem to accept another patch:
"  mtd/gpmi : do not include the mxs.h".

My other patches depend on this patch. And they have been delayed for some time.

thanks
Huang Shijie

>
> --
> Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? | Wolfram Sang ? ? ? ? ? ? ? ?|
> Industrial Linux Solutions ? ? ? ? ? ? ? ? | http://www.pengutronix.de/ ?|

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

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

> emm..
> If you think this version is ok, I will ask Artem to accept another patch:
> "  mtd/gpmi : do not include the mxs.h".
> 
> My other patches depend on this patch. And they have been delayed for some time.

Please wait at least somebody acknowledged that this has been picked up.
linux-next will break otherwise.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

> emm..
> If you think this version is ok, I will ask Artem to accept another patch:
> "  mtd/gpmi : do not include the mxs.h".
> 
> My other patches depend on this patch. And they have been delayed for some time.

Please wait at least somebody acknowledged that this has been picked up.
linux-next will break otherwise.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120407/4af3e0d2/attachment-0001.sig>

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Huang Shijie]

On Sat, Apr 7, 2012 at 4:00 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> emm..
>> If you think this version is ok, I will ask Artem to accept another patch:
>> "  mtd/gpmi : do not include the mxs.h".
>>
>> My other patches depend on this patch. And they have been delayed for some time.
>
> Please wait at least somebody acknowledged that this has been picked up.
> linux-next will break otherwise.
>

ok, thanks.

Huang Shijie
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH V2 1/3] lib: add support for stmp-style devices

[Huang Shijie]

On Sat, Apr 7, 2012 at 4:00 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> emm..
>> If you think this version is ok, I will ask Artem to accept another patch:
>> " ?mtd/gpmi : do not include the mxs.h".
>>
>> My other patches depend on this patch. And they have been delayed for some time.
>
> Please wait at least somebody acknowledged that this has been picked up.
> linux-next will break otherwise.
>

ok, thanks.

Huang Shijie
> --
> Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? | Wolfram Sang ? ? ? ? ? ? ? ?|
> Industrial Linux Solutions ? ? ? ? ? ? ? ? | http://www.pengutronix.de/ ?|

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Andrew Morton]

On Wed, 21 Mar 2012 23:21:57 +0100 Wolfram Sang <w.sang@pengutronix.de> wrote:

> MX23/28 use IP cores which follow a register layout I have first seen on
> STMP3xxx SoCs. In this layout, every register actually has four u32:
> 
>  1.) to store a value directly
>  2.) a SET register where every 1-bit sets the corresponding bit,
>      others are unaffected
>  3.) same with a CLR register
>  4.) same with a TOG (toggle) register
> 
> Also, the 2 MSBs in register 0 are always the same and can be used to reset
> the IP core.
> 
> All this is strictly speaking not mach-specific (but IP core specific) and,
> thus, doesn't need to be in mach-mxs/include. At least, mx6 and mx50 also
> utilize IP cores following this stmp-style. So:
> 
> Introduce a stmp-style device, put the code and defines for that in a public
> place (lib/), and let drivers for stmp-style devices select that code.
> To avoid regressions and ease reviewing, the actual code is simply copied from
> mach-mxs. It definately wants updates, but those need a seperate patch series.
> 
> Voila, mach dependency gone, reusable code introduced. Note that I didn't
> remove the duplicated code from mach-mxs yet, first the drivers have to be
> converted.
> 
> ...
>
>  include/linux/stmp_device.h |   20 +++++++++++
>  lib/Kconfig                 |    3 ++
>  lib/Makefile                |    2 +
>  lib/stmp_device.c           |   80 +++++++++++++++++++++++++++++++++++++++++++

It's good that this is being presented as library code, rather than
being buried in some device-specific directory then copied and pasted
ten times.

But ./lib/ does seem rather a strange place for it.  Perhaps we need a
drivers/lib/ or something.  We can use ./lib/ for now - it can always
be moved later on.

> --- /dev/null
> +++ b/lib/stmp_device.c

The functions in this file look terribly racy on SMP, or even with
preemption or interrupts.  What happens if two CPUs or threads run
stmp_reset_block() against the same device at the same time?

Perhaps the caller is supposed to prevent that, and the documentation
which isn't there forgot to mention it ;)

[PATCH V2 1/3] lib: add support for stmp-style devices

[Andrew Morton]

On Wed, 21 Mar 2012 23:21:57 +0100 Wolfram Sang <w.sang@pengutronix.de> wrote:

> MX23/28 use IP cores which follow a register layout I have first seen on
> STMP3xxx SoCs. In this layout, every register actually has four u32:
> 
>  1.) to store a value directly
>  2.) a SET register where every 1-bit sets the corresponding bit,
>      others are unaffected
>  3.) same with a CLR register
>  4.) same with a TOG (toggle) register
> 
> Also, the 2 MSBs in register 0 are always the same and can be used to reset
> the IP core.
> 
> All this is strictly speaking not mach-specific (but IP core specific) and,
> thus, doesn't need to be in mach-mxs/include. At least, mx6 and mx50 also
> utilize IP cores following this stmp-style. So:
> 
> Introduce a stmp-style device, put the code and defines for that in a public
> place (lib/), and let drivers for stmp-style devices select that code.
> To avoid regressions and ease reviewing, the actual code is simply copied from
> mach-mxs. It definately wants updates, but those need a seperate patch series.
> 
> Voila, mach dependency gone, reusable code introduced. Note that I didn't
> remove the duplicated code from mach-mxs yet, first the drivers have to be
> converted.
> 
> ...
>
>  include/linux/stmp_device.h |   20 +++++++++++
>  lib/Kconfig                 |    3 ++
>  lib/Makefile                |    2 +
>  lib/stmp_device.c           |   80 +++++++++++++++++++++++++++++++++++++++++++

It's good that this is being presented as library code, rather than
being buried in some device-specific directory then copied and pasted
ten times.

But ./lib/ does seem rather a strange place for it.  Perhaps we need a
drivers/lib/ or something.  We can use ./lib/ for now - it can always
be moved later on.

> --- /dev/null
> +++ b/lib/stmp_device.c

The functions in this file look terribly racy on SMP, or even with
preemption or interrupts.  What happens if two CPUs or threads run
stmp_reset_block() against the same device at the same time?

Perhaps the caller is supposed to prevent that, and the documentation
which isn't there forgot to mention it ;)

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

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

On Sun, Apr 22, 2012 at 11:55:06PM -0700, Andrew Morton wrote:
> On Wed, 21 Mar 2012 23:21:57 +0100 Wolfram Sang <w.sang@pengutronix.de> wrote:
> 
> > MX23/28 use IP cores which follow a register layout I have first seen on
> > STMP3xxx SoCs. In this layout, every register actually has four u32:
> > 
> >  1.) to store a value directly
> >  2.) a SET register where every 1-bit sets the corresponding bit,
> >      others are unaffected
> >  3.) same with a CLR register
> >  4.) same with a TOG (toggle) register
> > 
> > Also, the 2 MSBs in register 0 are always the same and can be used to reset
> > the IP core.
> > 
> > All this is strictly speaking not mach-specific (but IP core specific) and,
> > thus, doesn't need to be in mach-mxs/include. At least, mx6 and mx50 also
> > utilize IP cores following this stmp-style. So:
> > 
> > Introduce a stmp-style device, put the code and defines for that in a public
> > place (lib/), and let drivers for stmp-style devices select that code.
> > To avoid regressions and ease reviewing, the actual code is simply copied from
> > mach-mxs. It definately wants updates, but those need a seperate patch series.
> > 
> > Voila, mach dependency gone, reusable code introduced. Note that I didn't
> > remove the duplicated code from mach-mxs yet, first the drivers have to be
> > converted.
> > 
> > ...
> >
> >  include/linux/stmp_device.h |   20 +++++++++++
> >  lib/Kconfig                 |    3 ++
> >  lib/Makefile                |    2 +
> >  lib/stmp_device.c           |   80 +++++++++++++++++++++++++++++++++++++++++++
> 
> It's good that this is being presented as library code, rather than
> being buried in some device-specific directory then copied and pasted
> ten times.
> 
> But ./lib/ does seem rather a strange place for it.  Perhaps we need a
> drivers/lib/ or something.  We can use ./lib/ for now - it can always
> be moved later on.

The first version had it in drivers/base/ but that also felt strange.
I like drivers/lib/. Arnd, do you agree? Shall I resend?

> 
> > --- /dev/null
> > +++ b/lib/stmp_device.c
> 
> The functions in this file look terribly racy on SMP, or even with
> preemption or interrupts.  What happens if two CPUs or threads run
> stmp_reset_block() against the same device at the same time?
> 
> Perhaps the caller is supposed to prevent that, and the documentation
> which isn't there forgot to mention it ;)

The code needs even more cleanups, timeouts are terrible as well. But as
mentioned above, this series only moves existing code to a central
place. Improvements would be the next, seperate step.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH V2 1/3] lib: add support for stmp-style devices

[Wolfram Sang]

On Sun, Apr 22, 2012 at 11:55:06PM -0700, Andrew Morton wrote:
> On Wed, 21 Mar 2012 23:21:57 +0100 Wolfram Sang <w.sang@pengutronix.de> wrote:
> 
> > MX23/28 use IP cores which follow a register layout I have first seen on
> > STMP3xxx SoCs. In this layout, every register actually has four u32:
> > 
> >  1.) to store a value directly
> >  2.) a SET register where every 1-bit sets the corresponding bit,
> >      others are unaffected
> >  3.) same with a CLR register
> >  4.) same with a TOG (toggle) register
> > 
> > Also, the 2 MSBs in register 0 are always the same and can be used to reset
> > the IP core.
> > 
> > All this is strictly speaking not mach-specific (but IP core specific) and,
> > thus, doesn't need to be in mach-mxs/include. At least, mx6 and mx50 also
> > utilize IP cores following this stmp-style. So:
> > 
> > Introduce a stmp-style device, put the code and defines for that in a public
> > place (lib/), and let drivers for stmp-style devices select that code.
> > To avoid regressions and ease reviewing, the actual code is simply copied from
> > mach-mxs. It definately wants updates, but those need a seperate patch series.
> > 
> > Voila, mach dependency gone, reusable code introduced. Note that I didn't
> > remove the duplicated code from mach-mxs yet, first the drivers have to be
> > converted.
> > 
> > ...
> >
> >  include/linux/stmp_device.h |   20 +++++++++++
> >  lib/Kconfig                 |    3 ++
> >  lib/Makefile                |    2 +
> >  lib/stmp_device.c           |   80 +++++++++++++++++++++++++++++++++++++++++++
> 
> It's good that this is being presented as library code, rather than
> being buried in some device-specific directory then copied and pasted
> ten times.
> 
> But ./lib/ does seem rather a strange place for it.  Perhaps we need a
> drivers/lib/ or something.  We can use ./lib/ for now - it can always
> be moved later on.

The first version had it in drivers/base/ but that also felt strange.
I like drivers/lib/. Arnd, do you agree? Shall I resend?

> 
> > --- /dev/null
> > +++ b/lib/stmp_device.c
> 
> The functions in this file look terribly racy on SMP, or even with
> preemption or interrupts.  What happens if two CPUs or threads run
> stmp_reset_block() against the same device at the same time?
> 
> Perhaps the caller is supposed to prevent that, and the documentation
> which isn't there forgot to mention it ;)

The code needs even more cleanups, timeouts are terrible as well. But as
mentioned above, this series only moves existing code to a central
place. Improvements would be the next, seperate step.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120423/fc45f0e7/attachment.sig>

Re: [PATCH V2 1/3] lib: add support for stmp-style devices

[Arnd Bergmann]

On Monday 23 April 2012, Wolfram Sang wrote:
> > But ./lib/ does seem rather a strange place for it.  Perhaps we need a
> > drivers/lib/ or something.  We can use ./lib/ for now - it can always
> > be moved later on.
> 
> The first version had it in drivers/base/ but that also felt strange.
> I like drivers/lib/. Arnd, do you agree? Shall I resend?

I think lib/ is fine, but I also suggested putting it there ;-)

If Andrew or someone else feels strongly about it, we can of course
change it again before I submit it to Linus.

	Arnd

[PATCH V2 1/3] lib: add support for stmp-style devices

[Arnd Bergmann]

On Monday 23 April 2012, Wolfram Sang wrote:
> > But ./lib/ does seem rather a strange place for it.  Perhaps we need a
> > drivers/lib/ or something.  We can use ./lib/ for now - it can always
> > be moved later on.
> 
> The first version had it in drivers/base/ but that also felt strange.
> I like drivers/lib/. Arnd, do you agree? Shall I resend?

I think lib/ is fine, but I also suggested putting it there ;-)

If Andrew or someone else feels strongly about it, we can of course
change it again before I submit it to Linus.

	Arnd

[PATCH 2/3] i2c: mxs: use global reset function

[Wolfram Sang]

The former mach specific reset_block function has been converted to a global
one. Use the new one to remove mach dependency from the driver.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/i2c/busses/Kconfig   |    1 +
 drivers/i2c/busses/i2c-mxs.c |    9 ++-------
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 3101dd5..d4378cf 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -467,6 +467,7 @@ config I2C_MV64XXX
 config I2C_MXS
 	tristate "Freescale i.MX28 I2C interface"
 	depends on SOC_IMX28
+	select STMP_DEVICE
 	help
 	  Say Y here if you want to use the I2C bus controller on
 	  the Freescale i.MX28 processors.
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 3d471d5..0834802 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -26,8 +26,7 @@
 #include <linux/platform_device.h>
 #include <linux/jiffies.h>
 #include <linux/io.h>
-
-#include <mach/common.h>
+#include <linux/stmp_device.h>
 
 #define DRIVER_NAME "mxs-i2c"
 
@@ -111,13 +110,9 @@ struct mxs_i2c_dev {
 	struct i2c_adapter adapter;
 };
 
-/*
- * TODO: check if calls to here are really needed. If not, we could get rid of
- * mxs_reset_block and the mach-dependency. Needs an I2C analyzer, probably.
- */
 static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
 {
-	mxs_reset_block(i2c->regs);
+	stmp_reset_block(i2c->regs);
 	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
 	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
-- 
1.7.2.5

[PATCH 2/3] i2c: mxs: use global reset function

[Wolfram Sang]

The former mach specific reset_block function has been converted to a global
one. Use the new one to remove mach dependency from the driver.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/i2c/busses/Kconfig   |    1 +
 drivers/i2c/busses/i2c-mxs.c |    9 ++-------
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 3101dd5..d4378cf 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -467,6 +467,7 @@ config I2C_MV64XXX
 config I2C_MXS
 	tristate "Freescale i.MX28 I2C interface"
 	depends on SOC_IMX28
+	select STMP_DEVICE
 	help
 	  Say Y here if you want to use the I2C bus controller on
 	  the Freescale i.MX28 processors.
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 3d471d5..0834802 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -26,8 +26,7 @@
 #include <linux/platform_device.h>
 #include <linux/jiffies.h>
 #include <linux/io.h>
-
-#include <mach/common.h>
+#include <linux/stmp_device.h>
 
 #define DRIVER_NAME "mxs-i2c"
 
@@ -111,13 +110,9 @@ struct mxs_i2c_dev {
 	struct i2c_adapter adapter;
 };
 
-/*
- * TODO: check if calls to here are really needed. If not, we could get rid of
- * mxs_reset_block and the mach-dependency. Needs an I2C analyzer, probably.
- */
 static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
 {
-	mxs_reset_block(i2c->regs);
+	stmp_reset_block(i2c->regs);
 	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
 	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
-- 
1.7.2.5

[PATCH 3/3] rtc: stmp3xxx: use global stmp_device functionality

[Wolfram Sang]

The former mach specific reset_block function has been converted to a global
one. Use the new one to remove mach dependency from the driver. Also simplify
code using the new STMP_REG_* macros.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/rtc/Kconfig        |    1 +
 drivers/rtc/rtc-stmp3xxx.c |   29 ++++++++++-------------------
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 3a125b8..e124f93 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -983,6 +983,7 @@ config RTC_DRV_COH901331
 config RTC_DRV_STMP
 	tristate "Freescale STMP3xxx/i.MX23/i.MX28 RTC"
 	depends on ARCH_MXS
+	select STMP_DEVICE
 	help
 	  If you say yes here you will get support for the onboard
 	  STMP3xxx/i.MX23/i.MX28 RTC.
diff --git a/drivers/rtc/rtc-stmp3xxx.c b/drivers/rtc/rtc-stmp3xxx.c
index 1028786..e755d3e 100644
--- a/drivers/rtc/rtc-stmp3xxx.c
+++ b/drivers/rtc/rtc-stmp3xxx.c
@@ -25,11 +25,9 @@
 #include <linux/interrupt.h>
 #include <linux/rtc.h>
 #include <linux/slab.h>
-
-#include <mach/common.h>
+#include <linux/stmp_device.h>
 
 #define STMP3XXX_RTC_CTRL			0x0
-#define STMP3XXX_RTC_CTRL_SET			0x4
 #define STMP3XXX_RTC_CTRL_CLR			0x8
 #define STMP3XXX_RTC_CTRL_ALARM_IRQ_EN		0x00000001
 #define STMP3XXX_RTC_CTRL_ONEMSEC_IRQ_EN	0x00000002
@@ -44,7 +42,6 @@
 #define STMP3XXX_RTC_ALARM			0x40
 
 #define STMP3XXX_RTC_PERSISTENT0		0x60
-#define STMP3XXX_RTC_PERSISTENT0_SET		0x64
 #define STMP3XXX_RTC_PERSISTENT0_CLR		0x68
 #define STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE_EN	0x00000002
 #define STMP3XXX_RTC_PERSISTENT0_ALARM_EN	0x00000004
@@ -106,20 +103,14 @@ static irqreturn_t stmp3xxx_rtc_interrupt(int irq, void *dev_id)
 static int stmp3xxx_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
 	struct stmp3xxx_rtc_data *rtc_data = dev_get_drvdata(dev);
+	u32 set_clr_offset = enabled ? STMP_OFFSET_REG_SET : STMP_OFFSET_REG_CLR;
+
+	writel(STMP3XXX_RTC_PERSISTENT0_ALARM_EN |
+		STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE_EN,
+		rtc_data->io + STMP3XXX_RTC_PERSISTENT0 + set_clr_offset);
+	writel(STMP3XXX_RTC_CTRL_ALARM_IRQ_EN,
+		rtc_data->io + STMP3XXX_RTC_CTRL + set_clr_offset);
 
-	if (enabled) {
-		writel(STMP3XXX_RTC_PERSISTENT0_ALARM_EN |
-				STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE_EN,
-				rtc_data->io + STMP3XXX_RTC_PERSISTENT0_SET);
-		writel(STMP3XXX_RTC_CTRL_ALARM_IRQ_EN,
-				rtc_data->io + STMP3XXX_RTC_CTRL_SET);
-	} else {
-		writel(STMP3XXX_RTC_PERSISTENT0_ALARM_EN |
-				STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE_EN,
-				rtc_data->io + STMP3XXX_RTC_PERSISTENT0_CLR);
-		writel(STMP3XXX_RTC_CTRL_ALARM_IRQ_EN,
-				rtc_data->io + STMP3XXX_RTC_CTRL_CLR);
-	}
 	return 0;
 }
 
@@ -206,7 +197,7 @@ static int stmp3xxx_rtc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, rtc_data);
 
-	mxs_reset_block(rtc_data->io);
+	stmp_reset_block(rtc_data->io);
 	writel(STMP3XXX_RTC_PERSISTENT0_ALARM_EN |
 			STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE_EN |
 			STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE,
@@ -253,7 +244,7 @@ static int stmp3xxx_rtc_resume(struct platform_device *dev)
 {
 	struct stmp3xxx_rtc_data *rtc_data = platform_get_drvdata(dev);
 
-	mxs_reset_block(rtc_data->io);
+	stmp_reset_block(rtc_data->io);
 	writel(STMP3XXX_RTC_PERSISTENT0_ALARM_EN |
 			STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE_EN |
 			STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE,
-- 
1.7.2.5

[PATCH 3/3] rtc: stmp3xxx: use global stmp_device functionality

[Wolfram Sang]

The former mach specific reset_block function has been converted to a global
one. Use the new one to remove mach dependency from the driver. Also simplify
code using the new STMP_REG_* macros.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/rtc/Kconfig        |    1 +
 drivers/rtc/rtc-stmp3xxx.c |   29 ++++++++++-------------------
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 3a125b8..e124f93 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -983,6 +983,7 @@ config RTC_DRV_COH901331
 config RTC_DRV_STMP
 	tristate "Freescale STMP3xxx/i.MX23/i.MX28 RTC"
 	depends on ARCH_MXS
+	select STMP_DEVICE
 	help
 	  If you say yes here you will get support for the onboard
 	  STMP3xxx/i.MX23/i.MX28 RTC.
diff --git a/drivers/rtc/rtc-stmp3xxx.c b/drivers/rtc/rtc-stmp3xxx.c
index 1028786..e755d3e 100644
--- a/drivers/rtc/rtc-stmp3xxx.c
+++ b/drivers/rtc/rtc-stmp3xxx.c
@@ -25,11 +25,9 @@
 #include <linux/interrupt.h>
 #include <linux/rtc.h>
 #include <linux/slab.h>
-
-#include <mach/common.h>
+#include <linux/stmp_device.h>
 
 #define STMP3XXX_RTC_CTRL			0x0
-#define STMP3XXX_RTC_CTRL_SET			0x4
 #define STMP3XXX_RTC_CTRL_CLR			0x8
 #define STMP3XXX_RTC_CTRL_ALARM_IRQ_EN		0x00000001
 #define STMP3XXX_RTC_CTRL_ONEMSEC_IRQ_EN	0x00000002
@@ -44,7 +42,6 @@
 #define STMP3XXX_RTC_ALARM			0x40
 
 #define STMP3XXX_RTC_PERSISTENT0		0x60
-#define STMP3XXX_RTC_PERSISTENT0_SET		0x64
 #define STMP3XXX_RTC_PERSISTENT0_CLR		0x68
 #define STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE_EN	0x00000002
 #define STMP3XXX_RTC_PERSISTENT0_ALARM_EN	0x00000004
@@ -106,20 +103,14 @@ static irqreturn_t stmp3xxx_rtc_interrupt(int irq, void *dev_id)
 static int stmp3xxx_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
 	struct stmp3xxx_rtc_data *rtc_data = dev_get_drvdata(dev);
+	u32 set_clr_offset = enabled ? STMP_OFFSET_REG_SET : STMP_OFFSET_REG_CLR;
+
+	writel(STMP3XXX_RTC_PERSISTENT0_ALARM_EN |
+		STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE_EN,
+		rtc_data->io + STMP3XXX_RTC_PERSISTENT0 + set_clr_offset);
+	writel(STMP3XXX_RTC_CTRL_ALARM_IRQ_EN,
+		rtc_data->io + STMP3XXX_RTC_CTRL + set_clr_offset);
 
-	if (enabled) {
-		writel(STMP3XXX_RTC_PERSISTENT0_ALARM_EN |
-				STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE_EN,
-				rtc_data->io + STMP3XXX_RTC_PERSISTENT0_SET);
-		writel(STMP3XXX_RTC_CTRL_ALARM_IRQ_EN,
-				rtc_data->io + STMP3XXX_RTC_CTRL_SET);
-	} else {
-		writel(STMP3XXX_RTC_PERSISTENT0_ALARM_EN |
-				STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE_EN,
-				rtc_data->io + STMP3XXX_RTC_PERSISTENT0_CLR);
-		writel(STMP3XXX_RTC_CTRL_ALARM_IRQ_EN,
-				rtc_data->io + STMP3XXX_RTC_CTRL_CLR);
-	}
 	return 0;
 }
 
@@ -206,7 +197,7 @@ static int stmp3xxx_rtc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, rtc_data);
 
-	mxs_reset_block(rtc_data->io);
+	stmp_reset_block(rtc_data->io);
 	writel(STMP3XXX_RTC_PERSISTENT0_ALARM_EN |
 			STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE_EN |
 			STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE,
@@ -253,7 +244,7 @@ static int stmp3xxx_rtc_resume(struct platform_device *dev)
 {
 	struct stmp3xxx_rtc_data *rtc_data = platform_get_drvdata(dev);
 
-	mxs_reset_block(rtc_data->io);
+	stmp_reset_block(rtc_data->io);
 	writel(STMP3XXX_RTC_PERSISTENT0_ALARM_EN |
 			STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE_EN |
 			STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE,
-- 
1.7.2.5