[PATCHv2 0/2] Add big endian support

[PATCHv2 0/2] Add big endian support

[Xiubo Li]

This patches are preparing for Vybird, LS1 and LS2. And on LS1 the IP will
in BE mode.

And this has been test on Vybird.

Changes in V2:
- Add the detail information in the commit comment.
- 'big-endians' --> 'big-endian'.


Xiubo Li (2):
  watchdog: imx2_wdt: Sort the header files alphabetically
  watchdog: imx2_wdt: Add big-endian support

 drivers/watchdog/imx2_wdt.c | 52 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 15 deletions(-)

-- 
1.8.4

[PATCHv2 1/2] watchdog: imx2_wdt: Sort the header files alphabetically

[Xiubo Li]

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
Cc: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/imx2_wdt.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index dd51d95..1795922 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -21,19 +21,19 @@
  * Halt on suspend:	Manual		Can be automatic
  */
 
+#include <linux/clk.h>
+#include <linux/fs.h>
 #include <linux/init.h>
+#include <linux/io.h>
+#include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/platform_device.h>
-#include <linux/watchdog.h>
-#include <linux/clk.h>
-#include <linux/fs.h>
-#include <linux/io.h>
-#include <linux/uaccess.h>
 #include <linux/timer.h>
-#include <linux/jiffies.h>
+#include <linux/uaccess.h>
+#include <linux/watchdog.h>
 
 #define DRIVER_NAME "imx2-wdt"
 
-- 
1.8.4

[PATCHv2 2/2] watchdog: imx2_wdt: Add big-endian support

[Xiubo Li]

For the platforms that this IP driver now supports:
SoC        CPU        Watchdog     Need 'big-endian'?
------------------------------------------------------
Vybird     little     little       No
LS1        little     big          Yes
LS2        little     little       No
IMX+       little     little       No

So for the LS1 SoCs, we need to do the big endianness converting.

And this will also support the following case, for example:
SoC        CPU        Watchdog     Need 'big-endian'?
------------------------------------------------------
PowerPC    big        big          Yes

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
Cc: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/imx2_wdt.c | 40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 1795922..32f8874 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -30,6 +30,7 @@
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/timer.h>
 #include <linux/uaccess.h>
@@ -64,9 +65,26 @@ static struct {
 	void __iomem *base;
 	unsigned timeout;
 	unsigned long status;
+	bool big_endian;
 	struct timer_list timer;	/* Pings the watchdog when closed */
 } imx2_wdt;
 
