Re: [PATCH] ARM: S3C2443: Workaround for 2443 EXTINT error

Re: [PATCH] ARM: S3C2443: Workaround for 2443 EXTINT error

[Heiko Stübner]

sorry, forgot half of it [inline]

Am Freitag, 23. November 2012, 08:10:15 schrieb Alexander Varnin:
> Please take a look at this document. It describes the problem with
> EXTINTn registers on 2443.
> In fact, the irqext_set function for s3c2443 differs from common
> starting from "//Hack for 2443 error workaround" comment.

wow ... this is a crazy bug :-)

[...]

> >> diff --git a/arch/arm/mach-s3c24xx/s3c2443.c
> >> b/arch/arm/mach-s3c24xx/s3c2443.c index ab648ad..99eef31 100644
> >> --- a/arch/arm/mach-s3c24xx/s3c2443.c
> >> +++ b/arch/arm/mach-s3c24xx/s3c2443.c

[...]

> >> +#if defined(CONFIG_CPU_S3C2443)
> >> +int
> >> +s3c2443_irqext_type(struct irq_data *data, unsigned int type)
> >> +{

[...]

> >> +	value = __raw_readl(extint_reg);
> >> +	//Hack for 2443 error workaround
> >> +	fixed = 0;
> >> +	if(extint_reg == S3C24XX_EXTINT1 || extint_reg == S3C24XX_EXTINT2)
> >> +		for(i=0; i<7;i++)
> >> +		    	fixed |= (((value >> ((7-i)*4+1)) & 7) | ((value >> ((7-
i)*4-3))
> >> & 8)) << i*4; +	else
> >> +		for(i=0; i<7;i++)
> >> +		    	fixed |= ( (value >> (7-i)*4) & 0xf )  << i*4;
> >> +        fixed |= (((value>>1) & 7) | ((value<<3) & 8)) << 27;
> >> +	value = fixed;
> > 
> > What does this do or what should it do? Also it gets calculated but
> > never used?
> > 
> > And please use scripts/checkpatch.pl to verify your patch follows
> > coding guidelines, as this block is especially hard to read.

So essentially register-reads somehow returned transformed data, but the write 
is done according to the datasheet.


It would definitely be better to integrate it into the existing _irqext_type 
function instead of introducing a second one.

The cpu_id is present in the samsung_cpu_id var and the list of cpus including 
the s3c2443 can be found in common.c. With this it would be possible to 
identify when the irq code is run on a s3c2443 machine and the original 
_irqext_type function could change the behaviour accordingly.

Not sure if it would make sense to introduce soc_is_s3c2443() etc macros for 
this.

And of course the actual block doing the transformation on read would need a 
more elaborate comment on the why and how, because in 3 years someone might 
not directly see what this does and why it was necessary.


> >> +	value = (value & ~(7 << extint_offset)) | (newvalue << 
extint_offset);
> >> +	__raw_writel(value, extint_reg);
> >> +
> >> +	return 0;
> >> +}
> >> +#endif //defined(CONFIG_CPU_S3C2443)
> >> 
> >>   static struct irq_chip s3c_irqext_chip = {
> >>   
> >>   	.name		= "s3c-ext",
> >>   	.irq_mask	= s3c_irqext_mask,

Re: [PATCH] ARM: S3C2443: Workaround for 2443 EXTINT error

[Alexander Varnin]

I've done it within another function, because otherwise users of other
chips would pay for a one more runtime check, which they don't need. On
the other hand, this function get called not so frequently, to make it
valueable. The first version of my patch i've used internally worked as
you said, so i can resend it.

I want to ask more experienced users of s3c2443. If this problem occures
on all s3c2443 chips, or only with some series of it? Maybe we need some
more checks, not to break working cases.

24.11.2012 04:16, Heiko Stübner пишет:
>>> What does this do or what should it do? Also it gets calculated but
>>> never used?
>>>
>>> And please use scripts/checkpatch.pl to verify your patch follows
>>> coding guidelines, as this block is especially hard to read.
> So essentially register-reads somehow returned transformed data, but the write 
> is done according to the datasheet.
>
>
> It would definitely be better to integrate it into the existing _irqext_type 
> function instead of introducing a second one.
>
> The cpu_id is present in the samsung_cpu_id var and the list of cpus including 
> the s3c2443 can be found in common.c. With this it would be possible to 
> identify when the irq code is run on a s3c2443 machine and the original 
> _irqext_type function could change the behaviour accordingly.
>
> Not sure if it would make sense to introduce soc_is_s3c2443() etc macros for 
> this.
>
> And of course the actual block doing the transformation on read would need a 
> more elaborate comment on the why and how, because in 3 years someone might 
> not directly see what this does and why it was necessary.
>

Re: [PATCH] ARM: S3C2443: Workaround for 2443 EXTINT error

[Heiko Stübner]

Am Samstag, 24. November 2012, 10:01:31 schrieb Alexander Varnin:
> I've done it within another function, because otherwise users of other
> chips would pay for a one more runtime check, which they don't need. On
> the other hand, this function get called not so frequently, to make it
> valueable. The first version of my patch i've used internally worked as
> you said, so i can resend it.

that would be cool, because as you said, this function isn't called this often 
and adding another function here seems kinda counter-productive to all the 
consolidation effort.

Your could even do a

#ifdef CONFIG_CPU_S3C2443
if (samsung_cpu_id == ...)
	the fix
#endif

so only people building multi-platform kernels will be affected.


> I want to ask more experienced users of s3c2443. If this problem occures
> on all s3c2443 chips, or only with some series of it? Maybe we need some
> more checks, not to break working cases.

Finding other s3c2443 mainline-users seems kinda hard - you're the first one I 
see :-) . In all the things I did to the s3c2416 I tried to pull the s3c2443 
along (blindly) most of the time, because both SoCs are so similar [0]. And 
during all the changes no-one ever complained or spoke up.


