Messages in this thread | | | Date | Mon, 20 Feb 2012 07:45:48 -0800 | From | Mark Brown <> | Subject | Re: [PATCH v5 1/5] Extcon (external connector): import Android's switch class and modify. |
| |
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. [unhandled content-type:application/pgp-signature] | |