Messages in this thread | | | Date | Wed, 30 Aug 2023 14:28:18 +1200 | From | Luke Jones <> | Subject | Re: [PATCH] platform/x86: asus-wmi: corrections to egpu safety check |
| |
On Mon, Aug 28 2023 at 11:58:51 AM +02:00:00, Hans de Goede <hdegoede@redhat.com> wrote: > Hi Luke, > > On 8/28/23 01:15, Luke D. Jones wrote: >> An incorrect if statement was preventing the enablement of the egpu. >> >> Fixes: 1bddf53ccac0 ("platform/x86: asus-wmi: don't allow eGPU >> switching if eGPU not connected") >> Signed-off-by: Luke D. Jones <luke@ljones.dev> > > Not sure which tree/branch this is based on but the commit-id for > your Fixes: > is not correct. > > in my tree this is commit d49f4d1a30ac (platform/x86: asus-wmi: don't > allow > eGPU switching if eGPU not connected") . > > > >> --- >> drivers/platform/x86/asus-wmi.c | 15 +++++++-------- >> 1 file changed, 7 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/platform/x86/asus-wmi.c >> b/drivers/platform/x86/asus-wmi.c >> index 6db2d18a61e0..b51586986450 100644 >> --- a/drivers/platform/x86/asus-wmi.c >> +++ b/drivers/platform/x86/asus-wmi.c >> @@ -731,19 +731,18 @@ static ssize_t egpu_enable_store(struct >> device *dev, >> return -EINVAL; >> >> result = asus_wmi_get_devstate_simple(asus, >> ASUS_WMI_DEVID_EGPU_CONNECTED); > > And this does not apply, because the: > > result = asus_wmi_get_devstate_simple() > > you have here has been fixed in my tree to be > > err = asus_wmi_get_devstate_simple() >
Fixed.
> to match the error checks below which actually check "err" not > "result" > I guess that a fix for this being squashed into my tree is also what > has resulted in the different commit ids. > > Please rebase on top of: > > https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=for-next > > and submit a new version. > > And please also Cc: platform-driver-x86@vger.kernel.org for the next > version.
Thank you for the review Hans.
> >> - if (err < 0) >> - return err; >> - if (err < 1) { >> - err = -ENODEV; >> - pr_warn("Failed to set egpu disable: %d\n", err); >> + if (err < 0) { >> + pr_warn("Failed to get egpu connection status: %d\n", err); >> return err; >> } >> >> if (asus->gpu_mux_mode_available) { >> result = asus_wmi_get_devstate_simple(asus, >> ASUS_WMI_DEVID_GPU_MUX); >> - if (result < 0) >> + if (result < 0) { >> /* An error here may signal greater failure of GPU handling */ >> + pr_warn("Failed to get gpu mux status: %d\n", err); >> return result; >> + } >> if (!result && enable) { >> err = -ENODEV; >> pr_warn("Can not enable eGPU when the MUX is in dGPU mode: >> %d\n", err); >> @@ -753,12 +752,12 @@ static ssize_t egpu_enable_store(struct >> device *dev, >> >> err = asus_wmi_set_devstate(ASUS_WMI_DEVID_EGPU, enable, &result); >> if (err) { >> - pr_warn("Failed to set egpu disable: %d\n", err); >> + pr_warn("Failed to set egpu state: %d\n", err); >> return err; >> } >> >> if (result > 1) { >> - pr_warn("Failed to set egpu disable (retval): 0x%x\n", result); >> + pr_warn("Failed to set egpu state (retval): 0x%x\n", result); >> return -EIO; >> } >> >
| |