Move the setting of the lp->cfg pointer to the chip specific lp855x_device_config struct from lp855x_configure() to lp855x_probe(), before calling lp855x_parse_dt().
This is a preperation patch for adding ACPI enumeration support.
Reviewed-by: Daniel Thompson daniel.thompson@linaro.org Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/video/backlight/lp855x_bl.c | 32 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c index e94932c69f54..808ff00b2003 100644 --- a/drivers/video/backlight/lp855x_bl.c +++ b/drivers/video/backlight/lp855x_bl.c @@ -170,22 +170,6 @@ static int lp855x_configure(struct lp855x *lp) int i, ret; struct lp855x_platform_data *pd = lp->pdata;
- switch (lp->chip_id) { - case LP8550: - case LP8551: - case LP8552: - case LP8553: - case LP8556: - lp->cfg = &lp855x_dev_cfg; - break; - case LP8555: - case LP8557: - lp->cfg = &lp8557_dev_cfg; - break; - default: - return -EINVAL; - } - if (lp->cfg->pre_init_device) { ret = lp->cfg->pre_init_device(lp); if (ret) { @@ -413,6 +397,22 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id) lp->chip_id = id->driver_data; lp->pdata = dev_get_platdata(&cl->dev);
+ switch (lp->chip_id) { + case LP8550: + case LP8551: + case LP8552: + case LP8553: + case LP8556: + lp->cfg = &lp855x_dev_cfg; + break; + case LP8555: + case LP8557: + lp->cfg = &lp8557_dev_cfg; + break; + default: + return -EINVAL; + } + if (!lp->pdata) { ret = lp855x_parse_dt(lp); if (ret < 0)
Add a dev local variable to the lp855x_probe(), to replace "&cl->dev" and "lp->dev" in various places.
Also switch to dev_err_probe() in one case which takes care of not printing -EPROBE_DEFER errors for us.
This is mostly a preparation for adding ACPI enumeration support which will use the new "dev" variable more.
Reviewed-by: Daniel Thompson daniel.thompson@linaro.org Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/video/backlight/lp855x_bl.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c index 808ff00b2003..d1d27d5eb0f2 100644 --- a/drivers/video/backlight/lp855x_bl.c +++ b/drivers/video/backlight/lp855x_bl.c @@ -381,21 +381,22 @@ static int lp855x_parse_dt(struct lp855x *lp)
static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id) { + struct device *dev = &cl->dev; struct lp855x *lp; int ret;
if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) return -EIO;
- lp = devm_kzalloc(&cl->dev, sizeof(struct lp855x), GFP_KERNEL); + lp = devm_kzalloc(dev, sizeof(struct lp855x), GFP_KERNEL); if (!lp) return -ENOMEM;
lp->client = cl; - lp->dev = &cl->dev; + lp->dev = dev; lp->chipname = id->name; lp->chip_id = id->driver_data; - lp->pdata = dev_get_platdata(&cl->dev); + lp->pdata = dev_get_platdata(dev);
switch (lp->chip_id) { case LP8550: @@ -424,30 +425,27 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id) else lp->mode = REGISTER_BASED;
- lp->supply = devm_regulator_get(lp->dev, "power"); + lp->supply = devm_regulator_get(dev, "power"); if (IS_ERR(lp->supply)) { if (PTR_ERR(lp->supply) == -EPROBE_DEFER) return -EPROBE_DEFER; lp->supply = NULL; }
- lp->enable = devm_regulator_get_optional(lp->dev, "enable"); + lp->enable = devm_regulator_get_optional(dev, "enable"); if (IS_ERR(lp->enable)) { ret = PTR_ERR(lp->enable); if (ret == -ENODEV) { lp->enable = NULL; } else { - if (ret != -EPROBE_DEFER) - dev_err(lp->dev, "error getting enable regulator: %d\n", - ret); - return ret; + return dev_err_probe(dev, ret, "getting enable regulator\n"); } }
if (lp->supply) { ret = regulator_enable(lp->supply); if (ret < 0) { - dev_err(&cl->dev, "failed to enable supply: %d\n", ret); + dev_err(dev, "failed to enable supply: %d\n", ret); return ret; } } @@ -455,7 +453,7 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id) if (lp->enable) { ret = regulator_enable(lp->enable); if (ret < 0) { - dev_err(lp->dev, "failed to enable vddio: %d\n", ret); + dev_err(dev, "failed to enable vddio: %d\n", ret); goto disable_supply; }
@@ -470,20 +468,19 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
ret = lp855x_configure(lp); if (ret) { - dev_err(lp->dev, "device config err: %d", ret); + dev_err(dev, "device config err: %d", ret); goto disable_vddio; }
ret = lp855x_backlight_register(lp); if (ret) { - dev_err(lp->dev, - "failed to register backlight. err: %d\n", ret); + dev_err(dev, "failed to register backlight. err: %d\n", ret); goto disable_vddio; }
- ret = sysfs_create_group(&lp->dev->kobj, &lp855x_attr_group); + ret = sysfs_create_group(&dev->kobj, &lp855x_attr_group); if (ret) { - dev_err(lp->dev, "failed to register sysfs. err: %d\n", ret); + dev_err(dev, "failed to register sysfs. err: %d\n", ret); goto disable_vddio; }
On Tue, 02 Nov 2021, Hans de Goede wrote:
Add a dev local variable to the lp855x_probe(), to replace "&cl->dev" and "lp->dev" in various places.
Also switch to dev_err_probe() in one case which takes care of not printing -EPROBE_DEFER errors for us.
This is mostly a preparation for adding ACPI enumeration support which will use the new "dev" variable more.
Reviewed-by: Daniel Thompson daniel.thompson@linaro.org Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/video/backlight/lp855x_bl.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-)
Applied, thanks.
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(-)
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,
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.
On Tue, 02 Nov 2021, Hans de Goede wrote:
Move the setting of the lp->cfg pointer to the chip specific lp855x_device_config struct from lp855x_configure() to lp855x_probe(), before calling lp855x_parse_dt().
This is a preperation patch for adding ACPI enumeration support.
Reviewed-by: Daniel Thompson daniel.thompson@linaro.org Signed-off-by: Hans de Goede hdegoede@redhat.com
Please keep sign-offs in chronological order.
Applied, thanks.
dri-devel@lists.freedesktop.org