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
