Re: [PATCH v2 1/4] firmware: dmi_scan: Introduce the dmi_get_bios_year() helper

[Lukas Wunner <lukas@wunner.de>]

On Thu, Mar 01, 2018 at 08:02:17PM +0200, Andy Shevchenko wrote:
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -1015,6 +1015,17 @@ bool dmi_get_date(int field, int *yearp, int *monthp, int *dayp)
>  }
>  EXPORT_SYMBOL(dmi_get_date);
>  
> +int dmi_get_bios_year(void)
> +{
> +	bool exists;
> +	int year;
> +
> +	exists = dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL);
> +
> +	return exists ? year : -ENODATA;
> +}
> +EXPORT_SYMBOL(dmi_get_bios_year);

It would be good if kerneldoc was added to this function.  One thing
to mention is that direct usage of the function in a conditional only
works reliably when asserting an exact or minimum BIOS date.  It doesn't
work reliably when asserting a maximum BIOS date unless the return
value is explicitly checked for -ENODATA.  (Fortunately that use case
seems to be rare, but still worth mentioning IMHO.)


> +static inline int dmi_get_bios_year(void) { return -ENXIO; }

Shouldn't this be -ENODATA as well for consistency?  Otherwise one would
have to check for -ENODATA *and* -ENXIO.

Thanks,

Lukas