* [PATCH v2 2/6] mfd: lm3533: Convert to use OF bindings
From: Svyatoslav Ryhel @ 2026-05-28 13:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
In-Reply-To: <20260528135123.103745-1-clamor95@gmail.com>
Since there are no users of this driver via platform data, remove the
platform data support and switch to using Device Tree bindings.
Additionally, optimize functions used only by platform data.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/iio/light/lm3533-als.c | 95 ++++------
drivers/leds/leds-lm3533.c | 51 ++++--
drivers/mfd/lm3533-core.c | 268 ++++++++++------------------
drivers/video/backlight/lm3533_bl.c | 52 ++++--
include/linux/mfd/lm3533.h | 51 +-----
5 files changed, 212 insertions(+), 305 deletions(-)
diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
index 99f0b903018c..cbd337b73bd9 100644
--- a/drivers/iio/light/lm3533-als.c
+++ b/drivers/iio/light/lm3533-als.c
@@ -16,15 +16,18 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/mfd/core.h>
+#include <linux/mod_devicetable.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
+#include <linux/units.h>
#include <linux/mfd/lm3533.h>
-#define LM3533_ALS_RESISTOR_MIN 1
-#define LM3533_ALS_RESISTOR_MAX 127
+#define LM3533_ALS_RESISTOR_MIN 1575
+#define LM3533_ALS_RESISTOR_MAX 200000
#define LM3533_ALS_CHANNEL_CURRENT_MAX 2
#define LM3533_ALS_THRESH_MAX 3
#define LM3533_ALS_ZONE_MAX 4
@@ -56,6 +59,9 @@ struct lm3533_als {
atomic_t zone;
struct mutex thresh_mutex;
+
+ bool pwm_mode;
+ u32 r_select;
};
@@ -714,59 +720,33 @@ static const struct attribute_group lm3533_als_attribute_group = {
.attrs = lm3533_als_attributes
};
-static int lm3533_als_set_input_mode(struct lm3533_als *als, bool pwm_mode)
+static int lm3533_als_setup(struct lm3533_als *als)
{
- u8 mask = LM3533_ALS_INPUT_MODE_MASK;
- u8 val;
+ struct device *dev = &als->pdev.dev;
int ret;
- if (pwm_mode)
- val = mask; /* pwm input */
- else
- val = 0; /* analog input */
-
- ret = lm3533_update(als->lm3533, LM3533_REG_ALS_CONF, val, mask);
- if (ret) {
- dev_err(&als->pdev->dev, "failed to set input mode %d\n",
- pwm_mode);
- return ret;
- }
-
- return 0;
-}
-
-static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
-{
- int ret;
-
- if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX) {
- dev_err(&als->pdev->dev, "invalid resistor value\n");
- return -EINVAL;
- }
-
- ret = lm3533_write(als->lm3533, LM3533_REG_ALS_RESISTOR_SELECT, val);
- if (ret) {
- dev_err(&als->pdev->dev, "failed to set resistor\n");
- return ret;
- }
+ device_property_read_u32(dev, "ti,resistor-value-ohm",
+ &als->r_select);
- return 0;
-}
+ als->r_select = clamp(als->r_select, LM3533_ALS_RESISTOR_MIN,
+ LM3533_ALS_RESISTOR_MAX);
+ als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * als->r_select);
-static int lm3533_als_setup(struct lm3533_als *als,
- const struct lm3533_als_platform_data *pdata)
-{
- int ret;
+ als->pwm_mode = device_property_read_bool(dev, "ti,pwm-mode");
- ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
+ ret = lm3533_update(lm3533, LM3533_REG_ALS_CONF, als->pwm_mode ?
+ LM3533_ALS_INPUT_MODE_MASK : 0,
+ LM3533_ALS_INPUT_MODE_MASK);
if (ret)
- return ret;
+ return dev_err_probe(dev, ret, "failed to set input mode %d\n",
+ als->pwm_mode);
/* ALS input is always high impedance in PWM-mode. */
- if (!pdata->pwm_mode) {
- ret = lm3533_als_set_resistor(als, pdata->r_select);
+ if (!als->pwm_mode) {
+ ret = lm3533_write(lm3533, LM3533_REG_ALS_RESISTOR_SELECT,
+ (u8)als->r_select);
if (ret)
- return ret;
+ return dev_err_probe(dev, ret, "failed to set resistor\n");
}
return 0;
@@ -828,7 +808,6 @@ static const struct iio_info lm3533_als_info = {
static int lm3533_als_probe(struct platform_device *pdev)
{
- const struct lm3533_als_platform_data *pdata;
struct lm3533 *lm3533;
struct lm3533_als *als;
struct iio_dev *indio_dev;
@@ -838,12 +817,6 @@ static int lm3533_als_probe(struct platform_device *pdev)
if (!lm3533)
return -EINVAL;
- pdata = dev_get_platdata(&pdev->dev);
- if (!pdata) {
- dev_err(&pdev->dev, "no platform data\n");
- return -EINVAL;
- }
-
indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*als));
if (!indio_dev)
return -ENOMEM;
@@ -852,25 +825,28 @@ static int lm3533_als_probe(struct platform_device *pdev)
indio_dev->channels = lm3533_als_channels;
indio_dev->num_channels = ARRAY_SIZE(lm3533_als_channels);
indio_dev->name = dev_name(&pdev->dev);
- iio_device_set_parent(indio_dev, pdev->dev.parent);
indio_dev->modes = INDIO_DIRECT_MODE;
als = iio_priv(indio_dev);
als->lm3533 = lm3533;
als->pdev = pdev;
- als->irq = lm3533->irq;
+ als->irq = platform_get_irq_optional(pdev, 0);
+
+ if (als->irq == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
atomic_set(&als->zone, 0);
mutex_init(&als->thresh_mutex);
platform_set_drvdata(pdev, indio_dev);
- if (als->irq) {
+ if (als->irq > 0) {
ret = lm3533_als_setup_irq(als, indio_dev);
if (ret)
return ret;
}
- ret = lm3533_als_setup(als, pdata);
+ ret = lm3533_als_setup(als);
if (ret)
goto err_free_irq;
@@ -907,9 +883,16 @@ static void lm3533_als_remove(struct platform_device *pdev)
free_irq(als->irq, indio_dev);
}
+static const struct of_device_id lm3533_als_match_table[] = {
+ { .compatible = "ti,lm3533-als" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, lm3533_als_match_table);
+
static struct platform_driver lm3533_als_driver = {
.driver = {
.name = "lm3533-als",
+ .of_match_table = lm3533_als_match_table,
},
.probe = lm3533_als_probe,
.remove = lm3533_als_remove,
diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index 45795f2a1042..d707d43d5526 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -10,8 +10,10 @@
#include <linux/module.h>
#include <linux/leds.h>
#include <linux/mfd/core.h>
+#include <linux/mod_devicetable.h>
#include <linux/mutex.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/slab.h>
#include <linux/mfd/lm3533.h>
@@ -48,6 +50,9 @@ struct lm3533_led {
struct mutex mutex;
unsigned long flags;
+
+ u32 max_current;
+ u32 pwm;
};
@@ -632,22 +637,20 @@ static const struct attribute_group *lm3533_led_attribute_groups[] = {
NULL
};
-static int lm3533_led_setup(struct lm3533_led *led,
- struct lm3533_led_platform_data *pdata)
+static int lm3533_led_setup(struct lm3533_led *led)
{
int ret;
- ret = lm3533_ctrlbank_set_max_current(&led->cb, pdata->max_current);
+ ret = lm3533_ctrlbank_set_max_current(&led->cb, led->max_current);
if (ret)
return ret;
- return lm3533_ctrlbank_set_pwm(&led->cb, pdata->pwm);
+ return lm3533_ctrlbank_set_pwm(&led->cb, (u8)led->pwm);
}
static int lm3533_led_probe(struct platform_device *pdev)
{
struct lm3533 *lm3533;
- struct lm3533_led_platform_data *pdata;
struct lm3533_led *led;
int ret;
@@ -657,12 +660,6 @@ static int lm3533_led_probe(struct platform_device *pdev)
if (!lm3533)
return -EINVAL;
- pdata = dev_get_platdata(&pdev->dev);
- if (!pdata) {
- dev_err(&pdev->dev, "no platform data\n");
- return -EINVAL;
- }
-
if (pdev->id < 0 || pdev->id >= LM3533_LVCTRLBANK_COUNT) {
dev_err(&pdev->dev, "illegal LED id %d\n", pdev->id);
return -EINVAL;
@@ -673,8 +670,6 @@ static int lm3533_led_probe(struct platform_device *pdev)
return -ENOMEM;
led->lm3533 = lm3533;
- led->cdev.name = pdata->name;
- led->cdev.default_trigger = pdata->default_trigger;
led->cdev.brightness_set_blocking = lm3533_led_set;
led->cdev.brightness_get = lm3533_led_get;
led->cdev.blink_set = lm3533_led_blink_set;
@@ -682,6 +677,15 @@ static int lm3533_led_probe(struct platform_device *pdev)
led->cdev.groups = lm3533_led_attribute_groups;
led->id = pdev->id;
+ led->cdev.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-%d",
+ pdev->name, led->id);
+ if (!led->cdev.name)
+ return -ENOMEM;
+
+ led->cdev.default_trigger = "none";
+ device_property_read_string(&pdev->dev, "linux,default-trigger",
+ &led->cdev.default_trigger);
+
mutex_init(&led->mutex);
/* The class framework makes a callback to get brightness during
@@ -694,15 +698,23 @@ static int lm3533_led_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, led);
- ret = led_classdev_register(pdev->dev.parent, &led->cdev);
+ ret = led_classdev_register(&pdev->dev, &led->cdev);
if (ret) {
- dev_err(&pdev->dev, "failed to register LED %d\n", pdev->id);
+ dev_err(&pdev->dev, "failed to register LED %d\n", led->id);
return ret;
}
led->cb.dev = led->cdev.dev;
- ret = lm3533_led_setup(led, pdata);
+ device_property_read_u32(&pdev->dev, "led-max-microamp",
+ &led->max_current);
+ led->max_current = clamp(led->max_current, LM3533_LED_MAX_CURRENT_MIN,
+ LM3533_LED_MAX_CURRENT_MAX);
+
+ led->pwm = 0;
+ device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
+
+ ret = lm3533_led_setup(led);
if (ret)
goto err_deregister;
@@ -739,9 +751,16 @@ static void lm3533_led_shutdown(struct platform_device *pdev)
lm3533_led_set(&led->cdev, LED_OFF); /* disable blink */
}
+static const struct of_device_id lm3533_led_match_table[] = {
+ { .compatible = "ti,lm3533-leds" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, lm3533_led_match_table);
+
static struct platform_driver lm3533_led_driver = {
.driver = {
.name = "lm3533-leds",
+ .of_match_table = lm3533_led_match_table,
},
.probe = lm3533_led_probe,
.remove = lm3533_led_remove,
diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index 0a2409d00b2e..8495e9119871 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -14,19 +14,26 @@
#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/mfd/core.h>
+#include <linux/mod_devicetable.h>
+#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
+#include <linux/units.h>
#include <linux/mfd/lm3533.h>
#define LM3533_BOOST_OVP_MASK 0x06
#define LM3533_BOOST_OVP_SHIFT 1
+#define LM3533_BOOST_OVP_MIN 16000000
+#define LM3533_BOOST_OVP_MAX 40000000
#define LM3533_BOOST_FREQ_MASK 0x01
#define LM3533_BOOST_FREQ_SHIFT 0
+#define LM3533_BOOST_FREQ_MIN 500000
+#define LM3533_BOOST_FREQ_MAX 1000000
#define LM3533_BL_ID_MASK 1
#define LM3533_LED_ID_MASK 3
@@ -35,6 +42,7 @@
#define LM3533_HVLED_ID_MAX 2
#define LM3533_LVLED_ID_MAX 5
+#define LM3533_CELLS_MAX 7
#define LM3533_REG_OUTPUT_CONF1 0x10
#define LM3533_REG_OUTPUT_CONF2 0x11
@@ -42,44 +50,6 @@
#define LM3533_REG_MAX 0xb2
-
-static struct mfd_cell lm3533_als_devs[] = {
- {
- .name = "lm3533-als",
- .id = -1,
- },
-};
-
-static struct mfd_cell lm3533_bl_devs[] = {
- {
- .name = "lm3533-backlight",
- .id = 0,
- },
- {
- .name = "lm3533-backlight",
- .id = 1,
- },
-};
-
-static struct mfd_cell lm3533_led_devs[] = {
- {
- .name = "lm3533-leds",
- .id = 0,
- },
- {
- .name = "lm3533-leds",
- .id = 1,
- },
- {
- .name = "lm3533-leds",
- .id = 2,
- },
- {
- .name = "lm3533-leds",
- .id = 3,
- },
-};
-
int lm3533_read(struct lm3533 *lm3533, u8 reg, u8 *val)
{
int tmp;
@@ -132,35 +102,6 @@ int lm3533_update(struct lm3533 *lm3533, u8 reg, u8 val, u8 mask)
}
EXPORT_SYMBOL_GPL(lm3533_update);
-static int lm3533_set_boost_freq(struct lm3533 *lm3533,
- enum lm3533_boost_freq freq)
-{
- int ret;
-
- ret = lm3533_update(lm3533, LM3533_REG_BOOST_PWM,
- freq << LM3533_BOOST_FREQ_SHIFT,
- LM3533_BOOST_FREQ_MASK);
- if (ret)
- dev_err(lm3533->dev, "failed to set boost frequency\n");
-
- return ret;
-}
-
-
-static int lm3533_set_boost_ovp(struct lm3533 *lm3533,
- enum lm3533_boost_ovp ovp)
-{
- int ret;
-
- ret = lm3533_update(lm3533, LM3533_REG_BOOST_PWM,
- ovp << LM3533_BOOST_OVP_SHIFT,
- LM3533_BOOST_OVP_MASK);
- if (ret)
- dev_err(lm3533->dev, "failed to set boost ovp\n");
-
- return ret;
-}
-
/*
* HVLED output config -- output hvled controlled by backlight bl
*/
@@ -376,135 +317,93 @@ static struct attribute_group lm3533_attribute_group = {
.attrs = lm3533_attributes
};
-static int lm3533_device_als_init(struct lm3533 *lm3533)
+static int lm3533_device_init(struct lm3533 *lm3533)
{
- struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
+ struct device *dev = lm3533->dev;
+ struct mfd_cell lm3533_cells[LM3533_CELLS_MAX];
+ u32 count = 0, reg;
int ret;
- if (!pdata->als)
- return 0;
-
- lm3533_als_devs[0].platform_data = pdata->als;
- lm3533_als_devs[0].pdata_size = sizeof(*pdata->als);
-
- ret = mfd_add_devices(lm3533->dev, 0, lm3533_als_devs, 1, NULL,
- 0, NULL);
- if (ret) {
- dev_err(lm3533->dev, "failed to add ALS device\n");
- return ret;
- }
-
- lm3533->have_als = 1;
-
- return 0;
-}
+ lm3533_enable(lm3533);
-static int lm3533_device_bl_init(struct lm3533 *lm3533)
-{
- struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
- int i;
- int ret;
+ device_for_each_child_node_scoped(lm3533->dev, child) {
+ if (!fwnode_device_is_available(child))
+ continue;
- if (!pdata->backlights || pdata->num_backlights == 0)
- return 0;
+ if (count >= LM3533_CELLS_MAX)
+ break;
- if (pdata->num_backlights > ARRAY_SIZE(lm3533_bl_devs))
- pdata->num_backlights = ARRAY_SIZE(lm3533_bl_devs);
+ if (fwnode_device_is_compatible(child, "ti,lm3533-als")) {
+ lm3533_cells[count].name = "lm3533-als";
+ lm3533_cells[count].id = PLATFORM_DEVID_NONE;
+ lm3533_cells[count].of_compatible = "ti,lm3533-als";
- for (i = 0; i < pdata->num_backlights; ++i) {
- lm3533_bl_devs[i].platform_data = &pdata->backlights[i];
- lm3533_bl_devs[i].pdata_size = sizeof(pdata->backlights[i]);
- }
+ lm3533->have_als = true;
+ }
- ret = mfd_add_devices(lm3533->dev, 0, lm3533_bl_devs,
- pdata->num_backlights, NULL, 0, NULL);
- if (ret) {
- dev_err(lm3533->dev, "failed to add backlight devices\n");
- return ret;
- }
+ if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) {
+ ret = fwnode_property_read_u32(child, "reg", ®);
+ if (ret || reg > LM3533_HVLED_ID_MAX) {
+ dev_err(dev, "invalid backlight reg %d\n", reg);
+ continue;
+ }
- lm3533->have_backlights = 1;
+ lm3533_cells[count].name = "lm3533-backlight";
+ lm3533_cells[count].id = reg;
+ lm3533_cells[count].of_compatible = "ti,lm3533-backlight";
- return 0;
-}
+ lm3533->have_backlights = true;
+ }
-static int lm3533_device_led_init(struct lm3533 *lm3533)
-{
- struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
- int i;
- int ret;
+ if (fwnode_device_is_compatible(child, "ti,lm3533-leds")) {
+ ret = fwnode_property_read_u32(child, "reg", ®);
+ if (ret || reg < LM3533_HVLED_ID_MAX ||
+ reg > LM3533_LVLED_ID_MAX) {
+ dev_err(dev, "invalid LED reg %d\n", reg);
+ continue;
+ }
- if (!pdata->leds || pdata->num_leds == 0)
- return 0;
+ lm3533_cells[count].name = "lm3533-leds";
+ lm3533_cells[count].id = reg - LM3533_HVLED_ID_MAX;
+ lm3533_cells[count].of_compatible = "ti,lm3533-leds";
- if (pdata->num_leds > ARRAY_SIZE(lm3533_led_devs))
- pdata->num_leds = ARRAY_SIZE(lm3533_led_devs);
+ lm3533->have_leds = true;
+ }
- for (i = 0; i < pdata->num_leds; ++i) {
- lm3533_led_devs[i].platform_data = &pdata->leds[i];
- lm3533_led_devs[i].pdata_size = sizeof(pdata->leds[i]);
+ count++;
}
- ret = mfd_add_devices(lm3533->dev, 0, lm3533_led_devs,
- pdata->num_leds, NULL, 0, NULL);
+ ret = lm3533_update(lm3533, LM3533_REG_BOOST_PWM,
+ lm3533->boost_freq << LM3533_BOOST_FREQ_SHIFT,
+ LM3533_BOOST_FREQ_MASK);
if (ret) {
- dev_err(lm3533->dev, "failed to add LED devices\n");
- return ret;
+ dev_err(dev, "failed to set boost frequency\n");
+ goto err_disable;
}
- lm3533->have_leds = 1;
-
- return 0;
-}
-
-static int lm3533_device_setup(struct lm3533 *lm3533,
- struct lm3533_platform_data *pdata)
-{
- int ret;
-
- ret = lm3533_set_boost_freq(lm3533, pdata->boost_freq);
- if (ret)
- return ret;
-
- return lm3533_set_boost_ovp(lm3533, pdata->boost_ovp);
-}
-
-static int lm3533_device_init(struct lm3533 *lm3533)
-{
- struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
- int ret;
-
- dev_dbg(lm3533->dev, "%s\n", __func__);
-
- if (!pdata) {
- dev_err(lm3533->dev, "no platform data\n");
- return -EINVAL;
+ ret = lm3533_update(lm3533, LM3533_REG_BOOST_PWM,
+ lm3533->boost_ovp << LM3533_BOOST_OVP_SHIFT,
+ LM3533_BOOST_OVP_MASK);
+ if (ret) {
+ dev_err(dev, "failed to set boost ovp\n");
+ goto err_disable;
}
- lm3533->hwen = devm_gpiod_get(lm3533->dev, NULL, GPIOD_OUT_LOW);
- if (IS_ERR(lm3533->hwen))
- return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->hwen), "failed to request HWEN GPIO\n");
- gpiod_set_consumer_name(lm3533->hwen, "lm3533-hwen");
-
- lm3533_enable(lm3533);
-
- ret = lm3533_device_setup(lm3533, pdata);
- if (ret)
+ ret = mfd_add_devices(dev, 0, lm3533_cells, count, NULL, 0, NULL);
+ if (ret) {
+ dev_err(dev, "failed to add MFD devices: %d\n", ret);
goto err_disable;
+ }
- lm3533_device_als_init(lm3533);
- lm3533_device_bl_init(lm3533);
- lm3533_device_led_init(lm3533);
-
- ret = sysfs_create_group(&lm3533->dev->kobj, &lm3533_attribute_group);
- if (ret < 0) {
- dev_err(lm3533->dev, "failed to create sysfs attributes\n");
- goto err_unregister;
+ ret = sysfs_create_group(&dev->kobj, &lm3533_attribute_group);
+ if (ret) {
+ dev_err(dev, "failed to create sysfs attributes\n");
+ goto err_remove_devices;
}
return 0;
-err_unregister:
+err_remove_devices:
mfd_remove_devices(lm3533->dev);
err_disable:
lm3533_disable(lm3533);
@@ -517,8 +416,6 @@ static void lm3533_device_exit(struct lm3533 *lm3533)
dev_dbg(lm3533->dev, "%s\n", __func__);
sysfs_remove_group(&lm3533->dev->kobj, &lm3533_attribute_group);
-
- mfd_remove_devices(lm3533->dev);
lm3533_disable(lm3533);
}
@@ -589,7 +486,26 @@ static int lm3533_i2c_probe(struct i2c_client *i2c)
return PTR_ERR(lm3533->regmap);
lm3533->dev = &i2c->dev;
- lm3533->irq = i2c->irq;
+
+ lm3533->hwen = devm_gpiod_get_optional(lm3533->dev, "enable",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(lm3533->hwen))
+ return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->hwen),
+ "failed to get HWEN GPIO\n");
+
+ device_property_read_u32(lm3533->dev, "ti,boost-ovp-microvolt",
+ &lm3533->boost_ovp);
+
+ lm3533->boost_ovp = clamp(lm3533->boost_ovp, LM3533_BOOST_OVP_MIN,
+ LM3533_BOOST_OVP_MAX);
+ lm3533->boost_ovp = lm3533->boost_ovp / (8 * MICRO) - 2;
+
+ device_property_read_u32(lm3533->dev, "ti,boost-freq-hz",
+ &lm3533->boost_freq);
+
+ lm3533->boost_freq = clamp(lm3533->boost_freq, LM3533_BOOST_FREQ_MIN,
+ LM3533_BOOST_FREQ_MAX);
+ lm3533->boost_freq = lm3533->boost_freq / (500 * KILO) - 1;
return lm3533_device_init(lm3533);
}
@@ -600,9 +516,16 @@ static void lm3533_i2c_remove(struct i2c_client *i2c)
dev_dbg(&i2c->dev, "%s\n", __func__);
+ mfd_remove_devices(lm3533->dev);
lm3533_device_exit(lm3533);
}
+static const struct of_device_id lm3533_match_table[] = {
+ { .compatible = "ti,lm3533" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, lm3533_match_table);
+
static const struct i2c_device_id lm3533_i2c_ids[] = {
{ "lm3533" },
{ }
@@ -612,6 +535,7 @@ MODULE_DEVICE_TABLE(i2c, lm3533_i2c_ids);
static struct i2c_driver lm3533_i2c_driver = {
.driver = {
.name = "lm3533",
+ .of_match_table = lm3533_match_table,
},
.id_table = lm3533_i2c_ids,
.probe = lm3533_i2c_probe,
diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index babfd3ceec86..42da652df58d 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -9,7 +9,9 @@
#include <linux/module.h>
#include <linux/init.h>
+#include <linux/mod_devicetable.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/backlight.h>
#include <linux/slab.h>
@@ -27,6 +29,9 @@ struct lm3533_bl {
struct lm3533_ctrlbank cb;
struct backlight_device *bd;
int id;
+
+ u32 max_current;
+ u32 pwm;
};
@@ -246,25 +251,24 @@ static struct attribute_group lm3533_bl_attribute_group = {
.attrs = lm3533_bl_attributes
};
-static int lm3533_bl_setup(struct lm3533_bl *bl,
- struct lm3533_bl_platform_data *pdata)
+static int lm3533_bl_setup(struct lm3533_bl *bl)
{
int ret;
- ret = lm3533_ctrlbank_set_max_current(&bl->cb, pdata->max_current);
+ ret = lm3533_ctrlbank_set_max_current(&bl->cb, bl->max_current);
if (ret)
return ret;
- return lm3533_ctrlbank_set_pwm(&bl->cb, pdata->pwm);
+ return lm3533_ctrlbank_set_pwm(&bl->cb, bl->pwm);
}
static int lm3533_bl_probe(struct platform_device *pdev)
{
struct lm3533 *lm3533;
- struct lm3533_bl_platform_data *pdata;
struct lm3533_bl *bl;
struct backlight_device *bd;
struct backlight_properties props;
+ char *name = NULL;
int ret;
dev_dbg(&pdev->dev, "%s\n", __func__);
@@ -273,12 +277,6 @@ static int lm3533_bl_probe(struct platform_device *pdev)
if (!lm3533)
return -EINVAL;
- pdata = dev_get_platdata(&pdev->dev);
- if (!pdata) {
- dev_err(&pdev->dev, "no platform data\n");
- return -EINVAL;
- }
-
if (pdev->id < 0 || pdev->id >= LM3533_HVCTRLBANK_COUNT) {
dev_err(&pdev->dev, "illegal backlight id %d\n", pdev->id);
return -EINVAL;
@@ -295,13 +293,20 @@ static int lm3533_bl_probe(struct platform_device *pdev)
bl->cb.id = lm3533_bl_get_ctrlbank_id(bl);
bl->cb.dev = NULL; /* until registered */
+ name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-%d",
+ pdev->name, pdev->id);
+ if (!name)
+ return -ENOMEM;
+
memset(&props, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
props.max_brightness = LM3533_BL_MAX_BRIGHTNESS;
- props.brightness = pdata->default_brightness;
- bd = devm_backlight_device_register(&pdev->dev, pdata->name,
- pdev->dev.parent, bl, &lm3533_bl_ops,
- &props);
+ props.brightness = LM3533_BL_MAX_BRIGHTNESS;
+ device_property_read_u32(&pdev->dev, "default-brightness",
+ &props.brightness);
+
+ bd = devm_backlight_device_register(&pdev->dev, name, &pdev->dev,
+ bl, &lm3533_bl_ops, &props);
if (IS_ERR(bd)) {
dev_err(&pdev->dev, "failed to register backlight device\n");
return PTR_ERR(bd);
@@ -320,7 +325,15 @@ static int lm3533_bl_probe(struct platform_device *pdev)
backlight_update_status(bd);
- ret = lm3533_bl_setup(bl, pdata);
+ device_property_read_u32(&pdev->dev, "led-max-microamp",
+ &bl->max_current);
+ bl->max_current = clamp(bl->max_current, LM3533_LED_MAX_CURRENT_MIN,
+ LM3533_LED_MAX_CURRENT_MAX);
+
+ bl->pwm = 0;
+ device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
+
+ ret = lm3533_bl_setup(bl);
if (ret)
goto err_sysfs_remove;
@@ -381,10 +394,17 @@ static void lm3533_bl_shutdown(struct platform_device *pdev)
lm3533_ctrlbank_disable(&bl->cb);
}
+static const struct of_device_id lm3533_bl_match_table[] = {
+ { .compatible = "ti,lm3533-backlight" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, lm3533_bl_match_table);
+
static struct platform_driver lm3533_bl_driver = {
.driver = {
.name = "lm3533-backlight",
.pm = &lm3533_bl_pm_ops,
+ .of_match_table = lm3533_bl_match_table,
},
.probe = lm3533_bl_probe,
.remove = lm3533_bl_remove,
diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h
index 69059a7a2ce5..3aa962d4c747 100644
--- a/include/linux/mfd/lm3533.h
+++ b/include/linux/mfd/lm3533.h
@@ -15,6 +15,9 @@
#define LM3533_ATTR_RW(_name) \
DEVICE_ATTR(_name, S_IRUGO | S_IWUSR , show_##_name, store_##_name)
+#define LM3533_LED_MAX_CURRENT_MIN 5000
+#define LM3533_LED_MAX_CURRENT_MAX 29800
+
struct device;
struct gpio_desc;
struct regmap;
@@ -25,7 +28,9 @@ struct lm3533 {
struct regmap *regmap;
struct gpio_desc *hwen;
- int irq;
+
+ u32 boost_ovp;
+ u32 boost_freq;
unsigned have_als:1;
unsigned have_backlights:1;
@@ -38,50 +43,6 @@ struct lm3533_ctrlbank {
int id;
};
-struct lm3533_als_platform_data {
- unsigned pwm_mode:1; /* PWM input mode (default analog) */
- u8 r_select; /* 1 - 127 (ignored in PWM-mode) */
-};
-
-struct lm3533_bl_platform_data {
- char *name;
- u16 max_current; /* 5000 - 29800 uA (800 uA step) */
- u8 default_brightness; /* 0 - 255 */
- u8 pwm; /* 0 - 0x3f */
-};
-
-struct lm3533_led_platform_data {
- char *name;
- const char *default_trigger;
- u16 max_current; /* 5000 - 29800 uA (800 uA step) */
- u8 pwm; /* 0 - 0x3f */
-};
-
-enum lm3533_boost_freq {
- LM3533_BOOST_FREQ_500KHZ,
- LM3533_BOOST_FREQ_1000KHZ,
-};
-
-enum lm3533_boost_ovp {
- LM3533_BOOST_OVP_16V,
- LM3533_BOOST_OVP_24V,
- LM3533_BOOST_OVP_32V,
- LM3533_BOOST_OVP_40V,
-};
-
-struct lm3533_platform_data {
- enum lm3533_boost_ovp boost_ovp;
- enum lm3533_boost_freq boost_freq;
-
- struct lm3533_als_platform_data *als;
-
- struct lm3533_bl_platform_data *backlights;
- int num_backlights;
-
- struct lm3533_led_platform_data *leds;
- int num_leds;
-};
-
extern int lm3533_ctrlbank_enable(struct lm3533_ctrlbank *cb);
extern int lm3533_ctrlbank_disable(struct lm3533_ctrlbank *cb);
--
2.51.0
^ permalink raw reply related
* [PATCH v2 3/6] mfd: lm3533: Add support for VIN power supply
From: Svyatoslav Ryhel @ 2026-05-28 13:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
In-Reply-To: <20260528135123.103745-1-clamor95@gmail.com>
Add support for 2.7V-5.5V VIN power supply.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/mfd/lm3533-core.c | 23 +++++++++++++++++++++--
include/linux/mfd/lm3533.h | 2 ++
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index 8495e9119871..519f8c16a3f3 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -17,6 +17,7 @@
#include <linux/mod_devicetable.h>
#include <linux/property.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
@@ -164,14 +165,25 @@ static int lm3533_set_lvled_config(struct lm3533 *lm3533, u8 lvled, u8 led)
return ret;
}
-static void lm3533_enable(struct lm3533 *lm3533)
+static int lm3533_enable(struct lm3533 *lm3533)
{
+ int ret;
+
+ ret = regulator_enable(lm3533->vin_supply);
+ if (ret) {
+ dev_err(lm3533->dev, "failed to enable vin power supply\n");
+ return ret;
+ }
+
gpiod_set_value(lm3533->hwen, 1);
+
+ return 0;
}
static void lm3533_disable(struct lm3533 *lm3533)
{
gpiod_set_value(lm3533->hwen, 0);
+ regulator_disable(lm3533->vin_supply);
}
enum lm3533_attribute_type {
@@ -324,7 +336,9 @@ static int lm3533_device_init(struct lm3533 *lm3533)
u32 count = 0, reg;
int ret;
- lm3533_enable(lm3533);
+ ret = lm3533_enable(lm3533);
+ if (ret)
+ return ret;
device_for_each_child_node_scoped(lm3533->dev, child) {
if (!fwnode_device_is_available(child))
@@ -493,6 +507,11 @@ static int lm3533_i2c_probe(struct i2c_client *i2c)
return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->hwen),
"failed to get HWEN GPIO\n");
+ lm3533->vin_supply = devm_regulator_get(lm3533->dev, "vin");
+ if (IS_ERR(lm3533->vin_supply))
+ return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->vin_supply),
+ "failed to get vin-supply\n");
+
device_property_read_u32(lm3533->dev, "ti,boost-ovp-microvolt",
&lm3533->boost_ovp);
diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h
index 3aa962d4c747..d751773fc92d 100644
--- a/include/linux/mfd/lm3533.h
+++ b/include/linux/mfd/lm3533.h
@@ -21,6 +21,7 @@
struct device;
struct gpio_desc;
struct regmap;
+struct regulator;
struct lm3533 {
struct device *dev;
@@ -28,6 +29,7 @@ struct lm3533 {
struct regmap *regmap;
struct gpio_desc *hwen;
+ struct regulator *vin_supply;
u32 boost_ovp;
u32 boost_freq;
--
2.51.0
^ permalink raw reply related
* [PATCH v2 4/6] mfd: lm3533: Set DMA mask
From: Svyatoslav Ryhel @ 2026-05-28 13:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
In-Reply-To: <20260528135123.103745-1-clamor95@gmail.com>
Missing coherent_dma_mask assigning triggers the following warning in
dmesg:
[ 3.287872] platform lm3533-backlight.0: DMA mask not set
Since this warning might be elevated to an error in the future, set
coherent_dma_mask to zero because both the core and cells do not utilize
DMA.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/mfd/lm3533-core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index 519f8c16a3f3..3cfdebf5fb52 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -526,6 +526,10 @@ static int lm3533_i2c_probe(struct i2c_client *i2c)
LM3533_BOOST_FREQ_MAX);
lm3533->boost_freq = lm3533->boost_freq / (500 * KILO) - 1;
+ /* LM3533 and child devices do not use DMA */
+ i2c->dev.coherent_dma_mask = 0;
+ i2c->dev.dma_mask = &i2c->dev.coherent_dma_mask;
+
return lm3533_device_init(lm3533);
}
--
2.51.0
^ permalink raw reply related
* [PATCH v2 5/6] video: backlight: lm3533_bl: Set initial mapping mode from DT
From: Svyatoslav Ryhel @ 2026-05-28 13:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
In-Reply-To: <20260528135123.103745-1-clamor95@gmail.com>
Add support to obtain the initial mapping mode from DT instead of leaving
it unconfigured. Additionally, update the linear sysfs code, which uses a
similar coding pattern.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/video/backlight/lm3533_bl.c | 50 ++++++++++++++++-------------
1 file changed, 27 insertions(+), 23 deletions(-)
diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index 42da652df58d..c03d0d1667e4 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -22,6 +22,7 @@
#define LM3533_BL_MAX_BRIGHTNESS 255
#define LM3533_REG_CTRLBANK_AB_BCONF 0x1a
+#define CTRLBANK_AB_BCONF_MODE(n) BIT(2 * (n) + 1)
struct lm3533_bl {
@@ -32,6 +33,7 @@ struct lm3533_bl {
u32 max_current;
u32 pwm;
+ bool linear;
};
@@ -135,8 +137,9 @@ static ssize_t show_linear(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct lm3533_bl *bl = dev_get_drvdata(dev);
+ int id = lm3533_bl_get_ctrlbank_id(bl);
+ u8 mask = CTRLBANK_AB_BCONF_MODE(id);
u8 val;
- u8 mask;
int linear;
int ret;
@@ -144,8 +147,6 @@ static ssize_t show_linear(struct device *dev,
if (ret)
return ret;
- mask = 1 << (2 * lm3533_bl_get_ctrlbank_id(bl) + 1);
-
if (val & mask)
linear = 1;
else
@@ -159,23 +160,16 @@ static ssize_t store_linear(struct device *dev,
const char *buf, size_t len)
{
struct lm3533_bl *bl = dev_get_drvdata(dev);
+ int id = lm3533_bl_get_ctrlbank_id(bl);
unsigned long linear;
- u8 mask;
- u8 val;
int ret;
if (kstrtoul(buf, 0, &linear))
return -EINVAL;
- mask = 1 << (2 * lm3533_bl_get_ctrlbank_id(bl) + 1);
-
- if (linear)
- val = mask;
- else
- val = 0;
-
- ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF, val,
- mask);
+ ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF,
+ linear ? CTRLBANK_AB_BCONF_MODE(id) : 0,
+ CTRLBANK_AB_BCONF_MODE(id));
if (ret)
return ret;
@@ -253,8 +247,15 @@ static struct attribute_group lm3533_bl_attribute_group = {
static int lm3533_bl_setup(struct lm3533_bl *bl)
{
+ int id = lm3533_bl_get_ctrlbank_id(bl);
int ret;
+ ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF,
+ bl->linear ? CTRLBANK_AB_BCONF_MODE(id) : 0,
+ CTRLBANK_AB_BCONF_MODE(id));
+ if (ret)
+ return ret;
+
ret = lm3533_ctrlbank_set_max_current(&bl->cb, bl->max_current);
if (ret)
return ret;
@@ -317,14 +318,6 @@ static int lm3533_bl_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, bl);
- ret = sysfs_create_group(&bd->dev.kobj, &lm3533_bl_attribute_group);
- if (ret < 0) {
- dev_err(&pdev->dev, "failed to create sysfs attributes\n");
- return ret;
- }
-
- backlight_update_status(bd);
-
device_property_read_u32(&pdev->dev, "led-max-microamp",
&bl->max_current);
bl->max_current = clamp(bl->max_current, LM3533_LED_MAX_CURRENT_MIN,
@@ -333,9 +326,20 @@ static int lm3533_bl_probe(struct platform_device *pdev)
bl->pwm = 0;
device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
+ bl->linear = device_property_read_bool(&pdev->dev,
+ "ti,linear-mapping-mode");
+
ret = lm3533_bl_setup(bl);
if (ret)
- goto err_sysfs_remove;
+ return ret;
+
+ ret = sysfs_create_group(&bd->dev.kobj, &lm3533_bl_attribute_group);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to create sysfs attributes\n");
+ return ret;
+ }
+
+ backlight_update_status(bd);
ret = lm3533_ctrlbank_enable(&bl->cb);
if (ret)
--
2.51.0
^ permalink raw reply related
* [PATCH v2 6/6] video: leds: backlight: lm3533: Support getting LED sources from DT
From: Svyatoslav Ryhel @ 2026-05-28 13:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
In-Reply-To: <20260528135123.103745-1-clamor95@gmail.com>
Add Control Bank to HVLED/LVLED muxing support based on the led-sources
defined in the device tree.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/leds/leds-lm3533.c | 55 ++++++++++++++++++++++++++++-
drivers/video/backlight/lm3533_bl.c | 37 ++++++++++++++++++-
2 files changed, 90 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index d707d43d5526..07390bba9a48 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -7,6 +7,7 @@
* Author: Johan Hovold <jhovold@gmail.com>
*/
+#include <linux/bits.h>
#include <linux/module.h>
#include <linux/leds.h>
#include <linux/mfd/core.h>
@@ -26,6 +27,12 @@
#define LM3533_ALS_CHANNEL_LV_MIN 1
#define LM3533_ALS_CHANNEL_LV_MAX 2
+#define LM3533_REG_OUTPUT_CONF1 0x10
+#define OUTPUT_CONF1_MASK GENMASK(7, 2)
+#define OUTPUT_CONF1_SHIFT 2
+#define LM3533_REG_OUTPUT_CONF2 0x11
+#define OUTPUT_CONF2_MASK GENMASK(3, 0)
+#define OUTPUT_CONF2_SHIFT 6
#define LM3533_REG_CTRLBANK_BCONF_BASE 0x1b
#define LM3533_REG_PATTERN_ENABLE 0x28
#define LM3533_REG_PATTERN_LOW_TIME_BASE 0x71
@@ -53,6 +60,9 @@ struct lm3533_led {
u32 max_current;
u32 pwm;
+
+ int num_leds;
+ u32 leds[LM3533_LVCTRLBANK_MAX];
};
@@ -639,7 +649,33 @@ static const struct attribute_group *lm3533_led_attribute_groups[] = {
static int lm3533_led_setup(struct lm3533_led *led)
{
- int ret;
+ u32 output_cfg_shift = 0;
+ u32 output_cfg_val = 0;
+ int ret, i;
+
+ if (led->num_leds) {
+ for (i = 0; i < led->num_leds; i++) {
+ if (led->leds[i] > LM3533_LVCTRLBANK_MAX)
+ continue;
+
+ output_cfg_shift = led->leds[i] * 2;
+ output_cfg_val |= led->id << output_cfg_shift;
+ }
+
+ /* LVLED1, LVLED2 and LVLED3 */
+ ret = lm3533_update(led->lm3533, LM3533_REG_OUTPUT_CONF1,
+ output_cfg_val << OUTPUT_CONF1_SHIFT,
+ OUTPUT_CONF1_MASK);
+ if (ret)
+ return ret;
+
+ /* LVLED4 and LVLED5 */
+ ret = lm3533_update(led->lm3533, LM3533_REG_OUTPUT_CONF2,
+ output_cfg_val >> OUTPUT_CONF2_SHIFT,
+ OUTPUT_CONF2_MASK);
+ if (ret)
+ return ret;
+ }
ret = lm3533_ctrlbank_set_max_current(&led->cb, led->max_current);
if (ret)
@@ -714,6 +750,23 @@ static int lm3533_led_probe(struct platform_device *pdev)
led->pwm = 0;
device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
+ led->num_leds = device_property_count_u32(&pdev->dev, "led-sources");
+
+ /*
+ * If led-sources property is not set then either this Control Bank uses
+ * its default LVLED or is not linked to any LVLED at all.
+ */
+ if (led->num_leds > 0 && led->num_leds <= LM3533_LVCTRLBANK_MAX) {
+ ret = device_property_read_u32_array(&pdev->dev, "led-sources",
+ led->leds, led->num_leds);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to get led-sources\n");
+ goto err_deregister;
+ }
+ } else {
+ led->num_leds = 0;
+ }
+
ret = lm3533_led_setup(led);
if (ret)
goto err_deregister;
diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index c03d0d1667e4..82b46a531dd2 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -7,6 +7,7 @@
* Author: Johan Hovold <jhovold@gmail.com>
*/
+#include <linux/bits.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/mod_devicetable.h>
@@ -21,6 +22,8 @@
#define LM3533_HVCTRLBANK_COUNT 2
#define LM3533_BL_MAX_BRIGHTNESS 255
+#define LM3533_REG_OUTPUT_CONF1 0x10
+#define OUTPUT_CONF1_MASK GENMASK(1, 0)
#define LM3533_REG_CTRLBANK_AB_BCONF 0x1a
#define CTRLBANK_AB_BCONF_MODE(n) BIT(2 * (n) + 1)
@@ -34,6 +37,9 @@ struct lm3533_bl {
u32 max_current;
u32 pwm;
bool linear;
+
+ u32 num_leds;
+ u32 led_strings[LM3533_HVCTRLBANK_COUNT];
};
@@ -248,7 +254,8 @@ static struct attribute_group lm3533_bl_attribute_group = {
static int lm3533_bl_setup(struct lm3533_bl *bl)
{
int id = lm3533_bl_get_ctrlbank_id(bl);
- int ret;
+ u32 output_cfg_val = 0;
+ int ret, i;
ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF,
bl->linear ? CTRLBANK_AB_BCONF_MODE(id) : 0,
@@ -256,6 +263,16 @@ static int lm3533_bl_setup(struct lm3533_bl *bl)
if (ret)
return ret;
+ if (bl->num_leds) {
+ for (i = 0; i < bl->num_leds; i++)
+ output_cfg_val |= id << bl->led_strings[i];
+
+ ret = lm3533_update(bl->lm3533, LM3533_REG_OUTPUT_CONF1,
+ output_cfg_val, OUTPUT_CONF1_MASK);
+ if (ret)
+ return ret;
+ }
+
ret = lm3533_ctrlbank_set_max_current(&bl->cb, bl->max_current);
if (ret)
return ret;
@@ -329,6 +346,24 @@ static int lm3533_bl_probe(struct platform_device *pdev)
bl->linear = device_property_read_bool(&pdev->dev,
"ti,linear-mapping-mode");
+ bl->num_leds = device_property_count_u32(&pdev->dev, "led-sources");
+
+ /*
+ * If led-sources property is not set then either this Control Bank uses
+ * its default HVLED or is not linked to any HVLED at all.
+ */
+ if (bl->num_leds > 0 && bl->num_leds <= LM3533_HVCTRLBANK_COUNT) {
+ ret = device_property_read_u32_array(&pdev->dev, "led-sources",
+ bl->led_strings,
+ bl->num_leds);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to get led-sources\n");
+ goto err_sysfs_remove;
+ }
+ } else {
+ bl->num_leds = 0;
+ }
+
ret = lm3533_bl_setup(bl);
if (ret)
return ret;
--
2.51.0
^ permalink raw reply related
* Re: [PATCH 2/2] staging: sm750fb: rename setAllEngOff
From: Dan Carpenter @ 2026-05-28 14:14 UTC (permalink / raw)
To: Onish Sharma
Cc: sudipm.mukherjee, gregkh, linux-staging, linux-fbdev,
linux-kernel
In-Reply-To: <20260528133627.10850-2-neharora23587@gmail.com>
On Thu, May 28, 2026 at 07:06:27PM +0530, Onish Sharma wrote:
> Rename setAllEngOff to set_all_eng_off to comply with kernel coding style
> and improve readability.
>
> Signed-off-by: Onish Sharma <neharora23587@gmail.com>
> ---
> drivers/staging/sm750fb/sm750.c | 2 +-
> drivers/staging/sm750fb/sm750.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index 8f01b3c63fe8..716a8935f58d 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -848,7 +848,7 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, char *src)
> sm750_dev->init_parm.mem_clk = 0;
> sm750_dev->init_parm.master_clk = 0;
> sm750_dev->init_parm.power_mode = 0;
> - sm750_dev->init_parm.setAllEngOff = 0;
> + sm750_dev->init_parm.set_all_eng_off = 0;
> sm750_dev->init_parm.reset_memory = 1;
>
> /* defaultly turn g_hwcursor on for both view */
> diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> index 0492b1afbb11..e8885133da2e 100644
> --- a/drivers/staging/sm750fb/sm750.h
> +++ b/drivers/staging/sm750fb/sm750.h
> @@ -44,7 +44,7 @@ struct init_status {
> ushort chip_clk;
> ushort mem_clk;
> ushort master_clk;
> - ushort setAllEngOff;
> + ushort set_all_eng_off;
> ushort reset_memory;
> };
There are no users? Why not just delete it.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 1/2] staging: sm750fb: rename pvReg to pv_reg
From: Dan Carpenter @ 2026-05-28 14:15 UTC (permalink / raw)
To: Onish Sharma
Cc: sudipm.mukherjee, gregkh, linux-staging, linux-fbdev,
linux-kernel
In-Reply-To: <20260528133627.10850-1-neharora23587@gmail.com>
On Thu, May 28, 2026 at 07:06:26PM +0530, Onish Sharma wrote:
> Rename pvReg to pv_reg to comply with kernel coding style (checkpatch.pl)
> and improve readability.
>
> Signed-off-by: Onish Sharma <neharora23587@gmail.com>
pv stands for pointer void. It's Hungarian notation and it's not
allowed.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v2 2/6] mfd: lm3533: Convert to use OF bindings
From: Jonathan Cameron @ 2026-05-28 14:50 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner, Nuno Sá,
Andy Shevchenko, Helge Deller, Johan Hovold, dri-devel,
linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev
In-Reply-To: <20260528135123.103745-3-clamor95@gmail.com>
On Thu, 28 May 2026 16:51:19 +0300
Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> Since there are no users of this driver via platform data, remove the
> platform data support and switch to using Device Tree bindings.
> Additionally, optimize functions used only by platform data.
At least the IIO ones would have made much the same amount of sense for
dt, just that they weren't having in the first place. I'd prefer that
as a precursor patch to make the rest much more readable.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
I only looked in detail at the iio bit. A few changes requested.
> ---
> drivers/iio/light/lm3533-als.c | 95 ++++------
> drivers/leds/leds-lm3533.c | 51 ++++--
> drivers/mfd/lm3533-core.c | 268 ++++++++++------------------
> drivers/video/backlight/lm3533_bl.c | 52 ++++--
> include/linux/mfd/lm3533.h | 51 +-----
> 5 files changed, 212 insertions(+), 305 deletions(-)
>
> diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> index 99f0b903018c..cbd337b73bd9 100644
> --- a/drivers/iio/light/lm3533-als.c
> +++ b/drivers/iio/light/lm3533-als.c
> @@ -714,59 +720,33 @@ static const struct attribute_group lm3533_als_attribute_group = {
> .attrs = lm3533_als_attributes
> };
>
> -static int lm3533_als_set_input_mode(struct lm3533_als *als, bool pwm_mode)
> +static int lm3533_als_setup(struct lm3533_als *als)
> {
> - u8 mask = LM3533_ALS_INPUT_MODE_MASK;
> - u8 val;
> + struct device *dev = &als->pdev.dev;
> int ret;
>
> - if (pwm_mode)
> - val = mask; /* pwm input */
> - else
> - val = 0; /* analog input */
> -
> - ret = lm3533_update(als->lm3533, LM3533_REG_ALS_CONF, val, mask);
> - if (ret) {
> - dev_err(&als->pdev->dev, "failed to set input mode %d\n",
> - pwm_mode);
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> -static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
> -{
> - int ret;
> -
> - if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX) {
> - dev_err(&als->pdev->dev, "invalid resistor value\n");
> - return -EINVAL;
> - }
> -
> - ret = lm3533_write(als->lm3533, LM3533_REG_ALS_RESISTOR_SELECT, val);
> - if (ret) {
> - dev_err(&als->pdev->dev, "failed to set resistor\n");
> - return ret;
> - }
> + device_property_read_u32(dev, "ti,resistor-value-ohm",
> + &als->r_select);
Does this have a default? If so the pattern we've recently be setting on for IIO
is
if (device_property_present(dev, "ti,resistor-value-ohm"))
ret = device_property_read_u32();
if (ret) //corrupt property in some fashion
return ret;
} else {
//set default
}
If there is no default then check it unconditionally.
>
> - return 0;
> -}
> + als->r_select = clamp(als->r_select, LM3533_ALS_RESISTOR_MIN,
> + LM3533_ALS_RESISTOR_MAX);
> + als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * als->r_select);
>
> -static int lm3533_als_setup(struct lm3533_als *als,
> - const struct lm3533_als_platform_data *pdata)
> -{
> - int ret;
> + als->pwm_mode = device_property_read_bool(dev, "ti,pwm-mode");
>
> - ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> + ret = lm3533_update(lm3533, LM3533_REG_ALS_CONF, als->pwm_mode ?
> + LM3533_ALS_INPUT_MODE_MASK : 0,
That's ugly. Better as
ret = lm3533_update(lm3533, LM3533_REG_ALS_CONF,
als->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0,
Though if there wasn't a layer hiding the regmap, it could just have been
ret = regmap_assign_bits(lm3533->regmap, LM3533_REG_ALS_CONF,
LM3533_ALS_INPUT_MODE_MASK, als->pwm_mode);;
which would have been nicer.
I'm not particularly keen on the swashing of the helpers being in a patch
that is about switching the binding type as feels largely unrelated.
Should really have been a precursor, easier to review patch.
> + LM3533_ALS_INPUT_MODE_MASK);
> if (ret)
> - return ret;
> + return dev_err_probe(dev, ret, "failed to set input mode %d\n",
> + als->pwm_mode);
>
> /* ALS input is always high impedance in PWM-mode. */
> - if (!pdata->pwm_mode) {
> - ret = lm3533_als_set_resistor(als, pdata->r_select);
> + if (!als->pwm_mode) {
> + ret = lm3533_write(lm3533, LM3533_REG_ALS_RESISTOR_SELECT,
> + (u8)als->r_select);
Same applies here. Mostly an unrelated change as the only thing switching that
is related to the patch is one parameter.
> if (ret)
> - return ret;
> + return dev_err_probe(dev, ret, "failed to set resistor\n");
> }
>
> return 0;
> @@ -852,25 +825,28 @@ static int lm3533_als_probe(struct platform_device *pdev)
> indio_dev->channels = lm3533_als_channels;
> indio_dev->num_channels = ARRAY_SIZE(lm3533_als_channels);
> indio_dev->name = dev_name(&pdev->dev);
> - iio_device_set_parent(indio_dev, pdev->dev.parent);
I'm not sure why this was there in the first place. Hence not sure if it
is safe to remove.
> diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
> index 45795f2a1042..d707d43d5526 100644
> --- a/drivers/leds/leds-lm3533.c
> +++ b/drivers/leds/leds-lm3533.c
>
> led->cb.dev = led->cdev.dev;
>
> - ret = lm3533_led_setup(led, pdata);
> + device_property_read_u32(&pdev->dev, "led-max-microamp",
> + &led->max_current);
I'd prefer explicit setting of the default to be visible before this, or
the property_present pattern I mention in the IIO review above.
> + led->max_current = clamp(led->max_current, LM3533_LED_MAX_CURRENT_MIN,
> + LM3533_LED_MAX_CURRENT_MAX);
I didn't look any further (busy day!)
^ permalink raw reply
* Re: [PATCH v2 1/6] dt-bindings: leds: Document TI LM3533 LED controller
From: Jonathan Cameron @ 2026-05-28 14:54 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner, Nuno Sá,
Andy Shevchenko, Helge Deller, Johan Hovold, dri-devel,
linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev
In-Reply-To: <20260528135123.103745-2-clamor95@gmail.com>
On Thu, 28 May 2026 16:51:18 +0300
Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> Document the LM3533 - a complete power source for backlight, keypad and
> indicator LEDs in smartphone handsets. The high-voltage inductive boost
> converter provides the power for two series LED strings display backlight
> and keypad functions.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
The light sensor binding looks fine to me. I didn't check the rest.
Reviewed-by: Jonathan Cameron <jic23@kernel.org> #for light sensor
^ permalink raw reply
* Re: [PATCH v2 2/6] mfd: lm3533: Convert to use OF bindings
From: Svyatoslav Ryhel @ 2026-05-28 15:03 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner, Nuno Sá,
Andy Shevchenko, Helge Deller, Johan Hovold, dri-devel,
linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev
In-Reply-To: <20260528155001.2bcb7003@jic23-huawei>
чт, 28 трав. 2026 р. о 17:50 Jonathan Cameron <jic23@kernel.org> пише:
>
> On Thu, 28 May 2026 16:51:19 +0300
> Svyatoslav Ryhel <clamor95@gmail.com> wrote:
>
> > Since there are no users of this driver via platform data, remove the
> > platform data support and switch to using Device Tree bindings.
> > Additionally, optimize functions used only by platform data.
>
>
> At least the IIO ones would have made much the same amount of sense for
> dt, just that they weren't having in the first place. I'd prefer that
> as a precursor patch to make the rest much more readable.
>
I can add you preferences into this commit, I don't mind.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>
> I only looked in detail at the iio bit. A few changes requested.
>
> > ---
> > drivers/iio/light/lm3533-als.c | 95 ++++------
> > drivers/leds/leds-lm3533.c | 51 ++++--
> > drivers/mfd/lm3533-core.c | 268 ++++++++++------------------
> > drivers/video/backlight/lm3533_bl.c | 52 ++++--
> > include/linux/mfd/lm3533.h | 51 +-----
> > 5 files changed, 212 insertions(+), 305 deletions(-)
> >
> > diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> > index 99f0b903018c..cbd337b73bd9 100644
> > --- a/drivers/iio/light/lm3533-als.c
> > +++ b/drivers/iio/light/lm3533-als.c
>
> > @@ -714,59 +720,33 @@ static const struct attribute_group lm3533_als_attribute_group = {
> > .attrs = lm3533_als_attributes
> > };
> >
> > -static int lm3533_als_set_input_mode(struct lm3533_als *als, bool pwm_mode)
> > +static int lm3533_als_setup(struct lm3533_als *als)
> > {
> > - u8 mask = LM3533_ALS_INPUT_MODE_MASK;
> > - u8 val;
> > + struct device *dev = &als->pdev.dev;
> > int ret;
> >
> > - if (pwm_mode)
> > - val = mask; /* pwm input */
> > - else
> > - val = 0; /* analog input */
> > -
> > - ret = lm3533_update(als->lm3533, LM3533_REG_ALS_CONF, val, mask);
> > - if (ret) {
> > - dev_err(&als->pdev->dev, "failed to set input mode %d\n",
> > - pwm_mode);
> > - return ret;
> > - }
> > -
> > - return 0;
> > -}
> > -
> > -static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
> > -{
> > - int ret;
> > -
> > - if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX) {
> > - dev_err(&als->pdev->dev, "invalid resistor value\n");
> > - return -EINVAL;
> > - }
> > -
> > - ret = lm3533_write(als->lm3533, LM3533_REG_ALS_RESISTOR_SELECT, val);
> > - if (ret) {
> > - dev_err(&als->pdev->dev, "failed to set resistor\n");
> > - return ret;
> > - }
> > + device_property_read_u32(dev, "ti,resistor-value-ohm",
> > + &als->r_select);
> Does this have a default? If so the pattern we've recently be setting on for IIO
> is
> if (device_property_present(dev, "ti,resistor-value-ohm"))
> ret = device_property_read_u32();
> if (ret) //corrupt property in some fashion
> return ret;
> } else {
> //set default
> }
> If there is no default then check it unconditionally.
default value is LM3533_ALS_RESISTOR_MIN and if no property is present
clamp will ensure that als->r_select will be set to
LM3533_ALS_RESISTOR_MIN
>
> >
> > - return 0;
> > -}
> > + als->r_select = clamp(als->r_select, LM3533_ALS_RESISTOR_MIN,
> > + LM3533_ALS_RESISTOR_MAX);
> > + als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * als->r_select);
> >
> > -static int lm3533_als_setup(struct lm3533_als *als,
> > - const struct lm3533_als_platform_data *pdata)
> > -{
> > - int ret;
> > + als->pwm_mode = device_property_read_bool(dev, "ti,pwm-mode");
> >
> > - ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> > + ret = lm3533_update(lm3533, LM3533_REG_ALS_CONF, als->pwm_mode ?
> > + LM3533_ALS_INPUT_MODE_MASK : 0,
>
> That's ugly. Better as
>
> ret = lm3533_update(lm3533, LM3533_REG_ALS_CONF,
> als->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0,
>
Yes sure, just followed 80 char limit.
> Though if there wasn't a layer hiding the regmap, it could just have been
>
> ret = regmap_assign_bits(lm3533->regmap, LM3533_REG_ALS_CONF,
> LM3533_ALS_INPUT_MODE_MASK, als->pwm_mode);;
>
> which would have been nicer.
>
> I'm not particularly keen on the swashing of the helpers being in a patch
> that is about switching the binding type as feels largely unrelated.
> Should really have been a precursor, easier to review patch.
>
Removing of lm3533_update layer is not the scope of this patchset.
>
> > + LM3533_ALS_INPUT_MODE_MASK);
> > if (ret)
> > - return ret;
> > + return dev_err_probe(dev, ret, "failed to set input mode %d\n",
> > + als->pwm_mode);
> >
> > /* ALS input is always high impedance in PWM-mode. */
> > - if (!pdata->pwm_mode) {
> > - ret = lm3533_als_set_resistor(als, pdata->r_select);
> > + if (!als->pwm_mode) {
> > + ret = lm3533_write(lm3533, LM3533_REG_ALS_RESISTOR_SELECT,
> > + (u8)als->r_select);
>
> Same applies here. Mostly an unrelated change as the only thing switching that
> is related to the patch is one parameter.
>
Removing of lm3533_write layer is not the scope of this patchset.
> > if (ret)
> > - return ret;
> > + return dev_err_probe(dev, ret, "failed to set resistor\n");
> > }
> >
> > return 0;
>
> > @@ -852,25 +825,28 @@ static int lm3533_als_probe(struct platform_device *pdev)
> > indio_dev->channels = lm3533_als_channels;
> > indio_dev->num_channels = ARRAY_SIZE(lm3533_als_channels);
> > indio_dev->name = dev_name(&pdev->dev);
> > - iio_device_set_parent(indio_dev, pdev->dev.parent);
>
> I'm not sure why this was there in the first place. Hence not sure if it
> is safe to remove.
>
This is directly related to OF conversion. The iio_device_set_parent
bound indio_dev to parent, and it causes problems with OF now since
als output has its own node and binding it to parent if wrong. Same
story for backlight and leds btw.
>
> > diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
> > index 45795f2a1042..d707d43d5526 100644
> > --- a/drivers/leds/leds-lm3533.c
> > +++ b/drivers/leds/leds-lm3533.c
>
> >
> > led->cb.dev = led->cdev.dev;
> >
> > - ret = lm3533_led_setup(led, pdata);
> > + device_property_read_u32(&pdev->dev, "led-max-microamp",
> > + &led->max_current);
>
> I'd prefer explicit setting of the default to be visible before this, or
> the property_present pattern I mention in the IIO review above.
>
clamp will ensure that led->max_current will be set to
LM3533_LED_MAX_CURRENT_MIN regardless if it it present
> > + led->max_current = clamp(led->max_current, LM3533_LED_MAX_CURRENT_MIN,
> > + LM3533_LED_MAX_CURRENT_MAX);
>
> I didn't look any further (busy day!)
^ permalink raw reply
* [PATCH] staging: sm750fb: rename Bpp parameter to bpp
From: Gabry @ 2026-05-28 18:36 UTC (permalink / raw)
To: sudipm.mukherjee@gmail.com, teddy.wang@siliconmotion.com,
gregkh@linuxfoundation.org
Cc: linux-fbdev@vger.kernel.org, linux-staging@lists.linux.dev,
linux-kernel@vger.kernel.org
The Linux kernel coding style prefers snake_case over CamelCase foridentifier names.
Rename the 'Bpp' parameter (bytes per pixel) of sm750_hw_fillrect()
and sm750_hw_copyarea() to 'bpp' to comply with this standard. Update
the function prototypes in sm750_accel.h and the corresponding
kernel-doc descriptions accordingly.
This is a pure rename with no functional change, and addresses a
checkpatch.pl warning:
CHECK: Avoid CamelCase: <Bpp>
Signed-off-by: Gabriele Rizzo <gabry.256@proton.me>
---
drivers/staging/sm750fb/sm750_accel.c | 22 +++++++++++-----------
drivers/staging/sm750fb/sm750_accel.h | 6 +++---
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index 0f94d859e91c..4beabe1053f9 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -85,7 +85,7 @@ void sm750_hw_set2dformat(struct lynx_accel *accel, int fmt)
}
int sm750_hw_fillrect(struct lynx_accel *accel,
- u32 base, u32 pitch, u32 Bpp,
+ u32 base, u32 pitch, u32 bpp,
u32 x, u32 y, u32 width, u32 height,
u32 color, u32 rop)
{
@@ -102,14 +102,14 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
write_dpr(accel, DE_WINDOW_DESTINATION_BASE, base); /* dpr40 */
write_dpr(accel, DE_PITCH,
- ((pitch / Bpp << DE_PITCH_DESTINATION_SHIFT) &
+ ((pitch / bpp << DE_PITCH_DESTINATION_SHIFT) &
DE_PITCH_DESTINATION_MASK) |
- (pitch / Bpp & DE_PITCH_SOURCE_MASK)); /* dpr10 */
+ (pitch / bpp & DE_PITCH_SOURCE_MASK)); /* dpr10 */
write_dpr(accel, DE_WINDOW_WIDTH,
- ((pitch / Bpp << DE_WINDOW_WIDTH_DST_SHIFT) &
+ ((pitch / bpp << DE_WINDOW_WIDTH_DST_SHIFT) &
DE_WINDOW_WIDTH_DST_MASK) |
- (pitch / Bpp & DE_WINDOW_WIDTH_SRC_MASK)); /* dpr44 */
+ (pitch / bpp & DE_WINDOW_WIDTH_SRC_MASK)); /* dpr44 */
write_dpr(accel, DE_FOREGROUND, color); /* DPR14 */
@@ -138,7 +138,7 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
* @sy: Starting y coordinate of source surface
* @dest_base: Address of destination: offset in frame buffer
* @dest_pitch: Pitch value of destination surface in BYTE
- * @Bpp: Color depth of destination surface
+ * @bpp: Color depth of destination surface
* @dx: Starting x coordinate of destination surface
* @dy: Starting y coordinate of destination surface
* @width: width of rectangle in pixel value
@@ -149,7 +149,7 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
unsigned int source_base, unsigned int source_pitch,
unsigned int sx, unsigned int sy,
unsigned int dest_base, unsigned int dest_pitch,
- unsigned int Bpp, unsigned int dx, unsigned int dy,
+ unsigned int bpp, unsigned int dx, unsigned int dy,
unsigned int width, unsigned int height,
unsigned int rop2)
{
@@ -249,9 +249,9 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
* pixel values. Need Byte to pixel conversion.
*/
write_dpr(accel, DE_PITCH,
- ((dest_pitch / Bpp << DE_PITCH_DESTINATION_SHIFT) &
+ ((dest_pitch / bpp << DE_PITCH_DESTINATION_SHIFT) &
DE_PITCH_DESTINATION_MASK) |
- (source_pitch / Bpp & DE_PITCH_SOURCE_MASK)); /* dpr10 */
+ (source_pitch / bpp & DE_PITCH_SOURCE_MASK)); /* dpr10 */
/*
* Screen Window width in Pixels.
@@ -259,9 +259,9 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
* for a given point.
*/
write_dpr(accel, DE_WINDOW_WIDTH,
- ((dest_pitch / Bpp << DE_WINDOW_WIDTH_DST_SHIFT) &
+ ((dest_pitch / bpp << DE_WINDOW_WIDTH_DST_SHIFT) &
DE_WINDOW_WIDTH_DST_MASK) |
- (source_pitch / Bpp & DE_WINDOW_WIDTH_SRC_MASK)); /* dpr3c */
+ (source_pitch / bpp & DE_WINDOW_WIDTH_SRC_MASK)); /* dpr3c */
if (accel->de_wait() != 0)
return -1;
diff --git a/drivers/staging/sm750fb/sm750_accel.h b/drivers/staging/sm750fb/sm750_accel.h
index 2c79cb730a0a..d15a40cacb84 100644
--- a/drivers/staging/sm750fb/sm750_accel.h
+++ b/drivers/staging/sm750fb/sm750_accel.h
@@ -190,7 +190,7 @@ void sm750_hw_set2dformat(struct lynx_accel *accel, int fmt);
void sm750_hw_de_init(struct lynx_accel *accel);
int sm750_hw_fillrect(struct lynx_accel *accel,
- u32 base, u32 pitch, u32 Bpp,
+ u32 base, u32 pitch, u32 bpp,
u32 x, u32 y, u32 width, u32 height,
u32 color, u32 rop);
@@ -202,7 +202,7 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
* @sy: Starting y coordinate of source surface
* @dBase: Address of destination: offset in frame buffer
* @dPitch: Pitch value of destination surface in BYTE
- * @Bpp: Color depth of destination surface
+ * @bpp: Color depth of destination surface
* @dx: Starting x coordinate of destination surface
* @dy: Starting y coordinate of destination surface
* @width: width of rectangle in pixel value
@@ -213,7 +213,7 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
unsigned int sBase, unsigned int sPitch,
unsigned int sx, unsigned int sy,
unsigned int dBase, unsigned int dPitch,
- unsigned int Bpp, unsigned int dx, unsigned int dy,
+ unsigned int bpp, unsigned int dx, unsigned int dy,
unsigned int width, unsigned int height,
unsigned int rop2);
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v1 0/8] zorro: Improve handling of pointers in zorro_device_id::driver_data
From: patchwork-bot+netdevbpf @ 2026-05-29 1:20 UTC (permalink / raw)
To: =?utf-8?q?Uwe_Kleine-K=C3=B6nig_=28The_Capable_Hub=29_=3Cu=2Ekleine-koenig?=,
=?utf-8?q?=40baylibre=2Ecom=3E?=
Cc: geert, dlemoal, cassel, James.Bottomley, martin.petersen,
andrew+netdev, davem, edumazet, kuba, pabeni, tglx, mingo, max,
andi.shyti, deller, linux-ide, linux-m68k, linux-kernel,
linux-scsi, netdev, linux-i2c, linux-fbdev, dri-devel,
christian.ehrhardt, lk
In-Reply-To: <cover.1779803053.git.u.kleine-koenig@baylibre.com>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 26 May 2026 16:17:26 +0200 you wrote:
> Hello,
>
> this series is about improving the handling of pointers in struct
> zorro_device_id's driver_data.
>
> While it's ok on all current Linux platforms to store a pointer in an
> unsigned long variable, it involves casting that loses type information.
> This can be nicely seen in patch #7 where after profiting from patch #6
> the compiler notices a missing const.
>
> [...]
Here is the summary with links:
- [v1,net-next,3/8] net: Use named initializer for zorro_device_id arrays
https://git.kernel.org/netdev/net-next/c/4933a658369a
- [v1,6/8] zorro: Simplify storing pointers in device id struct
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v2 2/6] mfd: lm3533: Convert to use OF bindings
From: Jonathan Cameron @ 2026-05-29 9:08 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner, Nuno Sá,
Andy Shevchenko, Helge Deller, Johan Hovold, dri-devel,
linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev
In-Reply-To: <CAPVz0n0qCekQVGGyAyBuYv+RKC6bpydYBLJNGfPrgTYjtOJOuA@mail.gmail.com>
On Thu, 28 May 2026 18:03:31 +0300
Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> чт, 28 трав. 2026 р. о 17:50 Jonathan Cameron <jic23@kernel.org> пише:
> >
> > On Thu, 28 May 2026 16:51:19 +0300
> > Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> >
> > > Since there are no users of this driver via platform data, remove the
> > > platform data support and switch to using Device Tree bindings.
> > > Additionally, optimize functions used only by platform data.
> >
> >
> > At least the IIO ones would have made much the same amount of sense for
> > dt, just that they weren't having in the first place. I'd prefer that
Gah. I write gibberish after too much reviewing. having/helping!
> > as a precursor patch to make the rest much more readable.
> >
>
> I can add you preferences into this commit, I don't mind.
>
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> >
> > I only looked in detail at the iio bit. A few changes requested.
> >
> > > ---
> > > drivers/iio/light/lm3533-als.c | 95 ++++------
> > > drivers/leds/leds-lm3533.c | 51 ++++--
> > > drivers/mfd/lm3533-core.c | 268 ++++++++++------------------
> > > drivers/video/backlight/lm3533_bl.c | 52 ++++--
> > > include/linux/mfd/lm3533.h | 51 +-----
> > > 5 files changed, 212 insertions(+), 305 deletions(-)
> > >
> > > diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> > > index 99f0b903018c..cbd337b73bd9 100644
> > > --- a/drivers/iio/light/lm3533-als.c
> > > +++ b/drivers/iio/light/lm3533-als.c
> >
> > > @@ -714,59 +720,33 @@ static const struct attribute_group lm3533_als_attribute_group = {
> > > .attrs = lm3533_als_attributes
> > > };
> > >
> > > -static int lm3533_als_set_input_mode(struct lm3533_als *als, bool pwm_mode)
> > > +static int lm3533_als_setup(struct lm3533_als *als)
> > > {
> > > - u8 mask = LM3533_ALS_INPUT_MODE_MASK;
> > > - u8 val;
> > > + struct device *dev = &als->pdev.dev;
> > > int ret;
> > >
> > > - if (pwm_mode)
> > > - val = mask; /* pwm input */
> > > - else
> > > - val = 0; /* analog input */
> > > -
> > > - ret = lm3533_update(als->lm3533, LM3533_REG_ALS_CONF, val, mask);
> > > - if (ret) {
> > > - dev_err(&als->pdev->dev, "failed to set input mode %d\n",
> > > - pwm_mode);
> > > - return ret;
> > > - }
> > > -
> > > - return 0;
> > > -}
> > > -
> > > -static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
> > > -{
> > > - int ret;
> > > -
> > > - if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX) {
> > > - dev_err(&als->pdev->dev, "invalid resistor value\n");
> > > - return -EINVAL;
> > > - }
> > > -
> > > - ret = lm3533_write(als->lm3533, LM3533_REG_ALS_RESISTOR_SELECT, val);
> > > - if (ret) {
> > > - dev_err(&als->pdev->dev, "failed to set resistor\n");
> > > - return ret;
> > > - }
> > > + device_property_read_u32(dev, "ti,resistor-value-ohm",
> > > + &als->r_select);
> > Does this have a default? If so the pattern we've recently be setting on for IIO
> > is
> > if (device_property_present(dev, "ti,resistor-value-ohm"))
> > ret = device_property_read_u32();
> > if (ret) //corrupt property in some fashion
> > return ret;
> > } else {
> > //set default
> > }
> > If there is no default then check it unconditionally.
>
> default value is LM3533_ALS_RESISTOR_MIN and if no property is present
> clamp will ensure that als->r_select will be set to
> LM3533_ALS_RESISTOR_MIN
I don't see that default in the binding doc and relying in the 0 being clamped
isn't particularly readable - I'd set it explicitly.
>
> >
> > >
> > > - return 0;
> > > -}
> > > + als->r_select = clamp(als->r_select, LM3533_ALS_RESISTOR_MIN,
> > > + LM3533_ALS_RESISTOR_MAX);
> > > + als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * als->r_select);
> > >
> > > -static int lm3533_als_setup(struct lm3533_als *als,
> > > - const struct lm3533_als_platform_data *pdata)
> > > -{
> > > - int ret;
> > > + als->pwm_mode = device_property_read_bool(dev, "ti,pwm-mode");
> > >
> > > - ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> > > + ret = lm3533_update(lm3533, LM3533_REG_ALS_CONF, als->pwm_mode ?
> > > + LM3533_ALS_INPUT_MODE_MASK : 0,
> >
> > That's ugly. Better as
> >
> > ret = lm3533_update(lm3533, LM3533_REG_ALS_CONF,
> > als->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0,
> >
>
> Yes sure, just followed 80 char limit.
>
> > Though if there wasn't a layer hiding the regmap, it could just have been
> >
> > ret = regmap_assign_bits(lm3533->regmap, LM3533_REG_ALS_CONF,
> > LM3533_ALS_INPUT_MODE_MASK, als->pwm_mode);;
> >
> > which would have been nicer.
> >
> > I'm not particularly keen on the swashing of the helpers being in a patch
smashing. (this definitely wasn't my best effort at English!)
> > that is about switching the binding type as feels largely unrelated.
> > Should really have been a precursor, easier to review patch.
> >
>
> Removing of lm3533_update layer is not the scope of this patchset.
Understood. I'm fine with just the refactor you are doing brought out as a precursor
patch.
>
> >
> > > + LM3533_ALS_INPUT_MODE_MASK);
> > > if (ret)
> > > - return ret;
> > > + return dev_err_probe(dev, ret, "failed to set input mode %d\n",
> > > + als->pwm_mode);
> > >
> > > /* ALS input is always high impedance in PWM-mode. */
> > > - if (!pdata->pwm_mode) {
> > > - ret = lm3533_als_set_resistor(als, pdata->r_select);
> > > + if (!als->pwm_mode) {
> > > + ret = lm3533_write(lm3533, LM3533_REG_ALS_RESISTOR_SELECT,
> > > + (u8)als->r_select);
> >
> > Same applies here. Mostly an unrelated change as the only thing switching that
> > is related to the patch is one parameter.
> >
>
> Removing of lm3533_write layer is not the scope of this patchset.
>
> > > if (ret)
> > > - return ret;
> > > + return dev_err_probe(dev, ret, "failed to set resistor\n");
> > > }
> > >
> > > return 0;
> >
> > > @@ -852,25 +825,28 @@ static int lm3533_als_probe(struct platform_device *pdev)
> > > indio_dev->channels = lm3533_als_channels;
> > > indio_dev->num_channels = ARRAY_SIZE(lm3533_als_channels);
> > > indio_dev->name = dev_name(&pdev->dev);
> > > - iio_device_set_parent(indio_dev, pdev->dev.parent);
> >
> > I'm not sure why this was there in the first place. Hence not sure if it
> > is safe to remove.
> >
>
> This is directly related to OF conversion. The iio_device_set_parent
> bound indio_dev to parent, and it causes problems with OF now since
> als output has its own node and binding it to parent if wrong. Same
> story for backlight and leds btw.
Is there any risk anyone was using the canonical path to get to the iio dev?
/sys/bus/platform/devices/..../iio\:deviceX
This is technically an ABI change be it a subtle one.
>
> >
> > > diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
> > > index 45795f2a1042..d707d43d5526 100644
> > > --- a/drivers/leds/leds-lm3533.c
> > > +++ b/drivers/leds/leds-lm3533.c
> >
> > >
> > > led->cb.dev = led->cdev.dev;
> > >
> > > - ret = lm3533_led_setup(led, pdata);
> > > + device_property_read_u32(&pdev->dev, "led-max-microamp",
> > > + &led->max_current);
> >
> > I'd prefer explicit setting of the default to be visible before this, or
> > the property_present pattern I mention in the IIO review above.
> >
>
> clamp will ensure that led->max_current will be set to
> LM3533_LED_MAX_CURRENT_MIN regardless if it it present
As above, I'd prefer it set explicitly.
>
> > > + led->max_current = clamp(led->max_current, LM3533_LED_MAX_CURRENT_MIN,
> > > + LM3533_LED_MAX_CURRENT_MAX);
> >
> > I didn't look any further (busy day!)
>
^ permalink raw reply
* Re: [PATCH v2 2/6] mfd: lm3533: Convert to use OF bindings
From: Svyatoslav Ryhel @ 2026-05-29 9:39 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner, Nuno Sá,
Andy Shevchenko, Helge Deller, Johan Hovold, dri-devel,
linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev
In-Reply-To: <20260529100819.1823ebb3@jic23-huawei>
пт, 29 трав. 2026 р. о 12:08 Jonathan Cameron <jic23@kernel.org> пише:
>
> On Thu, 28 May 2026 18:03:31 +0300
> Svyatoslav Ryhel <clamor95@gmail.com> wrote:
>
> > чт, 28 трав. 2026 р. о 17:50 Jonathan Cameron <jic23@kernel.org> пише:
> > >
> > > On Thu, 28 May 2026 16:51:19 +0300
> > > Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> > >
> > > > Since there are no users of this driver via platform data, remove the
> > > > platform data support and switch to using Device Tree bindings.
> > > > Additionally, optimize functions used only by platform data.
> > >
> > >
> > > At least the IIO ones would have made much the same amount of sense for
> > > dt, just that they weren't having in the first place. I'd prefer that
>
> Gah. I write gibberish after too much reviewing. having/helping!
>
> > > as a precursor patch to make the rest much more readable.
> > >
> >
> > I can add you preferences into this commit, I don't mind.
> >
> > > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > >
> > > I only looked in detail at the iio bit. A few changes requested.
> > >
> > > > ---
> > > > drivers/iio/light/lm3533-als.c | 95 ++++------
> > > > drivers/leds/leds-lm3533.c | 51 ++++--
> > > > drivers/mfd/lm3533-core.c | 268 ++++++++++------------------
> > > > drivers/video/backlight/lm3533_bl.c | 52 ++++--
> > > > include/linux/mfd/lm3533.h | 51 +-----
> > > > 5 files changed, 212 insertions(+), 305 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> > > > index 99f0b903018c..cbd337b73bd9 100644
> > > > --- a/drivers/iio/light/lm3533-als.c
> > > > +++ b/drivers/iio/light/lm3533-als.c
> > >
> > > > @@ -714,59 +720,33 @@ static const struct attribute_group lm3533_als_attribute_group = {
> > > > .attrs = lm3533_als_attributes
> > > > };
> > > >
> > > > -static int lm3533_als_set_input_mode(struct lm3533_als *als, bool pwm_mode)
> > > > +static int lm3533_als_setup(struct lm3533_als *als)
> > > > {
> > > > - u8 mask = LM3533_ALS_INPUT_MODE_MASK;
> > > > - u8 val;
> > > > + struct device *dev = &als->pdev.dev;
> > > > int ret;
> > > >
> > > > - if (pwm_mode)
> > > > - val = mask; /* pwm input */
> > > > - else
> > > > - val = 0; /* analog input */
> > > > -
> > > > - ret = lm3533_update(als->lm3533, LM3533_REG_ALS_CONF, val, mask);
> > > > - if (ret) {
> > > > - dev_err(&als->pdev->dev, "failed to set input mode %d\n",
> > > > - pwm_mode);
> > > > - return ret;
> > > > - }
> > > > -
> > > > - return 0;
> > > > -}
> > > > -
> > > > -static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
> > > > -{
> > > > - int ret;
> > > > -
> > > > - if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX) {
> > > > - dev_err(&als->pdev->dev, "invalid resistor value\n");
> > > > - return -EINVAL;
> > > > - }
> > > > -
> > > > - ret = lm3533_write(als->lm3533, LM3533_REG_ALS_RESISTOR_SELECT, val);
> > > > - if (ret) {
> > > > - dev_err(&als->pdev->dev, "failed to set resistor\n");
> > > > - return ret;
> > > > - }
> > > > + device_property_read_u32(dev, "ti,resistor-value-ohm",
> > > > + &als->r_select);
> > > Does this have a default? If so the pattern we've recently be setting on for IIO
> > > is
> > > if (device_property_present(dev, "ti,resistor-value-ohm"))
> > > ret = device_property_read_u32();
> > > if (ret) //corrupt property in some fashion
> > > return ret;
> > > } else {
> > > //set default
> > > }
> > > If there is no default then check it unconditionally.
> >
> > default value is LM3533_ALS_RESISTOR_MIN and if no property is present
> > clamp will ensure that als->r_select will be set to
> > LM3533_ALS_RESISTOR_MIN
>
> I don't see that default in the binding doc and relying in the 0 being clamped
> isn't particularly readable - I'd set it explicitly.
>
Oh, ye, my bad. Schema enforces one of props to be present and if pwn
is present then resistor is ignored. What if I move resistor reading,
clamping and conversion under !als->pwm_mode check? Then resistor must
be present and hence must be checked unconditionally.
Additionally, I can comment original lm3533_als_setup with #if 0
#endif then git formatting will be much cleaner and easier to review,
and once we all come to result I will just remove entire commented
block and Lee can pick clean commits.
>
> >
> > >
> > > >
> > > > - return 0;
> > > > -}
> > > > + als->r_select = clamp(als->r_select, LM3533_ALS_RESISTOR_MIN,
> > > > + LM3533_ALS_RESISTOR_MAX);
> > > > + als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * als->r_select);
> > > >
> > > > -static int lm3533_als_setup(struct lm3533_als *als,
> > > > - const struct lm3533_als_platform_data *pdata)
> > > > -{
> > > > - int ret;
> > > > + als->pwm_mode = device_property_read_bool(dev, "ti,pwm-mode");
> > > >
> > > > - ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> > > > + ret = lm3533_update(lm3533, LM3533_REG_ALS_CONF, als->pwm_mode ?
> > > > + LM3533_ALS_INPUT_MODE_MASK : 0,
> > >
> > > That's ugly. Better as
> > >
> > > ret = lm3533_update(lm3533, LM3533_REG_ALS_CONF,
> > > als->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0,
> > >
> >
> > Yes sure, just followed 80 char limit.
> >
> > > Though if there wasn't a layer hiding the regmap, it could just have been
> > >
> > > ret = regmap_assign_bits(lm3533->regmap, LM3533_REG_ALS_CONF,
> > > LM3533_ALS_INPUT_MODE_MASK, als->pwm_mode);;
> > >
> > > which would have been nicer.
> > >
> > > I'm not particularly keen on the swashing of the helpers being in a patch
>
> smashing. (this definitely wasn't my best effort at English!)
>
> > > that is about switching the binding type as feels largely unrelated.
> > > Should really have been a precursor, easier to review patch.
> > >
> >
> > Removing of lm3533_update layer is not the scope of this patchset.
>
> Understood. I'm fine with just the refactor you are doing brought out as a precursor
> patch.
>
I have looked into removing wrappers too. That seems to be less a
hassle that I anticipated, so I will include regmap switch in the v2.
> >
> > >
> > > > + LM3533_ALS_INPUT_MODE_MASK);
> > > > if (ret)
> > > > - return ret;
> > > > + return dev_err_probe(dev, ret, "failed to set input mode %d\n",
> > > > + als->pwm_mode);
> > > >
> > > > /* ALS input is always high impedance in PWM-mode. */
> > > > - if (!pdata->pwm_mode) {
> > > > - ret = lm3533_als_set_resistor(als, pdata->r_select);
> > > > + if (!als->pwm_mode) {
> > > > + ret = lm3533_write(lm3533, LM3533_REG_ALS_RESISTOR_SELECT,
> > > > + (u8)als->r_select);
> > >
> > > Same applies here. Mostly an unrelated change as the only thing switching that
> > > is related to the patch is one parameter.
> > >
> >
> > Removing of lm3533_write layer is not the scope of this patchset.
> >
> > > > if (ret)
> > > > - return ret;
> > > > + return dev_err_probe(dev, ret, "failed to set resistor\n");
> > > > }
> > > >
> > > > return 0;
> > >
> > > > @@ -852,25 +825,28 @@ static int lm3533_als_probe(struct platform_device *pdev)
> > > > indio_dev->channels = lm3533_als_channels;
> > > > indio_dev->num_channels = ARRAY_SIZE(lm3533_als_channels);
> > > > indio_dev->name = dev_name(&pdev->dev);
> > > > - iio_device_set_parent(indio_dev, pdev->dev.parent);
> > >
> > > I'm not sure why this was there in the first place. Hence not sure if it
> > > is safe to remove.
> > >
> >
> > This is directly related to OF conversion. The iio_device_set_parent
> > bound indio_dev to parent, and it causes problems with OF now since
> > als output has its own node and binding it to parent if wrong. Same
> > story for backlight and leds btw.
>
> Is there any risk anyone was using the canonical path to get to the iio dev?
> /sys/bus/platform/devices/..../iio\:deviceX
> This is technically an ABI change be it a subtle one.
>
Linux kernel has no users of this driver, and it is in "stale" state
for more then 2 years (maybe even longer). I have cc'd Johan Hovold.
https://lore.kernel.org/lkml/ZmBcvtLCzllQDWVX@hovoldconsulting.com/
This this 2 y. o. discussion and there were no actions ore movements.
I assume this driver in its current form has no more users. This does
not mean that it cannot be revived though.
>
> >
> > >
> > > > diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
> > > > index 45795f2a1042..d707d43d5526 100644
> > > > --- a/drivers/leds/leds-lm3533.c
> > > > +++ b/drivers/leds/leds-lm3533.c
> > >
> > > >
> > > > led->cb.dev = led->cdev.dev;
> > > >
> > > > - ret = lm3533_led_setup(led, pdata);
> > > > + device_property_read_u32(&pdev->dev, "led-max-microamp",
> > > > + &led->max_current);
> > >
> > > I'd prefer explicit setting of the default to be visible before this, or
> > > the property_present pattern I mention in the IIO review above.
> > >
> >
> > clamp will ensure that led->max_current will be set to
> > LM3533_LED_MAX_CURRENT_MIN regardless if it it present
>
> As above, I'd prefer it set explicitly.
>
I understand your position and I am not denying it for ALS part, but
LEDs don't belong to IIO subsystem and different subsystem maintainers
may have drastically different preferences and requirements (ugh, PTSD
in its full glory).
> >
> > > > + led->max_current = clamp(led->max_current, LM3533_LED_MAX_CURRENT_MIN,
> > > > + LM3533_LED_MAX_CURRENT_MAX);
> > >
> > > I didn't look any further (busy day!)
> >
>
^ permalink raw reply
* Re: [PATCH v2 1/6] dt-bindings: leds: Document TI LM3533 LED controller
From: Daniel Thompson @ 2026-05-29 9:51 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
In-Reply-To: <20260528135123.103745-2-clamor95@gmail.com>
On Thu, May 28, 2026 at 04:51:18PM +0300, Svyatoslav Ryhel wrote:
> Document the LM3533 - a complete power source for backlight, keypad and
> indicator LEDs in smartphone handsets. The high-voltage inductive boost
> converter provides the power for two series LED strings display backlight
> and keypad functions.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
> .../leds/backlight/ti,lm3533-backlight.yaml | 68 +++++++
> .../bindings/leds/ti,lm3533-leds.yaml | 66 +++++++
> .../devicetree/bindings/leds/ti,lm3533.yaml | 170 ++++++++++++++++++
> 3 files changed, 304 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
> create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml
> create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3533.yaml
>
> diff --git a/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
> new file mode 100644
> index 000000000000..866b0fb8ed04
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/ti,lm3533-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI LM3533 high voltage series LED strings
> +
> +description:
> + This is part of the TI LM3533 MFD device. It represents two high voltage series
> + LED strings for display backlight controlled by the TI LM3533.
> +
> +maintainers:
> + - Svyatoslav Ryhel <clamor95@gmail.com>
> +
> +allOf:
> + - $ref: /schemas/leds/backlight/common.yaml#
> +
> +properties:
> + compatible:
> + const: ti,lm3533-backlight
> +
> + reg:
> + description: Control bank selection (0 = bank A, 1 = bank B).
> + maximum: 1
> <snip>
> + ti,pwm-config-mask:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Control Bank PWM Configuration Register mask that allows to configure
> + PWM input in Zones 0-4
> + BIT(0) - PWM Input is enabled
> + BIT(1) - PWM Input is enabled in Zone 0
> + BIT(2) - PWM Input is enabled in Zone 1
> + BIT(3) - PWM Input is enabled in Zone 2
> + BIT(4) - PWM Input is enabled in Zone 3
> + BIT(5) - PWM Input is enabled in Zone 4
This is optional and the drive implements a default (zero) that is not
documented here.
Is zero a sane default from a DT binding point of view?
Daniel.
^ permalink raw reply
* [PATCH v2] staging: sm750fb: rename pv_reg to io_base
From: Onish Sharma @ 2026-05-29 9:52 UTC (permalink / raw)
To: sudipm.mukherjee
Cc: teddy.wang, gregkh, linux-fbdev, linux-staging, linux-kernel,
Onish Sharma
In-Reply-To: <ahhN_kiSb-yRWfiI@stanley.mountain>
Rename pv_reg to io_base to follow kernel naming style and improve
readability.
No functional changes intended.
---
drivers/staging/sm750fb/sm750.c | 4 ++--
drivers/staging/sm750fb/sm750.h | 2 +-
drivers/staging/sm750fb/sm750_hw.c | 12 ++++++------
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 716a8935f58d..c2d2864f135b 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -743,7 +743,7 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
* must be set after crtc member initialized
*/
crtc->cursor.offset = crtc->o_screen + crtc->vidmem_size - 1024;
- crtc->cursor.mmio = sm750_dev->pv_reg +
+ crtc->cursor.mmio = sm750_dev->io_base +
0x800f0 + (int)crtc->channel * 0x140;
crtc->cursor.max_h = 64;
@@ -1047,7 +1047,7 @@ static void lynxfb_pci_remove(struct pci_dev *pdev)
sm750fb_framebuffer_release(sm750_dev);
arch_phys_wc_del(sm750_dev->mtrr.vram);
- iounmap(sm750_dev->pv_reg);
+ iounmap(sm750_dev->io_base);
iounmap(sm750_dev->vmem);
pci_release_region(pdev, 1);
kfree(g_settings);
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index e8885133da2e..c42800313c6a 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -97,7 +97,7 @@ struct sm750_dev {
unsigned long vidreg_start;
__u32 vidmem_size;
__u32 vidreg_size;
- void __iomem *pv_reg;
+ void __iomem *io_base;
unsigned char __iomem *vmem;
/* locks*/
spinlock_t slock;
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index 95f797e5776a..dc1118808b4f 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -23,18 +23,18 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
}
/* now map mmio and vidmem */
- sm750_dev->pv_reg =
+ sm750_dev->io_base =
ioremap(sm750_dev->vidreg_start, sm750_dev->vidreg_size);
- if (!sm750_dev->pv_reg) {
+ if (!sm750_dev->io_base) {
dev_err(&pdev->dev, "mmio failed\n");
ret = -EFAULT;
goto err_release_region;
}
- sm750_dev->accel.dpr_base = sm750_dev->pv_reg + DE_BASE_ADDR_TYPE1;
- sm750_dev->accel.dp_port_base = sm750_dev->pv_reg + DE_PORT_ADDR_TYPE1;
+ sm750_dev->accel.dpr_base = sm750_dev->io_base + DE_BASE_ADDR_TYPE1;
+ sm750_dev->accel.dp_port_base = sm750_dev->io_base + DE_PORT_ADDR_TYPE1;
- mmio750 = sm750_dev->pv_reg;
+ mmio750 = sm750_dev->io_base;
sm750_set_chip_type(sm750_dev->devid, sm750_dev->revid);
sm750_dev->vidmem_start = pci_resource_start(pdev, 0);
@@ -58,7 +58,7 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
return 0;
err_unmap_reg:
- iounmap(sm750_dev->pv_reg);
+ iounmap(sm750_dev->io_base);
err_release_region:
pci_release_region(pdev, 1);
return ret;
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] backlight: Use named initializers for arrays of i2c_device_data
From: Daniel Thompson @ 2026-05-29 10:01 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Lee Jones, Jingoo Han, Michael Hennerich, Helge Deller,
Junjie Cao, Jianhua Lu, Flavio Suligoi, dri-devel, linux-fbdev,
linux-kernel
In-Reply-To: <20260518111203.639603-2-u.kleine-koenig@baylibre.com>
On Mon, May 18, 2026 at 01:12:03PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> While being less compact, using named initializers allows to more easily
> see which members of the structs are assigned which value without having
> to lookup the declaration of the struct. And it's also more robust
> against changes to the struct definition.
>
> The mentioned robustness is relevant for a planned change to struct
> i2c_device_id that replaces .driver_data by an anonymous union.
>
> While touching all these arrays, unify usage of whitespace in the list
> terminator.
>
> This patch doesn't modify the compiled arrays, only their representation
> in source form benefits. The former was confirmed with x86 and arm64
> builds.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Daniel.
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver
From: Daniel Thompson @ 2026-05-29 10:07 UTC (permalink / raw)
To: Neil Armstrong
Cc: Lee Jones, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, dri-devel,
linux-leds, devicetree, linux-kernel, linux-fbdev, KancyJoe,
Krzysztof Kozlowski
In-Reply-To: <20260519-topic-sm8650-ayaneo-pocket-s2-sy7758-v3-1-ec8194bbc885@linaro.org>
On Tue, May 19, 2026 at 10:43:38AM +0200, Neil Armstrong wrote:
> Document the Silergy SY7758 6-channel High Efficiency LED Driver
> used for backlight brightness control.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
> .../bindings/leds/backlight/silergy,sy7758.yaml | 53 ++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/leds/backlight/silergy,sy7758.yaml b/Documentation/devicetree/bindings/leds/backlight/silergy,sy7758.yaml
> new file mode 100644
> index 000000000000..80e978d691c2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/silergy,sy7758.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/silergy,sy7758.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Silergy SY7758 6-channel High Efficiency LED Driver
> +
> +maintainers:
> + - Neil Armstrong <neil.armstrong@linaro.org>
> +
> +description:
> + Silergy SY7758 is a high efficiency 6-channels LED backlight
> + driver with I2C brightness control.
> +
> +allOf:
> + - $ref: common.yaml#
> +
> +properties:
> + compatible:
> + const: silergy,sy7758
> +
> + reg:
> + maxItems: 1
> +
> + vddio-supply: true
> +
> + enable-gpios:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - vddio-supply
Sorry for missing this in v2 but is vddio-supply really a required
property?
It's unusual for supplies to be mandatory (and the it is not mandatory
in the driver implementation).
Daniel.
^ permalink raw reply
* Re: [PATCH v2 1/6] dt-bindings: leds: Document TI LM3533 LED controller
From: Svyatoslav Ryhel @ 2026-05-29 10:07 UTC (permalink / raw)
To: Daniel Thompson
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
In-Reply-To: <ahlhinOh3NxB7FY_@aspen.lan>
пт, 29 трав. 2026 р. о 12:51 Daniel Thompson <daniel@riscstar.com> пише:
>
> On Thu, May 28, 2026 at 04:51:18PM +0300, Svyatoslav Ryhel wrote:
> > Document the LM3533 - a complete power source for backlight, keypad and
> > indicator LEDs in smartphone handsets. The high-voltage inductive boost
> > converter provides the power for two series LED strings display backlight
> > and keypad functions.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> > .../leds/backlight/ti,lm3533-backlight.yaml | 68 +++++++
> > .../bindings/leds/ti,lm3533-leds.yaml | 66 +++++++
> > .../devicetree/bindings/leds/ti,lm3533.yaml | 170 ++++++++++++++++++
> > 3 files changed, 304 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
> > create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml
> > create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3533.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
> > new file mode 100644
> > index 000000000000..866b0fb8ed04
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
> > @@ -0,0 +1,68 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/leds/backlight/ti,lm3533-backlight.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TI LM3533 high voltage series LED strings
> > +
> > +description:
> > + This is part of the TI LM3533 MFD device. It represents two high voltage series
> > + LED strings for display backlight controlled by the TI LM3533.
> > +
> > +maintainers:
> > + - Svyatoslav Ryhel <clamor95@gmail.com>
> > +
> > +allOf:
> > + - $ref: /schemas/leds/backlight/common.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: ti,lm3533-backlight
> > +
> > + reg:
> > + description: Control bank selection (0 = bank A, 1 = bank B).
> > + maximum: 1
> > <snip>
> > + ti,pwm-config-mask:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: |
> > + Control Bank PWM Configuration Register mask that allows to configure
> > + PWM input in Zones 0-4
> > + BIT(0) - PWM Input is enabled
> > + BIT(1) - PWM Input is enabled in Zone 0
> > + BIT(2) - PWM Input is enabled in Zone 1
> > + BIT(3) - PWM Input is enabled in Zone 2
> > + BIT(4) - PWM Input is enabled in Zone 3
> > + BIT(5) - PWM Input is enabled in Zone 4
>
> This is optional and the drive implements a default (zero) that is not
> documented here.
>
> Is zero a sane default from a DT binding point of view?
>
Yes, if property is missing then PWM input is disabled which is
equivalent to setting all bits to 0.
>
> Daniel.
^ permalink raw reply
* [PATCH v2] staging: sm750fb: remove unused variable
From: Onish Sharma @ 2026-05-29 10:12 UTC (permalink / raw)
To: sudipm.mukherjee
Cc: teddy.wang, gregkh, linux-fbdev, linux-staging, linux-kernel,
Onish Sharma, Dan Carpenter
In-Reply-To: <ahlXYIqzu4O5-u9J@stanley.mountain>
Remove the set_all_eng_off flag and its associated cleanup logic.
The variable is redundant as the hardware should be initialized to a
known state regardless of prior usage.
Suggested-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Onish Sharma <neharora23587@gmail.com>
---
drivers/staging/sm750fb/ddk750_chip.c | 28 ---------------------------
drivers/staging/sm750fb/ddk750_chip.h | 7 -------
drivers/staging/sm750fb/sm750.c | 1 -
drivers/staging/sm750fb/sm750.h | 1 -
4 files changed, 37 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index 0bb56bbec43f..553654a77170 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -262,34 +262,6 @@ int ddk750_init_hw(struct initchip_param *p_init_param)
reg |= MISC_CTRL_LOCALMEM_RESET;
poke32(MISC_CTRL, reg);
}
-
- if (p_init_param->set_all_eng_off == 1) {
- sm750_enable_2d_engine(0);
-
- /* Disable Overlay, if a former application left it on */
- reg = peek32(VIDEO_DISPLAY_CTRL);
- reg &= ~DISPLAY_CTRL_PLANE;
- poke32(VIDEO_DISPLAY_CTRL, reg);
-
- /* Disable video alpha, if a former application left it on */
- reg = peek32(VIDEO_ALPHA_DISPLAY_CTRL);
- reg &= ~DISPLAY_CTRL_PLANE;
- poke32(VIDEO_ALPHA_DISPLAY_CTRL, reg);
-
- /* Disable alpha plane, if a former application left it on */
- reg = peek32(ALPHA_DISPLAY_CTRL);
- reg &= ~DISPLAY_CTRL_PLANE;
- poke32(ALPHA_DISPLAY_CTRL, reg);
-
- /* Disable DMA Channel, if a former application left it on */
- reg = peek32(DMA_ABORT_INTERRUPT);
- reg |= DMA_ABORT_INTERRUPT_ABORT_1;
- poke32(DMA_ABORT_INTERRUPT, reg);
-
- /* Disable DMA Power, if a former application left it on */
- sm750_enable_dma(0);
- }
-
/* We can add more initialization as needed. */
return 0;
diff --git a/drivers/staging/sm750fb/ddk750_chip.h b/drivers/staging/sm750fb/ddk750_chip.h
index ee2e9d90f7dd..2a13debc179f 100644
--- a/drivers/staging/sm750fb/ddk750_chip.h
+++ b/drivers/staging/sm750fb/ddk750_chip.h
@@ -76,13 +76,6 @@ struct initchip_param {
*/
unsigned short master_clock;
- /*
- * 0 = leave all engine state untouched.
- * 1 = make sure they are off: 2D, Overlay,
- * video alpha, alpha, hardware cursors
- */
- unsigned short set_all_eng_off;
-
/*
* 0 = Do not reset the memory controller
* 1 = Reset the memory controller
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index c2d2864f135b..127db29883d2 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -848,7 +848,6 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, char *src)
sm750_dev->init_parm.mem_clk = 0;
sm750_dev->init_parm.master_clk = 0;
sm750_dev->init_parm.power_mode = 0;
- sm750_dev->init_parm.set_all_eng_off = 0;
sm750_dev->init_parm.reset_memory = 1;
/* defaultly turn g_hwcursor on for both view */
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index c42800313c6a..7ab13da5d7cc 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -44,7 +44,6 @@ struct init_status {
ushort chip_clk;
ushort mem_clk;
ushort master_clk;
- ushort set_all_eng_off;
ushort reset_memory;
};
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v3 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver
From: Neil Armstrong @ 2026-05-29 10:16 UTC (permalink / raw)
To: Daniel Thompson
Cc: Lee Jones, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, dri-devel,
linux-leds, devicetree, linux-kernel, linux-fbdev, KancyJoe,
Krzysztof Kozlowski
In-Reply-To: <ahllT_HVTAJ5MbkS@aspen.lan>
On 5/29/26 12:07, Daniel Thompson wrote:
> On Tue, May 19, 2026 at 10:43:38AM +0200, Neil Armstrong wrote:
>> Document the Silergy SY7758 6-channel High Efficiency LED Driver
>> used for backlight brightness control.
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>> .../bindings/leds/backlight/silergy,sy7758.yaml | 53 ++++++++++++++++++++++
>> 1 file changed, 53 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/backlight/silergy,sy7758.yaml b/Documentation/devicetree/bindings/leds/backlight/silergy,sy7758.yaml
>> new file mode 100644
>> index 000000000000..80e978d691c2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/backlight/silergy,sy7758.yaml
>> @@ -0,0 +1,53 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/leds/backlight/silergy,sy7758.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Silergy SY7758 6-channel High Efficiency LED Driver
>> +
>> +maintainers:
>> + - Neil Armstrong <neil.armstrong@linaro.org>
>> +
>> +description:
>> + Silergy SY7758 is a high efficiency 6-channels LED backlight
>> + driver with I2C brightness control.
>> +
>> +allOf:
>> + - $ref: common.yaml#
>> +
>> +properties:
>> + compatible:
>> + const: silergy,sy7758
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + vddio-supply: true
>> +
>> + enable-gpios:
>> + maxItems: 1
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - vddio-supply
>
> Sorry for missing this in v2 but is vddio-supply really a required
> property?
>
> It's unusual for supplies to be mandatory (and the it is not mandatory
> in the driver implementation).
This device is a little bit special, the VDDIO regulator is used to provide
power for the I/O via the enable input, so basically the enable gpio power
level is provided by VDDIO.
This is the recommended way from the datasheet, and I assume it will be used
like that on other platforms (if it exists...)
This is why it's mandatory and enabled first before setting the enable pin.
This should probably be a comment in the code.
Neil
>
>
> Daniel.
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver
From: Daniel Thompson @ 2026-05-29 10:35 UTC (permalink / raw)
To: Neil Armstrong
Cc: Lee Jones, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, dri-devel,
linux-leds, devicetree, linux-kernel, linux-fbdev, KancyJoe,
Krzysztof Kozlowski
In-Reply-To: <e3c99fe3-9279-4dfa-af69-d9366ab06837@linaro.org>
On Fri, May 29, 2026 at 12:16:07PM +0200, Neil Armstrong wrote:
> On 5/29/26 12:07, Daniel Thompson wrote:
> > On Tue, May 19, 2026 at 10:43:38AM +0200, Neil Armstrong wrote:
> > > Document the Silergy SY7758 6-channel High Efficiency LED Driver
> > > used for backlight brightness control.
> > >
> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > ---
> > > .../bindings/leds/backlight/silergy,sy7758.yaml | 53 ++++++++++++++++++++++
> > > 1 file changed, 53 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/leds/backlight/silergy,sy7758.yaml b/Documentation/devicetree/bindings/leds/backlight/silergy,sy7758.yaml
> > > new file mode 100644
> > > index 000000000000..80e978d691c2
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/leds/backlight/silergy,sy7758.yaml
> > > @@ -0,0 +1,53 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/leds/backlight/silergy,sy7758.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Silergy SY7758 6-channel High Efficiency LED Driver
> > > +
> > > +maintainers:
> > > + - Neil Armstrong <neil.armstrong@linaro.org>
> > > +
> > > +description:
> > > + Silergy SY7758 is a high efficiency 6-channels LED backlight
> > > + driver with I2C brightness control.
> > > +
> > > +allOf:
> > > + - $ref: common.yaml#
> > > +
> > > +properties:
> > > + compatible:
> > > + const: silergy,sy7758
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + vddio-supply: true
> > > +
> > > + enable-gpios:
> > > + maxItems: 1
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - vddio-supply
> >
> > Sorry for missing this in v2 but is vddio-supply really a required
> > property?
> >
> > It's unusual for supplies to be mandatory (and the it is not mandatory
> > in the driver implementation).
>
> This device is a little bit special, the VDDIO regulator is used to provide
> power for the I/O via the enable input, so basically the enable gpio power
> level is provided by VDDIO.
I don't follow. The EN pin acts as both VDDIO and as an enable but it's
still effectively a power rail isn't it (albeit one with very low current
draw).
It looked to me like the correct way to model to two power rails
going into the chip is vdd-supply (main power supply) and vddio-supply
(EN/VDDIO) I don't understand why a single pin needs both a regulator
*and* a GPIO in the DT bindings?
> This is the recommended way from the datasheet, and I assume it will be used
> like that on other platforms (if it exists...)
>
> This is why it's mandatory and enabled first before setting the enable pin.
It's not mandatory for the C implementation. devm_regulator_get_enable()
will provide a dummy regulator if the property is omitted.
Daniel.
^ permalink raw reply
* Re: [PATCH v2] staging: sm750fb: remove unused variable
From: Dan Carpenter @ 2026-05-29 10:39 UTC (permalink / raw)
To: Onish Sharma
Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260529101242.10189-1-neharora23587@gmail.com>
On Fri, May 29, 2026 at 03:42:42PM +0530, Onish Sharma wrote:
> Remove the set_all_eng_off flag and its associated cleanup logic.
> The variable is redundant as the hardware should be initialized to a
> known state regardless of prior usage.
>
> Suggested-by: Dan Carpenter <error27@gmail.com>
> Signed-off-by: Onish Sharma <neharora23587@gmail.com>
> ---
Sorry, miscommunication. This breaks the driver. This is also a bit
more involved than I thought...
There are two structs:
struct init_status {
ushort power_mode;
/* below three clocks are in unit of MHZ*/
ushort chip_clk;
ushort mem_clk;
ushort master_clk;
ushort setAllEngOff;
ushort reset_memory;
};
And struct initchip_param. The initchip_param is exactly the same but
with all the struct members renamed and comments added. They have to
match because we cast back and forth.
Why do we have two different struct that have to be the same? You might
think it is for API, but as near as I can see that is not the case.
Maybe it was at some point? We should get rid of one struct. Which
everyone is API is the one we should keep. If neither is API then get
rid of init_status and keep initchip_param.
After that we can talk about getting rid of setAllEngOff/set_all_eng_off.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v2] staging: sm750fb: rename pv_reg to io_base
From: Dan Carpenter @ 2026-05-29 10:41 UTC (permalink / raw)
To: Onish Sharma
Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260529095233.9015-1-neharora23587@gmail.com>
On Fri, May 29, 2026 at 03:22:33PM +0530, Onish Sharma wrote:
> Rename pv_reg to io_base to follow kernel naming style and improve
> readability.
>
> No functional changes intended.
Run your patches through checkpatch. Also v2 patches need to be sent
in a specific format. This should be [PATCH v2 1/2]
https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/
> ---
^^^
There needs to be a note here to say what changed.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v2] staging: sm750fb: remove unused variable
From: Dan Carpenter @ 2026-05-29 10:44 UTC (permalink / raw)
To: Onish Sharma
Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <ahlszyY6Nd9ANz-X@stanley.mountain>
On Fri, May 29, 2026 at 01:39:11PM +0300, Dan Carpenter wrote:
> Which everyone is API is the one we should keep.
s/everyone/ever one/
regards,
dan carpenter
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox