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 <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
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)
--
2.31.1
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 <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
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;
}
--
2.31.1
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 <[email protected]>
---
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,
--
2.31.1
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 <[email protected]>
Reviewed-by: Daniel Thompson <[email protected]>
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 <[email protected]>
>
> Reviewed-by: Daniel Thompson <[email protected]>
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 <[email protected]>
> >
> > Reviewed-by: Daniel Thompson <[email protected]>
>
> 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 <[email protected]>
>>>
>>> Reviewed-by: Daniel Thompson <[email protected]>
>>
>> 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 <[email protected]>
> > >
> > > Reviewed-by: Daniel Thompson <[email protected]>
> >
> > 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.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
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 <[email protected]>
>>>>
>>>> Reviewed-by: Daniel Thompson <[email protected]>
>>>
>>> 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:
> 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 <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>
Please keep sign-offs in chronological order.
Applied, thanks.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
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 <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>
> ---
> drivers/video/backlight/lp855x_bl.c | 29 +++++++++++++----------------
> 1 file changed, 13 insertions(+), 16 deletions(-)
Applied, thanks.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
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 <[email protected]>
> ---
> 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.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog