Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1721964 > unrolled thread

Re: [PATCH v4 1/5] ACPI / blacklist: add acpi_match_platform_list()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4 1/5] ACPI / blacklist: add acpi_match_platform_list() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-08-28 23:10 +0200
    Re: [PATCH v4 1/5] ACPI / blacklist: add acpi_match_platform_list() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-08-28 23:30 +0200
    Re: [PATCH v4 1/5] ACPI / blacklist: add acpi_match_platform_list() Borislav Petkov <bp@alien8.de> - 2017-08-28 23:30 +0200

#1721964 — Re: [PATCH v4 1/5] ACPI / blacklist: add acpi_match_platform_list()

On Thursday, August 24, 2017 9:53:48 AM CEST Borislav Petkov wrote:
> On Wed, Aug 23, 2017 at 04:54:43PM -0600, Toshi Kani wrote:
> > ACPI OEM ID / OEM Table ID / Revision can be used to identify
> > a platform based on ACPI firmware info.  acpi_blacklisted(),
> > intel_pstate_platform_pwr_mgmt_exists(), and some other funcs,
> > have been using similar check to detect a list of platforms
> > that require special handlings.
> > 
> > Move the platform check in acpi_blacklisted() to a new common
> > utility function, acpi_match_platform_list(), so that other
> > drivers do not have to implement their own version.
> > 
> > There is no change in functionality.
> > 
> > Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Cc: Borislav Petkov <bp@alien8.de>
> > ---
> >  drivers/acpi/blacklist.c |   83 ++++++++--------------------------------------
> >  drivers/acpi/utils.c     |   36 ++++++++++++++++++++
> >  include/linux/acpi.h     |   19 +++++++++++
> >  3 files changed, 69 insertions(+), 69 deletions(-)
> 
> ...
> 
> > diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> > index b9d956c..0a9e597 100644
> > --- a/drivers/acpi/utils.c
> > +++ b/drivers/acpi/utils.c
> > @@ -816,3 +816,39 @@ static int __init acpi_backlight(char *str)
> >  	return 1;
> >  }
> >  __setup("acpi_backlight=", acpi_backlight);
> > +
> > +/**
> > + * acpi_match_platform_list - Check if the system matches with a given list
> > + * @plat: pointer to acpi_platform_list table terminated by a NULL entry
> > + *
> > + * Return the matched index if the system is found in the platform list.
> > + * Otherwise, return a negative error code.
> > + */
> > +int acpi_match_platform_list(const struct acpi_platform_list *plat)
> > +{
> > +	struct acpi_table_header hdr;
> > +	int idx = 0;
> > +
> > +	if (acpi_disabled)
> > +		return -ENODEV;
> > +
> > +	for (; plat->oem_id[0]; plat++, idx++) {
> > +		if (ACPI_FAILURE(acpi_get_table_header(plat->table, 0, &hdr)))
> > +			continue;
> > +
> > +		if (strncmp(plat->oem_id, hdr.oem_id, ACPI_OEM_ID_SIZE))
> > +			continue;
> > +
> > +		if (strncmp(plat->oem_table_id, hdr.oem_table_id, ACPI_OEM_TABLE_ID_SIZE))
> > +			continue;
> > +
> > +		if ((plat->pred == all_versions) ||
> > +		    (plat->pred == less_than_or_equal && hdr.oem_revision <= plat->oem_revision) ||
> > +		    (plat->pred == greater_than_or_equal && hdr.oem_revision >= plat->oem_revision) ||
> > +		    (plat->pred == equal && hdr.oem_revision == plat->oem_revision))
> 
> If you align the second part of the test like this:
> 
>              if ((plat->pred == all_versions) ||
>                  (plat->pred == less_than_or_equal	&& hdr.oem_revision <= plat->oem_revision) ||
>                  (plat->pred == greater_than_or_equal	&& hdr.oem_revision >= plat->oem_revision) ||
>                  (plat->pred == equal			&& hdr.oem_revision == plat->oem_revision))
> 
> it gets maximally readable. But I'd leave that up to Rafael when committing
> - no need to send another version.
> 
> Other than that:
> 
> Reviewed-by: Borislav Petkov <bp@suse.de>

OK

So what about the [3-5/5] in this series?

My current plan is to apply them too and expose a branch with them, can I
go ahead with that?

Thanks,
Rafael

[toc] | [next] | [standalone]

#1721976

On Monday, August 28, 2017 11:21:52 PM CEST Borislav Petkov wrote:
> On Mon, Aug 28, 2017 at 10:55:07PM +0200, Rafael J. Wysocki wrote:
> > So what about the [3-5/5] in this series?
> > 
> > My current plan is to apply them too and expose a branch with them, can I
> > go ahead with that?
> 
> No, please expose a branch with only the ACPI patches, i.e., 1 and 2 and
> I can merge it and apply the EDAC patches ontop.
> 
> Then you and I need to sync up so that you send your pull requests first
> and I follow you so that Linus applies them in the proper order.
> 
> Makes sense?

Yup, works for me!

[toc] | [prev] | [next] | [standalone]

#1721977

On Mon, Aug 28, 2017 at 10:55:07PM +0200, Rafael J. Wysocki wrote:
> So what about the [3-5/5] in this series?
> 
> My current plan is to apply them too and expose a branch with them, can I
> go ahead with that?

No, please expose a branch with only the ACPI patches, i.e., 1 and 2 and
I can merge it and apply the EDAC patches ontop.

Then you and I need to sync up so that you send your pull requests first
and I follow you so that Linus applies them in the proper order.

Makes sense?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web