Re: [PATCH] Lenovo IdeaPAD ACPI driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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...

> 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.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@xxxxxxxxx                              Intel Corporation

--
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


[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux