於 三，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.
> >> +
> >> +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.
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
