Re: [PATCH v2 2/3] video: xilinxfb: Do not use out_be32 IO function

2013-05-31 Thread Michal Simek 
On 05/31/2013 12:04 AM, Arnd Bergmann wrote:
> On Thursday 30 May 2013 11:41:01 Michal Simek wrote:
>>   * To perform the read/write on the registers we need to check on
>>   * which bus its connected and call the appropriate write API.
>>   */
>> -static void xilinx_fb_out_be32(struct xilinxfb_drvdata *drvdata, u32 offset,
>> +static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
>> u32 val)
>>  {
>> if (drvdata->flags & PLB_ACCESS_FLAG)
>> -   out_be32(drvdata->regs + (offset << 2), val);
>> +   __raw_writel(val, drvdata->regs + (offset << 2));
>>  #ifdef CONFIG_PPC_DCR
>> else
>> dcr_write(drvdata->dcr_host, offset, val);
>>
> 
> This is probably missing barriers, and is wrong on systems on which
> the endianess of the device is different from the CPU.
> 
> You already have an indirection in there, so I guess it won't hurt
> to create a third case for little-endian registers and add
> another bit in drvdata->flags, or make it depend on the architecture,
> if the endianess of the device registers is known at compile time.

The PLB_ACCESS_FLAGS is incorrectly named. It means BUS_ACCESS.
But I will find a way how to autodetect endianess directly on IP
as I have done it for uartlite and will send v3.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform




signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 2/3] video: xilinxfb: Do not use out_be32 IO function

2013-05-31 Thread Michal Simek 
On 05/31/2013 12:04 AM, Arnd Bergmann wrote:
 On Thursday 30 May 2013 11:41:01 Michal Simek wrote:
   * To perform the read/write on the registers we need to check on
   * which bus its connected and call the appropriate write API.
   */
 -static void xilinx_fb_out_be32(struct xilinxfb_drvdata *drvdata, u32 offset,
 +static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
 u32 val)
  {
 if (drvdata-flags  PLB_ACCESS_FLAG)
 -   out_be32(drvdata-regs + (offset  2), val);
 +   __raw_writel(val, drvdata-regs + (offset  2));
  #ifdef CONFIG_PPC_DCR
 else
 dcr_write(drvdata-dcr_host, offset, val);

 
 This is probably missing barriers, and is wrong on systems on which
 the endianess of the device is different from the CPU.
 
 You already have an indirection in there, so I guess it won't hurt
 to create a third case for little-endian registers and add
 another bit in drvdata-flags, or make it depend on the architecture,
 if the endianess of the device registers is known at compile time.

The PLB_ACCESS_FLAGS is incorrectly named. It means BUS_ACCESS.
But I will find a way how to autodetect endianess directly on IP
as I have done it for uartlite and will send v3.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP - KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform




signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 2/3] video: xilinxfb: Do not use out_be32 IO function

2013-05-30 Thread Timur Tabi 
On Thu, May 30, 2013 at 5:04 PM, Arnd Bergmann  wrote:
>
> This is probably missing barriers, and is wrong on systems on which
> the endianess of the device is different from the CPU.


I suggest what was done in fsl_ssi.c:

#ifdef PPC
#define read_ssi(addr) in_be32(addr)
#define write_ssi(val, addr) out_be32(addr, val)
#define write_ssi_mask(addr, clear, set) clrsetbits_be32(addr, clear, set)
#elif defined ARM
#define read_ssi(addr) readl(addr)
#define write_ssi(val, addr) writel(val, addr)
/*
 * FIXME: Proper locking should be added at write_ssi_mask caller level
 * to ensure this register read/modify/write sequence is race free.
 */
static inline void write_ssi_mask(u32 __iomem *addr, u32 clear, u32 set)
{
u32 val = readl(addr);
val = (val & ~clear) | set;
writel(val, addr);
}
#endif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 2/3] video: xilinxfb: Do not use out_be32 IO function

2013-05-30 Thread Arnd Bergmann 
On Thursday 30 May 2013 11:41:01 Michal Simek wrote:
>   * To perform the read/write on the registers we need to check on
>   * which bus its connected and call the appropriate write API.
>   */
> -static void xilinx_fb_out_be32(struct xilinxfb_drvdata *drvdata, u32 offset,
> +static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
> u32 val)
>  {
> if (drvdata->flags & PLB_ACCESS_FLAG)
> -   out_be32(drvdata->regs + (offset << 2), val);
> +   __raw_writel(val, drvdata->regs + (offset << 2));
>  #ifdef CONFIG_PPC_DCR
> else
> dcr_write(drvdata->dcr_host, offset, val);
> 

This is probably missing barriers, and is wrong on systems on which
the endianess of the device is different from the CPU.

You already have an indirection in there, so I guess it won't hurt
to create a third case for little-endian registers and add
another bit in drvdata->flags, or make it depend on the architecture,
if the endianess of the device registers is known at compile time.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 2/3] video: xilinxfb: Do not use out_be32 IO function

