| Message ID | 20211102225504.18920-3-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/3] backlight: lp855x: Move device_config setting out of lp855x_configure() | expand |
On Tue, Nov 02, 2021 at 11:55:04PM +0100, Hans de Goede wrote: > The Xiaomi Mi Pad 2 tablet uses an ACPI enumerated LP8556 backlight > controller for its LCD-panel, with a Xiaomi specific ACPI HID of > "XMCC0001", add support for this. > > Note the new "if (id)" check also fixes a NULL pointer deref when a user > tries to manually bind the driver from sysfs. > > When CONFIG_ACPI is disabled acpi_match_device() will always return NULL, > so the lp855x_parse_acpi() call will get optimized away. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> Daniel.
Hi, On 11/3/21 18:17, Daniel Thompson wrote: > On Tue, Nov 02, 2021 at 11:55:04PM +0100, Hans de Goede wrote: >> The Xiaomi Mi Pad 2 tablet uses an ACPI enumerated LP8556 backlight >> controller for its LCD-panel, with a Xiaomi specific ACPI HID of >> "XMCC0001", add support for this. >> >> Note the new "if (id)" check also fixes a NULL pointer deref when a user >> tries to manually bind the driver from sysfs. >> >> When CONFIG_ACPI is disabled acpi_match_device() will always return NULL, >> so the lp855x_parse_acpi() call will get optimized away. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> Thank you. So what is the process for upstreaming backlight patches, do these go through drm-misc-next (in that case I can push the series myself), or will you pick these up ? Regards, Hans
On Wed, Nov 03, 2021 at 06:28:15PM +0100, Hans de Goede wrote: > Hi, > > On 11/3/21 18:17, Daniel Thompson wrote: > > On Tue, Nov 02, 2021 at 11:55:04PM +0100, Hans de Goede wrote: > >> The Xiaomi Mi Pad 2 tablet uses an ACPI enumerated LP8556 backlight > >> controller for its LCD-panel, with a Xiaomi specific ACPI HID of > >> "XMCC0001", add support for this. > >> > >> Note the new "if (id)" check also fixes a NULL pointer deref when a user > >> tries to manually bind the driver from sysfs. > >> > >> When CONFIG_ACPI is disabled acpi_match_device() will always return NULL, > >> so the lp855x_parse_acpi() call will get optimized away. > >> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > > > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> > > Thank you. > > So what is the process for upstreaming backlight patches, > do these go through drm-misc-next (in that case I can push > the series myself), or will you pick these up ? Lee Jones gathers up the backlight patches and sends a PR (but, except in exceptional cases, treats my R-b as a pre-requisite before doing so). Daniel.
Hi, On 11/3/21 18:31, Daniel Thompson wrote: > On Wed, Nov 03, 2021 at 06:28:15PM +0100, Hans de Goede wrote: >> Hi, >> >> On 11/3/21 18:17, Daniel Thompson wrote: >>> On Tue, Nov 02, 2021 at 11:55:04PM +0100, Hans de Goede wrote: >>>> The Xiaomi Mi Pad 2 tablet uses an ACPI enumerated LP8556 backlight >>>> controller for its LCD-panel, with a Xiaomi specific ACPI HID of >>>> "XMCC0001", add support for this. >>>> >>>> Note the new "if (id)" check also fixes a NULL pointer deref when a user >>>> tries to manually bind the driver from sysfs. >>>> >>>> When CONFIG_ACPI is disabled acpi_match_device() will always return NULL, >>>> so the lp855x_parse_acpi() call will get optimized away. >>>> >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> >>> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> >> >> Thank you. >> >> So what is the process for upstreaming backlight patches, >> do these go through drm-misc-next (in that case I can push >> the series myself), or will you pick these up ? > > Lee Jones gathers up the backlight patches and sends a PR (but, except > in exceptional cases, treats my R-b as a pre-requisite before doing so). Ok, thanks. Regards, Hans
On Wed, 03 Nov 2021, Daniel Thompson wrote: > On Wed, Nov 03, 2021 at 06:28:15PM +0100, Hans de Goede wrote: > > Hi, > > > > On 11/3/21 18:17, Daniel Thompson wrote: > > > On Tue, Nov 02, 2021 at 11:55:04PM +0100, Hans de Goede wrote: > > >> The Xiaomi Mi Pad 2 tablet uses an ACPI enumerated LP8556 backlight > > >> controller for its LCD-panel, with a Xiaomi specific ACPI HID of > > >> "XMCC0001", add support for this. > > >> > > >> Note the new "if (id)" check also fixes a NULL pointer deref when a user > > >> tries to manually bind the driver from sysfs. > > >> > > >> When CONFIG_ACPI is disabled acpi_match_device() will always return NULL, > > >> so the lp855x_parse_acpi() call will get optimized away. > > >> > > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > > > > > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> > > > > Thank you. > > > > So what is the process for upstreaming backlight patches, > > do these go through drm-misc-next (in that case I can push > > the series myself), or will you pick these up ? > > Lee Jones gathers up the backlight patches and sends a PR (but, except > in exceptional cases, treats my R-b as a pre-requisite before doing so). Also the merge-window is open, so this is headed for v5.17.
Hi, On 11/4/21 09:29, Lee Jones wrote: > On Wed, 03 Nov 2021, Daniel Thompson wrote: > >> On Wed, Nov 03, 2021 at 06:28:15PM +0100, Hans de Goede wrote: >>> Hi, >>> >>> On 11/3/21 18:17, Daniel Thompson wrote: >>>> On Tue, Nov 02, 2021 at 11:55:04PM +0100, Hans de Goede wrote: >>>>> The Xiaomi Mi Pad 2 tablet uses an ACPI enumerated LP8556 backlight >>>>> controller for its LCD-panel, with a Xiaomi specific ACPI HID of >>>>> "XMCC0001", add support for this. >>>>> >>>>> Note the new "if (id)" check also fixes a NULL pointer deref when a user >>>>> tries to manually bind the driver from sysfs. >>>>> >>>>> When CONFIG_ACPI is disabled acpi_match_device() will always return NULL, >>>>> so the lp855x_parse_acpi() call will get optimized away. >>>>> >>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>> >>>> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> >>> >>> Thank you. >>> >>> So what is the process for upstreaming backlight patches, >>> do these go through drm-misc-next (in that case I can push >>> the series myself), or will you pick these up ? >> >> Lee Jones gathers up the backlight patches and sends a PR (but, except >> in exceptional cases, treats my R-b as a pre-requisite before doing so). > > Also the merge-window is open, so this is headed for v5.17. Right, I didn't expect anything else. Regards, Hans
On Tue, 02 Nov 2021, Hans de Goede wrote: > The Xiaomi Mi Pad 2 tablet uses an ACPI enumerated LP8556 backlight > controller for its LCD-panel, with a Xiaomi specific ACPI HID of > "XMCC0001", add support for this. > > Note the new "if (id)" check also fixes a NULL pointer deref when a user > tries to manually bind the driver from sysfs. > > When CONFIG_ACPI is disabled acpi_match_device() will always return NULL, > so the lp855x_parse_acpi() call will get optimized away. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v2: > - Remove `lp->pdata = pdata` assignment from lp855x_parse_dt() > - Add "and is in register mode" to the comment in > lp855x_parse_acpi() > --- > drivers/video/backlight/lp855x_bl.c | 73 ++++++++++++++++++++++++----- > 1 file changed, 61 insertions(+), 12 deletions(-) Applied, thanks.
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c index d1d27d5eb0f2..2b9e2bbbb03e 100644 --- a/drivers/video/backlight/lp855x_bl.c +++ b/drivers/video/backlight/lp855x_bl.c @@ -5,6 +5,7 @@ * Copyright (C) 2011 Texas Instruments */ +#include <linux/acpi.h> #include <linux/module.h> #include <linux/slab.h> #include <linux/i2c.h> @@ -330,7 +331,7 @@ static int lp855x_parse_dt(struct lp855x *lp) { struct device *dev = lp->dev; struct device_node *node = dev->of_node; - struct lp855x_platform_data *pdata; + struct lp855x_platform_data *pdata = lp->pdata; int rom_length; if (!node) { @@ -338,10 +339,6 @@ static int lp855x_parse_dt(struct lp855x *lp) return -EINVAL; } - pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); - if (!pdata) - return -ENOMEM; - of_property_read_string(node, "bl-name", &pdata->name); of_property_read_u8(node, "dev-ctrl", &pdata->device_control); of_property_read_u8(node, "init-brt", &pdata->initial_brightness); @@ -368,8 +365,6 @@ static int lp855x_parse_dt(struct lp855x *lp) pdata->rom_data = &rom[0]; } - lp->pdata = pdata; - return 0; } #else @@ -379,8 +374,32 @@ static int lp855x_parse_dt(struct lp855x *lp) } #endif +static int lp855x_parse_acpi(struct lp855x *lp) +{ + int ret; + + /* + * On ACPI the device has already been initialized by the firmware + * and is in register mode, so we can read back the settings from + * the registers. + */ + ret = i2c_smbus_read_byte_data(lp->client, lp->cfg->reg_brightness); + if (ret < 0) + return ret; + + lp->pdata->initial_brightness = ret; + + ret = i2c_smbus_read_byte_data(lp->client, lp->cfg->reg_devicectrl); + if (ret < 0) + return ret; + + lp->pdata->device_control = ret; + return 0; +} + static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id) { + const struct acpi_device_id *acpi_id = NULL; struct device *dev = &cl->dev; struct lp855x *lp; int ret; @@ -394,10 +413,20 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id) lp->client = cl; lp->dev = dev; - lp->chipname = id->name; - lp->chip_id = id->driver_data; lp->pdata = dev_get_platdata(dev); + if (id) { + lp->chipname = id->name; + lp->chip_id = id->driver_data; + } else { + acpi_id = acpi_match_device(dev->driver->acpi_match_table, dev); + if (!acpi_id) + return -ENODEV; + + lp->chipname = acpi_id->id; + lp->chip_id = acpi_id->driver_data; + } + switch (lp->chip_id) { case LP8550: case LP8551: @@ -415,9 +444,19 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id) } if (!lp->pdata) { - ret = lp855x_parse_dt(lp); - if (ret < 0) - return ret; + lp->pdata = devm_kzalloc(dev, sizeof(*lp->pdata), GFP_KERNEL); + if (!lp->pdata) + return -ENOMEM; + + if (id) { + ret = lp855x_parse_dt(lp); + if (ret < 0) + return ret; + } else { + ret = lp855x_parse_acpi(lp); + if (ret < 0) + return ret; + } } if (lp->pdata->period_ns > 0) @@ -537,10 +576,20 @@ static const struct i2c_device_id lp855x_ids[] = { }; MODULE_DEVICE_TABLE(i2c, lp855x_ids); +#ifdef CONFIG_ACPI +static const struct acpi_device_id lp855x_acpi_match[] = { + /* Xiaomi specific HID used for the LP8556 on the Mi Pad 2 */ + { "XMCC0001", LP8556 }, + { } +}; +MODULE_DEVICE_TABLE(acpi, lp855x_acpi_match); +#endif + static struct i2c_driver lp855x_driver = { .driver = { .name = "lp855x", .of_match_table = of_match_ptr(lp855x_dt_ids), + .acpi_match_table = ACPI_PTR(lp855x_acpi_match), }, .probe = lp855x_probe, .remove = lp855x_remove,
The Xiaomi Mi Pad 2 tablet uses an ACPI enumerated LP8556 backlight controller for its LCD-panel, with a Xiaomi specific ACPI HID of "XMCC0001", add support for this. Note the new "if (id)" check also fixes a NULL pointer deref when a user tries to manually bind the driver from sysfs. When CONFIG_ACPI is disabled acpi_match_device() will always return NULL, so the lp855x_parse_acpi() call will get optimized away. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v2: - Remove `lp->pdata = pdata` assignment from lp855x_parse_dt() - Add "and is in register mode" to the comment in lp855x_parse_acpi() --- drivers/video/backlight/lp855x_bl.c | 73 ++++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 12 deletions(-)