Hi Darren, 2015-08-05 3:38 GMT-06:00 Darren Hart <dvhart@xxxxxxxxxxxxx>: > On Fri, Jul 31, 2015 at 09:58:13PM -0600, Azael Avalos wrote: >> Currently the driver prints "*not supported" if any of the features >> queried are in fact not supported, let us print the available >> features instead. >> >> This patch removes all instances pr_info printing "*not supported", >> and add a new function called "print_supported_features", which will >> print the available laptop features. >> >> Signed-off-by: Azael Avalos <coproscefalo@xxxxxxxxx> >> --- >> drivers/platform/x86/toshiba_acpi.c | 72 +++++++++++++++++++++++-------------- >> 1 file changed, 46 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c >> index d983dc4..66b596a 100644 >> --- a/drivers/platform/x86/toshiba_acpi.c >> +++ b/drivers/platform/x86/toshiba_acpi.c >> @@ -459,7 +459,7 @@ static void toshiba_illumination_available(struct toshiba_acpi_dev *dev) >> if (ACPI_FAILURE(status)) >> pr_err("ACPI call to query Illumination support failed\n"); >> else if (out[0] == TOS_NOT_SUPPORTED) >> - pr_info("Illumination device not available\n"); >> + return; >> else if (out[0] == TOS_SUCCESS) >> dev->illumination_supported = 1; >> } >> @@ -483,7 +483,6 @@ static void toshiba_illumination_set(struct led_classdev *cdev, >> pr_err("ACPI call for illumination failed\n"); >> return; >> } else if (result == TOS_NOT_SUPPORTED) { >> - pr_info("Illumination not supported\n"); >> return; >> } > > I mentioned this in the previous review. For several of these, we have an if > statement that checks for a condition, and then returns, which is exactly what > would happen if we didn't have the if statement at all. > > If the context is important, a comment should be sufficient. Is there a > compelling reason to add the redundant check? The "offending" lines are removed by patch 04, that's why I didn't included a comment or removed the lines on this patch, as I was trying to "abstract" what each patch do, which in this patch, only removes the pr_info. > > -- > Darren Hart > Intel Open Source Technology Center Cheers Azael -- -- 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