2013-05-30 Thread Michal Simek 
out_be32 IO function is not supported by ARM.
It is only available for PPC and Microblaze.

Remove all out_be32 references and start to use __raw_writel
function.

Signed-off-by: Michal Simek 
---
Changes in v2: None

 drivers/video/xilinxfb.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index aecd15d..000185a 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -57,7 +57,7 @@
  * In case of direct PLB access the second control register will be at
  * an offset of 4 as compared to the DCR access where the offset is 1
  * i.e. REG_CTRL. So this is taken care in the function
- * xilinx_fb_out_be32 where it left shifts the offset 2 times in case of
+ * xilinx_fb_out32 where it left shifts the offset 2 times in case of
  * direct PLB access.
  */
 #define NUM_REGS   2
@@ -150,11 +150,11 @@ struct xilinxfb_drvdata {
  * To perform the read/write on the registers we need to check on
  * which bus its connected and call the appropriate write API.
  */
-static void xilinx_fb_out_be32(struct xilinxfb_drvdata *drvdata, u32 offset,
+static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
u32 val)
 {
if (drvdata->flags & PLB_ACCESS_FLAG)
-   out_be32(drvdata->regs + (offset << 2), val);
+   __raw_writel(val, drvdata->regs + (offset << 2));
 #ifdef CONFIG_PPC_DCR
else
dcr_write(drvdata->dcr_host, offset, val);
@@ -197,7 +197,7 @@ xilinx_fb_blank(int blank_mode, struct fb_info *fbi)
switch (blank_mode) {
case FB_BLANK_UNBLANK:
/* turn on panel */
-   xilinx_fb_out_be32(drvdata, REG_CTRL, 
drvdata->reg_ctrl_default);
+   xilinx_fb_out32(drvdata, REG_CTRL, drvdata->reg_ctrl_default);
break;

case FB_BLANK_NORMAL:
@@ -205,7 +205,7 @@ xilinx_fb_blank(int blank_mode, struct fb_info *fbi)
case FB_BLANK_HSYNC_SUSPEND:
case FB_BLANK_POWERDOWN:
/* turn off panel */
-   xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+   xilinx_fb_out32(drvdata, REG_CTRL, 0);
default:
break;

@@ -280,13 +280,13 @@ static int xilinxfb_assign(struct device *dev,
memset_io((void __iomem *)drvdata->fb_virt, 0, fbsize);

/* Tell the hardware where the frame buffer is */
-   xilinx_fb_out_be32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
+   xilinx_fb_out32(drvdata, REG_FB_ADDR, drvdata->fb_phys);

/* Turn on the display */
drvdata->reg_ctrl_default = REG_CTRL_ENABLE;
if (pdata->rotate_screen)
drvdata->reg_ctrl_default |= REG_CTRL_ROTATE;
-   xilinx_fb_out_be32(drvdata, REG_CTRL,
+   xilinx_fb_out32(drvdata, REG_CTRL,
drvdata->reg_ctrl_default);

/* Fill struct fb_info */
@@ -345,7 +345,7 @@ err_cmap:
iounmap(drvdata->fb_virt);

/* Turn off the display */
-   xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+   xilinx_fb_out32(drvdata, REG_CTRL, 0);

 err_fbmem:
if (drvdata->flags & PLB_ACCESS_FLAG)
@@ -381,7 +381,7 @@ static int xilinxfb_release(struct device *dev)
iounmap(drvdata->fb_virt);

/* Turn off the display */
-   xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+   xilinx_fb_out32(drvdata, REG_CTRL, 0);

/* Release the resources, as allocated based on interface */
if (drvdata->flags & PLB_ACCESS_FLAG) {
--
1.8.2.3



pgp0hqygYyCAI.pgp
Description: PGP signature

[PATCH v2 2/3] video: xilinxfb: Do not use out_be32 IO function

2013-05-30 Thread Michal Simek 
out_be32 IO function is not supported by ARM.
It is only available for PPC and Microblaze.

Remove all out_be32 references and start to use __raw_writel
function.

Signed-off-by: Michal Simek michal.si...@xilinx.com
---
Changes in v2: None

 drivers/video/xilinxfb.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index aecd15d..000185a 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -57,7 +57,7 @@
  * In case of direct PLB access the second control register will be at
  * an offset of 4 as compared to the DCR access where the offset is 1
  * i.e. REG_CTRL. So this is taken care in the function
- * xilinx_fb_out_be32 where it left shifts the offset 2 times in case of
+ * xilinx_fb_out32 where it left shifts the offset 2 times in case of
  * direct PLB access.
  */
 #define NUM_REGS   2
@@ -150,11 +150,11 @@ struct xilinxfb_drvdata {
  * To perform the read/write on the registers we need to check on
  * which bus its connected and call the appropriate write API.
  */
-static void xilinx_fb_out_be32(struct xilinxfb_drvdata *drvdata, u32 offset,
+static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
u32 val)
 {
if (drvdata-flags  PLB_ACCESS_FLAG)
-   out_be32(drvdata-regs + (offset  2), val);
+   __raw_writel(val, drvdata-regs + (offset  2));
 #ifdef CONFIG_PPC_DCR
else
dcr_write(drvdata-dcr_host, offset, val);
@@ -197,7 +197,7 @@ xilinx_fb_blank(int blank_mode, struct fb_info *fbi)
switch (blank_mode) {
case FB_BLANK_UNBLANK:
/* turn on panel */
-   xilinx_fb_out_be32(drvdata, REG_CTRL, 
drvdata-reg_ctrl_default);
+   xilinx_fb_out32(drvdata, REG_CTRL, drvdata-reg_ctrl_default);
break;

case FB_BLANK_NORMAL:
@@ -205,7 +205,7 @@ xilinx_fb_blank(int blank_mode, struct fb_info *fbi)
case FB_BLANK_HSYNC_SUSPEND:
case FB_BLANK_POWERDOWN:
/* turn off panel */
-   xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+   xilinx_fb_out32(drvdata, REG_CTRL, 0);
default:
break;

@@ -280,13 +280,13 @@ static int xilinxfb_assign(struct device *dev,
memset_io((void __iomem *)drvdata-fb_virt, 0, fbsize);

/* Tell the hardware where the frame buffer is */
-   xilinx_fb_out_be32(drvdata, REG_FB_ADDR, drvdata-fb_phys);
+   xilinx_fb_out32(drvdata, REG_FB_ADDR, drvdata-fb_phys);

/* Turn on the display */
drvdata-reg_ctrl_default = REG_CTRL_ENABLE;
if (pdata-rotate_screen)
drvdata-reg_ctrl_default |= REG_CTRL_ROTATE;
-   xilinx_fb_out_be32(drvdata, REG_CTRL,
+   xilinx_fb_out32(drvdata, REG_CTRL,
drvdata-reg_ctrl_default);

/* Fill struct fb_info */
@@ -345,7 +345,7 @@ err_cmap:
iounmap(drvdata-fb_virt);

/* Turn off the display */
-   xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+   xilinx_fb_out32(drvdata, REG_CTRL, 0);

 err_fbmem:
if (drvdata-flags  PLB_ACCESS_FLAG)
@@ -381,7 +381,7 @@ static int xilinxfb_release(struct device *dev)
iounmap(drvdata-fb_virt);

/* Turn off the display */
-   xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+   xilinx_fb_out32(drvdata, REG_CTRL, 0);

/* Release the resources, as allocated based on interface */
if (drvdata-flags  PLB_ACCESS_FLAG) {
--
1.8.2.3



pgp0hqygYyCAI.pgp
Description: PGP signature

Re: [PATCH v2 2/3] video: xilinxfb: Do not use out_be32 IO function

2013-05-30 Thread Arnd Bergmann 
On Thursday 30 May 2013 11:41:01 Michal Simek wrote:
   * To perform the read/write on the registers we need to check on
   * which bus its connected and call the appropriate write API.
   */
 -static void xilinx_fb_out_be32(struct xilinxfb_drvdata *drvdata, u32 offset,
 +static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
 u32 val)
  {
 if (drvdata-flags  PLB_ACCESS_FLAG)
 -   out_be32(drvdata-regs + (offset  2), val);
 +   __raw_writel(val, drvdata-regs + (offset  2));
  #ifdef CONFIG_PPC_DCR
 else
 dcr_write(drvdata-dcr_host, offset, val);
 

This is probably missing barriers, and is wrong on systems on which
the endianess of the device is different from the CPU.

You already have an indirection in there, so I guess it won't hurt
to create a third case for little-endian registers and add
another bit in drvdata-flags, or make it depend on the architecture,
if the endianess of the device registers is known at compile time.

Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@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 2/3] video: xilinxfb: Do not use out_be32 IO function

2013-05-30 Thread Timur Tabi 
On Thu, May 30, 2013 at 5:04 PM, Arnd Bergmann a...@arndb.de wrote:

 This is probably missing barriers, and is wrong on systems on which
 the endianess of the device is different from the CPU.


I suggest what was done in fsl_ssi.c:

#ifdef PPC
#define read_ssi(addr) in_be32(addr)
#define write_ssi(val, addr) out_be32(addr, val)
#define write_ssi_mask(addr, clear, set) clrsetbits_be32(addr, clear, set)
#elif defined ARM
#define read_ssi(addr) readl(addr)
#define write_ssi(val, addr) writel(val, addr)
/*
 * FIXME: Proper locking should be added at write_ssi_mask caller level
 * to ensure this register read/modify/write sequence is race free.
 */
static inline void write_ssi_mask(u32 __iomem *addr, u32 clear, u32 set)
{
u32 val = readl(addr);
val = (val  ~clear) | set;
writel(val, addr);
}
#endif
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/