@Kgene do you have anybody near you, who can tell us if the problem of the 
swapped EXTINT bits is common to all s3c2443 SoCs or how to identify affected 
ones?


Heiko

[0] for example, you should be able to use the s3c2416 cpufreq driver also on 
the s3c2443, as the armdiv etc structure is the same.


> 24.11.2012 04:16, Heiko Stübner пишет:
> >>> What does this do or what should it do? Also it gets calculated but
> >>> never used?
> >>> 
> >>> And please use scripts/checkpatch.pl to verify your patch follows
> >>> coding guidelines, as this block is especially hard to read.
> > 
> > So essentially register-reads somehow returned transformed data, but the
> > write is done according to the datasheet.
> > 
> > 
> > It would definitely be better to integrate it into the existing
> > _irqext_type function instead of introducing a second one.
> > 
> > The cpu_id is present in the samsung_cpu_id var and the list of cpus
> > including the s3c2443 can be found in common.c. With this it would be
> > possible to identify when the irq code is run on a s3c2443 machine and
> > the original _irqext_type function could change the behaviour
> > accordingly.
> > 
> > Not sure if it would make sense to introduce soc_is_s3c2443() etc macros
> > for this.
> > 
> > And of course the actual block doing the transformation on read would
> > need a more elaborate comment on the why and how, because in 3 years
> > someone might not directly see what this does and why it was necessary.

Re: [PATCH] ARM: S3C2443: Workaround for 2443 EXTINT error

[Heiko Stübner]

Am Samstag, 24. November 2012, 12:24:49 schrieb Heiko Stübner:
> Am Samstag, 24. November 2012, 10:01:31 schrieb Alexander Varnin:
> > I've done it within another function, because otherwise users of other
> > chips would pay for a one more runtime check, which they don't need. On
> > the other hand, this function get called not so frequently, to make it
> > valueable. The first version of my patch i've used internally worked as
> > you said, so i can resend it.
> 
> that would be cool, because as you said, this function isn't called this
> often and adding another function here seems kinda counter-productive to
> all the consolidation effort.
> 
> Your could even do a
> 
> #ifdef CONFIG_CPU_S3C2443
> if (samsung_cpu_id == ...)
> 	the fix
> #endif

or alternatively introduce a soc_is_s3c2443 macro (plat-
samsung/include/plat/cpu.h) for the conditional, like the other Samsung SoCs 
have.

if (soc_is_s3c2443())
 	the fix

If done like the other SoCs, it would evaluate to 0, when CONFIG_CPU_S3C2443 
is not set and otherwise check the cpu_id.