+static inline u16 imx2_wdt_readw(void __iomem *addr)
+{
+	if (imx2_wdt.big_endian)
+		return ioread16be(addr);
+	else
+		return ioread16(addr);
+}
+
+static inline void imx2_wdt_writew(u16 val, void __iomem *addr)
+{
+	if (imx2_wdt.big_endian)
+		iowrite16be(val, addr);
+	else
+		iowrite16(val, addr);
+}
+
 static struct miscdevice imx2_wdt_miscdev;
 
 static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -87,7 +105,7 @@ static const struct watchdog_info imx2_wdt_info = {
 
 static inline void imx2_wdt_setup(void)
 {
-	u16 val = __raw_readw(imx2_wdt.base + IMX2_WDT_WCR);
+	u16 val = imx2_wdt_readw(imx2_wdt.base + IMX2_WDT_WCR);
 
 	/* Suspend timer in low power mode, write once-only */
 	val |= IMX2_WDT_WCR_WDZST;
@@ -100,17 +118,17 @@ static inline void imx2_wdt_setup(void)
 	/* Set the watchdog's Time-Out value */
 	val |= WDOG_SEC_TO_COUNT(imx2_wdt.timeout);
 
-	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
+	imx2_wdt_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
 
 	/* enable the watchdog */
 	val |= IMX2_WDT_WCR_WDE;
-	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
+	imx2_wdt_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
 }
 
 static inline void imx2_wdt_ping(void)
 {
-	__raw_writew(IMX2_WDT_SEQ1, imx2_wdt.base + IMX2_WDT_WSR);
-	__raw_writew(IMX2_WDT_SEQ2, imx2_wdt.base + IMX2_WDT_WSR);
+	imx2_wdt_writew(IMX2_WDT_SEQ1, imx2_wdt.base + IMX2_WDT_WSR);
+	imx2_wdt_writew(IMX2_WDT_SEQ2, imx2_wdt.base + IMX2_WDT_WSR);
 }
 
 static void imx2_wdt_timer_ping(unsigned long arg)
@@ -143,12 +161,12 @@ static void imx2_wdt_stop(void)
 
 static void imx2_wdt_set_timeout(int new_timeout)
 {
-	u16 val = __raw_readw(imx2_wdt.base + IMX2_WDT_WCR);
+	u16 val = imx2_wdt_readw(imx2_wdt.base + IMX2_WDT_WCR);
 
 	/* set the new timeout value in the WSR */
 	val &= ~IMX2_WDT_WCR_WT;
 	val |= WDOG_SEC_TO_COUNT(new_timeout);
-	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
+	imx2_wdt_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
 }
 
 static int imx2_wdt_open(struct inode *inode, struct file *file)
@@ -192,7 +210,7 @@ static long imx2_wdt_ioctl(struct file *file, unsigned int cmd,
 		return put_user(0, p);
 
 	case WDIOC_GETBOOTSTATUS:
-		val = __raw_readw(imx2_wdt.base + IMX2_WDT_WRSR);
+		val = imx2_wdt_readw(imx2_wdt.base + IMX2_WDT_WRSR);
 		new_value = val & IMX2_WDT_WRSR_TOUT ? WDIOF_CARDRESET : 0;
 		return put_user(new_value, p);
 
@@ -257,8 +275,12 @@ static struct miscdevice imx2_wdt_miscdev = {
 
 static int __init imx2_wdt_probe(struct platform_device *pdev)
 {
-	int ret;
+	struct device_node *np = pdev->dev.of_node;
 	struct resource *res;
+	int ret;
+
+	if (np)
+		imx2_wdt.big_endian = of_property_read_bool(np, "big-endian");
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	imx2_wdt.base = devm_ioremap_resource(&pdev->dev, res);
-- 
1.8.4

Re: [PATCHv2 2/2] watchdog: imx2_wdt: Add big-endian support

[Guenter Roeck]

On 03/25/2014 07:21 PM, Xiubo Li wrote:
> For the platforms that this IP driver now supports:
> SoC        CPU        Watchdog     Need 'big-endian'?
> ------------------------------------------------------
> Vybird     little     little       No
> LS1        little     big          Yes
> LS2        little     little       No
> IMX+       little     little       No
>
> So for the LS1 SoCs, we need to do the big endianness converting.
>
> And this will also support the following case, for example:
> SoC        CPU        Watchdog     Need 'big-endian'?
> ------------------------------------------------------
> PowerPC    big        big          Yes
>
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> Cc: Guenter Roeck <linux@roeck-us.net>

Is this patch intended to solve the problem generically ?

http://www.spinics.net/lists/kernel/msg1714375.html

Guenter

> ---
>   drivers/watchdog/imx2_wdt.c | 40 +++++++++++++++++++++++++++++++---------
>   1 file changed, 31 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
> index 1795922..32f8874 100644
> --- a/drivers/watchdog/imx2_wdt.c
> +++ b/drivers/watchdog/imx2_wdt.c
> @@ -30,6 +30,7 @@
>   #include <linux/miscdevice.h>
>   #include <linux/module.h>
>   #include <linux/moduleparam.h>
> +#include <linux/of.h>
>   #include <linux/platform_device.h>
>   #include <linux/timer.h>
>   #include <linux/uaccess.h>
> @@ -64,9 +65,26 @@ static struct {
>   	void __iomem *base;
>   	unsigned timeout;
>   	unsigned long status;
> +	bool big_endian;
>   	struct timer_list timer;	/* Pings the watchdog when closed */
>   } imx2_wdt;
>
> +static inline u16 imx2_wdt_readw(void __iomem *addr)
> +{
> +	if (imx2_wdt.big_endian)
> +		return ioread16be(addr);
> +	else
> +		return ioread16(addr);
> +}
> +
> +static inline void imx2_wdt_writew(u16 val, void __iomem *addr)
> +{
> +	if (imx2_wdt.big_endian)
> +		iowrite16be(val, addr);
> +	else
> +		iowrite16(val, addr);
> +}
> +
>   static struct miscdevice imx2_wdt_miscdev;
>
>   static bool nowayout = WATCHDOG_NOWAYOUT;
> @@ -87,7 +105,7 @@ static const struct watchdog_info imx2_wdt_info = {
>
>   static inline void imx2_wdt_setup(void)
>   {
> -	u16 val = __raw_readw(imx2_wdt.base + IMX2_WDT_WCR);
> +	u16 val = imx2_wdt_readw(imx2_wdt.base + IMX2_WDT_WCR);
>
>   	/* Suspend timer in low power mode, write once-only */
>   	val |= IMX2_WDT_WCR_WDZST;
> @@ -100,17 +118,17 @@ static inline void imx2_wdt_setup(void)
>   	/* Set the watchdog's Time-Out value */
>   	val |= WDOG_SEC_TO_COUNT(imx2_wdt.timeout);
>
> -	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> +	imx2_wdt_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
>
>   	/* enable the watchdog */
>   	val |= IMX2_WDT_WCR_WDE;
> -	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> +	imx2_wdt_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
>   }
>
>   static inline void imx2_wdt_ping(void)
>   {
> -	__raw_writew(IMX2_WDT_SEQ1, imx2_wdt.base + IMX2_WDT_WSR);
> -	__raw_writew(IMX2_WDT_SEQ2, imx2_wdt.base + IMX2_WDT_WSR);
> +	imx2_wdt_writew(IMX2_WDT_SEQ1, imx2_wdt.base + IMX2_WDT_WSR);
> +	imx2_wdt_writew(IMX2_WDT_SEQ2, imx2_wdt.base + IMX2_WDT_WSR);
>   }
>
>   static void imx2_wdt_timer_ping(unsigned long arg)
> @@ -143,12 +161,12 @@ static void imx2_wdt_stop(void)
>
>   static void imx2_wdt_set_timeout(int new_timeout)
>   {
> -	u16 val = __raw_readw(imx2_wdt.base + IMX2_WDT_WCR);
> +	u16 val = imx2_wdt_readw(imx2_wdt.base + IMX2_WDT_WCR);
>
>   	/* set the new timeout value in the WSR */
>   	val &= ~IMX2_WDT_WCR_WT;
>   	val |= WDOG_SEC_TO_COUNT(new_timeout);
> -	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> +	imx2_wdt_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
>   }
>
>   static int imx2_wdt_open(struct inode *inode, struct file *file)
> @@ -192,7 +210,7 @@ static long imx2_wdt_ioctl(struct file *file, unsigned int cmd,
>   		return put_user(0, p);
>
>   	case WDIOC_GETBOOTSTATUS:
> -		val = __raw_readw(imx2_wdt.base + IMX2_WDT_WRSR);
> +		val = imx2_wdt_readw(imx2_wdt.base + IMX2_WDT_WRSR);
>   		new_value = val & IMX2_WDT_WRSR_TOUT ? WDIOF_CARDRESET : 0;
>   		return put_user(new_value, p);
>
> @@ -257,8 +275,12 @@ static struct miscdevice imx2_wdt_miscdev = {
>
>   static int __init imx2_wdt_probe(struct platform_device *pdev)
>   {
> -	int ret;
> +	struct device_node *np = pdev->dev.of_node;
>   	struct resource *res;
> +	int ret;
> +
> +	if (np)
> +		imx2_wdt.big_endian = of_property_read_bool(np, "big-endian");
>
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	imx2_wdt.base = devm_ioremap_resource(&pdev->dev, res);
>

RE: [PATCHv2 2/2] watchdog: imx2_wdt: Add big-endian support

[Li.Xiubo]

> Subject: Re: [PATCHv2 2/2] watchdog: imx2_wdt: Add big-endian support
> 
> On 03/25/2014 07:21 PM, Xiubo Li wrote:
> > For the platforms that this IP driver now supports:
> > SoC        CPU        Watchdog     Need 'big-endian'?
> > ------------------------------------------------------
> > Vybird     little     little       No
> > LS1        little     big          Yes
> > LS2        little     little       No
> > IMX+       little     little       No
> >
> > So for the LS1 SoCs, we need to do the big endianness converting.
> >
> > And this will also support the following case, for example:
> > SoC        CPU        Watchdog     Need 'big-endian'?
> > ------------------------------------------------------
> > PowerPC    big        big          Yes
> >
> > Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> > Cc: Guenter Roeck <linux@roeck-us.net>
> 
> Is this patch intended to solve the problem generically ?
> 
> http://www.spinics.net/lists/kernel/msg1714375.html
>

It's actually the following ones first, which will support 16-bits
Values for regmap-mmio:

https://patchwork.kernel.org/patch/3896321/
https://patchwork.kernel.org/patch/3896331/
https://patchwork.kernel.org/patch/3901021/

And the then the following one, which will support the LE endian:
http://www.spinics.net/lists/kernel/msg1714375.html


Thanks :)

BRs
Xiubo




 
> Guenter
> 
> > ---
> >   drivers/watchdog/imx2_wdt.c | 40 +++++++++++++++++++++++++++++++---------
> >   1 file changed, 31 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
> > index 1795922..32f8874 100644
> > --- a/drivers/watchdog/imx2_wdt.c
> > +++ b/drivers/watchdog/imx2_wdt.c
> > @@ -30,6 +30,7 @@
> >   #include <linux/miscdevice.h>
> >   #include <linux/module.h>
> >   #include <linux/moduleparam.h>
> > +#include <linux/of.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/timer.h>
> >   #include <linux/uaccess.h>
> > @@ -64,9 +65,26 @@ static struct {
> >   	void __iomem *base;
> >   	unsigned timeout;
> >   	unsigned long status;
> > +	bool big_endian;
> >   	struct timer_list timer;	/* Pings the watchdog when closed */
> >   } imx2_wdt;
> >
> > +static inline u16 imx2_wdt_readw(void __iomem *addr)
> > +{
> > +	if (imx2_wdt.big_endian)
> > +		return ioread16be(addr);
> > +	else
> > +		return ioread16(addr);
> > +}
> > +
> > +static inline void imx2_wdt_writew(u16 val, void __iomem *addr)
> > +{
> > +	if (imx2_wdt.big_endian)
> > +		iowrite16be(val, addr);
> > +	else
> > +		iowrite16(val, addr);
> > +}
> > +
> >   static struct miscdevice imx2_wdt_miscdev;
> >
> >   static bool nowayout = WATCHDOG_NOWAYOUT;
> > @@ -87,7 +105,7 @@ static const struct watchdog_info imx2_wdt_info = {
> >
> >   static inline void imx2_wdt_setup(void)
> >   {
> > -	u16 val = __raw_readw(imx2_wdt.base + IMX2_WDT_WCR);
> > +	u16 val = imx2_wdt_readw(imx2_wdt.base + IMX2_WDT_WCR);
> >
> >   	/* Suspend timer in low power mode, write once-only */
> >   	val |= IMX2_WDT_WCR_WDZST;
> > @@ -100,17 +118,17 @@ static inline void imx2_wdt_setup(void)
> >   	/* Set the watchdog's Time-Out value */
> >   	val |= WDOG_SEC_TO_COUNT(imx2_wdt.timeout);
> >
> > -	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> > +	imx2_wdt_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> >
> >   	/* enable the watchdog */
> >   	val |= IMX2_WDT_WCR_WDE;
> > -	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> > +	imx2_wdt_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> >   }
> >
> >   static inline void imx2_wdt_ping(void)
> >   {
> > -	__raw_writew(IMX2_WDT_SEQ1, imx2_wdt.base + IMX2_WDT_WSR);
> > -	__raw_writew(IMX2_WDT_SEQ2, imx2_wdt.base + IMX2_WDT_WSR);
> > +	imx2_wdt_writew(IMX2_WDT_SEQ1, imx2_wdt.base + IMX2_WDT_WSR);
> > +	imx2_wdt_writew(IMX2_WDT_SEQ2, imx2_wdt.base + IMX2_WDT_WSR);
> >   }
> >
> >   static void imx2_wdt_timer_ping(unsigned long arg)
> > @@ -143,12 +161,12 @@ static void imx2_wdt_stop(void)
> >
> >   static void imx2_wdt_set_timeout(int new_timeout)
> >   {
> > -	u16 val = __raw_readw(imx2_wdt.base + IMX2_WDT_WCR);
> > +	u16 val = imx2_wdt_readw(imx2_wdt.base + IMX2_WDT_WCR);
> >
> >   	/* set the new timeout value in the WSR */
> >   	val &= ~IMX2_WDT_WCR_WT;
> >   	val |= WDOG_SEC_TO_COUNT(new_timeout);
> > -	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> > +	imx2_wdt_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> >   }
> >
> >   static int imx2_wdt_open(struct inode *inode, struct file *file)
> > @@ -192,7 +210,7 @@ static long imx2_wdt_ioctl(struct file *file, unsigned
> int cmd,
> >   		return put_user(0, p);
> >
> >   	case WDIOC_GETBOOTSTATUS:
> > -		val = __raw_readw(imx2_wdt.base + IMX2_WDT_WRSR);
> > +		val = imx2_wdt_readw(imx2_wdt.base + IMX2_WDT_WRSR);
> >   		new_value = val & IMX2_WDT_WRSR_TOUT ? WDIOF_CARDRESET : 0;
> >   		return put_user(new_value, p);
> >
> > @@ -257,8 +275,12 @@ static struct miscdevice imx2_wdt_miscdev = {
> >
> >   static int __init imx2_wdt_probe(struct platform_device *pdev)
> >   {
> > -	int ret;
> > +	struct device_node *np = pdev->dev.of_node;
> >   	struct resource *res;
> > +	int ret;
> > +
> > +	if (np)
> > +		imx2_wdt.big_endian = of_property_read_bool(np, "big-endian");
> >
> >   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >   	imx2_wdt.base = devm_ioremap_resource(&pdev->dev, res);
> >
> 
> 

Re: [PATCHv2 2/2] watchdog: imx2_wdt: Add big-endian support

[Guenter Roeck]

On 03/30/2014 09:18 PM, Li.Xiubo@freescale.com wrote:
>
>
>> Subject: Re: [PATCHv2 2/2] watchdog: imx2_wdt: Add big-endian support
>>
>> On 03/25/2014 07:21 PM, Xiubo Li wrote:
>>> For the platforms that this IP driver now supports:
>>> SoC        CPU        Watchdog     Need 'big-endian'?
>>> ------------------------------------------------------
>>> Vybird     little     little       No
>>> LS1        little     big          Yes
>>> LS2        little     little       No
>>> IMX+       little     little       No
>>>
>>> So for the LS1 SoCs, we need to do the big endianness converting.
>>>
>>> And this will also support the following case, for example:
>>> SoC        CPU        Watchdog     Need 'big-endian'?
>>> ------------------------------------------------------
>>> PowerPC    big        big          Yes
>>>
>>> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
>>> Cc: Guenter Roeck <linux@roeck-us.net>
>>
>> Is this patch intended to solve the problem generically ?
>>
>> http://www.spinics.net/lists/kernel/msg1714375.html
>>
>
> It's actually the following ones first, which will support 16-bits
> Values for regmap-mmio:
>
> https://patchwork.kernel.org/patch/3896321/
> https://patchwork.kernel.org/patch/3896331/
> https://patchwork.kernel.org/patch/3901021/
>
> And the then the following one, which will support the LE endian:
> http://www.spinics.net/lists/kernel/msg1714375.html
>
So what is the plan for this patch ? Seems to me it would make more sense
to convert the driver to regmap.

Guenter

>
> Thanks :)
>
> BRs
> Xiubo
>
>
>
>
>
>> Guenter
>>
>>> ---
>>>    drivers/watchdog/imx2_wdt.c | 40 +++++++++++++++++++++++++++++++---------
>>>    1 file changed, 31 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
>>> index 1795922..32f8874 100644
>>> --- a/drivers/watchdog/imx2_wdt.c
>>> +++ b/drivers/watchdog/imx2_wdt.c
>>> @@ -30,6 +30,7 @@
>>>    #include <linux/miscdevice.h>
>>>    #include <linux/module.h>
>>>    #include <linux/moduleparam.h>
>>> +#include <linux/of.h>
>>>    #include <linux/platform_device.h>
>>>    #include <linux/timer.h>
>>>    #include <linux/uaccess.h>
>>> @@ -64,9 +65,26 @@ static struct {
>>>    	void __iomem *base;
>>>    	unsigned timeout;
>>>    	unsigned long status;
>>> +	bool big_endian;
>>>    	struct timer_list timer;	/* Pings the watchdog when closed */
>>>    } imx2_wdt;
>>>
>>> +static inline u16 imx2_wdt_readw(void __iomem *addr)
>>> +{
>>> +	if (imx2_wdt.big_endian)
>>> +		return ioread16be(addr);
>>> +	else
>>> +		return ioread16(addr);
>>> +}
>>> +
>>> +static inline void imx2_wdt_writew(u16 val, void __iomem *addr)
>>> +{
>>> +	if (imx2_wdt.big_endian)
>>> +		iowrite16be(val, addr);
>>> +	else
>>> +		iowrite16(val, addr);
>>> +}
>>> +
>>>    static struct miscdevice imx2_wdt_miscdev;
>>>
>>>    static bool nowayout = WATCHDOG_NOWAYOUT;
>>> @@ -87,7 +105,7 @@ static const struct watchdog_info imx2_wdt_info = {
>>>
>>>    static inline void imx2_wdt_setup(void)
>>>    {
>>> -	u16 val = __raw_readw(imx2_wdt.base + IMX2_WDT_WCR);
>>> +	u16 val = imx2_wdt_readw(imx2_wdt.base + IMX2_WDT_WCR);
>>>
>>>    	/* Suspend timer in low power mode, write once-only */
>>>    	val |= IMX2_WDT_WCR_WDZST;
>>> @@ -100,17 +118,17 @@ static inline void imx2_wdt_setup(void)
>>>    	/* Set the watchdog's Time-Out value */
>>>    	val |= WDOG_SEC_TO_COUNT(imx2_wdt.timeout);
>>>
>>> -	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
>>> +	imx2_wdt_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
>>>
>>>    	/* enable the watchdog */
>>>    	val |= IMX2_WDT_WCR_WDE;
>>> -	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
>>> +	imx2_wdt_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
>>>    }
>>>
>>>    static inline void imx2_wdt_ping(void)
>>>    {
>>> -	__raw_writew(IMX2_WDT_SEQ1, imx2_wdt.base + IMX2_WDT_WSR);
>>> -	__raw_writew(IMX2_WDT_SEQ2, imx2_wdt.base + IMX2_WDT_WSR);
>>> +	imx2_wdt_writew(IMX2_WDT_SEQ1, imx2_wdt.base + IMX2_WDT_WSR);
>>> +	imx2_wdt_writew(IMX2_WDT_SEQ2, imx2_wdt.base + IMX2_WDT_WSR);
>>>    }
>>>
>>>    static void imx2_wdt_timer_ping(unsigned long arg)
>>> @@ -143,12 +161,12 @@ static void imx2_wdt_stop(void)
>>>
>>>    static void imx2_wdt_set_timeout(int new_timeout)
>>>    {
>>> -	u16 val = __raw_readw(imx2_wdt.base + IMX2_WDT_WCR);
>>> +	u16 val = imx2_wdt_readw(imx2_wdt.base + IMX2_WDT_WCR);
>>>
>>>    	/* set the new timeout value in the WSR */
>>>    	val &= ~IMX2_WDT_WCR_WT;
>>>    	val |= WDOG_SEC_TO_COUNT(new_timeout);
>>> -	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
>>> +	imx2_wdt_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
>>>    }
>>>
>>>    static int imx2_wdt_open(struct inode *inode, struct file *file)
>>> @@ -192,7 +210,7 @@ static long imx2_wdt_ioctl(struct file *file, unsigned
>> int cmd,
>>>    		return put_user(0, p);
>>>
>>>    	case WDIOC_GETBOOTSTATUS:
>>> -		val = __raw_readw(imx2_wdt.base + IMX2_WDT_WRSR);
>>> +		val = imx2_wdt_readw(imx2_wdt.base + IMX2_WDT_WRSR);
>>>    		new_value = val & IMX2_WDT_WRSR_TOUT ? WDIOF_CARDRESET : 0;
>>>    		return put_user(new_value, p);
>>>
>>> @@ -257,8 +275,12 @@ static struct miscdevice imx2_wdt_miscdev = {
>>>
>>>    static int __init imx2_wdt_probe(struct platform_device *pdev)
>>>    {
>>> -	int ret;
>>> +	struct device_node *np = pdev->dev.of_node;
>>>    	struct resource *res;
>>> +	int ret;
>>> +
>>> +	if (np)
>>> +		imx2_wdt.big_endian = of_property_read_bool(np, "big-endian");
>>>
>>>    	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>    	imx2_wdt.base = devm_ioremap_resource(&pdev->dev, res);
>>>
>>
>>
>
>
>

RE: [PATCHv2 2/2] watchdog: imx2_wdt: Add big-endian support

[Li.Xiubo]

> > It's actually the following ones first, which will support 16-bits
> > Values for regmap-mmio:
> >
> > https://patchwork.kernel.org/patch/3896321/
> > https://patchwork.kernel.org/patch/3896331/
> > https://patchwork.kernel.org/patch/3901021/
> >
> > And the then the following one, which will support the LE endian:
> > http://www.spinics.net/lists/kernel/msg1714375.html
> >
> So what is the plan for this patch ? Seems to me it would make more sense
> to convert the driver to regmap.
> 

@Guenter, @Mark,

Yes, I do think so too and there has already regmap-mmio version patch
Series about the IMX2 Watchdog driver, but those regmap patches for
supporting the 16-bit values are in Mark's tree and the IMX2 Watchdog
will depend on them.

How to deal with this situation ?


The regmap patches depended by IMX2 Watchdog:

1, https://patchwork.kernel.org/patch/3896321/   ===> <applied>
   regmap: mmio: add regmap_mmio_{regsize, count}_check. 
   https://patchwork.kernel.org/patch/3896331/   ===> <applied>
   regmap: mmio: Add support for 1/2/8 bytes wide register address.
   https://patchwork.kernel.org/patch/3901021/   ===> <applied>
   regmap: mmio: Add regmap_mmio_regbits_check.


2, The above three and this one are a must.
   https://patchwork.kernel.org/patch/3912671/   ===> <pending>
   regmap: mmio: Fix the bug of 'offset' value parsing.


3, The following ones are not a must, but will make the IMX2
   Watchdog much simplifier.
   https://patchwork.kernel.org/patch/3912401/   ===> <pending>
   regmap: implement LE formatting/parsing for 16/32-bit values.
   https://patchwork.kernel.org/patch/3912751/   ===> <pending>
   regmap: add DT endianness binding support.
   https://patchwork.kernel.org/patch/3912741/   ===> <pending>
   regmap: Add the DT binding documentation for endianness


The above all have been test using SAI(32-bit regmap-mmio), 
SGTL5000(codec 16-bit regmap-i2c) and IMX2 Watchdog(16-bit
regmap-mmio).


I'll send the regmap-mmio based patch series for IMX2 Watchdog.


Thanks,

Best Regards,
Xiubo

Re: [PATCHv2 2/2] watchdog: imx2_wdt: Add big-endian support

[Mark Brown]

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

On Tue, Apr 01, 2014 at 02:24:12AM +0000, Li.Xiubo@freescale.com wrote:

> Yes, I do think so too and there has already regmap-mmio version patch
> Series about the IMX2 Watchdog driver, but those regmap patches for
> supporting the 16-bit values are in Mark's tree and the IMX2 Watchdog
> will depend on them.

> How to deal with this situation ?

Well, anything that's already applied went to Linus already so is in
-rc1.  Anything else we can either do a cross tree merge or I can apply
the driver.

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

RE: [PATCHv2 2/2] watchdog: imx2_wdt: Add big-endian support

[Li.Xiubo]

> Subject: Re: [PATCHv2 2/2] watchdog: imx2_wdt: Add big-endian support
> 
> On Tue, Apr 01, 2014 at 02:24:12AM +0000, Li.Xiubo@freescale.com wrote:
> 
> > Yes, I do think so too and there has already regmap-mmio version patch
> > Series about the IMX2 Watchdog driver, but those regmap patches for
> > supporting the 16-bit values are in Mark's tree and the IMX2 Watchdog
> > will depend on them.
> 
> > How to deal with this situation ?
> 
> Well, anything that's already applied went to Linus already so is in
> -rc1.  Anything else we can either do a cross tree merge or I can apply
> the driver.

That's great.

Thanks very much,

BRs
Xiubo