Re: [PATCH v1 1/4] dmi: Introduce dmi_get_bios_year() helper

Re: [PATCH v1 1/4] dmi: Introduce dmi_get_bios_year() helper

[Rafael J. Wysocki]

On Sun, Feb 25, 2018 at 2:23 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Fri, 2018-02-23 at 15:35 -0600, Bjorn Helgaas wrote:
>> On Thu, Feb 22, 2018 at 02:59:20PM +0200, Andy Shevchenko wrote:
>> > It's used in several places and more users may come.
>> > By using this helper they may create a slightly cleaner code.
>> >
>> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> > ---
>> >  include/linux/dmi.h | 7 +++++++
>> >  1 file changed, 7 insertions(+)
>> >
>> > diff --git a/include/linux/dmi.h b/include/linux/dmi.h
>> > index 46e151172d95..241c27008c70 100644
>> > --- a/include/linux/dmi.h
>> > +++ b/include/linux/dmi.h
>> > @@ -147,4 +147,11 @@ static inline const struct dmi_system_id *
>> >
>> >  #endif
>> >
>> > +static inline int dmi_get_bios_year(void)
>> > +{
>> > +   int year;
>> > +   dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL);
>> > +   return year;
>> > +}
>>
>> I don't really care personally, and I assume this series will go via a
>> non-PCI tree, but making this inline looks similar to this, which
>> wasn't well-received:
>>
>> https://lkml.kernel.org/r/CA+55aFypU331cQy-
>> 6ZJ6szF=2KVLqcbwCUGd+gTwPViRqRWN+g@mail.gmail.com
>
> "Yes, that header file is already full of random inline functions, but
> they are generally wrapper functions that don't really do anything, ..."
>
> I think the function above is exactly from the "wrapper that doesn't
> really do anything" category.

Yes, but honestly does it need to be inline even so?

Why don't you simply put the wrapper into dmi_scan.c and export it?

Re: [PATCH v1 1/4] dmi: Introduce dmi_get_bios_year() helper

[Andy Shevchenko]

On Mon, 2018-02-26 at 10:29 +0100, Rafael J. Wysocki wrote:
> On Sun, Feb 25, 2018 at 2:23 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, 2018-02-23 at 15:35 -0600, Bjorn Helgaas wrote:

> > > but making this inline looks similar to this, which
> > > wasn't well-received:
> > > 
> > > https://lkml.kernel.org/r/CA+55aFypU331cQy-
> > > 6ZJ6szF=2KVLqcbwCUGd+gTwPViRqRWN+g@mail.gmail.com
> > 
> > "Yes, that header file is already full of random inline functions,
> > but
> > they are generally wrapper functions that don't really do anything,
> > ..."
> > 
> > I think the function above is exactly from the "wrapper that doesn't
> > really do anything" category.
> 
> Yes, but honestly does it need to be inline even so?
> 
> Why don't you simply put the wrapper into dmi_scan.c and export it?

Taking into consideration that there are few comments to be addressed
and series had been applied, I will send a fix up series where I move
this to be a normal function.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v1 2/4] x86/pci: Re-use new dmi_get_bios_year() helper

[Jean Delvare]

Hi Andy,

