* [PATCH v2] fbdev: omap2: fix use-after-free in omapfb_mmap
From: Hongling Zeng @ 2026-06-02 8:54 UTC (permalink / raw)
To: deller, kees
Cc: linux-omap, linux-fbdev, dri-devel, linux-kernel, zhongling0719,
Hongling Zeng, stable
omapfb_mmap() has a race condition with OMAPFB_SETUP_PLANE ioctl that
can lead to use-after-free:
The fb_mmap() entry point holds mm_lock but not lock (fb_info->lock),
while ioctl handlers like OMAPFB_SETUP_PLANE hold lock but not mm_lock.
This allows concurrent execution.
In omapfb_mmap():
1. rg = omapfb_get_mem_region(ofbi->region); // Get old region ref
2. start = omapfb_get_region_paddr(ofbi); // Read from NEW region
3. len = fix->smem_len; // Read from NEW region
4. vm_iomap_memory(vma, start, len); // Map NEW region memory
5. atomic_inc(&rg->map_count); // Increment OLD region!
Concurrently, OMAPFB_SETUP_PLANE can:
- Reassign ofbi->region = new_rg
- Update fix->smem_len
- OMAPFB_SETUP_MEM then checks NEW region's map_count (0!) and frees it
This leaves userspace with a mapping to freed physical memory.
The fix is to read all required values (start, len) from the same
region reference (rg) that will have its map_count incremented,
preventing the region from being freed while still mapped.
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
Change in V2:
-Restore fix->smem_len to maintain VRFB sparse mapping.
-Increment map_count before mapping to prevent use-after-free
on driver unload
-Add proper error handling for map_count
---
drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
index d70deb6a9150..046892682fc6 100644
--- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
@@ -1099,7 +1099,11 @@ static int omapfb_mmap(struct fb_info *fbi, struct vm_area_struct *vma)
rg = omapfb_get_mem_region(ofbi->region);
- start = omapfb_get_region_paddr(ofbi);
+ if (ofbi->rotation_type == OMAP_DSS_ROT_VRFB)
+ start = rg->vrfb.paddr[0];
+ else
+ start = rg->paddr;
+
len = fix->smem_len;
DBG("user mmap region start %lx, len %d, off %lx\n", start, len,
@@ -1109,6 +1113,8 @@ static int omapfb_mmap(struct fb_info *fbi, struct vm_area_struct *vma)
vma->vm_ops = &mmap_user_ops;
vma->vm_private_data = rg;
+ atomic_inc(&rg->map_count);
+
r = vm_iomap_memory(vma, start, len);
if (r)
goto error;
@@ -1121,6 +1127,7 @@ static int omapfb_mmap(struct fb_info *fbi, struct vm_area_struct *vma)
return 0;
error:
+ atomic_dec(&rg->map_count);
omapfb_put_mem_region(rg);
return r;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v3 02/11] mfd: lm3533: Remove driver specific regmap wrappers
From: Svyatoslav Ryhel @ 2026-06-02 10:18 UTC (permalink / raw)
To: Andy Shevchenko
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: <ah6O1h8SPwjf3rV1@ashevche-desk.local>
вт, 2 черв. 2026 р. о 11:05 Andy Shevchenko <andriy.shevchenko@intel.com> пише:
>
> On Mon, Jun 01, 2026 at 06:18:22PM +0300, Svyatoslav Ryhel wrote:
> > Remove driver-specific regmap wrappers in favor of using regmap helpers
> > directly. The wrappers are mostly equivalent to the standard helpers, with
> > two exceptions: regmap_read requires an unsigned int pointer, and
> > regmap_update_bits has the mask and value arguments swapped. These
> > differences were accounted for and adjusted accordingly.
>
> We refer to functions as func(), exempli gratia, regmap_read().
>
Noted.
> ...
>
> > static int lm3533_als_get_current(struct iio_dev *indio_dev, unsigned channel,
> > int *val)
> > {
> > u8 zone;
> > - u8 target;
> > + u32 target;
> > int ret;
>
> While at it, move towards reversed xmas tree order
>
> u32 target;
> u8 zone;
> int ret;
>
Noted.
>
> ...
>
> > - ret = lm3533_update(als->lm3533, LM3533_REG_ALS_ZONE_INFO, val, mask);
> > + ret = regmap_update_bits(als->lm3533->regmap, LM3533_REG_ALS_ZONE_INFO,
> > + val, mask);
>
> It's better to replace this to use _set_bits()/_clear_bits() or even move from
> the above conditional (not in this context) to _assign_bits().
>
I will take a look.
> ...
>
> > else
> > val = 0; /* analog input */
> >
> > - ret = lm3533_update(als->lm3533, LM3533_REG_ALS_CONF, val, mask);
> > + ret = regmap_update_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
> > + mask, val);
>
> Ditto.
>
> > if (ret) {
> > dev_err(&als->pdev->dev, "failed to set input mode %d\n",
> > pwm_mode);
>
> ...
>
> > /* Make sure interrupts are disabled. */
> > - ret = lm3533_update(als->lm3533, LM3533_REG_ALS_ZONE_INFO, 0, mask);
> > + ret = regmap_update_bits(als->lm3533->regmap, LM3533_REG_ALS_ZONE_INFO,
> > + mask, 0);
>
> _clear_bits().
>
> > if (ret) {
> > dev_err(&als->pdev->dev, "failed to disable interrupts\n");
> > return ret;
>
> ...
>
> > u8 mask = LM3533_ALS_ENABLE_MASK;
> > int ret;
> >
> > - ret = lm3533_update(als->lm3533, LM3533_REG_ALS_CONF, mask, mask);
> > + ret = regmap_update_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
> > + mask, mask);
>
> _set_bits()
>
> > if (ret)
> > dev_err(&als->pdev->dev, "failed to enable ALS\n");
> >
>
> ...
>
> > u8 mask = LM3533_ALS_ENABLE_MASK;
> > int ret;
> >
> > - ret = lm3533_update(als->lm3533, LM3533_REG_ALS_CONF, 0, mask);
> > + ret = regmap_update_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
> > + mask, 0);
>
> _clear_bits()
>
> > if (ret)
> > dev_err(&als->pdev->dev, "failed to disable ALS\n");
>
> ...
>
> > else
> > val = 0;
> >
> > - ret = lm3533_update(led->lm3533, LM3533_REG_PATTERN_ENABLE, val, mask);
> > + ret = regmap_update_bits(led->lm3533->regmap,
> > + LM3533_REG_PATTERN_ENABLE, mask, val);
>
> _assign_bits() and so on...
>
> > if (ret) {
> > dev_err(led->cdev.dev, "failed to enable pattern %d (%d)\n",
> > pattern, enable);
>
> ...
>
> > extern int lm3533_ctrlbank_set_brightness(struct lm3533_ctrlbank *cb, u8 val);
> > -extern int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u8 *val);
> > +extern int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u32 *val);
>
> We don't need to keep 'extern' for ages.
>
I will no inflate this patchset further
> > extern int lm3533_ctrlbank_set_max_current(struct lm3533_ctrlbank *cb,
> > u16 imax);
> > extern int lm3533_ctrlbank_set_pwm(struct lm3533_ctrlbank *cb, u8 val);
> > -extern int lm3533_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u8 *val);
> > -
> > -extern int lm3533_read(struct lm3533 *lm3533, u8 reg, u8 *val);
> > -extern int lm3533_write(struct lm3533 *lm3533, u8 reg, u8 val);
> > -extern int lm3533_update(struct lm3533 *lm3533, u8 reg, u8 val, u8 mask);
> > +extern int lm3533_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u32 *val);
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v3 08/11] video: backlight: lm3533_bl: Improve linear sysfs logic
From: Svyatoslav Ryhel @ 2026-06-02 10:19 UTC (permalink / raw)
To: Andy Shevchenko
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: <ah6PxFtoJUWkd79P@ashevche-desk.local>
вт, 2 черв. 2026 р. о 11:09 Andy Shevchenko <andriy.shevchenko@intel.com> пише:
>
> On Mon, Jun 01, 2026 at 06:18:28PM +0300, Svyatoslav Ryhel wrote:
> > Simplify the sysfs logic of the linear property by switching to a macro
> > and a ternary operator.
>
> ...
>
> > if (kstrtoul(buf, 0, &linear))
> > return -EINVAL;
>
> Besides _assign_bits() in the below, side note here to unshadow error codes:
>
> ret = kstrtoul(buf, 0, &linear);
> if (ret)
> return ret;
>
> (obviously in a separate change).
Won't happen in this patches.
>
> ...
>
> > ret = regmap_update_bits(bl->lm3533->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
> > - mask, val);
> > + CTRLBANK_AB_BCONF_MODE(id),
> > + linear ? CTRLBANK_AB_BCONF_MODE(id) : 0);
> > if (ret)
> > return ret;
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v3 05/11] mfd: lm3533: Convert to use OF bindings
From: Svyatoslav Ryhel @ 2026-06-02 10:31 UTC (permalink / raw)
To: Andy Shevchenko
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: <ah6TGjRNnDpQGO60@ashevche-desk.local>
вт, 2 черв. 2026 р. о 11:24 Andy Shevchenko <andriy.shevchenko@intel.com> пише:
>
> On Mon, Jun 01, 2026 at 06:18:25PM +0300, Svyatoslav Ryhel wrote:
> > Since there are no users of this driver via platform data, remove the
> > platform data support and switch to using Device Tree bindings.
>
> ...
>
> > @@ -57,6 +60,9 @@ struct lm3533_als {
> >
> > atomic_t zone;
> > struct mutex thresh_mutex;
> > +
> > + bool pwm_mode;
> > + u32 r_select;
> > };
>
> Have you run `pahole`? Does it agree with the layout you made here?
>
Noted.
> ...
>
> > - als->irq = lm3533->irq;
> > + als->irq = platform_get_irq_optional(pdev, 0);
>
> > +
>
> Redundant blank line.
>
Simplifies code perception, whatever.
> > + if (als->irq == -EPROBE_DEFER)
> > + return -EPROBE_DEFER;
>
> What about other error codes when IRQ is found by can't be retrieved for some
> reasons? IIRC we check against ENXIO in similar cases
>
Then we treat it as no IRQ. Original implementation cares only if IRQ
is present or no.
> als->irq = platform_get_irq_optional(pdev, 0);
> if (als->irq == -ENXIO)
> als->irq = 0;
> if (als->irq < 0)
> return als->irq;
>
> ...
>
> > + led->pwm = 0;
>
> Isn't it 0 by zalloc ?
It is, thanks.
>
> > + device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
>
> ...
>
> > #define LM3533_BOOST_FREQ_MASK 0x01
> > #define LM3533_BOOST_FREQ_SHIFT 0
> > +#define LM3533_BOOST_FREQ_MIN 500000
> > +#define LM3533_BOOST_FREQ_MAX 1000000
>
> HZ_PER_KHZ (since you included units.h)?
>
500 * HZ_PER_KHZ
1000 * HZ_PER_KHZ
You meant this? Sure.
> ...
>
> > + nchilds = device_get_child_node_count(dev);
> > + if (!nchilds || nchilds > LM3533_CELLS_MAX) {
> > + dev_err(dev, "num of child nodes is not supported\n");
> > + return -ENODEV;
>
> Why not dev_err_probe() here and elsewhere? It looks inconsistent with this
> patch.
>
I must have overlooked it, thanks. WDYM elsewhere, this is the only occurance.
> > }
>
> ...
>
> > + device_for_each_child_node_scoped(lm3533->dev, child) {
>
> > + if (!fwnode_device_is_available(child))
> > + continue;
>
> Do we need this check?
>
This is nice to have if the node is disabled. If we assume that there
are no disabled nodes, I can remove it.
> ...
>
> > + dev_err(dev, "invalid LED node %s\n",
> > + fwnode_get_name(child));
>
> %pfw
>
Noted.
> ...
>
> > + ret = sysfs_create_group(&dev->kobj, &lm3533_attribute_group);
>
> No way. You should use .dev_groups.
>
I did not change how driver does this, just swapped lm3533->dev to
dev. I will set is back as it was.
> > + if (ret) {
> > + dev_err(dev, "failed to create sysfs attributes\n");
> > goto err_unregister;
> > }
>
> ...
>
> Can you think on how to split this change to smaller steps? I believe it's
> possible.
>
No, I am done with tinkering with this patchset. It is broken enough
and it has inflated enough.
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v3 05/11] mfd: lm3533: Convert to use OF bindings
From: Andy Shevchenko @ 2026-06-02 11:05 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: <CAPVz0n21RGAaJc1sda4xyp1h0z+6R6FJ4=XWdOtB1mgtV8=RUA@mail.gmail.com>
On Tue, Jun 02, 2026 at 01:31:44PM +0300, Svyatoslav Ryhel wrote:
> вт, 2 черв. 2026 р. о 11:24 Andy Shevchenko <andriy.shevchenko@intel.com> пише:
> > On Mon, Jun 01, 2026 at 06:18:25PM +0300, Svyatoslav Ryhel wrote:
...
> > > + device_for_each_child_node_scoped(lm3533->dev, child) {
> >
> > > + if (!fwnode_device_is_available(child))
> > > + continue;
> >
> > Do we need this check?
>
> This is nice to have if the node is disabled. If we assume that there
> are no disabled nodes, I can remove it.
It's already implied. See
static struct fwnode_handle *
of_fwnode_get_next_child_node(const struct fwnode_handle *fwnode, struct fwnode_handle *child)
{
return of_fwnode_handle(of_get_next_available_child(to_of_node(fwnode), to_of_node(child)));
}
And I believe it's written somewhere in the documentation (if not, feel free to
patch that).
...
> > > + ret = sysfs_create_group(&dev->kobj, &lm3533_attribute_group);
> >
> > No way. You should use .dev_groups.
>
> I did not change how driver does this, just swapped lm3533->dev to
> dev. I will set is back as it was.
This is a serious race condition that needs to be addressed. Since you are
touching this driver the fixes against known issues probably are the first
things that have to be done.
> > > + if (ret) {
> > > + dev_err(dev, "failed to create sysfs attributes\n");
> > > goto err_unregister;
> > > }
...
> > Can you think on how to split this change to smaller steps? I believe it's
> > possible.
>
> No, I am done with tinkering with this patchset. It is broken enough
> and it has inflated enough.
Probably you don't want this to be reviewed then? I believe other kernel
developers and maintainers will ask you the same.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 08/11] video: backlight: lm3533_bl: Improve linear sysfs logic
From: Andy Shevchenko @ 2026-06-02 11:07 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: <CAPVz0n0P7Jk17cM2M1zuHZfySo2=Uibr5izwKU2tqiBpBcg0FQ@mail.gmail.com>
On Tue, Jun 02, 2026 at 01:19:00PM +0300, Svyatoslav Ryhel wrote:
> вт, 2 черв. 2026 р. о 11:09 Andy Shevchenko <andriy.shevchenko@intel.com> пише:
> > On Mon, Jun 01, 2026 at 06:18:28PM +0300, Svyatoslav Ryhel wrote:
...
> > > if (kstrtoul(buf, 0, &linear))
> > > return -EINVAL;
> >
> > Besides _assign_bits() in the below, side note here to unshadow error codes:
> >
> > ret = kstrtoul(buf, 0, &linear);
> > if (ret)
> > return ret;
> >
> > (obviously in a separate change).
>
> Won't happen in this patches.
You mean both suggestions or you are talking about kstrotoul() only? If it's
only about the latter, it's fine with me, but _assign_bits() makes sense to do
in this patch as you already change the parameters enough to make it better.
...
> > > ret = regmap_update_bits(bl->lm3533->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
> > > - mask, val);
> > > + CTRLBANK_AB_BCONF_MODE(id),
> > > + linear ? CTRLBANK_AB_BCONF_MODE(id) : 0);
> > > if (ret)
> > > return ret;
^^^ left for the context.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 08/11] video: backlight: lm3533_bl: Improve linear sysfs logic
From: Svyatoslav Ryhel @ 2026-06-02 11:59 UTC (permalink / raw)
To: Andy Shevchenko
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: <ah65hfgoM67V6-iR@ashevche-desk.local>
вт, 2 черв. 2026 р. о 14:07 Andy Shevchenko <andriy.shevchenko@intel.com> пише:
>
> On Tue, Jun 02, 2026 at 01:19:00PM +0300, Svyatoslav Ryhel wrote:
> > вт, 2 черв. 2026 р. о 11:09 Andy Shevchenko <andriy.shevchenko@intel.com> пише:
> > > On Mon, Jun 01, 2026 at 06:18:28PM +0300, Svyatoslav Ryhel wrote:
>
> ...
>
> > > > if (kstrtoul(buf, 0, &linear))
> > > > return -EINVAL;
> > >
> > > Besides _assign_bits() in the below, side note here to unshadow error codes:
> > >
> > > ret = kstrtoul(buf, 0, &linear);
> > > if (ret)
> > > return ret;
> > >
> > > (obviously in a separate change).
> >
> > Won't happen in this patches.
>
> You mean both suggestions or you are talking about kstrotoul() only? If it's
> only about the latter, it's fine with me, but _assign_bits() makes sense to do
> in this patch as you already change the parameters enough to make it better.
>
only kstrotoul()
> ...
>
> > > > ret = regmap_update_bits(bl->lm3533->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
> > > > - mask, val);
> > > > + CTRLBANK_AB_BCONF_MODE(id),
> > > > + linear ? CTRLBANK_AB_BCONF_MODE(id) : 0);
> > > > if (ret)
> > > > return ret;
>
> ^^^ left for the context.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v3 05/11] mfd: lm3533: Convert to use OF bindings
From: Svyatoslav Ryhel @ 2026-06-02 12:00 UTC (permalink / raw)
To: Andy Shevchenko
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: <ah648F2plc4UHTM1@ashevche-desk.local>
вт, 2 черв. 2026 р. о 14:05 Andy Shevchenko <andriy.shevchenko@intel.com> пише:
>
> On Tue, Jun 02, 2026 at 01:31:44PM +0300, Svyatoslav Ryhel wrote:
> > вт, 2 черв. 2026 р. о 11:24 Andy Shevchenko <andriy.shevchenko@intel.com> пише:
> > > On Mon, Jun 01, 2026 at 06:18:25PM +0300, Svyatoslav Ryhel wrote:
>
>
> ...
>
> > > > + device_for_each_child_node_scoped(lm3533->dev, child) {
> > >
> > > > + if (!fwnode_device_is_available(child))
> > > > + continue;
> > >
> > > Do we need this check?
> >
> > This is nice to have if the node is disabled. If we assume that there
> > are no disabled nodes, I can remove it.
>
> It's already implied. See
>
> static struct fwnode_handle *
> of_fwnode_get_next_child_node(const struct fwnode_handle *fwnode, struct fwnode_handle *child)
> {
> return of_fwnode_handle(of_get_next_available_child(to_of_node(fwnode), to_of_node(child)));
> }
>
> And I believe it's written somewhere in the documentation (if not, feel free to
> patch that).
>
Very nice. Thank you.
> ...
>
> > > > + ret = sysfs_create_group(&dev->kobj, &lm3533_attribute_group);
> > >
> > > No way. You should use .dev_groups.
> >
> > I did not change how driver does this, just swapped lm3533->dev to
> > dev. I will set is back as it was.
>
> This is a serious race condition that needs to be addressed. Since you are
> touching this driver the fixes against known issues probably are the first
> things that have to be done.
>
Fine, I will have a look.
> > > > + if (ret) {
> > > > + dev_err(dev, "failed to create sysfs attributes\n");
> > > > goto err_unregister;
> > > > }
>
> ...
>
> > > Can you think on how to split this change to smaller steps? I believe it's
> > > possible.
> >
> > No, I am done with tinkering with this patchset. It is broken enough
> > and it has inflated enough.
>
> Probably you don't want this to be reviewed then? I believe other kernel
> developers and maintainers will ask you the same.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v3 03/11] iio: light: lm3533-als: Remove redundant pdata helpers
From: Jonathan Cameron @ 2026-06-02 13:42 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: <20260601151831.76350-4-clamor95@gmail.com>
On Mon, 1 Jun 2026 18:18:23 +0300
Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> The lm3533_als_set_input_mode and lm3533_als_set_resistor functions are
> used only in lm3533_als_setup. Incorporate their code into
> lm3533_als_setup directly to simplify driver readability.
Minor stuff inline.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
> drivers/iio/light/lm3533-als.c | 61 +++++++++-------------------------
> 1 file changed, 16 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> index fb61904f110f..52136ca1abc9 100644
> --- a/drivers/iio/light/lm3533-als.c
> +++ b/drivers/iio/light/lm3533-als.c
> static int lm3533_als_setup(struct lm3533_als *als,
> const struct lm3533_als_platform_data *pdata)
> {
> + struct device *dev = &als->pdev->dev;
> int ret;
>
> - ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> + ret = regmap_update_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
Maybe a local struct regmap pointer given dereferenced in a couple of places.
> + LM3533_ALS_INPUT_MODE_MASK,
> + pdata->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0);
Andy raised this in previous patch but in the interests of being specific
regmap_assign_bits() is going to be cleaner here.
> if (ret)
> - return ret;
> + return dev_err_probe(dev, ret, "failed to set input mode %d\n",
> + pdata->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 (pdata->r_select < LM3533_ALS_RESISTOR_MIN ||
> + pdata->r_select > LM3533_ALS_RESISTOR_MAX) {
> + dev_err(&als->pdev->dev, "invalid resistor value\n");
> + return -EINVAL;
> + }
> +
> + ret = regmap_write(als->lm3533->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
> + pdata->r_select);
> if (ret)
> - return ret;
> + return dev_err_probe(dev, ret, "failed to set resistor\n");
> }
>
> return 0;
^ permalink raw reply
* Re: [PATCH v3 03/11] iio: light: lm3533-als: Remove redundant pdata helpers
From: Svyatoslav Ryhel @ 2026-06-02 13:45 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: <20260602144222.7a50a041@jic23-huawei>
вт, 2 черв. 2026 р. о 16:42 Jonathan Cameron <jic23@kernel.org> пише:
>
> On Mon, 1 Jun 2026 18:18:23 +0300
> Svyatoslav Ryhel <clamor95@gmail.com> wrote:
>
> > The lm3533_als_set_input_mode and lm3533_als_set_resistor functions are
> > used only in lm3533_als_setup. Incorporate their code into
> > lm3533_als_setup directly to simplify driver readability.
> Minor stuff inline.
>
>
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> > drivers/iio/light/lm3533-als.c | 61 +++++++++-------------------------
> > 1 file changed, 16 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> > index fb61904f110f..52136ca1abc9 100644
> > --- a/drivers/iio/light/lm3533-als.c
> > +++ b/drivers/iio/light/lm3533-als.c
>
> > static int lm3533_als_setup(struct lm3533_als *als,
> > const struct lm3533_als_platform_data *pdata)
> > {
> > + struct device *dev = &als->pdev->dev;
> > int ret;
> >
> > - ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> > + ret = regmap_update_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
>
> Maybe a local struct regmap pointer given dereferenced in a couple of places.
>
sure, why not
> > + LM3533_ALS_INPUT_MODE_MASK,
> > + pdata->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0);
>
> Andy raised this in previous patch but in the interests of being specific
> regmap_assign_bits() is going to be cleaner here.
>
I am currently adjusting accordingly.
> > if (ret)
> > - return ret;
> > + return dev_err_probe(dev, ret, "failed to set input mode %d\n",
> > + pdata->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 (pdata->r_select < LM3533_ALS_RESISTOR_MIN ||
> > + pdata->r_select > LM3533_ALS_RESISTOR_MAX) {
> > + dev_err(&als->pdev->dev, "invalid resistor value\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = regmap_write(als->lm3533->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
> > + pdata->r_select);
> > if (ret)
> > - return ret;
> > + return dev_err_probe(dev, ret, "failed to set resistor\n");
> > }
> >
> > return 0;
>
^ permalink raw reply
* Re: [PATCH v3 05/11] mfd: lm3533: Convert to use OF bindings
From: Jonathan Cameron @ 2026-06-02 13:46 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: <20260601151831.76350-6-clamor95@gmail.com>
On Mon, 1 Jun 2026 18:18:25 +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.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> index 52136ca1abc9..55b35467a722 100644
> --- a/drivers/iio/light/lm3533-als.c
> +++ b/drivers/iio/light/lm3533-als.c
> @@ -16,16 +16,19 @@
> #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/regmap.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
> @@ -57,6 +60,9 @@ struct lm3533_als {
>
> atomic_t zone;
> struct mutex thresh_mutex;
> +
> + bool pwm_mode;
> + u32 r_select;
> };
>
>
> @@ -411,7 +417,7 @@ static ssize_t show_thresh_either_en(struct device *dev,
> int enable;
> int ret;
>
> - if (als->irq) {
> + if (als->irq > 0) {
> ret = lm3533_als_get_int_mode(indio_dev, &enable);
> if (ret)
> return ret;
> @@ -716,30 +722,34 @@ static const struct attribute_group lm3533_als_attribute_group = {
> .attrs = lm3533_als_attributes
> };
>
> -static int lm3533_als_setup(struct lm3533_als *als,
> - const struct lm3533_als_platform_data *pdata)
> +static int lm3533_als_setup(struct lm3533_als *als)
> {
> struct device *dev = &als->pdev->dev;
> int ret;
>
> + als->pwm_mode = device_property_read_bool(dev, "ti,pwm-mode");
> +
> ret = regmap_update_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
> LM3533_ALS_INPUT_MODE_MASK,
> - pdata->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0);
> + als->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0);
> if (ret)
> return dev_err_probe(dev, ret, "failed to set input mode %d\n",
> - pdata->pwm_mode);
> -
> + als->pwm_mode);
>
> /* ALS input is always high impedance in PWM-mode. */
> - if (!pdata->pwm_mode) {
> - if (pdata->r_select < LM3533_ALS_RESISTOR_MIN ||
> - pdata->r_select > LM3533_ALS_RESISTOR_MAX) {
> - dev_err(&als->pdev->dev, "invalid resistor value\n");
> - return -EINVAL;
> - }
> + if (!als->pwm_mode) {
> + ret = device_property_read_u32(dev, "ti,resistor-value-ohms",
> + &als->r_select);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to ger resistor value\n");
> +
> + als->r_select = clamp(als->r_select, LM3533_ALS_RESISTOR_MIN,
> + LM3533_ALS_RESISTOR_MAX);
If we are getting garbage from DT I think I'd rather error out that paper over
that problem. So similar to before, check valid value and if not fail probe
so that hopefully someone goes and fixes it!
> + als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * als->r_select);
Why do we need this when we didn't before? The range checks are the same
so it smells like it shouldn't need transforming. I'd also rather we didn't do
rewriting of the meaning of r_select like this. Just use a local variable for
the intermediate result.
>
> ret = regmap_write(als->lm3533->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
> - pdata->r_select);
> + als->r_select);
> if (ret)
> return dev_err_probe(dev, ret, "failed to set resistor\n");
^ permalink raw reply
* Re: [PATCH v3 01/11] dt-bindings: leds: Document TI LM3533 LED controller
From: Daniel Thompson @ 2026-06-02 13:49 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, 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: <20260601151831.76350-2-clamor95@gmail.com>
On Mon, Jun 01, 2026 at 06:18:21PM +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>
> Reviewed-by: Jonathan Cameron <jic23@kernel.org> #for light sensor
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org> #for backlight
Daniel.
^ permalink raw reply
* Re: [PATCH v3 05/11] mfd: lm3533: Convert to use OF bindings
From: Daniel Thompson @ 2026-06-02 13:49 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, 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: <20260601151831.76350-6-clamor95@gmail.com>
On Mon, Jun 01, 2026 at 06:18:25PM +0300, Svyatoslav Ryhel wrote:
> Since there are no users of this driver via platform data, remove the
> platform data support and switch to using Device Tree bindings.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org> #for backlight
Daniel.
^ permalink raw reply
* Re: [PATCH v3 05/11] mfd: lm3533: Convert to use OF bindings
From: Svyatoslav Ryhel @ 2026-06-02 13:50 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: <20260602144640.433b4d35@jic23-huawei>
вт, 2 черв. 2026 р. о 16:46 Jonathan Cameron <jic23@kernel.org> пише:
>
> On Mon, 1 Jun 2026 18:18:25 +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.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>
>
> > index 52136ca1abc9..55b35467a722 100644
> > --- a/drivers/iio/light/lm3533-als.c
> > +++ b/drivers/iio/light/lm3533-als.c
> > @@ -16,16 +16,19 @@
> > #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/regmap.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
> > @@ -57,6 +60,9 @@ struct lm3533_als {
> >
> > atomic_t zone;
> > struct mutex thresh_mutex;
> > +
> > + bool pwm_mode;
> > + u32 r_select;
> > };
> >
> >
> > @@ -411,7 +417,7 @@ static ssize_t show_thresh_either_en(struct device *dev,
> > int enable;
> > int ret;
> >
> > - if (als->irq) {
> > + if (als->irq > 0) {
> > ret = lm3533_als_get_int_mode(indio_dev, &enable);
> > if (ret)
> > return ret;
> > @@ -716,30 +722,34 @@ static const struct attribute_group lm3533_als_attribute_group = {
> > .attrs = lm3533_als_attributes
> > };
> >
> > -static int lm3533_als_setup(struct lm3533_als *als,
> > - const struct lm3533_als_platform_data *pdata)
> > +static int lm3533_als_setup(struct lm3533_als *als)
> > {
> > struct device *dev = &als->pdev->dev;
> > int ret;
> >
> > + als->pwm_mode = device_property_read_bool(dev, "ti,pwm-mode");
> > +
> > ret = regmap_update_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
> > LM3533_ALS_INPUT_MODE_MASK,
> > - pdata->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0);
> > + als->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0);
> > if (ret)
> > return dev_err_probe(dev, ret, "failed to set input mode %d\n",
> > - pdata->pwm_mode);
> > -
> > + als->pwm_mode);
> >
> > /* ALS input is always high impedance in PWM-mode. */
> > - if (!pdata->pwm_mode) {
> > - if (pdata->r_select < LM3533_ALS_RESISTOR_MIN ||
> > - pdata->r_select > LM3533_ALS_RESISTOR_MAX) {
> > - dev_err(&als->pdev->dev, "invalid resistor value\n");
> > - return -EINVAL;
> > - }
> > + if (!als->pwm_mode) {
> > + ret = device_property_read_u32(dev, "ti,resistor-value-ohms",
> > + &als->r_select);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "failed to ger resistor value\n");
> > +
> > + als->r_select = clamp(als->r_select, LM3533_ALS_RESISTOR_MIN,
> > + LM3533_ALS_RESISTOR_MAX);
>
> If we are getting garbage from DT I think I'd rather error out that paper over
> that problem. So similar to before, check valid value and if not fail probe
> so that hopefully someone goes and fixes it!
>
sure
> > + als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * als->r_select);
> Why do we need this when we didn't before? The range checks are the same
> so it smells like it shouldn't need transforming. I'd also rather we didn't do
> rewriting of the meaning of r_select like this. Just use a local variable for
> the intermediate result.
>
before pdata passed resistor value as actual register value, not we
are getting the actual resistance in ohms from the tree and must
convert it into register value.
> >
> > ret = regmap_write(als->lm3533->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
> > - pdata->r_select);
> > + als->r_select);
> > if (ret)
> > return dev_err_probe(dev, ret, "failed to set resistor\n");
^ permalink raw reply
* Re: [PATCH v3 08/11] video: backlight: lm3533_bl: Improve linear sysfs logic
From: Daniel Thompson @ 2026-06-02 13:55 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, 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: <20260601151831.76350-9-clamor95@gmail.com>
On Mon, Jun 01, 2026 at 06:18:28PM +0300, Svyatoslav Ryhel wrote:
> Simplify the sysfs logic of the linear property by switching to a macro
> and a ternary operator.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
With the change to regmap_assign_bits():
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Daniel.
^ permalink raw reply
* Re: [PATCH v3 09/11] video: backlight: lm3533_bl: Set initial mapping mode from DT
From: Daniel Thompson @ 2026-06-02 14:05 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, 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: <20260601151831.76350-10-clamor95@gmail.com>
On Mon, Jun 01, 2026 at 06:18:29PM +0300, Svyatoslav Ryhel wrote:
> Add support to obtain the initial mapping mode from DT instead of leaving
> it unconfigured.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
> drivers/video/backlight/lm3533_bl.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
> index 36e6f027613a..f0d88b7bc229 100644
> --- a/drivers/video/backlight/lm3533_bl.c
> +++ b/drivers/video/backlight/lm3533_bl.c
> @@ -34,6 +34,7 @@ struct lm3533_bl {
>
> u32 max_current;
> u32 pwm;
> + bool linear;
> };
>
>
> @@ -247,8 +248,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 = regmap_update_bits(bl->lm3533->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
> + CTRLBANK_AB_BCONF_MODE(id),
> + bl->linear ? CTRLBANK_AB_BCONF_MODE(id) : 0);
I guess this is another candidate for regmap_assign_bits() but with that
change:
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Daniel.
^ permalink raw reply
* Re: [PATCH v3 10/11] video: backlight: lm3533_bl: Implement backlight_scale property
From: Daniel Thompson @ 2026-06-02 14:06 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, 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: <20260601151831.76350-11-clamor95@gmail.com>
On Mon, Jun 01, 2026 at 06:18:30PM +0300, Svyatoslav Ryhel wrote:
> Since the device supports linear and non-linear modes, implement the
> backlight_scale property to describe this state.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Thanks!
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Daniel.
^ permalink raw reply
* Re: [PATCH v3 05/11] mfd: lm3533: Convert to use OF bindings
From: Jonathan Cameron @ 2026-06-02 14:20 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: <CAPVz0n1r97d8-uzhPGBx0LFSp75A3_2mMXDQQ30utT-6NtpHNA@mail.gmail.com>
On Tue, 2 Jun 2026 16:50:16 +0300
Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> вт, 2 черв. 2026 р. о 16:46 Jonathan Cameron <jic23@kernel.org> пише:
> >
> > On Mon, 1 Jun 2026 18:18:25 +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.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> >
> >
> > > index 52136ca1abc9..55b35467a722 100644
> > > --- a/drivers/iio/light/lm3533-als.c
> > > +++ b/drivers/iio/light/lm3533-als.c
> > > @@ -16,16 +16,19 @@
> > > #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/regmap.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
> > > @@ -57,6 +60,9 @@ struct lm3533_als {
> > >
> > > atomic_t zone;
> > > struct mutex thresh_mutex;
> > > +
> > > + bool pwm_mode;
> > > + u32 r_select;
> > > };
> > >
> > >
> > > @@ -411,7 +417,7 @@ static ssize_t show_thresh_either_en(struct device *dev,
> > > int enable;
> > > int ret;
> > >
> > > - if (als->irq) {
> > > + if (als->irq > 0) {
> > > ret = lm3533_als_get_int_mode(indio_dev, &enable);
> > > if (ret)
> > > return ret;
> > > @@ -716,30 +722,34 @@ static const struct attribute_group lm3533_als_attribute_group = {
> > > .attrs = lm3533_als_attributes
> > > };
> > >
> > > -static int lm3533_als_setup(struct lm3533_als *als,
> > > - const struct lm3533_als_platform_data *pdata)
> > > +static int lm3533_als_setup(struct lm3533_als *als)
> > > {
> > > struct device *dev = &als->pdev->dev;
> > > int ret;
> > >
> > > + als->pwm_mode = device_property_read_bool(dev, "ti,pwm-mode");
> > > +
> > > ret = regmap_update_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
> > > LM3533_ALS_INPUT_MODE_MASK,
> > > - pdata->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0);
> > > + als->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0);
> > > if (ret)
> > > return dev_err_probe(dev, ret, "failed to set input mode %d\n",
> > > - pdata->pwm_mode);
> > > -
> > > + als->pwm_mode);
> > >
> > > /* ALS input is always high impedance in PWM-mode. */
> > > - if (!pdata->pwm_mode) {
> > > - if (pdata->r_select < LM3533_ALS_RESISTOR_MIN ||
> > > - pdata->r_select > LM3533_ALS_RESISTOR_MAX) {
> > > - dev_err(&als->pdev->dev, "invalid resistor value\n");
> > > - return -EINVAL;
> > > - }
> > > + if (!als->pwm_mode) {
> > > + ret = device_property_read_u32(dev, "ti,resistor-value-ohms",
> > > + &als->r_select);
> > > + if (ret)
> > > + return dev_err_probe(dev, ret,
> > > + "failed to ger resistor value\n");
> > > +
> > > + als->r_select = clamp(als->r_select, LM3533_ALS_RESISTOR_MIN,
> > > + LM3533_ALS_RESISTOR_MAX);
> >
> > If we are getting garbage from DT I think I'd rather error out that paper over
> > that problem. So similar to before, check valid value and if not fail probe
> > so that hopefully someone goes and fixes it!
> >
>
> sure
>
> > > + als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * als->r_select);
> > Why do we need this when we didn't before? The range checks are the same
> > so it smells like it shouldn't need transforming. I'd also rather we didn't do
> > rewriting of the meaning of r_select like this. Just use a local variable for
> > the intermediate result.
> >
>
> before pdata passed resistor value as actual register value, not we
> are getting the actual resistance in ohms from the tree and must
> convert it into register value.
ah. I missed the change of values. Can you make them explicitly now _OHMS or something
along those lines rather than reusing the macro name for a different thing.
>
> > >
> > > ret = regmap_write(als->lm3533->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
> > > - pdata->r_select);
> > > + als->r_select);
> > > if (ret)
> > > return dev_err_probe(dev, ret, "failed to set resistor\n");
^ permalink raw reply
* Re: [PATCH v3 05/11] mfd: lm3533: Convert to use OF bindings
From: Svyatoslav Ryhel @ 2026-06-02 14:28 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: <20260602152019.078cc40e@jic23-huawei>
вт, 2 черв. 2026 р. о 17:20 Jonathan Cameron <jic23@kernel.org> пише:
>
> On Tue, 2 Jun 2026 16:50:16 +0300
> Svyatoslav Ryhel <clamor95@gmail.com> wrote:
>
> > вт, 2 черв. 2026 р. о 16:46 Jonathan Cameron <jic23@kernel.org> пише:
> > >
> > > On Mon, 1 Jun 2026 18:18:25 +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.
> > > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > >
> > >
> > > > index 52136ca1abc9..55b35467a722 100644
> > > > --- a/drivers/iio/light/lm3533-als.c
> > > > +++ b/drivers/iio/light/lm3533-als.c
> > > > @@ -16,16 +16,19 @@
> > > > #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/regmap.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
> > > > @@ -57,6 +60,9 @@ struct lm3533_als {
> > > >
> > > > atomic_t zone;
> > > > struct mutex thresh_mutex;
> > > > +
> > > > + bool pwm_mode;
> > > > + u32 r_select;
> > > > };
> > > >
> > > >
> > > > @@ -411,7 +417,7 @@ static ssize_t show_thresh_either_en(struct device *dev,
> > > > int enable;
> > > > int ret;
> > > >
> > > > - if (als->irq) {
> > > > + if (als->irq > 0) {
> > > > ret = lm3533_als_get_int_mode(indio_dev, &enable);
> > > > if (ret)
> > > > return ret;
> > > > @@ -716,30 +722,34 @@ static const struct attribute_group lm3533_als_attribute_group = {
> > > > .attrs = lm3533_als_attributes
> > > > };
> > > >
> > > > -static int lm3533_als_setup(struct lm3533_als *als,
> > > > - const struct lm3533_als_platform_data *pdata)
> > > > +static int lm3533_als_setup(struct lm3533_als *als)
> > > > {
> > > > struct device *dev = &als->pdev->dev;
> > > > int ret;
> > > >
> > > > + als->pwm_mode = device_property_read_bool(dev, "ti,pwm-mode");
> > > > +
> > > > ret = regmap_update_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
> > > > LM3533_ALS_INPUT_MODE_MASK,
> > > > - pdata->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0);
> > > > + als->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0);
> > > > if (ret)
> > > > return dev_err_probe(dev, ret, "failed to set input mode %d\n",
> > > > - pdata->pwm_mode);
> > > > -
> > > > + als->pwm_mode);
> > > >
> > > > /* ALS input is always high impedance in PWM-mode. */
> > > > - if (!pdata->pwm_mode) {
> > > > - if (pdata->r_select < LM3533_ALS_RESISTOR_MIN ||
> > > > - pdata->r_select > LM3533_ALS_RESISTOR_MAX) {
> > > > - dev_err(&als->pdev->dev, "invalid resistor value\n");
> > > > - return -EINVAL;
> > > > - }
> > > > + if (!als->pwm_mode) {
> > > > + ret = device_property_read_u32(dev, "ti,resistor-value-ohms",
> > > > + &als->r_select);
> > > > + if (ret)
> > > > + return dev_err_probe(dev, ret,
> > > > + "failed to ger resistor value\n");
> > > > +
> > > > + als->r_select = clamp(als->r_select, LM3533_ALS_RESISTOR_MIN,
> > > > + LM3533_ALS_RESISTOR_MAX);
> > >
> > > If we are getting garbage from DT I think I'd rather error out that paper over
> > > that problem. So similar to before, check valid value and if not fail probe
> > > so that hopefully someone goes and fixes it!
> > >
> >
> > sure
> >
> > > > + als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * als->r_select);
> > > Why do we need this when we didn't before? The range checks are the same
> > > so it smells like it shouldn't need transforming. I'd also rather we didn't do
> > > rewriting of the meaning of r_select like this. Just use a local variable for
> > > the intermediate result.
> > >
> >
> > before pdata passed resistor value as actual register value, not we
> > are getting the actual resistance in ohms from the tree and must
> > convert it into register value.
>
> ah. I missed the change of values. Can you make them explicitly now _OHMS or something
> along those lines rather than reusing the macro name for a different thing.
>
Acknowledged.
> >
> > > >
> > > > ret = regmap_write(als->lm3533->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
> > > > - pdata->r_select);
> > > > + als->r_select);
> > > > if (ret)
> > > > return dev_err_probe(dev, ret, "failed to set resistor\n");
>
^ permalink raw reply
* [PATCH] staging: sm750fb: Add missing Kconfig dependency
From: Rong Zhang @ 2026-06-02 18:51 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman,
Javier Martinez Canillas, Thomas Zimmermann
Cc: linux-fbdev, linux-staging, linux-kernel, Rong Zhang
The sm750 frame buffer driver depends on FB_IOMEM_FOPS, but its Kconfig
somehow misses it.
Fix it by making FB_SM750 select FB_IOMEM_FOPS, as other frame buffer
drivers do.
Fixes: dc0ad215e5d8 ("staging/sm750fb: Initialize fb_ops with fbdev macros")
Signed-off-by: Rong Zhang <i@rong.moe>
---
drivers/staging/sm750fb/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/sm750fb/Kconfig b/drivers/staging/sm750fb/Kconfig
index 08bcccdd0f1c..25fe422f55f2 100644
--- a/drivers/staging/sm750fb/Kconfig
+++ b/drivers/staging/sm750fb/Kconfig
@@ -6,6 +6,7 @@ config FB_SM750
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
+ select FB_IOMEM_FOPS
help
Frame buffer driver for the Silicon Motion SM750 chip
with 2D acceleration and dual head support.
---
base-commit: e43ffb69e0438cddd72aaa30898b4dc446f664f8
change-id: f884501e-sm750-fb-iomem-kconfig-57814e818bca
Thanks,
Rong
^ permalink raw reply related
* [PATCH] staging: fbtft: Use sysfs_emit_at() to print to sysfs file
From: Dan Carpenter @ 2026-06-03 7:34 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Andy Shevchenko, Greg Kroah-Hartman, Helge Deller,
Thomas Zimmermann, Chintan Patel, dri-devel, linux-fbdev,
linux-staging, linux-kernel, kernel-janitors
This scnprintf() uses the wrong limit. It should be "PAGE_SIZE - len"
instead of just PAGE_SIZE. We're not going to hit the limit in real
life since we are printing at most FBTFT_GAMMA_MAX_VALUES_TOTAL (128)
u32 values, however, it's still worth fixing.
Use sysfs_emit_at() to fix this since this is a sysfs file.
Fixes: c296d5f9957c ("staging: fbtft: core support")
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
drivers/staging/fbtft/fbtft-sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c
index d05599d80011..343545e83a37 100644
--- a/drivers/staging/fbtft/fbtft-sysfs.c
+++ b/drivers/staging/fbtft/fbtft-sysfs.c
@@ -98,7 +98,7 @@ sprintf_gamma(struct fbtft_par *par, u32 *curves, char *buf)
mutex_lock(&par->gamma.lock);
for (i = 0; i < par->gamma.num_curves; i++) {
for (j = 0; j < par->gamma.num_values; j++)
- len += scnprintf(&buf[len], PAGE_SIZE,
+ len += sysfs_emit_at(buf, len,
"%04x ", curves[i * par->gamma.num_values + j]);
buf[len - 1] = '\n';
}
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] staging: fbtft: Use sysfs_emit_at() to print to sysfs file
From: Andy Shevchenko @ 2026-06-03 7:49 UTC (permalink / raw)
To: Dan Carpenter
Cc: Thomas Petazzoni, Andy Shevchenko, Greg Kroah-Hartman,
Helge Deller, Thomas Zimmermann, Chintan Patel, dri-devel,
linux-fbdev, linux-staging, linux-kernel, kernel-janitors
In-Reply-To: <ah_Y_Y2RtqeGxchF@stanley.mountain>
On Wed, Jun 03, 2026 at 10:34:21AM +0300, Dan Carpenter wrote:
> This scnprintf() uses the wrong limit. It should be "PAGE_SIZE - len"
> instead of just PAGE_SIZE. We're not going to hit the limit in real
> life since we are printing at most FBTFT_GAMMA_MAX_VALUES_TOTAL (128)
> u32 values, however, it's still worth fixing.
>
> Use sysfs_emit_at() to fix this since this is a sysfs file.
OK,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
...
> for (i = 0; i < par->gamma.num_curves; i++) {
> for (j = 0; j < par->gamma.num_values; j++)
> - len += scnprintf(&buf[len], PAGE_SIZE,
> + len += sysfs_emit_at(buf, len,
> "%04x ", curves[i * par->gamma.num_values + j]);
Can we switch to use hex_dump_to_buffer() at some point?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] staging: fbtft: Use sysfs_emit_at() to print to sysfs file
From: David Laight @ 2026-06-03 12:48 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Dan Carpenter, Thomas Petazzoni, Andy Shevchenko,
Greg Kroah-Hartman, Helge Deller, Thomas Zimmermann,
Chintan Patel, dri-devel, linux-fbdev, linux-staging,
linux-kernel, kernel-janitors
In-Reply-To: <ah_cd9Ax4fzOhBp7@ashevche-desk.local>
On Wed, 3 Jun 2026 10:49:11 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Wed, Jun 03, 2026 at 10:34:21AM +0300, Dan Carpenter wrote:
> > This scnprintf() uses the wrong limit. It should be "PAGE_SIZE - len"
> > instead of just PAGE_SIZE. We're not going to hit the limit in real
> > life since we are printing at most FBTFT_GAMMA_MAX_VALUES_TOTAL (128)
> > u32 values, however, it's still worth fixing.
> >
> > Use sysfs_emit_at() to fix this since this is a sysfs file.
>
> OK,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> ...
>
> > for (i = 0; i < par->gamma.num_curves; i++) {
> > for (j = 0; j < par->gamma.num_values; j++)
> > - len += scnprintf(&buf[len], PAGE_SIZE,
> > + len += sysfs_emit_at(buf, len,
> > "%04x ", curves[i * par->gamma.num_values + j]);
>
> Can we switch to use hex_dump_to_buffer() at some point?
>
That gets hard when you really want to aim for something nearer seq_printf().
Oh, the way the loops in the code are written doesn't look like
it gives the compiler much chance of optimising it very well.
This is probably equivalent:
for (i = par->gamma.num_curves; i--;) {
for (j = par->gamma.num_values; j--;)
len += sysfs_emit_at(buf, len, "%04x ", *curves++);
buf[len - 1] = '\n';
}
and will generate much better code.
(Although most of the cost inside the snprintf() function - which is horrid.)
num_values better be non-zero.
-- David
^ permalink raw reply
* Re: [PATCH] fbdev: grvga: Fix CLUT register address offset in comment
From: Helge Deller @ 2026-06-04 22:24 UTC (permalink / raw)
To: Eduardo Silva; +Cc: linux-fbdev
In-Reply-To: <20260601194644.275346-1-eduardo4silva@gmail.com>
On 6/1/26 21:46, Eduardo Silva wrote:
> The comment does not match the actual address offset. According
> to the GRLIB IP Library Reference Manual (p. 2119), the CLUT register
> is at offset 0x28, not the value stated in the comment.
>
> Signed-off-by: Eduardo Silva <eduardo4silva@gmail.com>
> ---
> drivers/video/fbdev/grvga.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH v1 0/8] zorro: Improve handling of pointers in zorro_device_id::driver_data
From: Helge Deller @ 2026-06-04 22:44 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: linux-ide, linux-m68k, linux-kernel, linux-scsi, netdev,
linux-i2c, linux-fbdev, dri-devel, Christian A. Ehrhardt,
Christian A. Ehrhardt
In-Reply-To: <cover.1779803053.git.u.kleine-koenig@baylibre.com>
Hi Uwe,
On 5/26/26 16:17, Uwe Kleine-König (The Capable Hub) 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.
>
> Preparing for that change, all zorro_device_ids are converted to use
> named initializers, which is also a nice cleanup that could stand for
> itself, as it improves readability for humans. (That is necessary
> because an anonymous union can be initialized by name, but not using a
> list initializer.)
>
> My motivation for this series is the CHERI hardware extension. With that
> pointers are bigger than longs and thus you cannot store pointers in
> zorro_device_id::driver_data. So this series is also about getting
> support for CHERI into the mainline, but I hope the clean up effects
> mentioned above are justification enough to accept this series.
>
> The dependencies in this series are as follows:
>
> - Patch #5 depends on #1, #2
> - Patches #7 and #8 depend on patch #6.
>
> So if the ata maintainers agreed to merge their patch #1 via scsi, and
> Geert agrees to patch #5 and that it's also merged via scsi, patches #1,
> #2, #6 and #7 can go in without further coordination.
>
> Patches #3, #4 and #5 are only about using the same initialization style
> for all zorro_device_id and can go in without coordination.
>
> Best regards
> Uwe
>
> Uwe Kleine-König (The Capable Hub) (8):
> ata: pata_budda: Use named initializer for zorro_device_id
> scsi: Use named initializer for zorro_device_id
> net: Use named initializer for zorro_device_id arrays
> i2c: icy: Use named initializer for zorro_device_id arrays
> video: fm2fb: Use named initializer for zorro_device_id array
> zorro: Simplify storing pointers in device id struct
> scsi: zorro7xx: Make use of struct zorro_device_id::driver_data_ptr
> video: cirrusfb: Make use of struct zorro_device_id::driver_data_ptr
>
> drivers/ata/pata_buddha.c | 8 ++++----
> drivers/i2c/busses/i2c-icy.c | 4 ++--
> drivers/net/ethernet/8390/hydra.c | 4 ++--
> drivers/net/ethernet/8390/xsurf100.c | 4 ++--
> drivers/net/ethernet/8390/zorro8390.c | 6 +++---
> drivers/net/ethernet/amd/a2065.c | 8 ++++----
> drivers/net/ethernet/amd/ariadne.c | 4 ++--
> drivers/scsi/a2091.c | 6 +++---
> drivers/scsi/gvp11.c | 17 ++++++++--------
> drivers/scsi/zorro7xx.c | 16 +++++++--------
> drivers/scsi/zorro_esp.c | 2 +-
> drivers/video/fbdev/cirrusfb.c | 28 +++++++++++++--------------
> drivers/video/fbdev/fm2fb.c | 6 +++---
> include/linux/mod_devicetable.h | 6 +++++-
> 14 files changed, 62 insertions(+), 57 deletions(-)
you may add to the series:
Acked-by: Helge Deller <deller@gmx.de>
Since it touches various subtrees, I assume you will merge it though your tree?
Helge
^ 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