Hi Darren, 2016-08-28 10:56 GMT-06:00 Darren Hart <dvhart@xxxxxxxxxxxxx>: > On Tue, Aug 16, 2016 at 12:06:16PM -0600, Azael Avalos wrote: >> Currently the success/error checking logic is intermixed, making the >> code a bit cumbersome to understand. >> >> This patch changes the affected functions to first check for errors >> and take appropriate actions, then check for the supported features. >> >> This patch also separates the error check from the acpi_status and >> the tci_raw function call error check, as those two are completely >> unrelated and were nested in if/else statements. > > Thanks, this is a good improvement. One questions below... > >> >> Signed-off-by: Azael Avalos <coproscefalo@xxxxxxxxx> >> --- >> drivers/platform/x86/toshiba_acpi.c | 222 ++++++++++++++++++++++-------------- >> 1 file changed, 135 insertions(+), 87 deletions(-) >> >> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c >> index c6fc5cc..2256cf5 100644 >> --- a/drivers/platform/x86/toshiba_acpi.c >> +++ b/drivers/platform/x86/toshiba_acpi.c >> @@ -476,10 +476,15 @@ static void toshiba_illumination_available(struct toshiba_acpi_dev *dev) >> >> status = tci_raw(dev, in, out); >> sci_close(dev); >> - if (ACPI_FAILURE(status)) >> + if (ACPI_FAILURE(status)) { >> pr_err("ACPI call to query Illumination support failed\n"); >> - else if (out[0] == TOS_SUCCESS) >> - dev->illumination_supported = 1; >> + return; >> + } >> + >> + if (out[0] != TOS_SUCCESS) > > Does this condition not merit a pr_err message? It reads like an error... > > There are several similar situations below which are equally silent. Is this a > deliberate decision? This was on purpose, since we are querying for all the supported features, the kernel log will contain a lot of error strings for not supported features, as not all laptop models support all of them. > > -- > Darren Hart > Intel Open Source Technology Center > -- > 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 -- -- El mundo apesta y vosotros apestais tambien -- -- 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