On Thu, 22 Feb 2018 14:59:21 +0200, Andy Shevchenko wrote:
> ...instead of open coding its functionality.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/pci/acpi.c            | 8 ++------
>  arch/x86/pci/direct.c          | 5 ++---
>  arch/x86/pci/mmconfig-shared.c | 9 ++-------
>  3 files changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index 7df49c40665e..00e60de30328 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -140,12 +140,8 @@ static const struct dmi_system_id pci_crs_quirks[] __initconst = {
>  
>  void __init pci_acpi_crs_quirks(void)
>  {
> -	int year;
> -
> -	if (dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL) && year < 2008) {
> -		if (iomem_resource.end <= 0xffffffff)
> -			pci_use_crs = false;
> -	}
> +	if ((dmi_get_bios_year() < 2008) && (iomem_resource.end <= 0xffffffff))
> +		pci_use_crs = false;

You are changing the behavior here, when DMI does not provide a BIOS
date. Beforehand, the test would fail and pci_use_crs would be left
alone. After your change, dmi_get_bios_year() will return 0, and
"0 < 2008" is true, so pci_use_crs is set to false.

I have no opinion on what this driver should do in such case, but I
certainly wouldn't expect a change of behavior from a "use a helper
instead of open-coding" kind of patch.

I don't think you can safely assume that the code calling
dmi_get_bios_year() will always do the right thing when 0 is being
returned. It's up to the calling code to decide what the default
behavior should be.

Also there are more parentheses than needed.

>  
>  	dmi_check_system(pci_crs_quirks);
>  
> diff --git a/arch/x86/pci/direct.c b/arch/x86/pci/direct.c
> index 2d9503323d10..a51074c55982 100644
> --- a/arch/x86/pci/direct.c
> +++ b/arch/x86/pci/direct.c
> @@ -195,14 +195,13 @@ static const struct pci_raw_ops pci_direct_conf2 = {
>  static int __init pci_sanity_check(const struct pci_raw_ops *o)
>  {
>  	u32 x = 0;
> -	int year, devfn;
> +	int devfn;
>  
>  	if (pci_probe & PCI_NO_CHECKS)
>  		return 1;
>  	/* Assume Type 1 works for newer systems.
>  	   This handles machines that don't have anything on PCI Bus 0. */
> -	dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL);
> -	if (year >= 2001)
> +	if (dmi_get_bios_year() >= 2001)
>  		return 1;
>  
>  	for (devfn = 0; devfn < 0x100; devfn++) {
> diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
> index 96684d0adcf9..0b40482578b8 100644
> --- a/arch/x86/pci/mmconfig-shared.c
> +++ b/arch/x86/pci/mmconfig-shared.c
> @@ -547,19 +547,14 @@ static void __init pci_mmcfg_reject_broken(int early)
>  static int __init acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg,
>  					struct acpi_mcfg_allocation *cfg)
>  {
> -	int year;
> -
>  	if (cfg->address < 0xFFFFFFFF)
>  		return 0;
>  
>  	if (!strncmp(mcfg->header.oem_id, "SGI", 3))
>  		return 0;
>  
> -	if (mcfg->header.revision >= 1) {
> -		if (dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL) &&
> -		    year >= 2010)
> -			return 0;
> -	}
> +	if ((mcfg->header.revision >= 1) && (dmi_get_bios_year() >= 2010))
> +		return 0;

Here too some parentheses are not needed.

>  
>  	pr_err(PREFIX "MCFG region for %04x [bus %02x-%02x] at %#llx "
>  	       "is above 4GB, ignored\n", cfg->pci_segment,


-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v1 2/4] x86/pci: Re-use new dmi_get_bios_year() helper

[Andy Shevchenko]

On Mon, 2018-02-26 at 17:28 +0100, Jean Delvare wrote:

> > -	if (dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL) && year
> > < 2008) {
> > -		if (iomem_resource.end <= 0xffffffff)
> > -			pci_use_crs = false;
> > -	}
> > +	if ((dmi_get_bios_year() < 2008) && (iomem_resource.end <=
> > 0xffffffff))
> > +		pci_use_crs = false;
> 
> You are changing the behavior here, when DMI does not provide a BIOS
> date. Beforehand, the test would fail and pci_use_crs would be left
> alone. After your change, dmi_get_bios_year() will return 0, and
> "0 < 2008" is true, so pci_use_crs is set to false.

Hmm... Indeed.

> I have no opinion on what this driver should do in such case,

I would assume that no BIOS date is related to prehistoric firmwares and
 using _CRS would sound weird on them.

>  but I
> certainly wouldn't expect a change of behavior from a "use a helper
> instead of open-coding" kind of patch.

Agree.

> I don't think you can safely assume that the code calling
> dmi_get_bios_year() will always do the right thing when 0 is being
> returned. It's up to the calling code to decide what the default
> behavior should be.

That's correct.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v1 2/4] x86/pci: Re-use new dmi_get_bios_year() helper

[Rafael J. Wysocki]

On Wed, Feb 28, 2018 at 11:29 AM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Mon, 2018-02-26 at 17:28 +0100, Jean Delvare wrote:
>
>> > -   if (dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL) && year
>> > < 2008) {
>> > -           if (iomem_resource.end <= 0xffffffff)
>> > -                   pci_use_crs = false;
>> > -   }
>> > +   if ((dmi_get_bios_year() < 2008) && (iomem_resource.end <=
>> > 0xffffffff))
>> > +           pci_use_crs = false;
>>
>> You are changing the behavior here, when DMI does not provide a BIOS
>> date. Beforehand, the test would fail and pci_use_crs would be left
>> alone. After your change, dmi_get_bios_year() will return 0, and
>> "0 < 2008" is true, so pci_use_crs is set to false.
>
> Hmm... Indeed.
>
>> I have no opinion on what this driver should do in such case,
>
> I would assume that no BIOS date is related to prehistoric firmwares and
>  using _CRS would sound weird on them.

Careful here.

You seem to be assuming that the DMI information is always valid
and/or complete which is know to not be the case sometimes.

Re: [PATCH v1 2/4] x86/pci: Re-use new dmi_get_bios_year() helper

[Jean Delvare]

On Wed, 28 Feb 2018 11:33:39 +0100, Rafael J. Wysocki wrote:
> On Wed, Feb 28, 2018 at 11:29 AM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > I would assume that no BIOS date is related to prehistoric firmwares and
> >  using _CRS would sound weird on them.  
> 
> Careful here.
> 
> You seem to be assuming that the DMI information is always valid
> and/or complete which is know to not be the case sometimes.

True. While the BIOS date is not the worst offender when it comes to
broken DMI data, you must remember that the date comes as a string, and
older SMBIOS specifications did not even recommend a specific format
for that string. As a matter of fact, my collection of DMI tables
includes a few creative samples like "Jul  7 2016" or "09-16-08" which
the kernel fails to parse.

So the default behavior at the driver level shouldn't be based on what
older systems are most likely to enjoy. The default behavior must be
the safest option, regardless of the age of the system.

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v1 2/4] x86/pci: Re-use new dmi_get_bios_year() helper

[Andy Shevchenko]

On Wed, 2018-02-28 at 20:21 +0100, Jean Delvare wrote:
> On Wed, 28 Feb 2018 11:33:39 +0100, Rafael J. Wysocki wrote:
> > On Wed, Feb 28, 2018 at 11:29 AM, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > I would assume that no BIOS date is related to prehistoric
> > > firmwares and
> > >  using _CRS would sound weird on them.  
> > 
> > Careful here.
> > 
> > You seem to be assuming that the DMI information is always valid
> > and/or complete which is know to not be the case sometimes.
> 
> True. While the BIOS date is not the worst offender when it comes to
> broken DMI data, you must remember that the date comes as a string,
> and
> older SMBIOS specifications did not even recommend a specific format
> for that string. As a matter of fact, my collection of DMI tables
> includes a few creative samples like "Jul  7 2016" or "09-16-08" which
> the kernel fails to parse.
> 
> So the default behavior at the driver level shouldn't be based on what
> older systems are most likely to enjoy. The default behavior must be
> the safest option, regardless of the age of the system.

Yep.

And here is a very good question which path is more safer: use _CRS, or
not?

Rafael, do you know any consequences of not using _CRS for PCI on older
and newer machines?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v1 3/4] ACPI / sleep: Re-use new dmi_get_bios_year() helper

[Jean Delvare]

On Thu, 22 Feb 2018 14:59:22 +0200, Andy Shevchenko wrote:
> ...instead of open coding its functionality.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/acpi/sleep.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index 46cde0912762..b35923e3a926 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -376,12 +376,10 @@ void __init acpi_sleep_no_blacklist(void)
>  
>  static void __init acpi_sleep_dmi_check(void)
>  {
> -	int year;
> -
>  	if (ignore_blacklist)
>  		return;
>  
> -	if (dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL) && year >= 2012)
> +	if (dmi_get_bios_year() >= 2012)
>  		acpi_nvs_nosave_s3();
>  
>  	dmi_check_system(acpisleep_dmi_table);

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v1 4/4] pci: Re-use new dmi_get_bios_year() helper

[Bjorn Helgaas]

On Sun, Feb 25, 2018 at 03:27:04PM +0200, Andy Shevchenko wrote:
> On Fri, 2018-02-23 at 15:40 -0600, Bjorn Helgaas wrote:
> > On Thu, Feb 22, 2018 at 02:59:23PM +0200, Andy Shevchenko wrote:
> > > ...instead of open coding its functionality.
> > 
> > Same comment about making the changelog complete, independent of the
> > subject.
> 
> Any suggestion how it would look like? (Same question for previous
> comment)

  PCI: Re-use new dmi_get_bios_year() helper

  Use new dmi_get_bios_year() helper instead of open-coding its
  functionality.