> so only people building multi-platform kernels will be affected.
> 
> > I want to ask more experienced users of s3c2443. If this problem occures
> > on all s3c2443 chips, or only with some series of it? Maybe we need some
> > more checks, not to break working cases.
> 
> Finding other s3c2443 mainline-users seems kinda hard - you're the first
> one I see :-) . In all the things I did to the s3c2416 I tried to pull the
> s3c2443 along (blindly) most of the time, because both SoCs are so similar
> [0]. And during all the changes no-one ever complained or spoke up.
> 
> 
> @Kgene do you have anybody near you, who can tell us if the problem of the
> swapped EXTINT bits is common to all s3c2443 SoCs or how to identify
> affected ones?
> 
> 
> Heiko
> 
> [0] for example, you should be able to use the s3c2416 cpufreq driver also
> on the s3c2443, as the armdiv etc structure is the same.
> 
> > 24.11.2012 04:16, Heiko Stübner пишет:
> > >>> What does this do or what should it do? Also it gets calculated but
> > >>> never used?
> > >>> 
> > >>> And please use scripts/checkpatch.pl to verify your patch follows
> > >>> coding guidelines, as this block is especially hard to read.
> > > 
> > > So essentially register-reads somehow returned transformed data, but
> > > the write is done according to the datasheet.
> > > 
> > > 
> > > It would definitely be better to integrate it into the existing
> > > _irqext_type function instead of introducing a second one.
> > > 
> > > The cpu_id is present in the samsung_cpu_id var and the list of cpus
> > > including the s3c2443 can be found in common.c. With this it would be
> > > possible to identify when the irq code is run on a s3c2443 machine and
> > > the original _irqext_type function could change the behaviour
> > > accordingly.
> > > 
> > > Not sure if it would make sense to introduce soc_is_s3c2443() etc
> > > macros for this.
> > > 
> > > And of course the actual block doing the transformation on read would
> > > need a more elaborate comment on the why and how, because in 3 years
> > > someone might not directly see what this does and why it was necessary.

[PATCH] ARM: S3C2443: Workaround for 2443 EXTINT error

[Alexander Varnin]

S3C2443 CPU has a problem with incorrect reading from EXTINTn
registers. So s3c_irqext_type function wrongly modifies them.
So add special function for s3c2443, to handle this case.

Signed-off-by: Alexander Varnin <fenixk19@mail.ru>
---
 arch/arm/mach-s3c24xx/s3c2443.c |    8 ++++
 arch/arm/plat-s3c24xx/irq.c     |   89 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-s3c24xx/s3c2443.c b/arch/arm/mach-s3c24xx/s3c2443.c
index ab648ad..99eef31 100644
--- a/arch/arm/mach-s3c24xx/s3c2443.c
+++ b/arch/arm/mach-s3c24xx/s3c2443.c
@@ -67,8 +67,11 @@ void s3c2443_restart(char mode, const char *cmd)
 	__raw_writel(S3C2443_SWRST_RESET, S3C2443_SWRST);
 }
 
+extern int s3c2443_irqext_type(struct irq_data *data, unsigned int type);
+
 int __init s3c2443_init(void)
 {
+	struct irq_chip * chip;
 	printk("S3C2443: Initialising architecture\n");
 
 	s3c_nand_setname("s3c2412-nand");
@@ -81,6 +84,11 @@ int __init s3c2443_init(void)
 	s3c_device_wdt.resource[1].start = IRQ_S3C2443_WDT;
 	s3c_device_wdt.resource[1].end   = IRQ_S3C2443_WDT;
 
+	chip = irq_get_chip(IRQ_EINT0);
+	chip->irq_set_type=s3c2443_irqext_type;
+	chip = irq_get_chip(IRQ_EINT4);
+	chip->irq_set_type=s3c2443_irqext_type;
+
 	return device_register(&s3c2443_dev);
 }
 
diff --git a/arch/arm/plat-s3c24xx/irq.c b/arch/arm/plat-s3c24xx/irq.c
index fe57bbb..9c815f3 100644
--- a/arch/arm/plat-s3c24xx/irq.c
+++ b/arch/arm/plat-s3c24xx/irq.c
@@ -224,7 +224,96 @@ s3c_irqext_type(struct irq_data *data, unsigned int type)
 
 	return 0;
 }
