* [PATCH] OMAP2+: ads7846_init: put gpio_pendown into pdata if it's provided
@ 2011-12-21 16:31 Ilya Yanok
2011-12-21 17:22 ` Igor Grinberg
0 siblings, 1 reply; 4+ messages in thread
From: Ilya Yanok @ 2011-12-21 16:31 UTC (permalink / raw)
To: linux-omap; +Cc: sasha_d, Ilya Yanok
If platform data is provided by the caller gpio_pendown is put into
unused static ads7846_config structure and effectively has no effect.
Of course caller can set gpio_pendown field in platform data himself
but it seems natural to do this in ads7846_init to remove duplication.
Signed-off-by: Ilya Yanok <yanok@emcraft.com>
---
arch/arm/mach-omap2/common-board-devices.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c
index 2d1d775..eb408dd 100644
--- a/arch/arm/mach-omap2/common-board-devices.c
+++ b/arch/arm/mach-omap2/common-board-devices.c
@@ -75,7 +75,10 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce,
gpio_set_debounce(gpio_pendown, gpio_debounce);
}
- ads7846_config.gpio_pendown = gpio_pendown;
+ if (!board_pdata)
+ ads7846_config.gpio_pendown = gpio_pendown;
+ else
+ board_pdata->gpio_pendown = gpio_pendown;
spi_bi->bus_num = bus_num;
spi_bi->irq = OMAP_GPIO_IRQ(gpio_pendown);
--
1.7.6.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] OMAP2+: ads7846_init: put gpio_pendown into pdata if it's provided
2011-12-21 16:31 [PATCH] OMAP2+: ads7846_init: put gpio_pendown into pdata if it's provided Ilya Yanok
@ 2011-12-21 17:22 ` Igor Grinberg
2011-12-21 18:06 ` Ilya Yanok
0 siblings, 1 reply; 4+ messages in thread
From: Igor Grinberg @ 2011-12-21 17:22 UTC (permalink / raw)
To: Ilya Yanok; +Cc: linux-omap, sasha_d
[-- Attachment #1: Type: text/plain, Size: 1506 bytes --]
Hi Ilya,
Please, Cc the linux-arm-kernel@lists.infradead.org for patches,
so Tony, or whoever will not need to resend them...
On 12/21/11 18:31, Ilya Yanok wrote:
> If platform data is provided by the caller gpio_pendown is put into
> unused static ads7846_config structure and effectively has no effect.
> Of course caller can set gpio_pendown field in platform data himself
> but it seems natural to do this in ads7846_init to remove duplication.
>
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> ---
> arch/arm/mach-omap2/common-board-devices.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c
> index 2d1d775..eb408dd 100644
> --- a/arch/arm/mach-omap2/common-board-devices.c
> +++ b/arch/arm/mach-omap2/common-board-devices.c
> @@ -75,7 +75,10 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce,
> gpio_set_debounce(gpio_pendown, gpio_debounce);
> }
>
> - ads7846_config.gpio_pendown = gpio_pendown;
> + if (!board_pdata)
> + ads7846_config.gpio_pendown = gpio_pendown;
> + else
> + board_pdata->gpio_pendown = gpio_pendown;
>
> spi_bi->bus_num = bus_num;
> spi_bi->irq = OMAP_GPIO_IRQ(gpio_pendown);
The fact that the ads7846_config has no effect in case
the board_pdata is provided does not really meter...
How about reusing the existing "if" instead of adding another one?
Like in the attached patch?
--
Regards,
Igor.
[-- Attachment #2: touch.patch --]
[-- Type: text/x-patch, Size: 595 bytes --]
diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c
index 94ccf46..e8a7368 100644
--- a/arch/arm/mach-omap2/common-board-devices.c
+++ b/arch/arm/mach-omap2/common-board-devices.c
@@ -102,8 +102,10 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce,
spi_bi->bus_num = bus_num;
spi_bi->irq = OMAP_GPIO_IRQ(gpio_pendown);
- if (board_pdata)
+ if (board_pdata) {
+ board_pdata->gpio_pendown = gpio_pendown;
spi_bi->platform_data = board_pdata;
+ }
spi_register_board_info(&ads7846_spi_board_info, 1);
}
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] OMAP2+: ads7846_init: put gpio_pendown into pdata if it's provided
2011-12-21 17:22 ` Igor Grinberg
@ 2011-12-21 18:06 ` Ilya Yanok
2011-12-22 8:49 ` Igor Grinberg
0 siblings, 1 reply; 4+ messages in thread
From: Ilya Yanok @ 2011-12-21 18:06 UTC (permalink / raw)
To: Igor Grinberg; +Cc: linux-omap, sasha_d
Hi Igor,
On 21.12.2011 21:22, Igor Grinberg wrote:
> Please, Cc the linux-arm-kernel@lists.infradead.org for patches,
> so Tony, or whoever will not need to resend them...
Uh.. Actually I thought that linux-omap ML is a good place for really
OMAP-specific patches like this one and there is no much sense posting
such patches into the main ARM list...
Ok, I will Cc linux-arm-kernel in future.
>> If platform data is provided by the caller gpio_pendown is put into
>> unused static ads7846_config structure and effectively has no effect.
>> Of course caller can set gpio_pendown field in platform data himself
>> but it seems natural to do this in ads7846_init to remove duplication.
>>
>> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
>> ---
>> arch/arm/mach-omap2/common-board-devices.c | 5 ++++-
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c
>> index 2d1d775..eb408dd 100644
>> --- a/arch/arm/mach-omap2/common-board-devices.c
>> +++ b/arch/arm/mach-omap2/common-board-devices.c
>> @@ -75,7 +75,10 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce,
>> gpio_set_debounce(gpio_pendown, gpio_debounce);
>> }
>>
>> - ads7846_config.gpio_pendown = gpio_pendown;
>> + if (!board_pdata)
>> + ads7846_config.gpio_pendown = gpio_pendown;
>> + else
>> + board_pdata->gpio_pendown = gpio_pendown;
>>
>> spi_bi->bus_num = bus_num;
>> spi_bi->irq = OMAP_GPIO_IRQ(gpio_pendown);
>
> The fact that the ads7846_config has no effect in case
> the board_pdata is provided does not really meter...
> How about reusing the existing "if" instead of adding another one?
> Like in the attached patch?
Yes, I think your version is a bit clearer. Probably it makes to add
"else" clause and move ads7846_config.gpio_pendown assignment under it.
Regards, Ilya.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] OMAP2+: ads7846_init: put gpio_pendown into pdata if it's provided
2011-12-21 18:06 ` Ilya Yanok
@ 2011-12-22 8:49 ` Igor Grinberg
0 siblings, 0 replies; 4+ messages in thread
From: Igor Grinberg @ 2011-12-22 8:49 UTC (permalink / raw)
To: Ilya Yanok; +Cc: linux-omap, sasha_d
On 12/21/11 20:06, Ilya Yanok wrote:
> Hi Igor,
>
> On 21.12.2011 21:22, Igor Grinberg wrote:
>> Please, Cc the linux-arm-kernel@lists.infradead.org for patches,
>> so Tony, or whoever will not need to resend them...
>
> Uh.. Actually I thought that linux-omap ML is a good place for really
> OMAP-specific patches like this one and there is no much sense posting
> such patches into the main ARM list...
>
> Ok, I will Cc linux-arm-kernel in future.
>
>>> If platform data is provided by the caller gpio_pendown is put into
>>> unused static ads7846_config structure and effectively has no effect.
>>> Of course caller can set gpio_pendown field in platform data himself
>>> but it seems natural to do this in ads7846_init to remove duplication.
>>>
>>> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
>>> ---
>>> arch/arm/mach-omap2/common-board-devices.c | 5 ++++-
>>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c
>>> index 2d1d775..eb408dd 100644
>>> --- a/arch/arm/mach-omap2/common-board-devices.c
>>> +++ b/arch/arm/mach-omap2/common-board-devices.c
>>> @@ -75,7 +75,10 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce,
>>> gpio_set_debounce(gpio_pendown, gpio_debounce);
>>> }
>>>
>>> - ads7846_config.gpio_pendown = gpio_pendown;
>>> + if (!board_pdata)
>>> + ads7846_config.gpio_pendown = gpio_pendown;
>>> + else
>>> + board_pdata->gpio_pendown = gpio_pendown;
>>>
>>> spi_bi->bus_num = bus_num;
>>> spi_bi->irq = OMAP_GPIO_IRQ(gpio_pendown);
>>
>> The fact that the ads7846_config has no effect in case
>> the board_pdata is provided does not really meter...
>> How about reusing the existing "if" instead of adding another one?
>> Like in the attached patch?
>
> Yes, I think your version is a bit clearer. Probably it makes to add
> "else" clause and move ads7846_config.gpio_pendown assignment under it.
Sounds fine, care to send a v2?
You can add my SOB.
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-12-22 8:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-21 16:31 [PATCH] OMAP2+: ads7846_init: put gpio_pendown into pdata if it's provided Ilya Yanok
2011-12-21 17:22 ` Igor Grinberg
2011-12-21 18:06 ` Ilya Yanok
2011-12-22 8:49 ` Igor Grinberg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.