The usual document structure is something like:

  TITLE

  This abstract contains a summary of the entire document, in a few
  paragraphs of complete sentences.

Where "TITLE" makes sense all by itself, even without reading the
body, and "Body" is a complete statement that also makes sense all by
itself without having to read "TITLE" first.

Granted, it's trivial, but following the convention improves
readability slightly because it fits the reader's expectations.

When the body is "...instead of open coding its functionality", it's a
bit of a hiccup because I have to start over and look back up to the
title to re-read the thing as a whole.

Bjorn

Re: [PATCH v1 4/4] pci: Re-use new dmi_get_bios_year() helper

[Andy Shevchenko]

On Mon, 2018-02-26 at 12:19 -0600, Bjorn Helgaas wrote:
> On Sun, Feb 25, 2018 at 03:27:04PM +0200, Andy Shevchenko wrote:
> > On Fri, 2018-02-23 at 15:40 -0600, Bjorn Helgaas wrote:
> > > On Thu, Feb 22, 2018 at 02:59:23PM +0200, Andy Shevchenko wrote:
> > > > ...instead of open coding its functionality.
> > > 
> > > Same comment about making the changelog complete, independent of
> > > the
> > > subject.
> > 
> > Any suggestion how it would look like? (Same question for previous
> > comment)
> 
>   PCI: Re-use new dmi_get_bios_year() helper
> 
>   Use new dmi_get_bios_year() helper instead of open-coding its
>   functionality.
> 
> The usual document structure is something like:
> 
>   TITLE
> 
>   This abstract contains a summary of the entire document, in a few
>   paragraphs of complete sentences.
> 
> Where "TITLE" makes sense all by itself, even without reading the
> body, and "Body" is a complete statement that also makes sense all by
> itself without having to read "TITLE" first.
> 

Thank you for a hint!

> Granted, it's trivial, but following the convention improves
> readability slightly because it fits the reader's expectations.

> When the body is "...instead of open coding its functionality", it's a
> bit of a hiccup because I have to start over and look back up to the
> title to re-read the thing as a whole.

OK, I got your point, though I don't like duplication in the subject and
body.


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v1 4/4] pci: Re-use new dmi_get_bios_year() helper

[Bjorn Helgaas]

On Wed, Feb 28, 2018 at 12:12:22PM +0200, Andy Shevchenko wrote:
> On Mon, 2018-02-26 at 12:19 -0600, Bjorn Helgaas wrote:
> > On Sun, Feb 25, 2018 at 03:27:04PM +0200, Andy Shevchenko wrote:
> > > On Fri, 2018-02-23 at 15:40 -0600, Bjorn Helgaas wrote:
> > > > On Thu, Feb 22, 2018 at 02:59:23PM +0200, Andy Shevchenko wrote:
> > > > > ...instead of open coding its functionality.
> > > > 
> > > > Same comment about making the changelog complete, independent of
> > > > the
> > > > subject.
> > > 
> > > Any suggestion how it would look like? (Same question for previous
> > > comment)
> > 
> >   PCI: Re-use new dmi_get_bios_year() helper
> > 
> >   Use new dmi_get_bios_year() helper instead of open-coding its
> >   functionality.
> > 
> > The usual document structure is something like:
> > 
> >   TITLE
> > 
> >   This abstract contains a summary of the entire document, in a few
> >   paragraphs of complete sentences.
> > 
> > Where "TITLE" makes sense all by itself, even without reading the
> > body, and "Body" is a complete statement that also makes sense all by
> > itself without having to read "TITLE" first.
> > 
> 
> Thank you for a hint!
> 
> > Granted, it's trivial, but following the convention improves
> > readability slightly because it fits the reader's expectations.
> 
> > When the body is "...instead of open coding its functionality", it's a
> > bit of a hiccup because I have to start over and look back up to the
> > title to re-read the thing as a whole.
> 
> OK, I got your point, though I don't like duplication in the subject and
> body.

Ah, I see.  I think of the subject and the body as serving two
distinct purposes, so for me there's no issue even if they happen to
contain exactly the same text.

Bjorn

Re: [PATCH v1 4/4] pci: Re-use new dmi_get_bios_year() helper

[Jean Delvare]

On Thu, 22 Feb 2018 14:59:23 +0200, Andy Shevchenko wrote:
> ...instead of open coding its functionality.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pci/pci.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index f6a4dd10d9b0..ae654e21439d 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2258,8 +2258,6 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev)
>   */
>  bool pci_bridge_d3_possible(struct pci_dev *bridge)
>  {
> -	unsigned int year;
> -
>  	if (!pci_is_pcie(bridge))
>  		return false;
>  
> @@ -2287,10 +2285,8 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
>  		 * It should be safe to put PCIe ports from 2015 or newer
>  		 * to D3.
>  		 */
> -		if (dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL) &&
> -		    year >= 2015) {
> +		if (dmi_get_bios_year() >= 2015)
>  			return true;
> -		}
>  		break;
>  	}
>  

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v1 1/4] dmi: Introduce dmi_get_bios_year() helper

[Jean Delvare]

Hi Andy,

On Thu, 22 Feb 2018 14:59:20 +0200, Andy Shevchenko wrote:
> It's used in several places and more users may come.
> By using this helper they may create a slightly cleaner code.

In general I'm not a big fan of arbitrary API shortcuts. I won't object
if you think it's a good one to have, however I'm a bit concerned about
the silent handling of the "error case", which could cause unexpected
default decisions as seen in patch 2/4 of this series. The fact that
dmi_get_bios_year() returns 0 if there is no DMI BIOS date provided
should at least be documented, to limit the risk.

I also wonder if dmi_get_date() itself couldn't be optimized a bit if
it turns out that most callers are only interested in the year.
Currently it will parse the whole string even if the caller isn't
interested in the month and day.

The fact that dmi_get_date() returns true even if it couldn't parse the
date string at all is also strange, although unrelated with your
current work.

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/dmi.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index 46e151172d95..241c27008c70 100644
> --- a/include/linux/dmi.h
> +++ b/include/linux/dmi.h
> @@ -147,4 +147,11 @@ static inline const struct dmi_system_id *
>  
>  #endif
>  
> +static inline int dmi_get_bios_year(void)
> +{
> +	int year;
> +	dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL);
> +	return year;
> +}

checkpatch complains:

WARNING: Missing a blank line after declarations
#61: FILE: include/linux/dmi.h:153:
+	int year;
+	dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL);

And I would tend to agree. Just because it is an inline function in a
header file doesn't mean we don't stick to the usual coding style
policy.

> +
>  #endif	/* __DMI_H__ */

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v1 1/4] dmi: Introduce dmi_get_bios_year() helper

[Andy Shevchenko]

On Tue, 2018-02-27 at 10:14 +0100, Jean Delvare wrote:
> Hi Andy,
> 
> On Thu, 22 Feb 2018 14:59:20 +0200, Andy Shevchenko wrote:
> > It's used in several places and more users may come.
> > By using this helper they may create a slightly cleaner code.
> 
> In general I'm not a big fan of arbitrary API shortcuts. I won't
> object
> if you think it's a good one to have, however I'm a bit concerned
> about
> the silent handling of the "error case", which could cause unexpected
> default decisions as seen in patch 2/4 of this series.

Yes, we better to return -ERRNO in such case and allow callers to behave
correspondingly.

>  The fact that
> dmi_get_bios_year() returns 0 if there is no DMI BIOS date provided
> should at least be documented, to limit the risk.
> 
> I also wonder if dmi_get_date() itself couldn't be optimized a bit if
> it turns out that most callers are only interested in the year.
> Currently it will parse the whole string even if the caller isn't
> interested in the month and day.
> 
> The fact that dmi_get_date() returns true even if it couldn't parse
> the
> date string at all is also strange, although unrelated with your
> current work.

So, what I consider is to
- move inline function to be regular one
- optimize it with current dmi_get_date()
- return error code when year is not parsable
- consider current use cases where we do compare for less than a given
year

Does it sound a correct approach?

> 
> checkpatch complains:
> 
> WARNING: Missing a blank line after declarations
> #61: FILE: include/linux/dmi.h:153:
> +	int year;
> +	dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL);
> 
> And I would tend to agree. Just because it is an inline function in a
> header file doesn't mean we don't stick to the usual coding style
> policy.

Ingo fixed that.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy