diff for duplicates of <4C200847.9010200@samsung.com> diff --git a/a/1.txt b/N1/1.txt index dc60677..5cdd42b 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -6,42 +6,42 @@ On 6/21/2010 8:16 PM, Russell King - ARM Linux 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; +>>>>> + ? ? ? struct samsung_keypad_platdata *npd; >>>>> + ->>>>> + � � � if (!pd) { ->>>>> + � � � � � � � printk(KERN_ERR "%s: no platform data\n", __func__); ->>>>> + � � � � � � � return; ->>>>> + � � � } +>>>>> + ? ? ? 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__); +>>>>> + ? ? ? 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 +>>>> ?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 +>>>> ?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; +>>>>> + ? ? ? 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. +>>> 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 +>>> 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?) >>> diff --git a/a/content_digest b/N1/content_digest index 9123b22..95cecae 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -3,18 +3,10 @@ "ref\020100621091952.GB7702@n2100.arm.linux.org.uk\0" "ref\0AANLkTilU1X1LR33Eed9BVGE2n-dqRuIuUFfbDd3gzEVc@mail.gmail.com\0" "ref\020100621111612.GJ7702@n2100.arm.linux.org.uk\0" - "From\0Joonyoung Shim <jy0922.shim@samsung.com>\0" - "Subject\0Re: [PATCH v5 1/3] ARM: SAMSUNG: Add keypad device support\0" + "From\0jy0922.shim@samsung.com (Joonyoung Shim)\0" + "Subject\0[PATCH v5 1/3] ARM: SAMSUNG: Add keypad device support\0" "Date\0Tue, 22 Jun 2010 09:48:07 +0900\0" - "To\0Russell King - ARM Linux <linux@arm.linux.org.uk>\0" - "Cc\0Eric Miao <eric.y.miao@gmail.com>" - linux-samsung-soc@vger.kernel.org - dmitry.torokhov@gmail.com - kyungmin.park@samsung.com - kgene.kim@samsung.com - ben-linux@fluff.org - linux-input@vger.kernel.org - " linux-arm-kernel@lists.infradead.org\0" + "To\0linux-arm-kernel@lists.infradead.org\0" "\00:1\0" "b\0" "On 6/21/2010 8:16 PM, Russell King - ARM Linux wrote:\n" @@ -25,42 +17,42 @@ ">>>> On Mon, Jun 21, 2010 at 2:26 PM, Joonyoung Shim <jy0922.shim@samsung.com> wrote:\n" ">>>>> +void __init samsung_keypad_set_platdata(struct samsung_keypad_platdata *pd)\n" ">>>>> +{\n" - ">>>>> + \357\277\275 \357\277\275 \357\277\275 struct samsung_keypad_platdata *npd;\n" + ">>>>> + ? ? ? struct samsung_keypad_platdata *npd;\n" ">>>>> +\n" - ">>>>> + \357\277\275 \357\277\275 \357\277\275 if (!pd) {\n" - ">>>>> + \357\277\275 \357\277\275 \357\277\275 \357\277\275 \357\277\275 \357\277\275 \357\277\275 printk(KERN_ERR \"%s: no platform data\\n\", __func__);\n" - ">>>>> + \357\277\275 \357\277\275 \357\277\275 \357\277\275 \357\277\275 \357\277\275 \357\277\275 return;\n" - ">>>>> + \357\277\275 \357\277\275 \357\277\275 }\n" + ">>>>> + ? ? ? if (!pd) {\n" + ">>>>> + ? ? ? ? ? ? ? printk(KERN_ERR \"%s: no platform data\\n\", __func__);\n" + ">>>>> + ? ? ? ? ? ? ? return;\n" + ">>>>> + ? ? ? }\n" ">>>>> +\n" - ">>>>> + \357\277\275 \357\277\275 \357\277\275 npd = kmemdup(pd, sizeof(struct samsung_keypad_platdata), GFP_KERNEL);\n" - ">>>>> + \357\277\275 \357\277\275 \357\277\275 if (!npd)\n" - ">>>>> + \357\277\275 \357\277\275 \357\277\275 \357\277\275 \357\277\275 \357\277\275 \357\277\275 printk(KERN_ERR \"%s: no memory for platform data\\n\", __func__);\n" + ">>>>> + ? ? ? npd = kmemdup(pd, sizeof(struct samsung_keypad_platdata), GFP_KERNEL);\n" + ">>>>> + ? ? ? if (!npd)\n" + ">>>>> + ? ? ? ? ? ? ? printk(KERN_ERR \"%s: no memory for platform data\\n\", __func__);\n" ">>>> This part of the code is actually duplicated again and again and again\n" ">>>> for each device, PXA and other legacy platforms are bad references for\n" ">>>> this. In arch/arm/mach-mmp/, it might be a bit cleaner, there are three\n" ">>>> major points:\n" ">>>>\n" - ">>>> \357\277\2751. A minimum 'struct pxa_device_desc' for a simple description of a\n" - ">>>> \357\277\275 \357\277\275 device (more than 90% of the devices can be described that way),\n" - ">>>> \357\277\275 \357\277\275 and avoid using a comparatively heavier weight platform_device,\n" - ">>>> \357\277\275 \357\277\275 which can be generated at run-time\n" + ">>>> ?1. A minimum 'struct pxa_device_desc' for a simple description of a\n" + ">>>> ? ? device (more than 90% of the devices can be described that way),\n" + ">>>> ? ? and avoid using a comparatively heavier weight platform_device,\n" + ">>>> ? ? which can be generated at run-time\n" ">>>>\n" - ">>>> \357\277\2752. pxa_register_device() to allocate and register the platform_device\n" - ">>>> \357\277\275 \357\277\275 at run-time, along with the platform data\n" + ">>>> ?2. pxa_register_device() to allocate and register the platform_device\n" + ">>>> ? ? at run-time, along with the platform data\n" ">>> It's a bad idea to make platform data be run-time discardable like this:\n" ">>>\n" ">>>>> +struct samsung_keypad_platdata {\n" - ">>>>> + \357\277\275 \357\277\275 \357\277\275 const struct matrix_keymap_data *keymap_data;\n" + ">>>>> + ? ? ? const struct matrix_keymap_data *keymap_data;\n" ">>> What you end up with is some platform data structures which must be kept\n" ">>> (those which have pointers to them from the platform data), and others\n" ">>> (the platform data itself) which can be discarded at runtime.\n" ">>>\n" ">>> We know that the __initdata attributations cause lots of problems -\n" - ">>> they're frequently wrong. \357\277\275Just see the constant hastle with __devinit\n" - ">>> et.al. \357\277\275The same issue happens with __initdata as well.\n" + ">>> they're frequently wrong. ?Just see the constant hastle with __devinit\n" + ">>> et.al. ?The same issue happens with __initdata as well.\n" ">>>\n" ">>> So why make things more complicated by allowing some platform data\n" - ">>> structures to be discardable and others not to be? \357\277\275Is their small\n" + ">>> structures to be discardable and others not to be? ?Is their small\n" ">>> size (maybe 6 words for this one) really worth the hastle of getting\n" ">>> __initdata attributations wrong (eg, on the keymap data?)\n" ">>>\n" @@ -79,4 +71,4 @@ "keymap area of input device and it is assigned from datas based on this\n" keymap_data. -1a1265d3850c4358568d2f0a4631f32b6ce7be206ef1ebe6ee65b995fb912c94 +b05cd1d1507b03c423a49a674946153cca9bb7748d90f017186bcc23529b0d67
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.