Hello, this change looks good, I have just minor comments about improving names of variables. On Thursday 15 August 2024 19:35:46 Andres Salomon wrote: > diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c > index 8cc05f0fab91..5df0944e5bf6 100644 > --- a/drivers/platform/x86/dell/dell-laptop.c > +++ b/drivers/platform/x86/dell/dell-laptop.c > @@ -935,43 +935,24 @@ static void dell_cleanup_rfkill(void) > static int dell_send_intensity(struct backlight_device *bd) > { > struct calling_interface_buffer buffer; > - struct calling_interface_token *token; > - int ret; > - > - token = dell_smbios_find_token(BRIGHTNESS_TOKEN); > - if (!token) > - return -ENODEV; > - > - dell_fill_request(&buffer, > - token->location, bd->props.brightness, 0, 0); > - if (power_supply_is_system_supplied() > 0) > - ret = dell_send_request(&buffer, > - CLASS_TOKEN_WRITE, SELECT_TOKEN_AC); > - else > - ret = dell_send_request(&buffer, > - CLASS_TOKEN_WRITE, SELECT_TOKEN_BAT); > + u16 pwr; > > - return ret; > + pwr = power_supply_is_system_supplied() > 0 ? I would suggest to call this variable "select" or something like that. Name "pwr" is too generic and does not express the fact that is stores token select id. > + SELECT_TOKEN_AC : SELECT_TOKEN_BAT; > + return dell_send_request_for_tokenid(&buffer, CLASS_TOKEN_WRITE, > + pwr, BRIGHTNESS_TOKEN, bd->props.brightness); > } ... > @@ -2171,21 +2142,11 @@ static int micmute_led_set(struct led_classdev *led_cdev, > enum led_brightness brightness) > { > struct calling_interface_buffer buffer; > - struct calling_interface_token *token; > int state = brightness != LED_OFF; > + u32 val; > > - if (state == 0) > - token = dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE); > - else > - token = dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE); > - > - if (!token) > - return -ENODEV; > - > - dell_fill_request(&buffer, token->location, token->value, 0, 0); > - dell_send_request(&buffer, CLASS_TOKEN_WRITE, SELECT_TOKEN_STD); > - > - return 0; > + val = state == 0 ? GLOBAL_MIC_MUTE_DISABLE : GLOBAL_MIC_MUTE_ENABLE; Here variable "val" is also too generic. The real meaning of this variable is tokenid. So I would suggest to use this name as it is used in other places in this file for storing token id. > + return dell_set_std_token_value(&buffer, val, USE_TVAL); > }