lkml.org 
[lkml]   [2022]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 5/7] platform/x86: ideapad-laptop: Expose camera_power only if supported
From
Hi Eray,

On 10/29/22 14:03, Eray Orçunus wrote:
> IdeaPads dropped support for VPCCMD_W_CAMERA somewhere between 2014-2016,
> none of the IdeaPads produced after that I tested supports it. Fortunately
> I found a way to check it; if the DSDT has camera device(s) defined, it
> shouldn't have working VPCCMD_W_CAMERA, thus camera_power shouldn't be
> exposed to sysfs. To accomplish this, walk the ACPI namespace in
> ideapad_check_features and check the devices starting with "CAM".
> Tested on 520-15IKB and Legion Y520, which successfully didn't expose
> the camera_power attribute.
>
> Link: https://www.spinics.net/lists/platform-driver-x86/msg26147.html
> Signed-off-by: Eray Orçunus <erayorcunus@gmail.com>
> ---
> drivers/platform/x86/ideapad-laptop.c | 52 ++++++++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index f3d4f2beda07..e8c088e7a53d 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -149,6 +149,7 @@ struct ideapad_private {
> bool fn_lock : 1;
> bool hw_rfkill_switch : 1;
> bool kbd_bl : 1;
> + bool cam_ctrl_via_ec : 1;
> bool touchpad_ctrl_via_ec : 1;
> bool usb_charging : 1;
> } features;
> @@ -163,6 +164,24 @@ static bool no_bt_rfkill;
> module_param(no_bt_rfkill, bool, 0444);
> MODULE_PARM_DESC(no_bt_rfkill, "No rfkill for bluetooth.");
>
> +static acpi_status acpi_find_device_callback(acpi_handle handle, u32 level,
> + void *context, void **return_value)
> +{
> + struct acpi_buffer ret_buf;
> + char buffer[8];
> +
> + ret_buf.length = sizeof(buffer);
> + ret_buf.pointer = buffer;
> +
> + if (ACPI_SUCCESS(acpi_get_name(handle, ACPI_SINGLE_NAME, &ret_buf)))
> + if (strstarts(ret_buf.pointer, context)) {
> + *return_value = handle;
> + return AE_CTRL_TERMINATE;
> + }
> +
> + return AE_OK;
> +}
> +
> /*
> * ACPI Helpers
> */
> @@ -675,7 +694,7 @@ static umode_t ideapad_is_visible(struct kobject *kobj,
> bool supported = true;
>
> if (attr == &dev_attr_camera_power.attr)
> - supported = test_bit(CFG_CAP_CAM_BIT, &priv->cfg);
> + supported = priv->features.cam_ctrl_via_ec;
> else if (attr == &dev_attr_conservation_mode.attr)
> supported = priv->features.conservation_mode;
> else if (attr == &dev_attr_fan_mode.attr)
> @@ -1527,6 +1546,37 @@ static void ideapad_check_features(struct ideapad_private *priv)
>
> priv->features.hw_rfkill_switch = dmi_check_system(hw_rfkill_list);
>
> + /*
> + * Some IdeaPads have camera switch via EC (mostly older ones),
> + * some don't. Fortunately we know that if DSDT contains device
> + * object for the camera, camera isn't switchable via EC.
> + * So, let's walk the namespace and try to find CAM* object.
> + * If we can't find it, set cam_ctrl_via_ec to true.
> + */
> +
> + priv->features.cam_ctrl_via_ec = false;

There is no need to explicitly set this to false since the entire
struct is allocated with kzalloc() and a bunch of other features
flags are also not explicitly set to false. Please drop this line.

> +
> + if (test_bit(CFG_CAP_CAM_BIT, &priv->cfg)) {
> + acpi_handle temp_handle = NULL;
> + acpi_handle pci_handle;
> + acpi_status status;
> +
> + status = acpi_get_handle(handle, "^^^", &pci_handle);
> + if (ACPI_SUCCESS(status)) {
> + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, pci_handle,
> + ACPI_UINT32_MAX,
> + acpi_find_device_callback,
> + NULL, "CAM",
> + &temp_handle);

Why not just use acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ... ?

The PCI root is usually pretty much the only object under the root anyways
and this way you can avoid the acpi_get_handle() call + its error handling,
so using ACPI_ROOT_OBJECT would lead to a nice cleanup.

> +
> + if (ACPI_SUCCESS(status) && temp_handle == NULL)
> + priv->features.cam_ctrl_via_ec = true;
> +
> + } else
> + dev_warn(&priv->platform_device->dev,
> + "Could not find PCI* node in the namespace\n");
> + }
> +
> /* Most ideapads with ELAN0634 touchpad don't use EC touchpad switch */
> priv->features.touchpad_ctrl_via_ec = !acpi_dev_present("ELAN0634", NULL, -1);
>

Regards,

Hans


\
 
 \ /
  Last update: 2022-11-15 21:45    [W:0.229 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site