From: jy0922.shim@samsung•com (Joonyoung Shim)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v5 1/3] ARM: SAMSUNG: Add keypad device support
Date: Tue, 22 Jun 2010 09:48:07 +0900 [thread overview]
Message-ID: <4C200847.9010200@samsung.com> (raw)
In-Reply-To: <20100621111612.GJ7702@n2100.arm.linux.org.uk>
On 6/21/2010 8:16 PM, Russell King - ARM Linux wrote:
> On Mon, Jun 21, 2010 at 06:39:10PM +0800, Eric Miao wrote:
>> On Mon, Jun 21, 2010 at 5:19 PM, Russell King - ARM Linux
>> <linux@arm•linux.org.uk> wrote:
>>> On Mon, Jun 21, 2010 at 05:05:34PM +0800, Eric Miao wrote:
>>>> On Mon, Jun 21, 2010 at 2:26 PM, Joonyoung Shim <jy0922.shim@samsung•com> wrote:
>>>>> +void __init samsung_keypad_set_platdata(struct samsung_keypad_platdata *pd)
>>>>> +{
>>>>> + ? ? ? struct samsung_keypad_platdata *npd;
>>>>> +
>>>>> + ? ? ? if (!pd) {
>>>>> + ? ? ? ? ? ? ? printk(KERN_ERR "%s: no platform data\n", __func__);
>>>>> + ? ? ? ? ? ? ? return;
>>>>> + ? ? ? }
>>>>> +
>>>>> + ? ? ? npd = kmemdup(pd, sizeof(struct samsung_keypad_platdata), GFP_KERNEL);
>>>>> + ? ? ? if (!npd)
>>>>> + ? ? ? ? ? ? ? printk(KERN_ERR "%s: no memory for platform data\n", __func__);
>>>> This part of the code is actually duplicated again and again and again
>>>> for each device, PXA and other legacy platforms are bad references for
>>>> this. In arch/arm/mach-mmp/, it might be a bit cleaner, there are three
>>>> major points:
>>>>
>>>> ?1. A minimum 'struct pxa_device_desc' for a simple description of a
>>>> ? ? device (more than 90% of the devices can be described that way),
>>>> ? ? and avoid using a comparatively heavier weight platform_device,
>>>> ? ? which can be generated at run-time
>>>>
>>>> ?2. pxa_register_device() to allocate and register the platform_device
>>>> ? ? at run-time, along with the platform data
>>> It's a bad idea to make platform data be run-time discardable like this:
>>>
>>>>> +struct samsung_keypad_platdata {
>>>>> + ? ? ? const struct matrix_keymap_data *keymap_data;
>>> What you end up with is some platform data structures which must be kept
>>> (those which have pointers to them from the platform data), and others
>>> (the platform data itself) which can be discarded at runtime.
>>>
>>> We know that the __initdata attributations cause lots of problems -
>>> they're frequently wrong. ?Just see the constant hastle with __devinit
>>> et.al. ?The same issue happens with __initdata as well.
>>>
>>> So why make things more complicated by allowing some platform data
>>> structures to be discardable and others not to be? ?Is their small
>>> size (maybe 6 words for this one) really worth the hastle of getting
>>> __initdata attributations wrong (eg, on the keymap data?)
>>>
>> Russell,
>>
>> The benefit I see is when multiple boards are compiled in, those
>> data not used can be automatically discarded.
>
> Yes, but only some of the data can be discarded. Continuing with the
> example in hand, while you can discard the six words which represent
> samsung_keypad_platdata, but the keymap_data can't be because that won't
> be re-allocated, which is probably a much larger data structure.
>
No. the keymap_data is possible too. The keypad driver allocates other
keymap area of input device and it is assigned from datas based on this
keymap_data.
next prev parent reply other threads:[~2010-06-22 0:48 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-06-21 6:26 [PATCH v5 1/3] ARM: SAMSUNG: Add keypad device support Joonyoung Shim
2010-06-21 6:26 ` [PATCH v5 2/3] ARM: S5PV210: Add keypad device helpers Joonyoung Shim
2010-06-21 6:26 ` [PATCH v5 3/3] input: samsung-keypad - Add samsung keypad driver Joonyoung Shim
2010-06-25 8:30 ` Dmitry Torokhov
2010-06-25 10:25 ` Joonyoung Shim
2010-06-25 10:39 ` Joonyoung Shim
2010-06-28 8:39 ` Joonyoung Shim
2010-06-28 9:01 ` Dmitry Torokhov
2010-06-21 9:05 ` [PATCH v5 1/3] ARM: SAMSUNG: Add keypad device support Eric Miao
2010-06-21 9:19 ` Russell King - ARM Linux
2010-06-21 10:39 ` Eric Miao
2010-06-21 11:16 ` Russell King - ARM Linux
2010-06-21 14:15 ` Eric Miao
2010-06-22 0:48 ` Joonyoung Shim [this message]
2010-06-22 3:02 ` Eric Miao
2010-06-22 3:27 ` Joonyoung Shim
2010-06-22 3:38 ` Eric Miao
2010-06-22 4:00 ` Joonyoung Shim
2010-06-22 7:15 ` Eric Miao
2010-06-22 7:33 ` Joonyoung Shim
2010-06-21 9:29 ` Marek Szyprowski
2010-06-21 10:43 ` Eric Miao
2010-06-21 11:21 ` Joonyoung Shim
2010-06-21 14:19 ` Eric Miao
2010-06-22 0:18 ` 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=4C200847.9010200@samsung.com \
--to=jy0922.shim@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