On Wed, Aug 11, 2010 at 3:39 PM, David Woodhouse <dwmw2@xxxxxxxxxxxxx> wrote: > On Wed, 2010-08-11 at 14:59 +0200, Corentin Chary wrote: >> I'd suggest to name it ideapad-laptop (IDEAPAD_LAPTOP). We try to use >> that name for laptop related drivers. > > OK. > >> But ... new WMI drivers are named with the *-wmi suffix. So I'm not >> really sure. Anyway, it's not really important. > > It doesn't do any WMI stuff... yet. I was looking at your discussion > from April and hoping you'd help make it work... I'd suggest you to read http://lwn.net/Articles/391230/ :) >> What's DEV_KILLSW ? a global kill switch ? > > Yes, that's a physical switch on the side of the laptop which > automatically kills all of wifi, bluetooth and 3g. > >> > +static struct rfkill *ideapad_rfkill[5]; >> > + >> > +static const char *ideapad_rfk_names[] = { >> > + "ideapad_camera", "ideapad_wlan", "ideapad_bluetooth", "ideapad_3g", "ideapad_rfkill" >> > +}; >> >> Hum, ideapad_camera inside rfkill stuff ? > > Element zero of the array doesn't get used. It was that or add a bunch > of '-1' in various places. I am entirely unconvinced which I like least. > It's the same for the types: > >> > +static const int ideapad_rfk_types[] = { >> > + 0, RFKILL_TYPE_WLAN, RFKILL_TYPE_BLUETOOTH, RFKILL_TYPE_WWAN, RFKILL_TYPE_WLAN >> > +}; > ... >> > +static ssize_t store_ideapad_cam(struct device *dev, >> > + struct device_attribute *attr, >> > + const char *buf, size_t count) >> > +{ >> > + int ret, state; >> > + >> > + if (!count) >> > + return 0; >> > + if (sscanf(buf, "%i", &state) != 1) >> > + return -EINVAL; >> > + ret = ideapad_dev_set_state(IDEAPAD_DEV_CAMERA, state); >> >> What does it do with values other than 1 and 0 ? maybe you could send >> !!state instead > > I could do. The ACPI code does so for itself anyway: > > If (Arg1) > { > Store (0x01, Local1) > } > Else > { > Store (0x00, Local1) > } > >> Also, it would be great not to use the "there will only be one of >> these device" anti-pattern. >> Of course, it will work, but I think it's better to get a clean, >> fully-reentrant driver. > > Agreed. Will fix. Alan did a great work on that for eeepc-laptop, and the new eeepc-wmi should be clean. You can take a look a these. And you should probably make it a platform driver (being the parent of acpi, wmi, input, backlight, rfkill, etc..). Is there a lot of thing hidden in ACPI / WMI on these models (like hotkeys, backlight, light sensors) ? Thanks, -- Corentin Chary http://xf.iksaif.net -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
