From: linux@armlinux.org.uk (Russell King - ARM Linux) Date: 2017-09-29 10:50:04
Hi Bart,
This series updates the sa1100fb fbdev driver's initialisation paths
to be more robust. In doing these updates, I realised that we omitted
to free some memory which was allocated for the framebuffer if
initialisation fails - something that was hidden due to the complex
cleanup that the driver performs. Switching to managed resources made
this more obvious.
Tested on H3600 iPAQ and Assabet.
drivers/video/fbdev/sa1100fb.c | 75 +++++++++++++++---------------------------
drivers/video/fbdev/sa1100fb.h | 2 ++
2 files changed, 29 insertions(+), 48 deletions(-)
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
From: Russell King <hidden> Date: 2017-09-29 10:50:56
Use devm_kzalloc() when allocating the private data for the framebuffer
device.
Signed-off-by: Russell King <redacted>
---
drivers/video/fbdev/sa1100fb.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
From: Russell King <hidden> Date: 2017-09-29 10:51:01
Use devm_clk_get() to get the clock for the LCD.
Signed-off-by: Russell King <redacted>
---
drivers/video/fbdev/sa1100fb.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
From: Russell King <hidden> Date: 2017-09-29 10:51:07
Use devm_ioremap_resource() to map the LCD controller memory region,
and remove the unnecessary cleanup for this.
Signed-off-by: Russell King <redacted>
---
drivers/video/fbdev/sa1100fb.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
From: Russell King <hidden> Date: 2017-09-29 10:51:12
Use devm_request_irq() to request the interrupt (a little earlier too)
so we can avoid having to manually clean this up.
Signed-off-by: Russell King <redacted>
---
drivers/video/fbdev/sa1100fb.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
From: Russell King <hidden> Date: 2017-09-29 10:51:17
Switch to using devm_gpio_request_one() to request the shannon gpio
and move the request before the video memory allocation, so we request
all device managed resources before this large allocation attempt.
Signed-off-by: Russell King <redacted>
---
drivers/video/fbdev/sa1100fb.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
@@ -1246,18 +1246,18 @@ static int sa1100fb_probe(struct platform_device *pdev)gotofailed;}-/* Initialize video memory */-ret=sa1100fb_map_video_memory(fbi);-if(ret)-gotofailed;-if(machine_is_shannon()){-ret=gpio_request_one(SHANNON_GPIO_DISP_EN,+ret=devm_gpio_request_one(&pdev->dev,SHANNON_GPIO_DISP_EN,GPIOF_OUT_INIT_LOW,"display enable");if(ret)gotofailed;}+/* Initialize video memory */+ret=sa1100fb_map_video_memory(fbi);+if(ret)+gotofailed;+/**Thismakessurethatourcolourbitfield*descriptorsarecorrectlyinitialised.
@@ -1268,7 +1268,7 @@ static int sa1100fb_probe(struct platform_device *pdev)ret=register_framebuffer(&fbi->fb);if(ret<0)-gotoerr_reg_fb;+gotofailed;#ifdef CONFIG_CPU_FREQfbi->freq_transition.notifier_call=sa1100fb_freq_transition;
@@ -1280,9 +1280,6 @@ static int sa1100fb_probe(struct platform_device *pdev)/* This driver cannot be unloaded at the moment */return0;-err_reg_fb:-if(machine_is_shannon())-gpio_free(SHANNON_GPIO_DISP_EN);failed:returnret;}
From: Russell King <hidden> Date: 2017-09-29 10:51:22
We merely return from the failed path, so remove all the gotos and use
return statements instead.
Signed-off-by: Russell King <redacted>
---
drivers/video/fbdev/sa1100fb.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
From: Russell King <hidden> Date: 2017-09-29 10:51:28
Don't leak the video memory allocation if register_framebuffer() fails.
Signed-off-by: Russell King <redacted>
---
drivers/video/fbdev/sa1100fb.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
@@ -1262,8 +1262,11 @@ static int sa1100fb_probe(struct platform_device *pdev)platform_set_drvdata(pdev,fbi);ret=register_framebuffer(&fbi->fb);-if(ret<0)-gotofailed;+if(ret<0){+dma_free_wc(fbi->dev,fbi->map_size,fbi->map_cpu,+fbi->map_dma);+returnret;+}#ifdef CONFIG_CPU_FREQfbi->freq_transition.notifier_call=sa1100fb_freq_transition;
@@ -1274,9 +1277,6 @@ static int sa1100fb_probe(struct platform_device *pdev)/* This driver cannot be unloaded at the moment */return0;--failed:-returnret;}staticstructplatform_driversa1100fb_driver={
From: Russell King <hidden> Date: 2017-09-29 10:51:33
Move the pseudo palette inside the driver private data structure so we
don't have to play tricks to cater for it.
Signed-off-by: Russell King <redacted>
---
drivers/video/fbdev/sa1100fb.c | 9 +++------
drivers/video/fbdev/sa1100fb.h | 2 ++
2 files changed, 5 insertions(+), 6 deletions(-)
On Friday, September 29, 2017 11:50:04 AM Russell King - ARM Linux wrote:
Hi Bart,
Hi Russell,
This series updates the sa1100fb fbdev driver's initialisation paths
to be more robust. In doing these updates, I realised that we omitted
to free some memory which was allocated for the framebuffer if
initialisation fails - something that was hidden due to the complex
cleanup that the driver performs. Switching to managed resources made
this more obvious.
Tested on H3600 iPAQ and Assabet.
drivers/video/fbdev/sa1100fb.c | 75 +++++++++++++++---------------------------
drivers/video/fbdev/sa1100fb.h | 2 ++
2 files changed, 29 insertions(+), 48 deletions(-)
I queued all patches for 4.15, thanks!
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
From: linux@armlinux.org.uk (Russell King - ARM Linux) Date: 2017-11-27 17:11:45
On Tue, Oct 17, 2017 at 03:32:30PM +0200, Bartlomiej Zolnierkiewicz wrote:
On Friday, September 29, 2017 11:50:04 AM Russell King - ARM Linux wrote:
quoted
Hi Bart,
Hi Russell,
quoted
This series updates the sa1100fb fbdev driver's initialisation paths
to be more robust. In doing these updates, I realised that we omitted
to free some memory which was allocated for the framebuffer if
initialisation fails - something that was hidden due to the complex
cleanup that the driver performs. Switching to managed resources made
this more obvious.
Tested on H3600 iPAQ and Assabet.
drivers/video/fbdev/sa1100fb.c | 75 +++++++++++++++---------------------------
drivers/video/fbdev/sa1100fb.h | 2 ++
2 files changed, 29 insertions(+), 48 deletions(-)
I queued all patches for 4.15, thanks!
Now that 4.15-rc1 is out, something tells me that was not actually
the case. Any ideas what happened?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
On Monday, November 27, 2017 05:11:45 PM Russell King - ARM Linux wrote:
On Tue, Oct 17, 2017 at 03:32:30PM +0200, Bartlomiej Zolnierkiewicz wrote:
quoted
On Friday, September 29, 2017 11:50:04 AM Russell King - ARM Linux wrote:
quoted
Hi Bart,
Hi Russell,
quoted
This series updates the sa1100fb fbdev driver's initialisation paths
to be more robust. In doing these updates, I realised that we omitted
to free some memory which was allocated for the framebuffer if
initialisation fails - something that was hidden due to the complex
cleanup that the driver performs. Switching to managed resources made
this more obvious.
Tested on H3600 iPAQ and Assabet.
drivers/video/fbdev/sa1100fb.c | 75 +++++++++++++++---------------------------
drivers/video/fbdev/sa1100fb.h | 2 ++
2 files changed, 29 insertions(+), 48 deletions(-)
I queued all patches for 4.15, thanks!
Now that 4.15-rc1 is out, something tells me that was not actually
the case. Any ideas what happened?
Could you please explain the issue that you are seeing a bit more?
For me it looks all fine:
$ git log --oneline v4.15-rc1 drivers/video/fbdev/sa1100fb.c
cb6bc3f video: sa1100fb: move pseudo palette into sa1100fb_info structure
0ab7658 video: sa1100fb: fix video memory allocation leak
c244f8e video: sa1100fb: clean up failure path
5634cba video: sa1100fb: use devm_gpio_request_one()
f6fc8c9 video: sa1100fb: use devm_request_irq()
df6b228 video: sa1100fb: use devm_ioremap_resource()
e43064c video: sa1100fb: use devm_clk_get()
ba1d36b video: sa1100fb: use devm_kzalloc()
...
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
From: linux@armlinux.org.uk (Russell King - ARM Linux) Date: 2017-11-27 17:40:28
On Mon, Nov 27, 2017 at 06:30:11PM +0100, Bartlomiej Zolnierkiewicz wrote:
On Monday, November 27, 2017 05:11:45 PM Russell King - ARM Linux wrote:
quoted
On Tue, Oct 17, 2017 at 03:32:30PM +0200, Bartlomiej Zolnierkiewicz wrote:
quoted
On Friday, September 29, 2017 11:50:04 AM Russell King - ARM Linux wrote:
quoted
Hi Bart,
Hi Russell,
quoted
This series updates the sa1100fb fbdev driver's initialisation paths
to be more robust. In doing these updates, I realised that we omitted
to free some memory which was allocated for the framebuffer if
initialisation fails - something that was hidden due to the complex
cleanup that the driver performs. Switching to managed resources made
this more obvious.
Tested on H3600 iPAQ and Assabet.
drivers/video/fbdev/sa1100fb.c | 75 +++++++++++++++---------------------------
drivers/video/fbdev/sa1100fb.h | 2 ++
2 files changed, 29 insertions(+), 48 deletions(-)
I queued all patches for 4.15, thanks!
Now that 4.15-rc1 is out, something tells me that was not actually
the case. Any ideas what happened?
Could you please explain the issue that you are seeing a bit more?
For me it looks all fine:
$ git log --oneline v4.15-rc1 drivers/video/fbdev/sa1100fb.c
cb6bc3f video: sa1100fb: move pseudo palette into sa1100fb_info structure
0ab7658 video: sa1100fb: fix video memory allocation leak
c244f8e video: sa1100fb: clean up failure path
5634cba video: sa1100fb: use devm_gpio_request_one()
f6fc8c9 video: sa1100fb: use devm_request_irq()
df6b228 video: sa1100fb: use devm_ioremap_resource()
e43064c video: sa1100fb: use devm_clk_get()
ba1d36b video: sa1100fb: use devm_kzalloc()
...
Looks like I hadn't rebased that particular branch, so they were still
showing up in my origin.. git log. Sorry about the false warning.
This is the problem of carrying close to 400 patches in separate
branches, and git log origin.. not able to show the commits that the
selection are based upon.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up