Re: [PATCH v5 1/5] Extcon (external connector): import Android's switch class and modify.

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

[-- Attachment #1: Type: text/plain, Size: 2058 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]