diff for duplicates of <4C202D8F.7030005@samsung.com> diff --git a/a/1.txt b/N1/1.txt index f156743..b279cf2 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -8,42 +8,42 @@ On 6/22/2010 12:02 PM, 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; +>>>>>>> + ? ? ? 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?) >>>>> @@ -76,7 +76,3 @@ current fact. if matrix_keypad_data is changed, i think the patchset should included change of related other parts using it. --- -To unsubscribe from this list: send the line "unsubscribe linux-input" in -the body of a message to majordomo@vger.kernel.org -More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/a/content_digest b/N1/content_digest index 5f31f73..3c3cc04 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -5,18 +5,10 @@ "ref\020100621111612.GJ7702@n2100.arm.linux.org.uk\0" "ref\04C200847.9010200@samsung.com\0" "ref\0AANLkTikbBkDOVF6ti_bTultATuW1KhciAmKc1Geg1OSH@mail.gmail.com\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 12:27:11 +0900\0" - "To\0Eric Miao <eric.y.miao@gmail.com>\0" - "Cc\0Russell King - ARM Linux <linux@arm.linux.org.uk>" - 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/22/2010 12:02 PM, Eric Miao wrote:\n" @@ -29,42 +21,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" @@ -96,10 +88,6 @@ "> \n" "\n" "if matrix_keypad_data is changed, i think the patchset should included\n" - "change of related other parts using it.\n" - "--\n" - "To unsubscribe from this list: send the line \"unsubscribe linux-input\" in\n" - "the body of a message to majordomo@vger.kernel.org\n" - More majordomo info at http://vger.kernel.org/majordomo-info.html + change of related other parts using it. -a67c1abba5763cdbd311af3dab18c0fc04c28dce4dcdcc71690bd25a17c0d2ab +7eb167335f1a383543bdb687d6756d23de1cad623dc00f402f9a7b03e5d41dde
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.