Re: [PATCH 1/2] platform/x86: Add new Dell UART backlight driver

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 5/12/24 9:35 PM, Andy Shevchenko wrote:
> On Sun, May 12, 2024 at 7:24 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Dell All In One (AIO) models released after 2017 use a backlight controller
>> board connected to an UART.
>>
>> In DSDT this uart port will be defined as:
>>
>>    Name (_HID, "DELL0501")
>>    Name (_CID, EisaId ("PNP0501")
>>
>> Instead of having a separate ACPI device with an UartSerialBusV2() resource
>> to model the backlight-controller, which would be the standard way to do
>> this.
>>
>> The acpi_quirk_skip_serdev_enumeration() has special handling for this
>> and it will make the serial port code create a serdev controller device
>> for the UART instead of a /dev/ttyS0 char-dev. It will also create
>> a dell-uart-backlight driver platform device for this driver to bind too.
>>
>> This new kernel module contains 2 drivers for this:
>>
>> 1. A simple platform driver which creates the actual serdev device
>>    (with the serdev controller device as parent)
>>
>> 2. A serdev driver for the created serdev device which exports
>>    the backlight functionality uses a standard backlight class device.
> 
> ...
> 
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> How is this being used?

#include "../serdev_helpers.h"

uses this.

> 
> ...
> 
>> +#include <linux/acpi.h>
> 
> + array_size.h
> 
>> +#include <linux/backlight.h>
>> +#include <linux/delay.h>
> 
> + device.h // devm_kzalloc(), dev_err() et al.
> 
> + err.h
> 
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/serdev.h>
> 
> + string.h
> + types.h
> 
>> +#include <linux/wait.h>

Ack, all includes added, thanks.

<snip>


>> +static int dell_uart_update_status(struct backlight_device *bd)
>> +{
>> +       struct dell_uart_backlight *dell_bl = bl_get_data(bd);
>> +       int ret;
>> +
>> +       ret = dell_uart_set_brightness(dell_bl, bd->props.brightness);
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (bd->props.power != dell_uart_get_bl_power(dell_bl))
>> +               ret = dell_uart_set_bl_power(dell_bl, bd->props.power);
> 
>     return ...;
> 
>> +       return ret;
> 
>   return 0;
> 
> ?

Ack, fixed for v2.

Regards,

Hans