| Message ID | a4006978064f91dd42ec0554a3d2164a28ac61de.1609079197.git.lukas@wunner.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | efi/apple-properties: Reinstate support for boolean properties | expand |
On Sun, Dec 27, 2020 at 03:30:32PM +0100, Lukas Wunner wrote: > Since commit 4466bf82821b ("efi/apple-properties: use > PROPERTY_ENTRY_U8_ARRAY_LEN"), my MacBook Pro issues a -ENODATA error > when trying to assign EFI properties to the discrete GPU: > > pci 0000:01:00.0: assigning 56 device properties > pci 0000:01:00.0: error -61 assigning properties > > That's because some of the properties have no value. They're booleans > whose presence can be checked by drivers, e.g. "use-backlight-blanking". > > Commit 6e98503dba64 ("efi/apple-properties: Remove redundant attribute > initialization from unmarshal_key_value_pairs()") used a trick to store > such booleans as u8 arrays (which is the data type used for all other > EFI properties on Macs): It cleared the property_entry's "is_array" > flag, thereby denoting that the value is stored inline in the > property_entry. > > Commit 4466bf82821b erroneously removed that trick. It was probably a > little fragile to begin with. Thanks for spotting this! Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> One nitpick below, though. > Reinstate support for boolean properties by explicitly using the > PROPERTY_ENTRY_BOOL() initializer for properties with zero-length value. > > Fixes: 4466bf82821b ("efi/apple-properties: use PROPERTY_ENTRY_U8_ARRAY_LEN") > Signed-off-by: Lukas Wunner <lukas@wunner.de> > Cc: stable@vger.kernel.org # v5.5+ > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/firmware/efi/apple-properties.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/firmware/efi/apple-properties.c b/drivers/firmware/efi/apple-properties.c > index 34f53d898acb..0f37877db641 100644 > --- a/drivers/firmware/efi/apple-properties.c > +++ b/drivers/firmware/efi/apple-properties.c > @@ -3,8 +3,9 @@ > * apple-properties.c - EFI device properties on Macs > * Copyright (C) 2016 Lukas Wunner <lukas@wunner.de> > * > - * Note, all properties are considered as u8 arrays. > - * To get a value of any of them the caller must use device_property_read_u8_array(). > + * Properties are stored either as: > + * booleans which can be queried with device_property_present() or > + * u8 arrays which can be retrieved with device_property_read_u8_array(). > */ > > #define pr_fmt(fmt) "apple-properties: " fmt > @@ -88,8 +89,11 @@ static void __init unmarshal_key_value_pairs(struct dev_header *dev_header, > > entry_data = ptr + key_len + sizeof(val_len); > entry_len = val_len - sizeof(val_len); > - entry[i] = PROPERTY_ENTRY_U8_ARRAY_LEN(key, entry_data, > - entry_len); > + if (!entry_len) > + entry[i] = PROPERTY_ENTRY_BOOL(key); > + else > + entry[i] = PROPERTY_ENTRY_U8_ARRAY_LEN(key, entry_data, > + entry_len); Can we use positive conditional, i.e. if (entry_len) entry[i] = PROPERTY_ENTRY_U8_ARRAY_LEN(key, entry_data, entry_len); else entry[i] = PROPERTY_ENTRY_BOOL(key); ? > if (dump_properties) { > dev_info(dev, "property: %s\n", key); > print_hex_dump(KERN_INFO, pr_fmt(), DUMP_PREFIX_OFFSET, > -- > 2.29.2 > -- With Best Regards, Andy Shevchenko
diff --git a/drivers/firmware/efi/apple-properties.c b/drivers/firmware/efi/apple-properties.c index 34f53d898acb..0f37877db641 100644 --- a/drivers/firmware/efi/apple-properties.c +++ b/drivers/firmware/efi/apple-properties.c @@ -3,8 +3,9 @@ * apple-properties.c - EFI device properties on Macs * Copyright (C) 2016 Lukas Wunner <lukas@wunner.de> * - * Note, all properties are considered as u8 arrays. - * To get a value of any of them the caller must use device_property_read_u8_array(). + * Properties are stored either as: + * booleans which can be queried with device_property_present() or + * u8 arrays which can be retrieved with device_property_read_u8_array(). */ #define pr_fmt(fmt) "apple-properties: " fmt @@ -88,8 +89,11 @@ static void __init unmarshal_key_value_pairs(struct dev_header *dev_header, entry_data = ptr + key_len + sizeof(val_len); entry_len = val_len - sizeof(val_len); - entry[i] = PROPERTY_ENTRY_U8_ARRAY_LEN(key, entry_data, - entry_len); + if (!entry_len) + entry[i] = PROPERTY_ENTRY_BOOL(key); + else + entry[i] = PROPERTY_ENTRY_U8_ARRAY_LEN(key, entry_data, + entry_len); if (dump_properties) { dev_info(dev, "property: %s\n", key); print_hex_dump(KERN_INFO, pr_fmt(), DUMP_PREFIX_OFFSET,
Since commit 4466bf82821b ("efi/apple-properties: use PROPERTY_ENTRY_U8_ARRAY_LEN"), my MacBook Pro issues a -ENODATA error when trying to assign EFI properties to the discrete GPU: pci 0000:01:00.0: assigning 56 device properties pci 0000:01:00.0: error -61 assigning properties That's because some of the properties have no value. They're booleans whose presence can be checked by drivers, e.g. "use-backlight-blanking". Commit 6e98503dba64 ("efi/apple-properties: Remove redundant attribute initialization from unmarshal_key_value_pairs()") used a trick to store such booleans as u8 arrays (which is the data type used for all other EFI properties on Macs): It cleared the property_entry's "is_array" flag, thereby denoting that the value is stored inline in the property_entry. Commit 4466bf82821b erroneously removed that trick. It was probably a little fragile to begin with. Reinstate support for boolean properties by explicitly using the PROPERTY_ENTRY_BOOL() initializer for properties with zero-length value. Fixes: 4466bf82821b ("efi/apple-properties: use PROPERTY_ENTRY_U8_ARRAY_LEN") Signed-off-by: Lukas Wunner <lukas@wunner.de> Cc: stable@vger.kernel.org # v5.5+ Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/firmware/efi/apple-properties.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)