+#if defined(CONFIG_CPU_S3C2443)
+int
+s3c2443_irqext_type(struct irq_data *data, unsigned int type)
+{
+	int i;
+	int fixed;
+	void __iomem *extint_reg;
+	void __iomem *gpcon_reg;
+	unsigned long gpcon_offset, extint_offset;
+	unsigned long newvalue = 0, value;
+
+	if ((data->irq >= IRQ_EINT0) && (data->irq <= IRQ_EINT3)) {
+		gpcon_reg = S3C2410_GPFCON;
+		extint_reg = S3C24XX_EXTINT0;
+		gpcon_offset = (data->irq - IRQ_EINT0) * 2;
+		extint_offset = (data->irq - IRQ_EINT0) * 4;
+	} else if ((data->irq >= IRQ_EINT4) && (data->irq <= IRQ_EINT7)) {
+		gpcon_reg = S3C2410_GPFCON;
+		extint_reg = S3C24XX_EXTINT0;
+		gpcon_offset = (data->irq - (EXTINT_OFF)) * 2;
+		extint_offset = (data->irq - (EXTINT_OFF)) * 4;
+	} else if ((data->irq >= IRQ_EINT8) && (data->irq <= IRQ_EINT15)) {
+		gpcon_reg = S3C2410_GPGCON;
+		extint_reg = S3C24XX_EXTINT1;
+		gpcon_offset = (data->irq - IRQ_EINT8) * 2;
+		extint_offset = (data->irq - IRQ_EINT8) * 4;
+	} else if ((data->irq >= IRQ_EINT16) && (data->irq <= IRQ_EINT23)) {
+		gpcon_reg = S3C2410_GPGCON;
+		extint_reg = S3C24XX_EXTINT2;
+		gpcon_offset = (data->irq - IRQ_EINT8) * 2;
+		extint_offset = (data->irq - IRQ_EINT16) * 4;
+	} else {
+		return -1;
+	}
 
+	/* Set the GPIO to external interrupt mode */
+	value = __raw_readl(gpcon_reg);
+	value = (value & ~(3 << gpcon_offset)) | (0x02 << gpcon_offset);
+	__raw_writel(value, gpcon_reg);
+
+	/* Set the external interrupt to pointed trigger type */
+	switch (type)
+	{
+		case IRQ_TYPE_NONE:
+			printk(KERN_WARNING "No edge setting!\n");
+			break;
+
+		case IRQ_TYPE_EDGE_RISING:
+			newvalue = S3C2410_EXTINT_RISEEDGE;
+			break;
+
+		case IRQ_TYPE_EDGE_FALLING:
+			newvalue = S3C2410_EXTINT_FALLEDGE;
+			break;
+
+		case IRQ_TYPE_EDGE_BOTH:
+			newvalue = S3C2410_EXTINT_BOTHEDGE;
+			break;
+
+		case IRQ_TYPE_LEVEL_LOW:
+			newvalue = S3C2410_EXTINT_LOWLEV;
+			break;
+
+		case IRQ_TYPE_LEVEL_HIGH:
+			newvalue = S3C2410_EXTINT_HILEV;
+			break;
+
+		default:
+			printk(KERN_ERR "No such irq type %d", type);
+			return -1;
+	}
+
+	value = __raw_readl(extint_reg);
+	//Hack for 2443 error workaround
+	fixed = 0;
+	if(extint_reg == S3C24XX_EXTINT1 || extint_reg == S3C24XX_EXTINT2)
+		for(i=0; i<7;i++)
+		    	fixed |= (((value >> ((7-i)*4+1)) & 7) | ((value >> ((7-i)*4-3)) & 8)) << i*4;
+	else
+		for(i=0; i<7;i++)
+		    	fixed |= ( (value >> (7-i)*4) & 0xf )  << i*4;
+        fixed |= (((value>>1) & 7) | ((value<<3) & 8)) << 27;
+	value = fixed;
+
+	value = (value & ~(7 << extint_offset)) | (newvalue << extint_offset);
+	__raw_writel(value, extint_reg);
+
+	return 0;
+}
+#endif //defined(CONFIG_CPU_S3C2443)
 static struct irq_chip s3c_irqext_chip = {
 	.name		= "s3c-ext",
 	.irq_mask	= s3c_irqext_mask,
-- 
1.7.2.5

Re: [PATCH] ARM: S3C2443: Workaround for 2443 EXTINT error

[Heiko Stübner]

Hi Alexander,


first of all could you elaborate a bit more on the error you experience,
because I currently have problems to understand it from the code alone :-) .

More inline.


