Re: [RFC] [PATCH v3 2/4] OMAP4: Keyboard device registration
* Dmitry Torokhov dmitry.torok...@gmail.com [100601 23:07]: On Tue, Jun 01, 2010 at 09:53:58PM +0300, Felipe Balbi wrote: On Tue, Jun 01, 2010 at 05:14:09PM +0200, ext Arce, Abraham wrote: I am using #ifdef CONFIG_ARCH_OMAP4 for this portion of code, what you are suggesting is to check at runtime? you need to add both checks. If you build omap3-only or omap2-only you don't want that code to be compiled, but if you build omap1-2-3-4 kernel, you want it to work correctly on all cases. It sould be nice if cpu_is_xxx were stubbed out for wrong arches withing the same group (like omap, etc) so we could reduce the #ifdef clutter. They are defined as stubs for for non-selected omaps. So in general a function doing this in the beginning of the function: if (!cpu_is_omap44xx()) return -ENODEV; Should already get optimized out if 44xx is not selected in Kconfig. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH v3 2/4] OMAP4: Keyboard device registration
On Mon, 31 May 2010 16:44:52 -0500 Arce, Abraham x0066...@ti.com wrote: + unsigned int length = 0, id = 0; + int hw_mod_name_len = 16; + char oh_name[hw_mod_name_len]; + char *name = omap4-keypad; + + length = snprintf(oh_name, hw_mod_name_len, kbd); + + oh = omap_hwmod_lookup(oh_name); + if (!oh) { + pr_err(Could not look up %s\n, oh_name); + return -EIO; + } Maybe I'm missing something here, but I don't see where length is being used, and why the snprintf()/oh_name thing is needed. What about: unsigned int id = 0; char *name = omap4-keypad; oh = omap_hwmod_lookup(kbd); if (!oh) { pr_err(Could not look up kbd\n); return -EIO; } Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC] [PATCH v3 2/4] OMAP4: Keyboard device registration
Thanks Thomas, + unsigned int length = 0, id = 0; + int hw_mod_name_len = 16; + char oh_name[hw_mod_name_len]; + char *name = omap4-keypad; + + length = snprintf(oh_name, hw_mod_name_len, kbd); + + oh = omap_hwmod_lookup(oh_name); + if (!oh) { + pr_err(Could not look up %s\n, oh_name); + return -EIO; + } Maybe I'm missing something here, but I don't see where length is being used, and why the snprintf()/oh_name thing is needed. What about: unsigned int id = 0; char *name = omap4-keypad; oh = omap_hwmod_lookup(kbd); if (!oh) { pr_err(Could not look up kbd\n); return -EIO; } I'll remove length variable and keep snprintf, below oh_name - kbd is used again, this will keep name defined in one single place WARN(IS_ERR(od), Could not build omap_device for %s %s\n, name, oh_name); Best Regards Abraham -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH v3 2/4] OMAP4: Keyboard device registration
On Wed, 2 Jun 2010 07:45:07 -0500 Arce, Abraham x0066...@ti.com wrote: I'll remove length variable and keep snprintf, below oh_name - kbd is used again, this will keep name defined in one single place WARN(IS_ERR(od), Could not build omap_device for %s %s\n, name, oh_name); In this case, why not: char *oh_name = kbd; There's really no point in using snprintf() for statically-defined strings. Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH v3 2/4] OMAP4: Keyboard device registration
* Arce, Abraham x0066...@ti.com [100601 00:39]: Register keyboard device with hwmod framework Please test this with omap3_defconfig too, and make sure it does not break things for omap2 and omap3. +int omap4_init_kp(struct omap4_keypad_platform_data *kp) +{ + struct omap_hwmod *oh; + struct omap_device *od; + struct omap4_keypad_platform_data *pdata; + + unsigned int length = 0, id = 0; + int hw_mod_name_len = 16; + char oh_name[hw_mod_name_len]; + char *name = omap4-keypad; if (!cpu_is_omap44xx()) return -ENODEV; Might be needed here. Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC] [PATCH v3 2/4] OMAP4: Keyboard device registration
Hi Tony, Please test this with omap3_defconfig too, and make sure it does not break things for omap2 and omap3. Tested, no problems... +int omap4_init_kp(struct omap4_keypad_platform_data *kp) +{ + struct omap_hwmod *oh; + struct omap_device *od; + struct omap4_keypad_platform_data *pdata; + + unsigned int length = 0, id = 0; + int hw_mod_name_len = 16; + char oh_name[hw_mod_name_len]; + char *name = omap4-keypad; if (!cpu_is_omap44xx()) return -ENODEV; I am using #ifdef CONFIG_ARCH_OMAP4 for this portion of code, what you are suggesting is to check at runtime? Best Regards Abraham -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH v3 2/4] OMAP4: Keyboard device registration
Felipe Balbi felipe.ba...@nokia.com writes: On Tue, Jun 01, 2010 at 06:47:34AM +0200, Balbi Felipe (Nokia-D/Helsinki) wrote: +pdata-device_enable = omap_device_enable; this is undefined at least here or previous patch. or does it come from omap_device layers ?? It's part of the omap_device layer. But, drivers no longer should be calling these hooks using platform_data. Instead, drivers should use the runtime PM API. The runtime PM core for OMAP will use the omap_device API. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH v3 2/4] OMAP4: Keyboard device registration
On Tue, Jun 01, 2010 at 05:14:09PM +0200, ext Arce, Abraham wrote: I am using #ifdef CONFIG_ARCH_OMAP4 for this portion of code, what you are suggesting is to check at runtime? you need to add both checks. If you build omap3-only or omap2-only you don't want that code to be compiled, but if you build omap1-2-3-4 kernel, you want it to work correctly on all cases. -- balbi DefectiveByDesign.org -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH v3 2/4] OMAP4: Keyboard device registration
On Tue, Jun 01, 2010 at 09:53:58PM +0300, Felipe Balbi wrote: On Tue, Jun 01, 2010 at 05:14:09PM +0200, ext Arce, Abraham wrote: I am using #ifdef CONFIG_ARCH_OMAP4 for this portion of code, what you are suggesting is to check at runtime? you need to add both checks. If you build omap3-only or omap2-only you don't want that code to be compiled, but if you build omap1-2-3-4 kernel, you want it to work correctly on all cases. It sould be nice if cpu_is_xxx were stubbed out for wrong arches withing the same group (like omap, etc) so we could reduce the #ifdef clutter. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH v3 2/4] OMAP4: Keyboard device registration
On Mon, May 31, 2010 at 11:44:52PM +0200, ext Arce, Abraham wrote: + pdata = kzalloc(sizeof(struct omap4_keypad_platform_data), GFP_KERNEL); you will never free this. [..] + pdata-device_enable = omap_device_enable; this is undefined at least here or previous patch. + pdata-device_idle = omap_device_idle; same for this + pdata-device_shutdown = omap_device_shutdown; and this. -- balbi DefectiveByDesign.org -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH v3 2/4] OMAP4: Keyboard device registration
On Tue, Jun 01, 2010 at 06:47:34AM +0200, Balbi Felipe (Nokia-D/Helsinki) wrote: + pdata-device_enable = omap_device_enable; this is undefined at least here or previous patch. or does it come from omap_device layers ?? -- balbi DefectiveByDesign.org -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html