On Mon, Feb 20, 2012 at 03:17:39PM +0900, MyungJoo Ham wrote: > On Mon, Feb 20, 2012 at 10:54 AM, Mark Brown > >> +             prop_buf = (char *)get_zeroed_page(GFP_KERNEL); > >> +             if (prop_buf) { > > Is the cast really needed here? > Unless we have that cast, we get: > drivers/extcon/extcon_class.c:89:12: warning: assignment makes pointer > from integer without a cast Oh, dear. That's a very sad API but it looks like your code is OK. > >> +             extcon_class->dev_attrs = extcon_attrs; > > I thought we were trying to remove classes, though I'm not sure if we're > > actually at the point where that's happening yet?  Greg? > Hmm.. I remember I was recommended to use classes some time ago (just > a few months ago) especially for adding sysfs entries. Things have > been changed already? There's class like buses but they're not actually classes - I'm not sure if the in kernel API has changed or not, though. Like I say Greg's the expert. > >> +static int create_extcon_class_for_android(void) > >> +{ > >> +     if (!extcon_class_for_android) { > >> +             extcon_class_for_android = class_create(THIS_MODULE, "switch"); > >> +             if (IS_ERR(extcon_class_for_android)) > >> +                     return PTR_ERR(extcon_class_for_android); > >> +             extcon_class_for_android->dev_attrs = extcon_attrs; > >> +     } > >> +     return 0; > >> +} > > Might be better to put this as a separate Kconfig option or just leave > > it as an out of tree patch (given how trivial it is).  We're going to > > end up renaming a bunch of the classes anyway I expect... > Then, would it be proper to put "for-android" features surrounded by > #ifdef CONFIG_ANDROID ? That's an idea. Though I'm pretty sure that for a simple change like this the Android userspace could just adapt and we will be changing some of the names of the directory entries I expect (I can't imagine we're going to keep h2w for example) so the rename would likely be useful to them.