public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: kgene.kim@samsung•com (Kukjin Kim)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v3 08/12] ARM: EXYNOS: support EINT for EXYNOS4 and EXYNOS5
Date: Tue, 13 Mar 2012 23:33:44 -0700	[thread overview]
Message-ID: <4F603BC8.3020900@samsung.com> (raw)
In-Reply-To: <20120314035203.GA2531@quad.lixom.net>

On 03/13/12 20:52, Olof Johansson wrote:
> Hi,
>
> A couple of comments below.
>
I agree, and will address comments from you.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung•com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

>
> On Tue, Mar 13, 2012 at 08:30:39AM -0700, Kukjin Kim wrote:
>> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
>> index 0b53018..72f21e4 100644
>> --- a/arch/arm/mach-exynos/common.c
>> +++ b/arch/arm/mach-exynos/common.c
>> @@ -777,39 +779,24 @@ static int exynos4_irq_eint_set_type(struct irq_data *data, unsigned int type)
>>   	mask = 0x7<<  shift;
>>
>>   	spin_lock(&eint_lock);
>> -	ctrl = __raw_readl(S5P_EINT_CON(EINT_REG_NR(data->irq)));
>> +	ctrl = __raw_readl(EINT_CON(exynos_eint_base, data->irq));
>>   	ctrl&= ~mask;
>>   	ctrl |= newvalue<<  shift;
>> -	__raw_writel(ctrl, S5P_EINT_CON(EINT_REG_NR(data->irq)));
>> +	__raw_writel(ctrl, EINT_CON(exynos_eint_base, data->irq));
>>   	spin_unlock(&eint_lock);
>>
>> -	switch (offs) {
>> -	case 0 ... 7:
>> -		s3c_gpio_cfgpin(EINT_GPIO_0(offs&  0x7), EINT_MODE);
>> -		break;
>> -	case 8 ... 15:
>> -		s3c_gpio_cfgpin(EINT_GPIO_1(offs&  0x7), EINT_MODE);
>> -		break;
>> -	case 16 ... 23:
>> -		s3c_gpio_cfgpin(EINT_GPIO_2(offs&  0x7), EINT_MODE);
>> -		break;
>> -	case 24 ... 31:
>> -		s3c_gpio_cfgpin(EINT_GPIO_3(offs&  0x7), EINT_MODE);
>> -		break;
>> -	default:
>> -		printk(KERN_ERR "No such irq number %d", offs);
>> -	}
>> +	s3c_gpio_cfgpin(irq_to_gpio(data->irq), S3C_GPIO_SFN(0xf));
>
> irq_to_gpio is going away, so it's not a good idea to add it under that name.
>
> Add a local function locally at the top of this file instead. and call
> it something else.
>
>> diff --git a/arch/arm/mach-exynos/include/mach/gpio.h b/arch/arm/mach-exynos/include/mach/gpio.h
>> index 80523ca..c09deb6 100644
>> --- a/arch/arm/mach-exynos/include/mach/gpio.h
>> +++ b/arch/arm/mach-exynos/include/mach/gpio.h
>> @@ -146,4 +146,33 @@ enum s5p_gpio_number {
>>   #define ARCH_NR_GPIOS		(EXYNOS4_GPZ(EXYNOS4_GPIO_Z_NR) +	\
>>   				 CONFIG_SAMSUNG_GPIO_EXTRA + 1)
>>
>> +#include<linux/types.h>
>> +#include<linux/err.h>
>> +#include<mach/irqs.h>
>> +#include<plat/cpu.h>
>> +
>> +static inline int irq_to_gpio(unsigned int irq)
>> +{
>> +	if (irq<  IRQ_EINT(0))
>> +		return -EINVAL;
>> +
>> +	irq -= IRQ_EINT(0);
>> +	if (irq<  8)
>> +		return soc_is_exynos5250() ?  EXYNOS5_GPX0(irq) :
>> +					 EXYNOS4_GPX0(irq);
>> +	irq -= 8;
>> +	if (irq<  8)
>> +		return soc_is_exynos5250() ?  EXYNOS5_GPX1(irq) :
>> +					 EXYNOS4_GPX1(irq);
>> +	irq -= 8;
>> +	if (irq<  8)
>> +		return soc_is_exynos5250() ?  EXYNOS5_GPX2(irq) :
>> +					 EXYNOS4_GPX2(irq);
>> +	irq -= 8;
>> +	if (irq<  8)
>> +		return soc_is_exynos5250() ?  EXYNOS5_GPX3(irq) :
>> +					 EXYNOS4_GPX3(irq);
>> +	return -EINVAL;
>> +}
>
> I think this would be cleaner as two small helpers and one check for SoC
> version instead.

  reply	other threads:[~2012-03-14  6:33 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-13 15:30 [PATCH v3 00/12] ARM: EXYNOS: add support new EXYNOS5250 Kukjin Kim
2012-03-13 15:30 ` [PATCH v3 01/12] ARM: EXYNOS: to declare static for mach-exynos/common.c Kukjin Kim
2012-03-13 15:30 ` [PATCH v3 02/12] ARM: EXYNOS: use exynos_init_uarts() instead of exynos4_init_uarts() Kukjin Kim
2012-03-13 15:30 ` [PATCH v3 03/12] ARM: EXYNOS: add clock part for EXYNOS5250 SoC Kukjin Kim
2012-03-13 15:30 ` [PATCH v3 04/12] ARM: EXYNOS: add initial setup-i2c0 for EXYNOS5 Kukjin Kim
2012-03-13 15:30 ` [PATCH v3 05/12] ARM: EXYNOS: add support uart for EXYNOS4 and EXYNOS5 Kukjin Kim
2012-03-13 16:13   ` Arnd Bergmann
2012-03-14  7:06     ` Kukjin Kim
2012-03-13 15:30 ` [PATCH v3 06/12] ARM: EXYNOS: add support for EXYNOS5250 SoC Kukjin Kim
2012-03-13 15:30 ` [PATCH v3 07/12] ARM: EXYNOS: add interrupt definitions for EXYNOS5250 Kukjin Kim
2012-03-14  3:42   ` Olof Johansson
2012-03-14  6:30     ` Kukjin Kim
2012-03-14  5:27   ` Thomas Abraham
2012-03-14  6:32     ` Kukjin Kim
2012-03-13 15:30 ` [PATCH v3 08/12] ARM: EXYNOS: support EINT for EXYNOS4 and EXYNOS5 Kukjin Kim
2012-03-14  3:52   ` Olof Johansson
2012-03-14  6:33     ` Kukjin Kim [this message]
2012-03-13 15:30 ` [PATCH v3 09/12] ARM: EXYNOS: add support get_core_count() for EXYNOS5250 Kukjin Kim
2012-03-13 16:15   ` Arnd Bergmann
2012-03-14  6:49     ` Kukjin Kim
2012-03-13 15:30 ` [PATCH v3 10/12] ARM: EXYNOS: add support ARCH_EXYNOS5 for EXYNOS5 SoCs Kukjin Kim
2012-03-13 15:30 ` [PATCH v3 11/12] ARM: EXYNOS: add support device tree enabled board file for EXYNOS5 Kukjin Kim
2012-03-13 15:30 ` [PATCH v3 12/12] ARM: dts: add initial dts file for EXYNOS5250, SMDK5250 Kukjin Kim
2012-03-14  3:56   ` Olof Johansson
2012-03-14  6:43     ` Kukjin Kim
2012-03-13 16:22 ` [PATCH v3 00/12] ARM: EXYNOS: add support new EXYNOS5250 Arnd Bergmann
2012-03-14  3:57   ` Olof Johansson
2012-03-14  7:08     ` Kukjin Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F603BC8.3020900@samsung.com \
    --to=kgene.kim@samsung$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox