| Message ID | 1434666968-1543-2-git-send-email-al.stone@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/19/2015 06:36 AM, Al Stone wrote: > Add the ACPI_SPEC_VERSION() macro to build a proper version number from > a major and minor revision number. Add also the ACPI_FADT_SPEC_VERSION > that constructs a proper version number from the entries in the current > FADT. > > These macros are added in order to simplify retrieving and comparing ACPI > specification version numbers, since this is becoming a more frequent need. > In particular, there are some architectures that require at least a certain > version of the spec, and there are differences in some structure sizes that > have changed with recent versions but can only be tracked by spec version > number. > > Signed-off-by: Al Stone <al.stone@linaro.org> > --- > include/linux/acpi.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index a4acb55..33ed313 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -48,6 +48,11 @@ > #include <acpi/acpi_io.h> > #include <asm/acpi.h> > > +#define ACPI_SPEC_VERSION(major, minor) ((major<<8)|minor) and minor comments for code style ((major<<8)|minor) - > ((major << 8) | minor) other than that: Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org> Thanks Hanjun > +#define ACPI_FADT_SPEC_VERSION \ > + ACPI_SPEC_VERSION(acpi_gbl_FADT.header.revision, \ > + acpi_gbl_FADT.minor_revision) > + > static inline acpi_handle acpi_device_handle(struct acpi_device *adev) > { > return adev ? adev->handle : NULL; >
On 06/19/2015 04:49 AM, Hanjun Guo wrote: > On 06/19/2015 06:36 AM, Al Stone wrote: >> Add the ACPI_SPEC_VERSION() macro to build a proper version number from >> a major and minor revision number. Add also the ACPI_FADT_SPEC_VERSION >> that constructs a proper version number from the entries in the current >> FADT. >> >> These macros are added in order to simplify retrieving and comparing ACPI >> specification version numbers, since this is becoming a more frequent need. >> In particular, there are some architectures that require at least a certain >> version of the spec, and there are differences in some structure sizes that >> have changed with recent versions but can only be tracked by spec version >> number. >> >> Signed-off-by: Al Stone <al.stone@linaro.org> >> --- >> include/linux/acpi.h | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/include/linux/acpi.h b/include/linux/acpi.h >> index a4acb55..33ed313 100644 >> --- a/include/linux/acpi.h >> +++ b/include/linux/acpi.h >> @@ -48,6 +48,11 @@ >> #include <acpi/acpi_io.h> >> #include <asm/acpi.h> >> >> +#define ACPI_SPEC_VERSION(major, minor) ((major<<8)|minor) > > and minor comments for code style > > ((major<<8)|minor) - > ((major << 8) | minor) Ah. Yeah, good idea. I can roll that into next version. > other than that: > > Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org> Thanks. > Thanks > Hanjun > >> +#define ACPI_FADT_SPEC_VERSION \ >> + ACPI_SPEC_VERSION(acpi_gbl_FADT.header.revision, \ >> + acpi_gbl_FADT.minor_revision) >> + >> static inline acpi_handle acpi_device_handle(struct acpi_device *adev) >> { >> return adev ? adev->handle : NULL; >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
Hi Al, On Fri, Jun 19, 2015 at 12:36 AM, Al Stone <al.stone@linaro.org> wrote: > Add the ACPI_SPEC_VERSION() macro to build a proper version number from > a major and minor revision number. Add also the ACPI_FADT_SPEC_VERSION > that constructs a proper version number from the entries in the current > FADT. > > These macros are added in order to simplify retrieving and comparing ACPI > specification version numbers, since this is becoming a more frequent need. > In particular, there are some architectures that require at least a certain > version of the spec, and there are differences in some structure sizes that > have changed with recent versions but can only be tracked by spec version > number. > > Signed-off-by: Al Stone <al.stone@linaro.org> > --- > include/linux/acpi.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index a4acb55..33ed313 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -48,6 +48,11 @@ > #include <acpi/acpi_io.h> > #include <asm/acpi.h> > > +#define ACPI_SPEC_VERSION(major, minor) ((major<<8)|minor) One nit here. acpi_gbl_FADT.header.revision is of type u8 originally, so shifting it by 8 bit positions only works due to some implicit type casting I suppose. Moreover, it is not entirely clear why the macro is specific to the computation of the ACPI spec version. So I'd drop ACPI_SPEC_VERSION and only define ACPI_FADT_SPEC_VERSION as something like #define ACPI_FADT_SPEC_VERSION (((unsigned int)acpi_gbl_FADT.header.revision << 8) | (unsigned int)acpi_gbl_FADT.minor_revision) Thanks, Rafael
On 06/30/2015 02:12 PM, Rafael J. Wysocki wrote: > Hi Al, > > On Fri, Jun 19, 2015 at 12:36 AM, Al Stone <al.stone@linaro.org> wrote: >> Add the ACPI_SPEC_VERSION() macro to build a proper version number from >> a major and minor revision number. Add also the ACPI_FADT_SPEC_VERSION >> that constructs a proper version number from the entries in the current >> FADT. >> >> These macros are added in order to simplify retrieving and comparing ACPI >> specification version numbers, since this is becoming a more frequent need. >> In particular, there are some architectures that require at least a certain >> version of the spec, and there are differences in some structure sizes that >> have changed with recent versions but can only be tracked by spec version >> number. >> >> Signed-off-by: Al Stone <al.stone@linaro.org> >> --- >> include/linux/acpi.h | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/include/linux/acpi.h b/include/linux/acpi.h >> index a4acb55..33ed313 100644 >> --- a/include/linux/acpi.h >> +++ b/include/linux/acpi.h >> @@ -48,6 +48,11 @@ >> #include <acpi/acpi_io.h> >> #include <asm/acpi.h> >> >> +#define ACPI_SPEC_VERSION(major, minor) ((major<<8)|minor) > > One nit here. > > acpi_gbl_FADT.header.revision is of type u8 originally, so shifting it > by 8 bit positions only works due to some implicit type casting I > suppose. Bah. That was being sloppy on my part. Sorry about that. Will fix. > Moreover, it is not entirely clear why the macro is specific to the > computation of the ACPI spec version. As far as I know, that's the only way to extract the spec version from tables; I don't recall there being any other table with that info. Since I will likely use this again, it seemed to make sense at the time. > So I'd drop ACPI_SPEC_VERSION and only define ACPI_FADT_SPEC_VERSION > as something like > > #define ACPI_FADT_SPEC_VERSION (((unsigned > int)acpi_gbl_FADT.header.revision << 8) | (unsigned > int)acpi_gbl_FADT.minor_revision) > > Thanks, > Rafael > Sure. That makes sense. It makes it clearer that this is the version just from the FADT. I'll do that.
On 2015/7/1 5:15, Al Stone wrote: > On 06/30/2015 02:12 PM, Rafael J. Wysocki wrote: >> Hi Al, >> >> On Fri, Jun 19, 2015 at 12:36 AM, Al Stone <al.stone@linaro.org> wrote: >>> Add the ACPI_SPEC_VERSION() macro to build a proper version number from >>> a major and minor revision number. Add also the ACPI_FADT_SPEC_VERSION >>> that constructs a proper version number from the entries in the current >>> FADT. >>> >>> These macros are added in order to simplify retrieving and comparing ACPI >>> specification version numbers, since this is becoming a more frequent need. >>> In particular, there are some architectures that require at least a certain >>> version of the spec, and there are differences in some structure sizes that >>> have changed with recent versions but can only be tracked by spec version >>> number. >>> >>> Signed-off-by: Al Stone <al.stone@linaro.org> >>> --- >>> include/linux/acpi.h | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h >>> index a4acb55..33ed313 100644 >>> --- a/include/linux/acpi.h >>> +++ b/include/linux/acpi.h >>> @@ -48,6 +48,11 @@ >>> #include <acpi/acpi_io.h> >>> #include <asm/acpi.h> >>> >>> +#define ACPI_SPEC_VERSION(major, minor) ((major<<8)|minor) >> One nit here. >> >> acpi_gbl_FADT.header.revision is of type u8 originally, so shifting it >> by 8 bit positions only works due to some implicit type casting I >> suppose. > Bah. That was being sloppy on my part. Sorry about that. Will fix. > >> Moreover, it is not entirely clear why the macro is specific to the >> computation of the ACPI spec version. > As far as I know, that's the only way to extract the spec version from > tables; I don't recall there being any other table with that info. Since > I will likely use this again, it seemed to make sense at the time. That's true from ACPI 5.1, as we discussed in ACPI spec working group, FADT Major Version and FADT Minor Version are recognized as ACPI Major/Minor version. yes, the spec itself didn't state that explicitly, maybe a ECR to make it explicit will be good, I can prepare one if that makes sense. Thanks Hanjun
diff --git a/include/linux/acpi.h b/include/linux/acpi.h index a4acb55..33ed313 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -48,6 +48,11 @@ #include <acpi/acpi_io.h> #include <asm/acpi.h> +#define ACPI_SPEC_VERSION(major, minor) ((major<<8)|minor) +#define ACPI_FADT_SPEC_VERSION \ + ACPI_SPEC_VERSION(acpi_gbl_FADT.header.revision, \ + acpi_gbl_FADT.minor_revision) + static inline acpi_handle acpi_device_handle(struct acpi_device *adev) { return adev ? adev->handle : NULL;
Add the ACPI_SPEC_VERSION() macro to build a proper version number from a major and minor revision number. Add also the ACPI_FADT_SPEC_VERSION that constructs a proper version number from the entries in the current FADT. These macros are added in order to simplify retrieving and comparing ACPI specification version numbers, since this is becoming a more frequent need. In particular, there are some architectures that require at least a certain version of the spec, and there are differences in some structure sizes that have changed with recent versions but can only be tracked by spec version number. Signed-off-by: Al Stone <al.stone@linaro.org> --- include/linux/acpi.h | 5 +++++ 1 file changed, 5 insertions(+)