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