Am Donnerstag, 22. November 2012, 14:00:01 schrieb Alexander Varnin:
> S3C2443 CPU has a problem with incorrect reading from EXTINTn
> registers. So s3c_irqext_type function wrongly modifies them.
> So add special function for s3c2443, to handle this case.
> 
> Signed-off-by: Alexander Varnin <fenixk19@mail.ru>
> ---
>  arch/arm/mach-s3c24xx/s3c2443.c |    8 ++++
>  arch/arm/plat-s3c24xx/irq.c     |   89
> +++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+),
> 0 deletions(-)
> 
> diff --git a/arch/arm/mach-s3c24xx/s3c2443.c
> b/arch/arm/mach-s3c24xx/s3c2443.c index ab648ad..99eef31 100644
> --- a/arch/arm/mach-s3c24xx/s3c2443.c
> +++ b/arch/arm/mach-s3c24xx/s3c2443.c
> @@ -67,8 +67,11 @@ void s3c2443_restart(char mode, const char *cmd)
>  	__raw_writel(S3C2443_SWRST_RESET, S3C2443_SWRST);
>  }
> 
> +extern int s3c2443_irqext_type(struct irq_data *data, unsigned int type);
> +
>  int __init s3c2443_init(void)
>  {
> +	struct irq_chip * chip;
>  	printk("S3C2443: Initialising architecture\n");
> 
>  	s3c_nand_setname("s3c2412-nand");
> @@ -81,6 +84,11 @@ int __init s3c2443_init(void)
>  	s3c_device_wdt.resource[1].start = IRQ_S3C2443_WDT;
>  	s3c_device_wdt.resource[1].end   = IRQ_S3C2443_WDT;
> 
> +	chip = irq_get_chip(IRQ_EINT0);
> +	chip->irq_set_type=s3c2443_irqext_type;
> +	chip = irq_get_chip(IRQ_EINT4);
> +	chip->irq_set_type=s3c2443_irqext_type;
> +
>  	return device_register(&s3c2443_dev);
>  }

both this and your new exttype function would be more suitable for
irq-s3c2443.c .



> diff --git a/arch/arm/plat-s3c24xx/irq.c b/arch/arm/plat-s3c24xx/irq.c
> index fe57bbb..9c815f3 100644
> --- a/arch/arm/plat-s3c24xx/irq.c
> +++ b/arch/arm/plat-s3c24xx/irq.c
> @@ -224,7 +224,96 @@ s3c_irqext_type(struct irq_data *data, unsigned int
> type)
> 
>  	return 0;
>  }
> +#if defined(CONFIG_CPU_S3C2443)
> +int
> +s3c2443_irqext_type(struct irq_data *data, unsigned int type)
> +{
> +	int i;
> +	int fixed;
> +	void __iomem *extint_reg;
> +	void __iomem *gpcon_reg;
> +	unsigned long gpcon_offset, extint_offset;
> +	unsigned long newvalue = 0, value;
> +
> +	if ((data->irq >= IRQ_EINT0) && (data->irq <= IRQ_EINT3)) {
> +		gpcon_reg = S3C2410_GPFCON;
> +		extint_reg = S3C24XX_EXTINT0;
> +		gpcon_offset = (data->irq - IRQ_EINT0) * 2;
> +		extint_offset = (data->irq - IRQ_EINT0) * 4;
> +	} else if ((data->irq >= IRQ_EINT4) && (data->irq <= IRQ_EINT7)) {
> +		gpcon_reg = S3C2410_GPFCON;
> +		extint_reg = S3C24XX_EXTINT0;
> +		gpcon_offset = (data->irq - (EXTINT_OFF)) * 2;
> +		extint_offset = (data->irq - (EXTINT_OFF)) * 4;
> +	} else if ((data->irq >= IRQ_EINT8) && (data->irq <= IRQ_EINT15)) {
> +		gpcon_reg = S3C2410_GPGCON;
> +		extint_reg = S3C24XX_EXTINT1;
> +		gpcon_offset = (data->irq - IRQ_EINT8) * 2;
> +		extint_offset = (data->irq - IRQ_EINT8) * 4;
> +	} else if ((data->irq >= IRQ_EINT16) && (data->irq <= IRQ_EINT23)) {
> +		gpcon_reg = S3C2410_GPGCON;
> +		extint_reg = S3C24XX_EXTINT2;
> +		gpcon_offset = (data->irq - IRQ_EINT8) * 2;
> +		extint_offset = (data->irq - IRQ_EINT16) * 4;
> +	} else {
> +		return -1;
> +	}
> 
> +	/* Set the GPIO to external interrupt mode */
> +	value = __raw_readl(gpcon_reg);
> +	value = (value & ~(3 << gpcon_offset)) | (0x02 << gpcon_offset);
> +	__raw_writel(value, gpcon_reg);
> +
> +	/* Set the external interrupt to pointed trigger type */
> +	switch (type)
> +	{
> +		case IRQ_TYPE_NONE:
> +			printk(KERN_WARNING "No edge setting!\n");
> +			break;
> +
> +		case IRQ_TYPE_EDGE_RISING:
> +			newvalue = S3C2410_EXTINT_RISEEDGE;
> +			break;
> +
> +		case IRQ_TYPE_EDGE_FALLING:
> +			newvalue = S3C2410_EXTINT_FALLEDGE;
> +			break;
> +
> +		case IRQ_TYPE_EDGE_BOTH:
> +			newvalue = S3C2410_EXTINT_BOTHEDGE;
> +			break;
> +
> +		case IRQ_TYPE_LEVEL_LOW:
> +			newvalue = S3C2410_EXTINT_LOWLEV;
> +			break;
> +
> +		case IRQ_TYPE_LEVEL_HIGH:
> +			newvalue = S3C2410_EXTINT_HILEV;
> +			break;
> +
> +		default:
> +			printk(KERN_ERR "No such irq type %d", type);
> +			return -1;
> +	}
> +
> +	value = __raw_readl(extint_reg);
> +	//Hack for 2443 error workaround
> +	fixed = 0;
> +	if(extint_reg == S3C24XX_EXTINT1 || extint_reg == S3C24XX_EXTINT2)
> +		for(i=0; i<7;i++)
> +		    	fixed |= (((value >> ((7-i)*4+1)) & 7) | ((value >> ((7-i)*4-3)) & 8)) << i*4;
> +	else
> +		for(i=0; i<7;i++)
> +		    	fixed |= ( (value >> (7-i)*4) & 0xf )  << i*4;
> +        fixed |= (((value>>1) & 7) | ((value<<3) & 8)) << 27;
> +	value = fixed;

What does this do or what should it do? Also it gets calculated but
never used?

And please use scripts/checkpatch.pl to verify your patch follows
coding guidelines, as this block is especially hard to read.


> +	value = (value & ~(7 << extint_offset)) | (newvalue << extint_offset);
> +	__raw_writel(value, extint_reg);
> +
> +	return 0;
> +}
> +#endif //defined(CONFIG_CPU_S3C2443)
>  static struct irq_chip s3c_irqext_chip = {
>  	.name		= "s3c-ext",
>  	.irq_mask	= s3c_irqext_mask,

Re: [PATCH] ARM: S3C2443: Workaround for 2443 EXTINT error

[Heiko Stübner]

Am Freitag, 23. November 2012, 08:10:15 schrieb Alexander Varnin:
> Please take a look at this document. It describes the problem with
> EXTINTn registers on 2443.
> In fact, the irqext_set function for s3c2443 differs from common
> starting from "//Hack for 2443 error workaround" comment.

wow ... this is a crazy bug :-)

