Re: [PATCH 4/6] msi-laptop: Add MSI Wind U90/U100 support

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

 



2012/11/29 joeyli <jlee@xxxxxxxx>:
> 於 三,2012-11-28 於 19:41 +0200,Maxim Mikityanskiy 提到:
>> 2012/11/28 joeyli <jlee@xxxxxxxx>:
>> > 於 日,2012-11-25 於 00:28 +0200,Maxim Mikityanskiy 提到:
>> >> Add MSI Wind U90/U100 to separate DMI table, add U90/U100 specific
>> >> workarounds and add some missing EC features support such as basic fan
>> >> control, turbo and ECO modes and touchpad state.
>> >>
>> >> Signed-off-by: Maxim Mikityanskiy <maxtram95@xxxxxxxxx>
>> >> ---
>> >>  drivers/platform/x86/msi-laptop.c | 199 ++++++++++++++++++++++++++++++++------
>> >>  1 file changed, 171 insertions(+), 28 deletions(-)
>> >>
>> >> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
>> >> index 3b6f494..16e9863 100644
>> >> --- a/drivers/platform/x86/msi-laptop.c
>> >> +++ b/drivers/platform/x86/msi-laptop.c
>> >> @@ -82,8 +82,19 @@
>> >>  #define MSI_STANDARD_EC_SCM_LOAD_ADDRESS     0x2d
>> >>  #define MSI_STANDARD_EC_SCM_LOAD_MASK                (1 << 0)
>> >>
>> >> -#define MSI_STANDARD_EC_TOUCHPAD_ADDRESS     0xe4
>> >> +#define MSI_STANDARD_EC_POWER_ADDRESS                0xe4
>> >
>> > If 0xE4 register is not just for touchpad and power, then I suggest use
>> > a more general name like: MSI_STANDARD_EC_FUNCTIONS_ADDRESS or other.
>>
>> OK, I will rename it.
>
> thanks
>
>> >> +static ssize_t show_touchpad(struct device *dev,
>> >> +     struct device_attribute *attr, char *buf)
>> >> +{
>> >> +
>> >> +     u8 rdata;
>> >> +     int result;
>> >> +
>> >> +     result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
>> >> +     if (result < 0)
>> >> +             return result;
>> >> +
>> >> +     return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_TOUCHPAD_MASK));
>> >> +}
>> >
>> > I don't think we need create a touchpad attribute interface because
>> > there already have key code raise the change to user space.
>>
>> Key codes only indicate touchpad state change. We cannot determine
>> initial touchpad state on boot using only key codes. I think, it might
>> be useful to have a file in sysfs that always keeps actual touchpad
>> state, so that we can get initial touchpad state on boot. Do you
>> disagree with that?
>>
>
> I agree this reason for add a sysfs interface for raise touchpad status
> to userland.
> But this interface will be a special api and I am not sure there already
> have standard way in input subsystem for raise touchpad status. If there
> have no standard api to raise it, I think add it is make sense.

AFAIK, there is no generic interface that exposes touchpad state. As
for now, the only way is to create such special platform-specific file
in sysfs. asus-wmi uses the same approach.

>> >> +
>> >> +static ssize_t show_turbo(struct device *dev,
>> >> +     struct device_attribute *attr, char *buf)
>> >> +{
> ...
>> >> -     if (!old_ec_model)
>> >> +     if (!old_ec_model) {
>> >>               get_threeg_exists();
>> >> +             if (dmi_check_system(msi_load_scm_models_dmi_table))
>> >> +                     load_scm_model = 1;
>> >> +             else if (dmi_check_system(msi_load_scm_ro_models_dmi_table)) {
>> >> +                     load_scm_model = 1;
>> >> +                     ec_read_only = 1;
>> >> +             }
>> >> +     }
>> >>
>> >>       if (!old_ec_model && dmi_check_system(msi_load_scm_models_dmi_table))
>> >>               load_scm_model = 1;
>> >
>> > hmm... the load_scm_model dmi table check 2 times? I think you can just
>> > remove the duplicate code.
>>
>> Oops, I forgot to remove the last two lines when I were preparing patches.
>>
>> Do I need to resend all patches after doing all needed fixes?
>
> Yes, please modify your patches for all changed and I suggest re-send
> them by add v2 to subject, like:
>         [PATCH 0/6 v2] msi-laptop: Add MSI Wind U90/U100 support
>         ...
>         [PATCH 4/6 v2] msi-laptop: Add MSI Wind U90/U100 support
>
> Please write down what's the v2 change for each patch on description of
> patch.

OK, I'll do that when v2 will be ready.

>
> Thanks a lot!
> Joey Lee
>
>
--
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