* [PATCH 1/3] drivers: base: add support for stmp-style devices
2012-03-07 22:28 [PATCH 0/3] introduce stmp-style devices Wolfram Sang
@ 2012-03-07 22:28 ` Wolfram Sang
2012-03-07 22:40 ` Fabio Estevam
` (2 more replies)
2012-03-07 22:28 ` [PATCH 2/3] i2c: mxs: use global reset function Wolfram Sang
` (2 subsequent siblings)
3 siblings, 3 replies; 10+ messages in thread
From: Wolfram Sang @ 2012-03-07 22:28 UTC (permalink / raw)
To: linux-arm-kernel
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 also uses IP cores
following this stmp-style. So:
Introduce a stmp-style device, put the code and defines for that in a public
place (drivers/base/), 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>
---
drivers/base/Kconfig | 3 ++
drivers/base/Makefile | 1 +
drivers/base/stmp_device.c | 80 +++++++++++++++++++++++++++++++++++++++++++
include/linux/stmp_device.h | 19 ++++++++++
4 files changed, 103 insertions(+), 0 deletions(-)
create mode 100644 drivers/base/stmp_device.c
create mode 100644 include/linux/stmp_device.h
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 7be9f79..bcd3356 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -176,6 +176,9 @@ config GENERIC_CPU_DEVICES
bool
default n
+config STMP_DEVICE
+ bool
+
source "drivers/base/regmap/Kconfig"
config DMA_SHARED_BUFFER
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 610f999..86e4134 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -18,6 +18,7 @@ ifeq ($(CONFIG_SYSFS),y)
obj-$(CONFIG_MODULES) += module.o
endif
obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o
+obj-$(CONFIG_STMP_DEVICE) += stmp_device.o
obj-$(CONFIG_REGMAP) += regmap/
ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
diff --git a/drivers/base/stmp_device.c b/drivers/base/stmp_device.c
new file mode 100644
index 0000000..8ac9bcc
--- /dev/null
+++ b/drivers/base/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);
diff --git a/include/linux/stmp_device.h b/include/linux/stmp_device.h
new file mode 100644
index 0000000..d088de2
--- /dev/null
+++ b/include/linux/stmp_device.h
@@ -0,0 +1,19 @@
+/*
+ * 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; version 2 of the License.
+ */
+
+#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__ */
--
1.7.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 1/3] drivers: base: add support for stmp-style devices
2012-03-07 22:28 ` [PATCH 1/3] drivers: base: add support for " Wolfram Sang
@ 2012-03-07 22:40 ` Fabio Estevam
2012-03-08 7:45 ` Wolfram Sang
2012-03-08 2:09 ` Huang Shijie
2012-03-08 12:14 ` Shawn Guo
2 siblings, 1 reply; 10+ messages in thread
From: Fabio Estevam @ 2012-03-07 22:40 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 7, 2012 at 7:28 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
...
> +static int stmp_clear_poll_bit(void __iomem *addr, u32 mask)
> +{
> + ? ? ? int timeout = 0x400;
Could you please add a proper timeout mechanism here?
Something like: http://patchwork.ozlabs.org/patch/137365/
,as you suggested in the first place ;-)
> +
> + ? ? ? 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;
Same here.
Regards,
Fabio Estevam
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] drivers: base: add support for stmp-style devices
2012-03-07 22:40 ` Fabio Estevam
@ 2012-03-08 7:45 ` Wolfram Sang
0 siblings, 0 replies; 10+ messages in thread
From: Wolfram Sang @ 2012-03-08 7:45 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 07, 2012 at 07:40:22PM -0300, Fabio Estevam wrote:
> On Wed, Mar 7, 2012 at 7:28 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> ...
> > +static int stmp_clear_poll_bit(void __iomem *addr, u32 mask)
> > +{
> > + ? ? ? int timeout = 0x400;
>
> Could you please add a proper timeout mechanism here?
>
> Something like: http://patchwork.ozlabs.org/patch/137365/
>From the patch description:
===
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.
===
Or is that considered a wrong path?
Thanks,
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/20120308/cb794861/attachment.sig>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] drivers: base: add support for stmp-style devices
2012-03-07 22:28 ` [PATCH 1/3] drivers: base: add support for " Wolfram Sang
2012-03-07 22:40 ` Fabio Estevam
@ 2012-03-08 2:09 ` Huang Shijie
2012-03-08 12:14 ` Shawn Guo
2 siblings, 0 replies; 10+ messages in thread
From: Huang Shijie @ 2012-03-08 2:09 UTC (permalink / raw)
To: linux-arm-kernel
? 2012?03?08? 06:28, Wolfram Sang ??:
> 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 also uses IP cores
> following this stmp-style. So:
>
mx50 also uses the same IP core.
BR
Huang Shijie
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] drivers: base: add support for stmp-style devices
2012-03-07 22:28 ` [PATCH 1/3] drivers: base: add support for " Wolfram Sang
2012-03-07 22:40 ` Fabio Estevam
2012-03-08 2:09 ` Huang Shijie
@ 2012-03-08 12:14 ` Shawn Guo
2 siblings, 0 replies; 10+ messages in thread
From: Shawn Guo @ 2012-03-08 12:14 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 07, 2012 at 11:28:32PM +0100, Wolfram Sang 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 also uses IP cores
> following this stmp-style. So:
>
> Introduce a stmp-style device, put the code and defines for that in a public
> place (drivers/base/), 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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] i2c: mxs: use global reset function
2012-03-07 22:28 [PATCH 0/3] introduce stmp-style devices Wolfram Sang
2012-03-07 22:28 ` [PATCH 1/3] drivers: base: add support for " Wolfram Sang
@ 2012-03-07 22:28 ` Wolfram Sang
2012-03-07 22:28 ` [PATCH 3/3] rtc: stmp3xxx: use global stmp_device functionality Wolfram Sang
2012-03-08 16:03 ` [PATCH 0/3] introduce stmp-style devices Arnd Bergmann
3 siblings, 0 replies; 10+ messages in thread
From: Wolfram Sang @ 2012-03-07 22:28 UTC (permalink / raw)
To: linux-arm-kernel
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.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/3] rtc: stmp3xxx: use global stmp_device functionality
2012-03-07 22:28 [PATCH 0/3] introduce stmp-style devices Wolfram Sang
2012-03-07 22:28 ` [PATCH 1/3] drivers: base: add support for " Wolfram Sang
2012-03-07 22:28 ` [PATCH 2/3] i2c: mxs: use global reset function Wolfram Sang
@ 2012-03-07 22:28 ` Wolfram Sang
2012-03-08 16:03 ` [PATCH 0/3] introduce stmp-style devices Arnd Bergmann
3 siblings, 0 replies; 10+ messages in thread
From: Wolfram Sang @ 2012-03-07 22:28 UTC (permalink / raw)
To: linux-arm-kernel
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_OFFSET_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.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 0/3] introduce stmp-style devices
2012-03-07 22:28 [PATCH 0/3] introduce stmp-style devices Wolfram Sang
` (2 preceding siblings ...)
2012-03-07 22:28 ` [PATCH 3/3] rtc: stmp3xxx: use global stmp_device functionality Wolfram Sang
@ 2012-03-08 16:03 ` Arnd Bergmann
2012-03-09 13:26 ` Wolfram Sang
3 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2012-03-08 16:03 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 07 March 2012, Wolfram Sang wrote:
>
> 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 RFC:
>
> * renamed the macros to STMP_OFFSET_*
> * rebased to 3.3-rc6
>
> Tested with a TX28 board.
I think having the stmp_device code is good, but I'm not sure about the
location in drivers/base, which is otherwise reserved for anything related
to 'struct device'. How about putting the same file into lib/stmpregs.c?
Arnd
^ permalink raw reply [flat|nested] 10+ messages in thread