Hi Peter, On 01/29/2019 07:51 PM, Peter Jones wrote: > The BGRT table's "status" field doesn't indicate "validity", but rather > if and how the image is being displayed. As such, we shouldn't decide > the table is invalid if status bits we don't understand are in use, and > it's better to expose the values we do understand directly. This goes against the conclusion of this discussion from 2015: https://patchwork.kernel.org/patch/6688521/ I wasn't involved with that discussion, so I have CC-ed the participants. You noted in another mail that the version field was not updated while the status field has been backwards-incompatibly changed. I honestly don't know what to think of this, because it shatters any meaning the words "must be zero" could have had. > This patch removes the validation of the status flag, and adds the files > "orientation" and "displayed" in sysfs. The "status" file in sysfs is > kept for compatibility with existing software. Greetings, Môshe > Signed-off-by: Peter Jones <pjones@xxxxxxxxxx> > --- > drivers/acpi/bgrt.c | 15 +++++++++++++++ > drivers/firmware/efi/efi-bgrt.c | 5 ----- > Documentation/ABI/testing/sysfs-firmware-acpi | 7 ++++++- > 3 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/drivers/acpi/bgrt.c b/drivers/acpi/bgrt.c > index 048413e0689..63d17fac2cc 100644 > --- a/drivers/acpi/bgrt.c > +++ b/drivers/acpi/bgrt.c > @@ -32,6 +32,21 @@ static ssize_t show_status(struct device *dev, > } > static DEVICE_ATTR(status, S_IRUGO, show_status, NULL); > > +static ssize_t show_orientation(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return snprintf(buf, PAGE_SIZE, "%d\n", > + ((bgrt_tab.status >> 1) & 0x3) * 90); > +} > +static DEVICE_ATTR(orientation, S_IRUGO, show_orientation, NULL); > + > +static ssize_t show_displayed(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return snprintf(buf, PAGE_SIZE, "%d\n", bgrt_tab.status & 0x1); > +} > +static DEVICE_ATTR(displayed, S_IRUGO, show_displayed, NULL); > + > static ssize_t show_type(struct device *dev, > struct device_attribute *attr, char *buf) > { > diff --git a/drivers/firmware/efi/efi-bgrt.c b/drivers/firmware/efi/efi-bgrt.c > index b2d98e16c7b..3a8797364b8 100644 > --- a/drivers/firmware/efi/efi-bgrt.c > +++ b/drivers/firmware/efi/efi-bgrt.c > @@ -120,11 +120,6 @@ void __init efi_bgrt_init(unsigned long rsdp_phys) > bgrt->version); > goto out; > } > - if (bgrt->status & 0xfe) { > - pr_notice("Ignoring BGRT: reserved status bits are non-zero %u\n", > - bgrt->status); > - goto out; > - } > if (bgrt->image_type != 0) { > pr_notice("Ignoring BGRT: invalid image type %u (expected 0)\n", > bgrt->image_type); > diff --git a/Documentation/ABI/testing/sysfs-firmware-acpi b/Documentation/ABI/testing/sysfs-firmware-acpi > index 613f42a9d5c..e4d3a1b5878 100644 > --- a/Documentation/ABI/testing/sysfs-firmware-acpi > +++ b/Documentation/ABI/testing/sysfs-firmware-acpi > @@ -10,13 +10,18 @@ Description: > transitions. > > image: The image bitmap. Currently a 32-bit BMP. > - status: 1 if the image is valid, 0 if firmware invalidated it. > type: 0 indicates image is in BMP format. > version: The version of the BGRT. Currently 1. > xoffset: The number of pixels between the left of the screen > and the left edge of the image. > yoffset: The number of pixels between the top of the screen > and the top edge of the image. > + status: The raw status flag of the image. The values are > + presented individually in the following files: > + displayed: 1 indicates the image was displayed, 0 indicates it > + wasn't. > + orientation: The orientation of the image in relation to its > + natural layout, in degrees rotated clockwise. > > What: /sys/firmware/acpi/hotplug/ > Date: February 2013 >
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
