From: Johan Hovold <hidden> Date: 2013-10-22 17:27:17
The driver supports 16-bit brightness values, but the value returned
from get_brightness was truncated to eight bits.
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <redacted>
---
drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Johan Hovold <hidden> Date: 2013-10-22 17:27:20
Add missing module alias which is needed for module autoloading.
Signed-off-by: Johan Hovold <redacted>
---
drivers/video/backlight/atmel-pwm-bl.c | 1 +
1 file changed, 1 insertion(+)
From: Johan Hovold <hidden> Date: 2013-10-22 17:27:24
Clean up probe error handling by checking parameters before any
allocations and removing an obsolete error label. Also remove
unnecessary reset of private gpio number.
Signed-off-by: Johan Hovold <redacted>
---
drivers/video/backlight/atmel-pwm-bl.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
@@ -126,40 +126,33 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)structatmel_pwm_bl*pwmbl;intretval;+pdata=dev_get_platdata(&pdev->dev);+if(!pdata)+return-ENODEV;++if(pdata->pwm_compare_max<pdata->pwm_duty_max||+pdata->pwm_duty_min>pdata->pwm_duty_max||+pdata->pwm_frequency=0)+return-EINVAL;+pwmbl=devm_kzalloc(&pdev->dev,sizeof(structatmel_pwm_bl),GFP_KERNEL);if(!pwmbl)return-ENOMEM;pwmbl->pdev=pdev;--pdata=dev_get_platdata(&pdev->dev);-if(!pdata){-retval=-ENODEV;-gotoerr_free_mem;-}--if(pdata->pwm_compare_max<pdata->pwm_duty_max||-pdata->pwm_duty_min>pdata->pwm_duty_max||-pdata->pwm_frequency=0){-retval=-EINVAL;-gotoerr_free_mem;-}-pwmbl->pdata=pdata;pwmbl->gpio_on=pdata->gpio_on;retval=pwm_channel_alloc(pdata->pwm_channel,&pwmbl->pwmc);if(retval)-gotoerr_free_mem;+returnretval;if(pwmbl->gpio_on!=-1){retval=devm_gpio_request(&pdev->dev,pwmbl->gpio_on,"gpio_atmel_pwm_bl");-if(retval){-pwmbl->gpio_on=-1;+if(retval)gotoerr_free_pwm;-}/* Turn display off by default. */retval=gpio_direction_output(pwmbl->gpio_on,
@@ -197,7 +190,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)err_free_pwm:pwm_channel_free(&pwmbl->pwmc);-err_free_mem:+returnretval;}
From: Johan Hovold <hidden> Date: 2013-10-22 17:27:25
Use gpio_is_valid rather than open coding the more restrictive != -1
test.
Signed-off-by: Johan Hovold <redacted>
---
drivers/video/backlight/atmel-pwm-bl.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
From: Johan Hovold <hidden> Date: 2013-10-22 17:28:39
Use devm_gpio_request_one rather than requesting and setting direction
in two calls.
Signed-off-by: Johan Hovold <redacted>
---
drivers/video/backlight/atmel-pwm-bl.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
@@ -124,6 +124,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)conststructatmel_pwm_bl_platform_data*pdata;structbacklight_device*bldev;structatmel_pwm_bl*pwmbl;+intflags;intretval;pdata=dev_get_platdata(&pdev->dev);
@@ -149,14 +150,14 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)returnretval;if(gpio_is_valid(pwmbl->gpio_on)){-retval=devm_gpio_request(&pdev->dev,pwmbl->gpio_on,-"gpio_atmel_pwm_bl");-if(retval)-gotoerr_free_pwm;-/* Turn display off by default. */-retval=gpio_direction_output(pwmbl->gpio_on,-0^pdata->on_active_low);+if(pdata->on_active_low)+flags=GPIOF_OUT_INIT_HIGH;+else+flags=GPIOF_OUT_INIT_LOW;++retval=devm_gpio_request_one(&pdev->dev,pwmbl->gpio_on,+flags,"gpio_atmel_pwm_bl");if(retval)gotoerr_free_pwm;}
From: Johan Hovold <hidden> Date: 2013-10-22 17:28:55
Add helper function to control the gpio_on signal.
Signed-off-by: Johan Hovold <redacted>
---
drivers/video/backlight/atmel-pwm-bl.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
From: Johan Hovold <hidden> Date: 2013-10-22 17:28:57
Make sure to honour gpio polarity also at remove so that the backlight
is actually disabled on boards with active-low enable pin.
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <redacted>
---
drivers/video/backlight/atmel-pwm-bl.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
From: Jingoo Han <hidden> Date: 2013-10-23 01:19:46
On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
These patches fix a few issues and clean up the atmel-pwm-bl driver
somewhat.
Johan
Johan Hovold (9):
backlight: atmel-pwm-bl: fix reported brightness
backlight: atmel-pwm-bl: fix gpio polarity in remove
backlight: atmel-pwm-bl: fix module autoload
backlight: atmel-pwm-bl: clean up probe error handling
backlight: atmel-pwm-bl: clean up get_intensity
backlight: atmel-pwm-bl: remove unused include
backlight: atmel-pwm-bl: use gpio_is_valid
backlight: atmel-pwm-bl: refactor gpio_on handling
backlight: atmel-pwm-bl: use gpio_request_one
++cc Andrew Morton, Tomi Valkeinen, Jean-Christophe Plagniol-Villard
Hi Johan Hovold,
Currently, because there is no git tree for backlight,
backlight patches have been merged to mm-tree by Andrew Morton.
Please, add Andrew Morton to CC list.
Also, there is another way.
If Nicolas Ferre wants to merge these patches, the patches can be
merged through ATMEL-SoC tree with my Acked-by.
Best regards,
Jingoo Han
From: Jingoo Han <hidden> Date: 2013-10-23 01:21:15
On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
quoted hunk
The driver supports 16-bit brightness values, but the value returned
from get_brightness was truncated to eight bits.
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <redacted>
---
drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Jingoo Han <hidden> Date: 2013-10-23 01:48:03
On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
Make sure to honour gpio polarity also at remove so that the backlight
is actually disabled on boards with active-low enable pin.
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <redacted>
Acked-by: Jingoo Han <redacted>
Best regards,
Jingoo Han
From: Jingoo Han <hidden> Date: 2013-10-23 01:50:19
On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
Clean up probe error handling by checking parameters before any
allocations and removing an obsolete error label. Also remove
unnecessary reset of private gpio number.
Signed-off-by: Johan Hovold <redacted>
Acked-by: Jingoo Han <redacted>
Best regards,
Jingoo Han
From: Nicolas Ferre <hidden> Date: 2013-10-23 07:57:57
On 23/10/2013 02:19, Jingoo Han :
On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
quoted
These patches fix a few issues and clean up the atmel-pwm-bl driver
somewhat.
Johan
Johan Hovold (9):
backlight: atmel-pwm-bl: fix reported brightness
backlight: atmel-pwm-bl: fix gpio polarity in remove
backlight: atmel-pwm-bl: fix module autoload
backlight: atmel-pwm-bl: clean up probe error handling
backlight: atmel-pwm-bl: clean up get_intensity
backlight: atmel-pwm-bl: remove unused include
backlight: atmel-pwm-bl: use gpio_is_valid
backlight: atmel-pwm-bl: refactor gpio_on handling
backlight: atmel-pwm-bl: use gpio_request_one
++cc Andrew Morton, Tomi Valkeinen, Jean-Christophe Plagniol-Villard
Hi Johan Hovold,
Currently, because there is no git tree for backlight,
backlight patches have been merged to mm-tree by Andrew Morton.
Please, add Andrew Morton to CC list.
Also, there is another way.
If Nicolas Ferre wants to merge these patches, the patches can be
merged through ATMEL-SoC tree with my Acked-by.
Hi,
As it's a driver without interaction with AT91 code, maybe routing this
patch series through mm-tree is the way to go.
If you find any issue in the process, please tell me. I would be happy
to ease the process.
Bye,
From: Johan Hovold <hidden> Date: 2013-10-23 08:51:39
On Wed, Oct 23, 2013 at 10:20:59AM +0900, Jingoo Han wrote:
On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
quoted
The driver supports 16-bit brightness values, but the value returned
from get_brightness was truncated to eight bits.
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <redacted>
---
drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
@@ -70,7 +70,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)staticintatmel_pwm_bl_get_intensity(structbacklight_device*bd){structatmel_pwm_bl*pwmbl=bl_get_data(bd);-u8intensity;+u32intensity;if(pwmbl->pdata->pwm_active_low){intensity=pwm_channel_readl(&pwmbl->pwmc,PWM_CDTY)-
@@ -80,7 +80,7 @@ static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)pwm_channel_readl(&pwmbl->pwmc,PWM_CDTY);}-returnintensity;+return(u16)intensity;
However, atmel_pwm_bl_get_intensity() should return 'int',
instead of 'u16'.
Yes, but the cast to int is implicit. Perhaps
return (intensity & 0xffff);
(or just a comment) would make it more clear why the cast is there.
Also, pwm_channel_readl() returns 'u32'.
Yes, (and only the 16 least-significant bits are used). That and the
fact that the platform-data limits are currently unsigned long (I was
considering fixing this later) was why I preferred keeping all register
value manipulation in u32 and do a single cast at the end.
However, if you feel strongly about this, I'll respin the series (a later
patch would likely no longer apply), use u16 and add casts to the two
assignments.
Thanks,
Johan
From: Jingoo Han <hidden> Date: 2013-10-23 09:35:48
On Wednesday, October 23, 2013 5:51 PM, Johan Hovold wrote:
On Wed, Oct 23, 2013 at 10:20:59AM +0900, Jingoo Han wrote:
quoted
On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
quoted
The driver supports 16-bit brightness values, but the value returned
from get_brightness was truncated to eight bits.
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <redacted>
---
drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
@@ -70,7 +70,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)staticintatmel_pwm_bl_get_intensity(structbacklight_device*bd){structatmel_pwm_bl*pwmbl=bl_get_data(bd);-u8intensity;+u32intensity;if(pwmbl->pdata->pwm_active_low){intensity=pwm_channel_readl(&pwmbl->pwmc,PWM_CDTY)-
@@ -80,7 +80,7 @@ static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)pwm_channel_readl(&pwmbl->pwmc,PWM_CDTY);}-returnintensity;+return(u16)intensity;
However, atmel_pwm_bl_get_intensity() should return 'int',
instead of 'u16'.
Yes, but the cast to int is implicit. Perhaps
return (intensity & 0xffff);
(or just a comment) would make it more clear why the cast is there.
quoted
Also, pwm_channel_readl() returns 'u32'.
Yes, (and only the 16 least-significant bits are used). That and the
fact that the platform-data limits are currently unsigned long (I was
considering fixing this later) was why I preferred keeping all register
value manipulation in u32 and do a single cast at the end.
However, if you feel strongly about this, I'll respin the series (a later
patch would likely no longer apply), use u16 and add casts to the two
assignments.
Thank you for your kind and detailed description. :-)
OK, I have no objection on your original patch.
Would you re-send these patches with my Acked-by with CC'ing Andrew Morton,
Tomi Valkeinen? Then, these patches can be merged through mm-tree.
Best regards,
Jingoo Han