[...]

> >> diff --git a/arch/arm/mach-s3c24xx/s3c2443.c
> >> b/arch/arm/mach-s3c24xx/s3c2443.c index ab648ad..99eef31 100644
> >> --- a/arch/arm/mach-s3c24xx/s3c2443.c
> >> +++ b/arch/arm/mach-s3c24xx/s3c2443.c

[...]

> >> +#if defined(CONFIG_CPU_S3C2443)
> >> +int
> >> +s3c2443_irqext_type(struct irq_data *data, unsigned int type)
> >> +{

[...]

> >> +	value = __raw_readl(extint_reg);
> >> +	//Hack for 2443 error workaround
> >> +	fixed = 0;
> >> +	if(extint_reg == S3C24XX_EXTINT1 || extint_reg == S3C24XX_EXTINT2)
> >> +		for(i=0; i<7;i++)
> >> +		    	fixed |= (((value >> ((7-i)*4+1)) & 7) | ((value >> ((7-
i)*4-3))
> >> & 8)) << i*4; +	else
> >> +		for(i=0; i<7;i++)
> >> +		    	fixed |= ( (value >> (7-i)*4) & 0xf )  << i*4;
> >> +        fixed |= (((value>>1) & 7) | ((value<<3) & 8)) << 27;
> >> +	value = fixed;
> > 
> > What does this do or what should it do? Also it gets calculated but
> > never used?
> > 
> > And please use scripts/checkpatch.pl to verify your patch follows
> > coding guidelines, as this block is especially hard to read.



> >> +	value = (value & ~(7 << extint_offset)) | (newvalue << 
extint_offset);
> >> +	__raw_writel(value, extint_reg);
> >> +
> >> +	return 0;
> >> +}
> >> +#endif //defined(CONFIG_CPU_S3C2443)
> >> 
> >>   static struct irq_chip s3c_irqext_chip = {
> >>   
> >>   	.name		= "s3c-ext",
> >>   	.irq_mask	= s3c_irqext_mask,