Re: [PATCH v2 1/2] [media] s5p-jpeg: Fix modification sequence of interrupt enable register

2015-01-07 Thread Jacek Anaszewski 

Hi Tony,

On 12/19/2014 08:37 AM, Tony K Nadackal wrote:

Fix the bug in modifying the interrupt enable register.


For Exynos4 this was not a bug as there are only five bit fields
used in the EXYNOS4_INT_EN_REG - all of them enable related
interrupt signal and EXYNOS4_INT_EN_ALL value is 0x1f which
just sets these bit fields to 1.

If for Exynos7 there are other bit fields in this register
and it has to be read prior setting to find out current
state then I'd parametrize this function with version argument
as you do it in the patch adding support for Exynos7, but
for Exynos4 case I'd left the behaviour as it is currenlty, i.e.
avoid reading the register and do it only for Exynos7 case.
Effectively, this patch is not required, as it doesn't fix
anything but adds redundant call to readl.


Signed-off-by: Tony K Nadackal tony...@samsung.com
---
  drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c 
b/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
index e53f13a..a61ff7e 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
@@ -155,7 +155,10 @@ void exynos4_jpeg_set_enc_out_fmt(void __iomem *base, 
unsigned int out_fmt)

  void exynos4_jpeg_set_interrupt(void __iomem *base)
  {
-   writel(EXYNOS4_INT_EN_ALL, base + EXYNOS4_INT_EN_REG);
+   unsigned int reg;
+
+   reg = readl(base + EXYNOS4_INT_EN_REG)  ~EXYNOS4_INT_EN_MASK;
+   writel(reg | EXYNOS4_INT_EN_ALL, base + EXYNOS4_INT_EN_REG);
  }

  unsigned int exynos4_jpeg_get_int_status(void __iomem *base)




--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 1/2] [media] s5p-jpeg: Fix modification sequence of interrupt enable register

2015-01-07 Thread Tony K Nadackal 
Hi Jacek,

On Wednesday, January 07, 2015 3:38 PM Jacek Anaszewski wrote,

 Hi Tony,
 
 On 12/19/2014 08:37 AM, Tony K Nadackal wrote:
  Fix the bug in modifying the interrupt enable register.
 
 For Exynos4 this was not a bug as there are only five bit fields used in the
 EXYNOS4_INT_EN_REG - all of them enable related interrupt signal and
 EXYNOS4_INT_EN_ALL value is 0x1f which just sets these bit fields to 1.
 

I agree that it is not a bug. 
I added the register read before modifying it to avoid any potential bugs in the
future.

 If for Exynos7 there are other bit fields in this register and it has to be
read prior
 setting to find out current state then I'd parametrize this function with
version
 argument as you do it in the patch adding support for Exynos7, but for Exynos4
 case I'd left the behaviour as it is currenlty, i.e.
 avoid reading the register and do it only for Exynos7 case.

Directly writing the value EXYNOS4_INT_EN_ALL (0x1B6 in Exynos7) to
EXYNOS4_INT_EN_REG works in case of Exynos7 too.
I  will parametrize this function with version to take care of the Exynos7 bit
fields.

 Effectively, this patch is not required, as it doesn't fix anything but adds
 redundant call to readl.
 
  Signed-off-by: Tony K Nadackal tony...@samsung.com
  ---
drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
  b/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
  index e53f13a..a61ff7e 100644
  --- a/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
  +++ b/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
  @@ -155,7 +155,10 @@ void exynos4_jpeg_set_enc_out_fmt(void __iomem
  *base, unsigned int out_fmt)
 
void exynos4_jpeg_set_interrupt(void __iomem *base)
{
  -   writel(EXYNOS4_INT_EN_ALL, base + EXYNOS4_INT_EN_REG);
  +   unsigned int reg;
  +
  +   reg = readl(base + EXYNOS4_INT_EN_REG)  ~EXYNOS4_INT_EN_MASK;
  +   writel(reg | EXYNOS4_INT_EN_ALL, base + EXYNOS4_INT_EN_REG);
}
 
unsigned int exynos4_jpeg_get_int_status(void __iomem *base)
 
 
 
 --
 Best Regards,
 Jacek Anaszewski

Thanks and Regards,
Tony

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] [media] s5p-jpeg: Fix modification sequence of interrupt enable register

2014-12-18 Thread Tony K Nadackal 
Fix the bug in modifying the interrupt enable register.

Signed-off-by: Tony K Nadackal tony...@samsung.com
---
 drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c 
b/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
index e53f13a..a61ff7e 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
@@ -155,7 +155,10 @@ void exynos4_jpeg_set_enc_out_fmt(void __iomem *base, 
unsigned int out_fmt)
 
 void exynos4_jpeg_set_interrupt(void __iomem *base)
 {
-   writel(EXYNOS4_INT_EN_ALL, base + EXYNOS4_INT_EN_REG);
+   unsigned int reg;
+
+   reg = readl(base + EXYNOS4_INT_EN_REG)  ~EXYNOS4_INT_EN_MASK;
+   writel(reg | EXYNOS4_INT_EN_ALL, base + EXYNOS4_INT_EN_REG);
 }
 
 unsigned int exynos4_jpeg_get_int_status(void __iomem *base)
-- 
2.2.0

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html