* Re: [PATCH] fbtft: Remove access to page->index
From: Matthew Wilcox @ 2025-03-05 17:49 UTC (permalink / raw)
To: Simona Vetter
Cc: Lorenzo Stoakes, Greg Kroah-Hartman, Thomas Zimmermann,
Jeff Johnson, Thomas Petazzoni, dri-devel, linux-fbdev,
linux-staging
In-Reply-To: <Z8f_pcnaC5iJMz-Z@phenom.ffwll.local>
On Wed, Mar 05, 2025 at 08:39:17AM +0100, Simona Vetter wrote:
> On Tue, Mar 04, 2025 at 04:21:30PM +0000, Matthew Wilcox wrote:
> > On Fri, Feb 21, 2025 at 05:53:16PM +0000, Lorenzo Stoakes wrote:
> > > On Fri, Feb 21, 2025 at 05:31:29PM +0000, Matthew Wilcox (Oracle) wrote:
> > > > There is no need to print out page->index as part of the debug message.
> > > >
> > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> > >
> > > LGTM from my side,
> > >
> > > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> >
> > This patch is not yet in linux-next; can I expect it to go in soon?
>
> Being both staging and fbdev it's double orphaned, I stuffed it into
> drm-misc-next now, thanks for the ping&patch.
Thanks!
^ permalink raw reply
* Re: video: au1100fb: Move a variable assignment behind a null pointer check in au1100fb_setmode()
From: Markus Elfring @ 2025-03-05 17:28 UTC (permalink / raw)
To: Helge Deller, Dan Carpenter, Uwe Kleine-König,
kernel-janitors, linux-fbdev, dri-devel
Cc: Antonino Daplas, Thomas Zimmermann, Yihao Han, cocci, LKML
In-Reply-To: <81a620bb-205f-45f7-9036-e8e44a8e7be9@gmx.de>
> No crash or anything can or will happen here.
>
> Markus, maybe you missed the "&" in front of "&fbdev->info" ?
Would you get into the mood to adjust development views
if you would take any feedback and further background information
(by David Svoboda for example) better into account?
https://wiki.sei.cmu.edu/confluence/display/c/EXP34-C.+Do+not+dereference+null+pointers?focusedCommentId=405504139#comment-405504139
Regards,
Markus
^ permalink raw reply
* Re: video: au1100fb: Move a variable assignment behind a null pointer check in au1100fb_setmode()
From: Helge Deller @ 2025-03-05 17:07 UTC (permalink / raw)
To: Markus Elfring, Dan Carpenter, Uwe Kleine-König,
kernel-janitors, linux-fbdev, dri-devel
Cc: Antonino Daplas, Thomas Zimmermann, Yihao Han, cocci, LKML
In-Reply-To: <9d042e6a-6d93-4ae4-8373-28b9dec21867@web.de>
On 3/5/25 13:14, Markus Elfring wrote:
>> Anyway, none of that applies here, because this is just pointer math.
> Which data processing do you expect to be generally supported at the discussed
> source code place (according to the rules of the programming language “C”)?
> https://en.cppreference.com/w/c/language/behavior
There is nothing to discuss.
Dan is correct.
We have:
struct au1100fb_device {
struct fb_info info;
...
so:
struct fb_info *info = &fbdev->info;
gets translated by the compiler to a trivial pointer math:
info = <value of fbdev> + 0 # 0 is there offset of "info" in the struct.
No crash or anything can or will happen here.
Markus, maybe you missed the "&" in front of "&fbdev->info" ?
Helge
^ permalink raw reply
* Re: [PATCH v3 2/2] mfd: lm3533: convert to use OF
From: Svyatoslav Ryhel @ 2025-03-05 14:18 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lars-Peter Clausen, Pavel Machek, Daniel Thompson, Jingoo Han,
Helge Deller, Andy Shevchenko, Uwe Kleine-König, devicetree,
linux-kernel, linux-iio, linux-leds, dri-devel, linux-fbdev
In-Reply-To: <20250305134455.2843f603@jic23-huawei>
ср, 5 бер. 2025 р. о 15:45 Jonathan Cameron <jic23@kernel.org> пише:
>
> On Fri, 28 Feb 2025 11:30:51 +0200
> Svyatoslav Ryhel <clamor95@gmail.com> wrote:
>
> > пт, 28 лют. 2025 р. о 10:59 Lee Jones <lee@kernel.org> пише:
> > >
> > > On Mon, 24 Feb 2025, Svyatoslav Ryhel wrote:
> > >
> > > > Remove platform data and fully relay on OF and device tree
> > > > parsing and binding devices.
> > > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > ---
> > > > drivers/iio/light/lm3533-als.c | 40 ++++---
> > > > drivers/leds/leds-lm3533.c | 46 +++++---
> > > > drivers/mfd/lm3533-core.c | 159 ++++++++--------------------
> > > > drivers/video/backlight/lm3533_bl.c | 71 ++++++++++---
> > > > include/linux/mfd/lm3533.h | 35 +-----
> > > > 5 files changed, 164 insertions(+), 187 deletions(-)
> > > >
...
> > > > /* 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_als_set_resistor(als, als->r_select);
> > >
> > > You're already passing 'als'.
> > >
> > > Just teach lm3533_als_set_resistor that 'r_select' is now contained.
> > >
> >
> > This is not scope of this patchset. I was already accused in too much
> > changes which make it unreadable. This patchset is dedicated to
> > swapping platform data to use of the device tree. NOT improving
> > functions, NOT rewriting arbitrary mechanics. If you feed a need for
> > this change, then propose a followup. I need from this driver only one
> > thing, that it could work with device tree. But it seems that it is
> > better that it just rots in the garbage bin until removed cause no one
> > cared.
>
> This is not an unreasonable request as you added r_select to als.
> Perhaps it belongs in a separate follow up patch.
I have just moved values used in pdata to private structs of each
driver. Without changing names or purpose.
> However
> it is worth remembering the motivation here is that you want get
> this code upstream, the maintainers don't have that motivation.
This driver is already upstream and it is useless and incompatible
with majority of supported devices. Maintainers should encourage those
who try to help and instead we have what? A total discouragement. Well
defined path into nowhere.
>
> Greg KH has given various talks on the different motivations in the
> past. It maybe worth a watch.
>
>
> >
> > > > if (ret)
> > > > return ret;
> > > > }
> > > > @@ -828,22 +833,16 @@ 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;
> > > > + u32 val;
> > >
> > > Value of what, potatoes?
> > >
> >
> > Oranges.
>
> A well named variable would avoid need for any discussion of
> what it is the value of.
>
This is temporary placeholder used to get values from the tree and
then pass it driver struct.
> >
> > > > int ret;
> > > >
> > > > lm3533 = dev_get_drvdata(pdev->dev.parent);
> > > > 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;
> > > > @@ -864,13 +863,21 @@ static int lm3533_als_probe(struct platform_device *pdev)
> > > >
> > > > platform_set_drvdata(pdev, indio_dev);
> > > >
> > > > + val = 200 * KILO; /* 200kOhm */
> > >
> > > Better to #define magic numbers; DEFAULT_{DESCRIPTION}_OHMS
> > >
> >
> > Why? that is not needed.
> If this variable had a more useful name there would be no need for
> the comment either.
>
> val_resitor_ohms = 200 * KILLO;
>
> or similar.
>
So I have to add a "reasonably" named variable for each property I
want to get from device tree? Why? It seems to be a bit of overkill,
no? Maybe I am not aware, have variables stopped being reusable?
> >
> > > > + device_property_read_u32(&pdev->dev, "ti,resistor-value-ohm", &val);
> > > > +
> > > > + /* Convert resitance into R_ALS value with 2v / 10uA * R */
> > >
> > > Because ...
> > >
> >
> > BACAUSE the device DOES NOT understand human readable values, only 0s
> > and 1s, hence mOhms must be converted into value lm3533 chip can
> > understand.
> A comment that gave the motivation would be much more useful than
> repeating the maths.
>
> /* Convert resistance to equivalent register value */
>
ok, this is reasonable.
> >
> > > > + als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * val);
> > > > +
> > > > + als->pwm_mode = device_property_read_bool(&pdev->dev, "ti,pwm-mode");
> > > > +
> > > > if (als->irq) {
> > > > 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 +914,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,
>
> Anyhow, I'm short on time so only looking at the IIO related part.
>
> Jonathan
^ permalink raw reply
* Re: [PATCH v3 2/2] mfd: lm3533: convert to use OF
From: Jonathan Cameron @ 2025-03-05 13:44 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lars-Peter Clausen, Pavel Machek, Daniel Thompson, Jingoo Han,
Helge Deller, Andy Shevchenko, Uwe Kleine-König, devicetree,
linux-kernel, linux-iio, linux-leds, dri-devel, linux-fbdev
In-Reply-To: <CAPVz0n0jaR=UM7WbBs3zM-cZzuaPVWBjf4Q7i82hvxtXg2oCzQ@mail.gmail.com>
On Fri, 28 Feb 2025 11:30:51 +0200
Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> пт, 28 лют. 2025 р. о 10:59 Lee Jones <lee@kernel.org> пише:
> >
> > On Mon, 24 Feb 2025, Svyatoslav Ryhel wrote:
> >
> > > Remove platform data and fully relay on OF and device tree
> > > parsing and binding devices.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > > drivers/iio/light/lm3533-als.c | 40 ++++---
> > > drivers/leds/leds-lm3533.c | 46 +++++---
> > > drivers/mfd/lm3533-core.c | 159 ++++++++--------------------
> > > drivers/video/backlight/lm3533_bl.c | 71 ++++++++++---
> > > include/linux/mfd/lm3533.h | 35 +-----
> > > 5 files changed, 164 insertions(+), 187 deletions(-)
> > >
> > > diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> > > index 99f0b903018c..cb52965e93c6 100644
> > > --- a/drivers/iio/light/lm3533-als.c
> > > +++ b/drivers/iio/light/lm3533-als.c
> > > @@ -16,9 +16,12 @@
> > > #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>
> > >
> > > @@ -56,6 +59,9 @@ struct lm3533_als {
> > >
> > > atomic_t zone;
> > > struct mutex thresh_mutex;
> > > +
> > > + unsigned pwm_mode:1; /* PWM input mode (default analog) */
> > > + u8 r_select; /* 1 - 127 (ignored in PWM-mode) */
> > > };
> > >
> > >
> > > @@ -753,18 +759,17 @@ static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
> > > return 0;
> > > }
> > >
> > > -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)
> > > {
> > > int ret;
> > >
> > > - ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> > > + ret = lm3533_als_set_input_mode(als, als->pwm_mode);
> > > if (ret)
> > > return ret;
> > >
> > > /* 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_als_set_resistor(als, als->r_select);
> >
> > You're already passing 'als'.
> >
> > Just teach lm3533_als_set_resistor that 'r_select' is now contained.
> >
>
> This is not scope of this patchset. I was already accused in too much
> changes which make it unreadable. This patchset is dedicated to
> swapping platform data to use of the device tree. NOT improving
> functions, NOT rewriting arbitrary mechanics. If you feed a need for
> this change, then propose a followup. I need from this driver only one
> thing, that it could work with device tree. But it seems that it is
> better that it just rots in the garbage bin until removed cause no one
> cared.
This is not an unreasonable request as you added r_select to als.
Perhaps it belongs in a separate follow up patch. However
it is worth remembering the motivation here is that you want get
this code upstream, the maintainers don't have that motivation.
Greg KH has given various talks on the different motivations in the
past. It maybe worth a watch.
>
> > > if (ret)
> > > return ret;
> > > }
> > > @@ -828,22 +833,16 @@ 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;
> > > + u32 val;
> >
> > Value of what, potatoes?
> >
>
> Oranges.
A well named variable would avoid need for any discussion of
what it is the value of.
>
> > > int ret;
> > >
> > > lm3533 = dev_get_drvdata(pdev->dev.parent);
> > > 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;
> > > @@ -864,13 +863,21 @@ static int lm3533_als_probe(struct platform_device *pdev)
> > >
> > > platform_set_drvdata(pdev, indio_dev);
> > >
> > > + val = 200 * KILO; /* 200kOhm */
> >
> > Better to #define magic numbers; DEFAULT_{DESCRIPTION}_OHMS
> >
>
> Why? that is not needed.
If this variable had a more useful name there would be no need for
the comment either.
val_resitor_ohms = 200 * KILLO;
or similar.
>
> > > + device_property_read_u32(&pdev->dev, "ti,resistor-value-ohm", &val);
> > > +
> > > + /* Convert resitance into R_ALS value with 2v / 10uA * R */
> >
> > Because ...
> >
>
> BACAUSE the device DOES NOT understand human readable values, only 0s
> and 1s, hence mOhms must be converted into value lm3533 chip can
> understand.
A comment that gave the motivation would be much more useful than
repeating the maths.
/* Convert resistance to equivalent register value */
>
> > > + als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * val);
> > > +
> > > + als->pwm_mode = device_property_read_bool(&pdev->dev, "ti,pwm-mode");
> > > +
> > > if (als->irq) {
> > > 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 +914,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,
Anyhow, I'm short on time so only looking at the IIO related part.
Jonathan
^ permalink raw reply
* Re: video: au1100fb: Move a variable assignment behind a null pointer check in au1100fb_setmode()
From: Markus Elfring @ 2025-03-05 12:14 UTC (permalink / raw)
To: Dan Carpenter, Uwe Kleine-König, kernel-janitors,
linux-fbdev, dri-devel
Cc: Antonino Daplas, Helge Deller, Thomas Zimmermann, Yihao Han,
cocci, LKML
In-Reply-To: <47c37d1a-5740-4f48-ac0f-635d8b6f51b2@stanley.mountain>
> Anyway, none of that applies here, because this is just pointer math.
Which data processing do you expect to be generally supported at the discussed
source code place (according to the rules of the programming language “C”)?
https://en.cppreference.com/w/c/language/behavior
Regards,
Markus
^ permalink raw reply
* Re: [PATCH] fbtft: Remove access to page->index
From: Simona Vetter @ 2025-03-05 7:39 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Lorenzo Stoakes, Greg Kroah-Hartman, Thomas Zimmermann,
Jeff Johnson, Thomas Petazzoni, dri-devel, linux-fbdev,
linux-staging
In-Reply-To: <Z8coiv3rn8loOltq@casper.infradead.org>
On Tue, Mar 04, 2025 at 04:21:30PM +0000, Matthew Wilcox wrote:
> On Fri, Feb 21, 2025 at 05:53:16PM +0000, Lorenzo Stoakes wrote:
> > On Fri, Feb 21, 2025 at 05:31:29PM +0000, Matthew Wilcox (Oracle) wrote:
> > > There is no need to print out page->index as part of the debug message.
> > >
> > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> >
> > LGTM from my side,
> >
> > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>
> This patch is not yet in linux-next; can I expect it to go in soon?
Being both staging and fbdev it's double orphaned, I stuffed it into
drm-misc-next now, thanks for the ping&patch.
-Sima
--
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH RESEND] video: au1100fb: Move a variable assignment behind a null pointer check in au1100fb_setmode()
From: Dan Carpenter @ 2025-03-03 10:08 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Markus Elfring, kernel-janitors, linux-fbdev, dri-devel,
Antonino Daplas, Helge Deller, Thomas Zimmermann, Yihao Han,
cocci, LKML
In-Reply-To: <ugymllbkcsg22ffgyofvkquh5afbvoyv2nna5udmy3xfhv2rjz@jhgghzldzm4u>
On Mon, Mar 03, 2025 at 10:19:06AM +0100, Uwe Kleine-König wrote:
> Hello,
>
> On Sun, Mar 02, 2025 at 07:02:12PM +0100, Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Thu, 13 Apr 2023 21:35:36 +0200
> >
> > The address of a data structure member was determined before
> > a corresponding null pointer check in the implementation of
> > the function “au1100fb_setmode”.
> >
> > Thus avoid the risk for undefined behaviour by moving the assignment
> > for the variable “info” behind the null pointer check
^ permalink raw reply
* Re: [PATCH] fbtft: Remove access to page->index
From: Matthew Wilcox @ 2025-03-04 16:21 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Greg Kroah-Hartman, Thomas Zimmermann, Jeff Johnson,
Thomas Petazzoni, dri-devel, linux-fbdev, linux-staging
In-Reply-To: <27cc53b9-0db7-451c-9136-5fdcf42145f7@lucifer.local>
On Fri, Feb 21, 2025 at 05:53:16PM +0000, Lorenzo Stoakes wrote:
> On Fri, Feb 21, 2025 at 05:31:29PM +0000, Matthew Wilcox (Oracle) wrote:
> > There is no need to print out page->index as part of the debug message.
> >
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>
> LGTM from my side,
>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
This patch is not yet in linux-next; can I expect it to go in soon?
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: remove adi,ad7606.yaml from SEPS525
From: Jonathan Cameron @ 2025-03-04 14:27 UTC (permalink / raw)
To: David Lechner
Cc: linux-kernel, linux-staging, linux-fbdev, linux-iio,
Michael Hennerich
In-Reply-To: <20250303-maintainers-remove-adi-ad7606-yaml-from-seps525-lcd-controller-v1-1-a4e4f1b824ab@baylibre.com>
On Mon, 03 Mar 2025 14:39:57 +0000
David Lechner <dlechner@baylibre.com> wrote:
> Remove Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml from
> STAGING - SEPS525 LCD CONTROLLER DRIVERS. This was likley a copy/paste
> mistake. There is no bindings file for SEPS525 since it is only in
> staging.
>
> The removed file matches Documentation/devicetree/bindings/iio/*/adi,*
> under ANALOG DEVICES INC IIO DRIVERS already so wasn't just misplaced.
>
> Signed-off-by: David Lechner <dlechner@baylibre.com>
I'll apply it.
Jonathan
> ---
> This falls under FBTFT which is currently orphaned, so someone else will
> have to volunteer to pick this up.
> ---
> MAINTAINERS | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8e0736dc2ee0e33544fa373a4978b7dae18c040c..215dbaeedced8473b5b339329b3596a2fbfd13b1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22512,7 +22512,6 @@ STAGING - SEPS525 LCD CONTROLLER DRIVERS
> M: Michael Hennerich <michael.hennerich@analog.com>
> L: linux-fbdev@vger.kernel.org
> S: Supported
> -F: Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> F: drivers/staging/fbtft/fb_seps525.c
>
> STAGING - SILICON MOTION SM750 FRAME BUFFER DRIVER
>
> ---
> base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
> change-id: 20250303-maintainers-remove-adi-ad7606-yaml-from-seps525-lcd-controller-b77d4c4bf54a
>
> Best regards,
^ permalink raw reply
* [PATCH] MAINTAINERS: remove adi,ad7606.yaml from SEPS525
From: David Lechner @ 2025-03-03 14:39 UTC (permalink / raw)
To: linux-kernel, linux-staging, linux-fbdev, linux-iio,
Michael Hennerich
Cc: David Lechner
Remove Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml from
STAGING - SEPS525 LCD CONTROLLER DRIVERS. This was likley a copy/paste
mistake. There is no bindings file for SEPS525 since it is only in
staging.
The removed file matches Documentation/devicetree/bindings/iio/*/adi,*
under ANALOG DEVICES INC IIO DRIVERS already so wasn't just misplaced.
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
This falls under FBTFT which is currently orphaned, so someone else will
have to volunteer to pick this up.
---
MAINTAINERS | 1 -
1 file changed, 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8e0736dc2ee0e33544fa373a4978b7dae18c040c..215dbaeedced8473b5b339329b3596a2fbfd13b1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22512,7 +22512,6 @@ STAGING - SEPS525 LCD CONTROLLER DRIVERS
M: Michael Hennerich <michael.hennerich@analog.com>
L: linux-fbdev@vger.kernel.org
S: Supported
-F: Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
F: drivers/staging/fbtft/fb_seps525.c
STAGING - SILICON MOTION SM750 FRAME BUFFER DRIVER
---
base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
change-id: 20250303-maintainers-remove-adi-ad7606-yaml-from-seps525-lcd-controller-b77d4c4bf54a
Best regards,
--
David Lechner <dlechner@baylibre.com>
^ permalink raw reply related
* Re: [PATCH RESEND] video: au1100fb: Move a variable assignment behind a null pointer check in au1100fb_setmode()
From: Dan Carpenter @ 2025-03-03 10:53 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Markus Elfring, kernel-janitors, linux-fbdev, dri-devel,
Antonino Daplas, Helge Deller, Thomas Zimmermann, Yihao Han,
cocci, LKML
In-Reply-To: <hwk2nf62owdo3olxrwt5tu7nwfpjkrr3yawizfpb3xn6ydeekx@xwz7nh5ece2c>
On Mon, Mar 03, 2025 at 11:30:46AM +0100, Uwe Kleine-König wrote:
> On Mon, Mar 03, 2025 at 01:08:29PM +0300, Dan Carpenter wrote:
> > On Mon, Mar 03, 2025 at 10:19:06AM +0100, Uwe Kleine-König wrote:
> > > Hello,
> > >
> > > On Sun, Mar 02, 2025 at 07:02:12PM +0100, Markus Elfring wrote:
> > > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > > Date: Thu, 13 Apr 2023 21:35:36 +0200
> > > >
> > > > The address of a data structure member was determined before
> > > > a corresponding null pointer check in the implementation of
> > > > the function “au1100fb_setmode”.
> > > >
> > > > Thus avoid the risk for undefined behaviour by moving the assignment
> > > > for the variable “info” behind the null pointer check.
> > > >
> > > > This issue was detected by using the Coccinelle software.
> > > >
> > > > Fixes: 3b495f2bb749 ("Au1100 FB driver uplift for 2.6.")
> > > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > >
> > > Acked-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> > >
> > > Should also get
> > >
> > > Cc: stable@vger.kernel.org
> > >
> > > to ensure this is backported to stable.
> >
> > It's not a bugfix, it's a cleanup. That's not a dereference, it's
> > just pointer math. It shouldn't have a Fixes tag.
> >
> > Real bugs where we dereference a pointer and then check for NULL don't
> > last long in the kernel. Most of the stuff Markus is sending is false
> > positives like this.
>
> I thought a compiler translating the code
>
> struct fb_info *info = &fbdev->info;
>
> if (!fbdev)
> return -EINVAL;
>
> is free (and expected) to just drop the if block.
If you dereference a pointer then it doesn't make sense to have a NULL
check after that because the kernel would already have crashed.
In 2009, we had the famous tun.c bug where there was a dereference
followed by a NULL check and the compiler deleted it as you say.
And then, it turned out that you could remap the NULL pointer to and so
the NULL dereference didn't lead to a crash and the missing NULL meant
the kernel kept running happily. The remapped memory was full of
function pointers to get root.
We changed min_mmap_addr so that we can't remap the NULL pointer and
we started using the -fno-delete-null-pointer-checks compiler option so
that it wouldn't remove the NULL check even in places where it didn't
make sense. We also started doing more static analysis. We've also
tried to randomize where functions are in the memory so it's trickier
to hardcode function pointers.
A couple years later we had another bug where it turned out you could
still remap NULL. I forget how the details. No one wrote an exploit
and it wasn't publicized as much.
Anyway, none of that applies here, because this is just pointer math.
regards,
dan carpenter
^ permalink raw reply
* Re: video: au1100fb: Move a variable assignment behind a null pointer check in au1100fb_setmode()
From: Markus Elfring @ 2025-03-03 10:36 UTC (permalink / raw)
To: Uwe Kleine-König, Dan Carpenter, kernel-janitors,
linux-fbdev, dri-devel
Cc: Antonino Daplas, Helge Deller, Thomas Zimmermann, Yihao Han,
cocci, LKML
In-Reply-To: <hwk2nf62owdo3olxrwt5tu7nwfpjkrr3yawizfpb3xn6ydeekx@xwz7nh5ece2c>
> struct fb_info *info = &fbdev->info;
>
> if (!fbdev)
> return -EINVAL;
Is such a null pointer check still relevant for the discussed function implementation?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH RESEND] video: au1100fb: Move a variable assignment behind a null pointer check in au1100fb_setmode()
From: Uwe Kleine-König @ 2025-03-03 10:30 UTC (permalink / raw)
To: Dan Carpenter
Cc: Markus Elfring, kernel-janitors, linux-fbdev, dri-devel,
Antonino Daplas, Helge Deller, Thomas Zimmermann, Yihao Han,
cocci, LKML
In-Reply-To: <eebf8c0c-7a6a-405f-aaab-2a8a8c2bd91f@stanley.mountain>
[-- Attachment #1: Type: text/plain, Size: 1855 bytes --]
On Mon, Mar 03, 2025 at 01:08:29PM +0300, Dan Carpenter wrote:
> On Mon, Mar 03, 2025 at 10:19:06AM +0100, Uwe Kleine-König wrote:
> > Hello,
> >
> > On Sun, Mar 02, 2025 at 07:02:12PM +0100, Markus Elfring wrote:
> > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > Date: Thu, 13 Apr 2023 21:35:36 +0200
> > >
> > > The address of a data structure member was determined before
> > > a corresponding null pointer check in the implementation of
> > > the function “au1100fb_setmode”.
> > >
> > > Thus avoid the risk for undefined behaviour by moving the assignment
> > > for the variable “info” behind the null pointer check.
> > >
> > > This issue was detected by using the Coccinelle software.
> > >
> > > Fixes: 3b495f2bb749 ("Au1100 FB driver uplift for 2.6.")
> > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> >
> > Acked-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> >
> > Should also get
> >
> > Cc: stable@vger.kernel.org
> >
> > to ensure this is backported to stable.
>
> It's not a bugfix, it's a cleanup. That's not a dereference, it's
> just pointer math. It shouldn't have a Fixes tag.
>
> Real bugs where we dereference a pointer and then check for NULL don't
> last long in the kernel. Most of the stuff Markus is sending is false
> positives like this.
I thought a compiler translating the code
struct fb_info *info = &fbdev->info;
if (!fbdev)
return -EINVAL;
is free (and expected) to just drop the if block. I wasn't aware that
this only applies when the pointer is actually dereferenced. Testing
that with arm-linux-gnueabihf-gcc 14.2.0 seems to confirm what you're
saying.
Thanks for letting me know. With that learned I agree that the Fixes tag
should be dropped (and Cc: stable not added).
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND] video: au1100fb: Move a variable assignment behind a null pointer check in au1100fb_setmode()
From: Dan Carpenter @ 2025-03-03 10:14 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Markus Elfring, kernel-janitors, linux-fbdev, dri-devel,
Antonino Daplas, Helge Deller, Thomas Zimmermann, Yihao Han,
cocci, LKML
In-Reply-To: <eebf8c0c-7a6a-405f-aaab-2a8a8c2bd91f@stanley.mountain>
On Mon, Mar 03, 2025 at 01:08:29PM +0300, Dan Carpenter wrote:
> Real bugs where we dereference a pointer and then check for NULL don't
> last long in the kernel. Most of the stuff Markus is sending is false
> positives like this.
Maybe I was too optimistic. Here are the Smatch warnings from the
Friday's linux-next.
Common false positives are that the pointer can sometimes be NULL but
there are other ways to determine without checking explicitly. For
example, maybe the caller passes a flag which means it's non-NULL.
regards,
dan carpenter
arch/arm64/kvm/../../../virt/kvm/kvm_main.c:1639 kvm_prepare_memory_region() warn: variable dereferenced before check 'new' (see line 1622)
arch/x86/kernel/fpu/xstate.c:1567 fpstate_realloc() warn: variable dereferenced before check 'curfps' (see line 1546)
arch/x86/kvm/../../../virt/kvm/kvm_main.c:1639 kvm_prepare_memory_region() warn: variable dereferenced before check 'new' (see line 1622)
drivers/base/dd.c:696 really_probe() warn: variable dereferenced before check 'dev->bus' (see line 640)
drivers/base/dd.c:720 really_probe() warn: variable dereferenced before check 'dev->bus' (see line 640)
drivers/block/drbd/drbd_worker.c:588 make_resync_request() warn: variable dereferenced before check 'peer_device' (see line 587)
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:12341 parse_edid_displayid_vrr() warn: variable dereferenced before check 'edid_ext' (see line 12337)
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn21/rn_clk_mgr.c:775 rn_clk_mgr_construct() warn: variable dereferenced before check 'ctx->dc_bios->integrated_info' (see line 743)
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn301/vg_clk_mgr.c:734 vg_clk_mgr_construct() warn: variable dereferenced before check 'ctx->dc_bios->integrated_info' (see line 720)
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn314/dcn314_clk_mgr.c:899 dcn314_clk_mgr_construct() warn: variable dereferenced before check 'ctx->dc_bios->integrated_info' (see line 838)
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c:715 dcn315_clk_mgr_construct() warn: variable dereferenced before check 'ctx->dc_bios->integrated_info' (see line 654)
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn316/dcn316_clk_mgr.c:655 dcn316_clk_mgr_construct() warn: variable dereferenced before check 'ctx->dc_bios->integrated_info' (see line 634)
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn31/dcn31_clk_mgr.c:789 dcn31_clk_mgr_construct() warn: variable dereferenced before check 'ctx->dc_bios->integrated_info' (see line 728)
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c:1380 dcn35_clk_mgr_construct() warn: variable dereferenced before check 'ctx->dc_bios->integrated_info' (see line 1307)
drivers/gpu/drm/i915/gem/i915_gem_shmem.c:174 shmem_sg_alloc_table() warn: variable dereferenced before check 'sg' (see line 163)
drivers/gpu/drm/i915/gt/gen6_ppgtt.c:274 gen6_ppgtt_cleanup() warn: variable dereferenced before check 'ppgtt->base.pd' (see line 271)
drivers/gpu/drm/i915/gt/intel_execlists_submission.c:3649 rcu_virtual_context_destroy() warn: variable dereferenced before check 've->base.sched_engine' (see line 3623)
drivers/gpu/drm/nouveau/nouveau_dp.c:494 nouveau_dp_irq() warn: variable dereferenced before check 'outp' (see line 489)
drivers/hid/hid-debug.c:3727 hid_debug_events_read() warn: variable dereferenced before check 'list->hdev' (see line 3713)
drivers/net/ethernet/amd/pcnet32.c:1923 pcnet32_probe1() warn: variable dereferenced before check 'pdev' (see line 1843)
drivers/net/ethernet/apm/xgene/xgene_enet_main.c:267 xgene_enet_tx_completion() warn: variable dereferenced before check 'skb' (see line 243)
drivers/net/ethernet/natsemi/ns83820.c:884 rx_irq() warn: variable dereferenced before check 'skb' (see line 883)
drivers/net/ethernet/pensando/ionic/ionic_txrx.c:205 ionic_rx_build_skb() warn: variable dereferenced before check 'buf_info->page' (see line 188)
drivers/net/wireless/intel/iwlwifi/dvm/mac80211.c:1236 iwl_setup_interface() warn: variable dereferenced before check 'priv->lib->bt_params' (see line 1229)
drivers/net/wireless/intel/iwlwifi/dvm/main.c:1114 iwl_init_drv() warn: variable dereferenced before check 'priv->lib->bt_params' (see line 1109)
drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c:307 mt76_connac2_mac_tx_rate_val() warn: variable dereferenced before check 'conf' (see line 300)
drivers/net/wireless/mediatek/mt76/mt7925/mac.c:851 mt7925_tx_check_aggr() warn: variable dereferenced before check 'sta' (see line 847)
drivers/nvme/host/ioctl.c:173 nvme_map_user_request() warn: variable dereferenced before check 'bio' (see line 162)
drivers/platform/mellanox/mlxreg-lc.c:903 mlxreg_lc_probe() warn: variable dereferenced before check 'data->notifier' (see line 828)
drivers/scsi/aacraid/commsup.c:2344 aac_command_thread() warn: variable dereferenced before check 'dev->queues' (see line 2332)
drivers/scsi/aic7xxx/aic79xx_osm.c:1837 ahd_done() warn: variable dereferenced before check 'cmd' (see line 1793)
drivers/scsi/csiostor/csio_mb.c:932 csio_fcoe_vnp_alloc_init_mb() warn: variable dereferenced before check 'vnport_wwnn' (see line 929)
drivers/scsi/ips.c:2560 ips_next() warn: variable dereferenced before check 'scb->scsi_cmd' (see line 2554)
drivers/scsi/ips.c:2568 ips_next() warn: variable dereferenced before check 'scb->scsi_cmd' (see line 2554)
drivers/scsi/ips.c:2593 ips_next() warn: variable dereferenced before check 'scb->scsi_cmd' (see line 2554)
drivers/scsi/libsas/sas_scsi_host.c:119 sas_scsi_task_done() warn: variable dereferenced before check 'sc' (see line 110)
drivers/scsi/lpfc/lpfc_bsg.c:1332 lpfc_bsg_hba_get_event() warn: variable dereferenced before check 'evt_dat' (see line 1299)
drivers/slimbus/qcom-ngd-ctrl.c:884 qcom_slim_ngd_xfer_msg() warn: variable dereferenced before check 'txn->msg' (see line 808)
drivers/spi/spi-offload.c:186 spi_offload_trigger_get() warn: variable dereferenced before check 'trigger->ops' (see line 176)
drivers/staging/media/atomisp/pci/isp/kernels/output/output_1.0/ia_css_output.host.c:58 ia_css_output_config() warn: variable dereferenced before check 'from->info' (see line 53)
drivers/usb/gadget/udc/tegra-xudc.c:2681 tegra_xudc_handle_transfer_completion() warn: variable dereferenced before check 'ep->desc' (see line 2679)
drivers/usb/musb/musb_core.c:964 musb_handle_intr_disconnect() warn: variable dereferenced before check 'musb->hcd' (see line 963)
fs/bcachefs/journal.c:1187 __bch2_set_nr_journal_buckets() warn: variable dereferenced before check 'c' (see line 1184)
fs/efs/inode.c:299 efs_map_block() warn: variable dereferenced before check 'bh' (see line 292)
fs/efs/inode.c:304 efs_map_block() warn: variable dereferenced before check 'bh' (see line 292)
fs/efs/inode.c:309 efs_map_block() warn: variable dereferenced before check 'bh' (see line 292)
fs/nfs/write.c:808 nfs_inode_remove_request() warn: variable dereferenced before check 'folio' (see line 805)
fs/ocfs2/dlm/dlmrecovery.c:1590 dlm_mig_lockres_worker() warn: variable dereferenced before check 'res' (see line 1563)
fs/ocfs2/move_extents.c:322 ocfs2_defrag_extent() warn: variable dereferenced before check 'context->data_ac' (see line 279)
fs/ocfs2/namei.c:1455 ocfs2_rename() warn: variable dereferenced before check 'newfe_bh' (see line 1452)
fs/ocfs2/namei.c:1637 ocfs2_rename() warn: variable dereferenced before check 'old_dir_bh' (see line 1618)
fs/smb/client/file.c:3085 cifs_oplock_break() warn: variable dereferenced before check 'inode' (see line 3056)
lib/reed_solomon/decode_rs.c:315 decode_rs16() warn: variable dereferenced before check 'par' (see line 81)
net/core/failover.c:85 failover_slave_register() warn: variable dereferenced before check 'fops' (see line 66)
net/mpls/mpls_iptunnel.c:156 mpls_xmit() warn: variable dereferenced before check 'out_dev' (see line 56)
^ permalink raw reply
* Re: [PATCH RESEND] video: au1100fb: Move a variable assignment behind a null pointer check in au1100fb_setmode()
From: Dan Carpenter @ 2025-03-03 10:08 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Markus Elfring, kernel-janitors, linux-fbdev, dri-devel,
Antonino Daplas, Helge Deller, Thomas Zimmermann, Yihao Han,
cocci, LKML
In-Reply-To: <ugymllbkcsg22ffgyofvkquh5afbvoyv2nna5udmy3xfhv2rjz@jhgghzldzm4u>
On Mon, Mar 03, 2025 at 10:19:06AM +0100, Uwe Kleine-König wrote:
> Hello,
>
> On Sun, Mar 02, 2025 at 07:02:12PM +0100, Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Thu, 13 Apr 2023 21:35:36 +0200
> >
> > The address of a data structure member was determined before
> > a corresponding null pointer check in the implementation of
> > the function “au1100fb_setmode”.
> >
> > Thus avoid the risk for undefined behaviour by moving the assignment
> > for the variable “info” behind the null pointer check.
> >
> > This issue was detected by using the Coccinelle software.
> >
> > Fixes: 3b495f2bb749 ("Au1100 FB driver uplift for 2.6.")
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>
> Acked-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
>
> Should also get
>
> Cc: stable@vger.kernel.org
>
> to ensure this is backported to stable.
It's not a bugfix, it's a cleanup. That's not a dereference, it's
just pointer math. It shouldn't have a Fixes tag.
Real bugs where we dereference a pointer and then check for NULL don't
last long in the kernel. Most of the stuff Markus is sending is false
positives like this.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH RESEND] video: au1100fb: Move a variable assignment behind a null pointer check in au1100fb_setmode()
From: Uwe Kleine-König @ 2025-03-03 9:19 UTC (permalink / raw)
To: Markus Elfring
Cc: kernel-janitors, linux-fbdev, dri-devel, Antonino Daplas,
Helge Deller, Thomas Zimmermann, Yihao Han, cocci, LKML
In-Reply-To: <3f1e7aaa-501a-44f1-8122-28e9efa0a33c@web.de>
[-- Attachment #1: Type: text/plain, Size: 850 bytes --]
Hello,
On Sun, Mar 02, 2025 at 07:02:12PM +0100, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 13 Apr 2023 21:35:36 +0200
>
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “au1100fb_setmode”.
>
> Thus avoid the risk for undefined behaviour by moving the assignment
> for the variable “info” behind the null pointer check.
>
> This issue was detected by using the Coccinelle software.
>
> Fixes: 3b495f2bb749 ("Au1100 FB driver uplift for 2.6.")
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Acked-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
Should also get
Cc: stable@vger.kernel.org
to ensure this is backported to stable.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH RESEND] video: au1100fb: Move a variable assignment behind a null pointer check in au1100fb_setmode()
From: Markus Elfring @ 2025-03-02 18:02 UTC (permalink / raw)
To: kernel-janitors, linux-fbdev, dri-devel, Antonino Daplas,
Helge Deller, Thomas Zimmermann, Uwe Kleine-König, Yihao Han
Cc: cocci, LKML
In-Reply-To: <86551e6f-d529-1ff6-6ce6-b9669d10e6cb@web.de>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 13 Apr 2023 21:35:36 +0200
The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function “au1100fb_setmode”.
Thus avoid the risk for undefined behaviour by moving the assignment
for the variable “info” behind the null pointer check.
This issue was detected by using the Coccinelle software.
Fixes: 3b495f2bb749 ("Au1100 FB driver uplift for 2.6.")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/video/fbdev/au1100fb.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
index cb317398e71a..fcb47b350bc9 100644
--- a/drivers/video/fbdev/au1100fb.c
+++ b/drivers/video/fbdev/au1100fb.c
@@ -137,13 +137,15 @@ static int au1100fb_fb_blank(int blank_mode, struct fb_info *fbi)
*/
int au1100fb_setmode(struct au1100fb_device *fbdev)
{
- struct fb_info *info = &fbdev->info;
+ struct fb_info *info;
u32 words;
int index;
if (!fbdev)
return -EINVAL;
+ info = &fbdev->info;
+
/* Update var-dependent FB info */
if (panel_is_active(fbdev->panel) || panel_is_color(fbdev->panel)) {
if (info->var.bits_per_pixel <= 8) {
--
2.40.0
^ permalink raw reply related
* [PATCH v3 1/2] fbdev: hyperv_fb: Simplify hvfb_putmem
From: Saurabh Sengar @ 2025-03-01 16:16 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, deller, akpm, linux-hyperv,
linux-fbdev, dri-devel, linux-kernel
Cc: ssengar, mhklinux
In-Reply-To: <1740845791-19977-1-git-send-email-ssengar@linux.microsoft.com>
The device object required in 'hvfb_release_phymem' function
for 'dma_free_coherent' can also be obtained from the 'info'
pointer, making 'hdev' parameter in 'hvfb_putmem' redundant.
Remove the unnecessary 'hdev' argument from 'hvfb_putmem'.
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
drivers/video/fbdev/hyperv_fb.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 363e4ccfcdb7..09fb025477f7 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -952,7 +952,7 @@ static phys_addr_t hvfb_get_phymem(struct hv_device *hdev,
}
/* Release contiguous physical memory */
-static void hvfb_release_phymem(struct hv_device *hdev,
+static void hvfb_release_phymem(struct device *device,
phys_addr_t paddr, unsigned int size)
{
unsigned int order = get_order(size);
@@ -960,7 +960,7 @@ static void hvfb_release_phymem(struct hv_device *hdev,
if (order <= MAX_PAGE_ORDER)
__free_pages(pfn_to_page(paddr >> PAGE_SHIFT), order);
else
- dma_free_coherent(&hdev->device,
+ dma_free_coherent(device,
round_up(size, PAGE_SIZE),
phys_to_virt(paddr),
paddr);
@@ -1074,7 +1074,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
}
/* Release the framebuffer */
-static void hvfb_putmem(struct hv_device *hdev, struct fb_info *info)
+static void hvfb_putmem(struct fb_info *info)
{
struct hvfb_par *par = info->par;
@@ -1083,7 +1083,7 @@ static void hvfb_putmem(struct hv_device *hdev, struct fb_info *info)
iounmap(par->mmio_vp);
vmbus_free_mmio(par->mem->start, screen_fb_size);
} else {
- hvfb_release_phymem(hdev, info->fix.smem_start,
+ hvfb_release_phymem(info->device, info->fix.smem_start,
screen_fb_size);
}
@@ -1197,7 +1197,7 @@ static int hvfb_probe(struct hv_device *hdev,
error:
fb_deferred_io_cleanup(info);
- hvfb_putmem(hdev, info);
+ hvfb_putmem(info);
error2:
vmbus_close(hdev->channel);
error1:
@@ -1226,7 +1226,7 @@ static void hvfb_remove(struct hv_device *hdev)
vmbus_close(hdev->channel);
hv_set_drvdata(hdev, NULL);
- hvfb_putmem(hdev, info);
+ hvfb_putmem(info);
framebuffer_release(info);
}
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/2] fbdev: hyperv_fb: Allow graceful removal of framebuffer
From: Saurabh Sengar @ 2025-03-01 16:16 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, deller, akpm, linux-hyperv,
linux-fbdev, dri-devel, linux-kernel
Cc: ssengar, mhklinux
In-Reply-To: <1740845791-19977-1-git-send-email-ssengar@linux.microsoft.com>
When a Hyper-V framebuffer device is unbind, hyperv_fb driver tries to
release the framebuffer forcefully. If this framebuffer is in use it
produce the following WARN and hence this framebuffer is never released.
[ 44.111220] WARNING: CPU: 35 PID: 1882 at drivers/video/fbdev/core/fb_info.c:70 framebuffer_release+0x2c/0x40
< snip >
[ 44.111289] Call Trace:
[ 44.111290] <TASK>
[ 44.111291] ? show_regs+0x6c/0x80
[ 44.111295] ? __warn+0x8d/0x150
[ 44.111298] ? framebuffer_release+0x2c/0x40
[ 44.111300] ? report_bug+0x182/0x1b0
[ 44.111303] ? handle_bug+0x6e/0xb0
[ 44.111306] ? exc_invalid_op+0x18/0x80
[ 44.111308] ? asm_exc_invalid_op+0x1b/0x20
[ 44.111311] ? framebuffer_release+0x2c/0x40
[ 44.111313] ? hvfb_remove+0x86/0xa0 [hyperv_fb]
[ 44.111315] vmbus_remove+0x24/0x40 [hv_vmbus]
[ 44.111323] device_remove+0x40/0x80
[ 44.111325] device_release_driver_internal+0x20b/0x270
[ 44.111327] ? bus_find_device+0xb3/0xf0
Fix this by moving the release of framebuffer and assosiated memory
to fb_ops.fb_destroy function, so that framebuffer framework handles
it gracefully.
While we fix this, also replace manual registrations/unregistration of
framebuffer with devm_register_framebuffer.
Fixes: 68a2d20b79b1 ("drivers/video: add Hyper-V Synthetic Video Frame Buffer Driver")
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
[V3]
- using simplified hvfb_putmem()
drivers/video/fbdev/hyperv_fb.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 09fb025477f7..76a42379c8df 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -282,6 +282,8 @@ static uint screen_depth;
static uint screen_fb_size;
static uint dio_fb_size; /* FB size for deferred IO */
+static void hvfb_putmem(struct fb_info *info);
+
/* Send message to Hyper-V host */
static inline int synthvid_send(struct hv_device *hdev,
struct synthvid_msg *msg)
@@ -862,6 +864,17 @@ static void hvfb_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width,
hvfb_ondemand_refresh_throttle(par, x, y, width, height);
}
+/*
+ * fb_ops.fb_destroy is called by the last put_fb_info() call at the end
+ * of unregister_framebuffer() or fb_release(). Do any cleanup related to
+ * framebuffer here.
+ */
+static void hvfb_destroy(struct fb_info *info)
+{
+ hvfb_putmem(info);
+ framebuffer_release(info);
+}
+
/*
* TODO: GEN1 codepaths allocate from system or DMA-able memory. Fix the
* driver to use the _SYSMEM_ or _DMAMEM_ helpers in these cases.
@@ -877,6 +890,7 @@ static const struct fb_ops hvfb_ops = {
.fb_set_par = hvfb_set_par,
.fb_setcolreg = hvfb_setcolreg,
.fb_blank = hvfb_blank,
+ .fb_destroy = hvfb_destroy,
};
/* Get options from kernel paramenter "video=" */
@@ -1172,7 +1186,7 @@ static int hvfb_probe(struct hv_device *hdev,
if (ret)
goto error;
- ret = register_framebuffer(info);
+ ret = devm_register_framebuffer(&hdev->device, info);
if (ret) {
pr_err("Unable to register framebuffer\n");
goto error;
@@ -1220,14 +1234,10 @@ static void hvfb_remove(struct hv_device *hdev)
fb_deferred_io_cleanup(info);
- unregister_framebuffer(info);
cancel_delayed_work_sync(&par->dwork);
vmbus_close(hdev->channel);
hv_set_drvdata(hdev, NULL);
-
- hvfb_putmem(info);
- framebuffer_release(info);
}
static int hvfb_suspend(struct hv_device *hdev)
--
2.43.0
^ permalink raw reply related
* [PATCH v3 0/2] fbdev: hyperv_fb: framebuffer release cleanup
From: Saurabh Sengar @ 2025-03-01 16:16 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, deller, akpm, linux-hyperv,
linux-fbdev, dri-devel, linux-kernel
Cc: ssengar, mhklinux
This patch series addresses an issue in the unbind path of the hyperv_fb
driver, which is resolved in the second patch of this series.
While working on this fix, it was observed that hvfb_putmem() and its
child functions could be cleaned up first to simplify the movement of
hvfb_putmem(). This cleanup is done in the first patch.
Although hvfb_getmem() could also be cleaned up, it depends on
vmbus_allocate_mmio(), which is used by multiple other drivers. Since
addressing hvfb_getmem() is independent of this fix, it will be handled
in a separate patch series.
[V3]
- Add a patch 1 for cleanup of hvfb_putmem()
- Use the simplified hvfb_putmem()
Saurabh Sengar (2):
fbdev: hyperv_fb: Simplify hvfb_putmem
fbdev: hyperv_fb: Allow graceful removal of framebuffer
drivers/video/fbdev/hyperv_fb.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH 0/7] gpu: ipu-v3: Remove unused functions
From: Dr. David Alan Gilbert @ 2025-03-01 12:35 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: p.zabel, airlied, simona, deller, shawnguo, s.hauer, kernel,
festevam, dri-devel, linux-fbdev, imx, linux-arm-kernel,
linux-kernel
In-Reply-To: <174082343297.2941786.11452976094065423321.b4-ty@linaro.org>
* Dmitry Baryshkov (dmitry.baryshkov@linaro.org) wrote:
> On Thu, 26 Dec 2024 02:27:45 +0000, linux@treblig.org wrote:
> > From: "Dr. David Alan Gilbert" <linux@treblig.org>
> >
> > Hi,
> > This set removes a bunch of functions in ipu-v3 that
> > have been unused for a long time (since 2012-2017).
> >
> > No changes to functions are made, just full deletions.
> >
> > [...]
>
> Applied to drm-misc-next, thanks!
Thanks!
Dave
> [1/7] gpu: ipu-v3: ipu-ic: Remove unused ipu_ic_task_graphics_init
> commit: 16e3bf497fb2d379f3d461fa0c85d14de0a3d183
> [2/7] gpu: ipu-v3: Remove unused ipu_rot_mode_to_degrees
> commit: a52ba18c254c0a3819e632e6371554f1c6f5bd16
> [3/7] gpu: ipu-v3: Remove unused ipu_idmac_channel_busy
> commit: 4f9c64e95c3510f4a5192bd401de5611c1dd5637
> [4/7] gpu: ipu-v3: Remove unused ipu_image_convert_* functions
> commit: 96e9d754b35e87a5be2de7dce3c810ffdd769c84
> [5/7] gpu: ipu-v3: Remove unused ipu_vdi_unsetup
> commit: 27985c86e283e1e5ac8a9809f189f03643a6f5f2
> [6/7] gpu: ipu-v3: ipu-csi: Remove unused functions
> commit: c687c3147d5de801ed835b077802b68fe85d8a3d
> [7/7] gpu: ipu-v3 ipu-cpmem: Remove unused functions
> commit: 2800028d5bdee8e9a3cda2fec782dadc32225d8d
>
> Best regards,
> --
> With best wishes
> Dmitry
>
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply
* Re: [PATCH 0/7] gpu: ipu-v3: Remove unused functions
From: Dmitry Baryshkov @ 2025-03-01 10:03 UTC (permalink / raw)
To: p.zabel, airlied, simona, deller, shawnguo, s.hauer, kernel,
festevam, linux
Cc: dri-devel, linux-fbdev, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20241226022752.219399-1-linux@treblig.org>
On Thu, 26 Dec 2024 02:27:45 +0000, linux@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <linux@treblig.org>
>
> Hi,
> This set removes a bunch of functions in ipu-v3 that
> have been unused for a long time (since 2012-2017).
>
> No changes to functions are made, just full deletions.
>
> [...]
Applied to drm-misc-next, thanks!
[1/7] gpu: ipu-v3: ipu-ic: Remove unused ipu_ic_task_graphics_init
commit: 16e3bf497fb2d379f3d461fa0c85d14de0a3d183
[2/7] gpu: ipu-v3: Remove unused ipu_rot_mode_to_degrees
commit: a52ba18c254c0a3819e632e6371554f1c6f5bd16
[3/7] gpu: ipu-v3: Remove unused ipu_idmac_channel_busy
commit: 4f9c64e95c3510f4a5192bd401de5611c1dd5637
[4/7] gpu: ipu-v3: Remove unused ipu_image_convert_* functions
commit: 96e9d754b35e87a5be2de7dce3c810ffdd769c84
[5/7] gpu: ipu-v3: Remove unused ipu_vdi_unsetup
commit: 27985c86e283e1e5ac8a9809f189f03643a6f5f2
[6/7] gpu: ipu-v3: ipu-csi: Remove unused functions
commit: c687c3147d5de801ed835b077802b68fe85d8a3d
[7/7] gpu: ipu-v3 ipu-cpmem: Remove unused functions
commit: 2800028d5bdee8e9a3cda2fec782dadc32225d8d
Best regards,
--
With best wishes
Dmitry
^ permalink raw reply
* Re: (subset) [PATCH v1 00/13] Convert to use devm_kmemdup_array()
From: Mark Brown @ 2025-02-28 21:22 UTC (permalink / raw)
To: perex, tiwai, lgirdwood, deller, andriy.shevchenko, sre,
sakari.ailus, mchehab, jdmason, fancer.lancer, Hans Verkuil,
Raag Jadav
Cc: linux-sound, linux-fbdev, linux-pm, linux-media, ntb,
linux-kernel
In-Reply-To: <20250221165333.2780888-1-raag.jadav@intel.com>
On Fri, 21 Feb 2025 22:23:20 +0530, Raag Jadav wrote:
> This series is the second wave of patches to add users of newly introduced
> devm_kmemdup_array() helper. Original series on [1].
>
> This depends on changes available on immutable tag[2]. Feel free to pick
> your subsystem patches with it, or share your preferred way to route them.
>
> [1] https://lore.kernel.org/r/20250212062513.2254767-1-raag.jadav@intel.com
> [2] https://lore.kernel.org/r/Z7cqCaME4LxTTBn6@black.fi.intel.com
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[09/13] regulator: devres: use devm_kmemdup_array()
commit: 6ddd1159825c516b8f64fda83177c161434141f5
[10/13] regulator: cros-ec: use devm_kmemdup_array()
commit: c5c4ce6612bb25ce6d6936d8ade96fcba635da54
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [syzbot] [fbdev?] KASAN: global-out-of-bounds Read in bit_putcs (3)
From: syzbot @ 2025-02-28 20:37 UTC (permalink / raw)
To: daniel, deller, dri-devel, linux-fbdev, linux-kernel, simona,
syzkaller-bugs
In-Reply-To: <000000000000e39b37061e898704@google.com>
syzbot has found a reproducer for the following issue on:
HEAD commit: 017f704fbfb1 Merge branch 'for-next/core' into for-kernelci
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci
console output: https://syzkaller.appspot.com/x/log.txt?x=12128864580000
kernel config: https://syzkaller.appspot.com/x/.config?x=d6b7e15dc5b5e776
dashboard link: https://syzkaller.appspot.com/bug?extid=793cf822d213be1a74f2
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
userspace arch: arm64
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1643dba0580000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16128864580000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/d34fb306468f/disk-017f704f.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/6f1126558a26/vmlinux-017f704f.xz
kernel image: https://storage.googleapis.com/syzbot-assets/6d9d912bee27/Image-017f704f.gz.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+793cf822d213be1a74f2@syzkaller.appspotmail.com
do_el0_svc+0x48/0x58 arch/arm64/kernel/syscall.c:151
el0_svc+0x54/0x168 arch/arm64/kernel/entry-common.c:744
el0t_64_sync_handler+0x84/0x108 arch/arm64/kernel/entry-common.c:762
el0t_64_sync+0x198/0x19c arch/arm64/kernel/entry.S:600
==================================================================
BUG: KASAN: global-out-of-bounds in __fb_pad_aligned_buffer include/linux/fb.h:644 [inline]
BUG: KASAN: global-out-of-bounds in bit_putcs_aligned drivers/video/fbdev/core/bitblit.c:96 [inline]
BUG: KASAN: global-out-of-bounds in bit_putcs+0x9b8/0xe30 drivers/video/fbdev/core/bitblit.c:185
Read of size 1 at addr ffff80008be20810 by task syz-executor101/6448
CPU: 1 UID: 0 PID: 6448 Comm: syz-executor101 Not tainted 6.14.0-rc4-syzkaller-g017f704fbfb1 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 12/27/2024
Call trace:
show_stack+0x2c/0x3c arch/arm64/kernel/stacktrace.c:466 (C)
__dump_stack lib/dump_stack.c:94 [inline]
dump_stack_lvl+0xe4/0x150 lib/dump_stack.c:120
print_address_description mm/kasan/report.c:408 [inline]
print_report+0x198/0x550 mm/kasan/report.c:521
kasan_report+0xd8/0x138 mm/kasan/report.c:634
__asan_report_load1_noabort+0x20/0x2c mm/kasan/report_generic.c:378
__fb_pad_aligned_buffer include/linux/fb.h:644 [inline]
bit_putcs_aligned drivers/video/fbdev/core/bitblit.c:96 [inline]
bit_putcs+0x9b8/0xe30 drivers/video/fbdev/core/bitblit.c:185
fbcon_putcs+0x390/0x5a0 drivers/video/fbdev/core/fbcon.c:1308
do_update_region+0x1e8/0x3d0 drivers/tty/vt/vt.c:609
update_region+0x1e0/0x478 drivers/tty/vt/vt.c:633
vcs_write+0x9e8/0x1180 drivers/tty/vt/vc_screen.c:698
do_loop_readv_writev fs/read_write.c:843 [inline]
vfs_writev+0x494/0x92c fs/read_write.c:1052
do_writev+0x178/0x304 fs/read_write.c:1096
__do_sys_writev fs/read_write.c:1164 [inline]
__se_sys_writev fs/read_write.c:1161 [inline]
__arm64_sys_writev+0x80/0x94 fs/read_write.c:1161
__invoke_syscall arch/arm64/kernel/syscall.c:35 [inline]
invoke_syscall+0x98/0x2b8 arch/arm64/kernel/syscall.c:49
el0_svc_common+0x130/0x23c arch/arm64/kernel/syscall.c:132
do_el0_svc+0x48/0x58 arch/arm64/kernel/syscall.c:151
el0_svc+0x54/0x168 arch/arm64/kernel/entry-common.c:744
el0t_64_sync_handler+0x84/0x108 arch/arm64/kernel/entry-common.c:762
el0t_64_sync+0x198/0x19c arch/arm64/kernel/entry.S:600
The buggy address belongs to the variable:
fontdata_8x16+0x1010/0x1480
The buggy address belongs to the virtual mapping at
[ffff80008b810000, ffff80008f6b0000) created by:
declare_kernel_vmas+0x58/0xb8 arch/arm64/mm/mmu.c:771
The buggy address belongs to the physical page:
page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x1a1820
flags: 0x5ffc00000002000(reserved|node=0|zone=2|lastcpupid=0x7ff)
raw: 05ffc00000002000 fffffdffc5860808 fffffdffc5860808 0000000000000000
raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff80008be20700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff80008be20780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff80008be20800: 00 00 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
^
ffff80008be20880: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
ffff80008be20900: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
==================================================================
---
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
^ 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