diff mbox

ACPI / Thermal / video: fix max_level incorrect value

Message ID 20160521075500.GA22025@aaronlu.sh.intel.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Aaron Lu May 21, 2016, 7:55 a.m. UTC
On Sat, May 21, 2016 at 11:29:33AM +0800, Aaron Lu wrote:
> My bad, I see the problem here: the acpi_video_device_lcd_query_levels
> called in acpi_video_run_bcl_for_osi didn't do the proper conversion.
> The acpi_handle is a "void *" so there is no warnings...

I think I have found the problem, please give the patch a test, thanks.

From: Aaron Lu <aaron.lu@intel.com>
Date: Sat, 21 May 2016 15:30:46 +0800
Subject: [PATCH] ACPI / Thermal / video: fix max_level incorrect value

commit 059500940def("ACPI/video: export acpi_video_get_levels")
mistakenly dropped the correct value of max_level and that caused the
set_level function following failed and the acpi_video backlight interface
didn't get created. Fix this by passing back the correct max_level value.

While at it, also fix the param used in acpi_video_device_lcd_query_levels
where acpi_handle is expected but acpi_video_device is passed.

Reported-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 drivers/acpi/acpi_video.c                         | 9 ++++++---
 drivers/thermal/int340x_thermal/int3406_thermal.c | 2 +-
 include/acpi/video.h                              | 6 ++++--
 3 files changed, 11 insertions(+), 6 deletions(-)

Comments

Aaron Lu May 25, 2016, 5:15 a.m. UTC | #1
Valdis, can you please give the patch a try? Thanks.

-Aaron

On Sat, May 21, 2016 at 03:55:00PM +0800, Aaron Lu wrote:
> I think I have found the problem, please give the patch a test, thanks.
> 
> From: Aaron Lu <aaron.lu@intel.com>
> Date: Sat, 21 May 2016 15:30:46 +0800
> Subject: [PATCH] ACPI / Thermal / video: fix max_level incorrect value
> 
> commit 059500940def("ACPI/video: export acpi_video_get_levels")
> mistakenly dropped the correct value of max_level and that caused the
> set_level function following failed and the acpi_video backlight interface
> didn't get created. Fix this by passing back the correct max_level value.
> 
> While at it, also fix the param used in acpi_video_device_lcd_query_levels
> where acpi_handle is expected but acpi_video_device is passed.
> 
> Reported-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> ---
>  drivers/acpi/acpi_video.c                         | 9 ++++++---
>  drivers/thermal/int340x_thermal/int3406_thermal.c | 2 +-
>  include/acpi/video.h                              | 6 ++++--
>  3 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index 3d5b8a099351..c1d138e128cb 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -754,7 +754,8 @@ static int acpi_video_bqc_quirk(struct acpi_video_device *device,
>  }
>  
>  int acpi_video_get_levels(struct acpi_device *device,
> -			  struct acpi_video_device_brightness **dev_br)
> +			  struct acpi_video_device_brightness **dev_br,
> +			  int *pmax_level)
>  {
>  	union acpi_object *obj = NULL;
>  	int i, max_level = 0, count = 0, level_ac_battery = 0;
> @@ -841,6 +842,8 @@ int acpi_video_get_levels(struct acpi_device *device,
>  
>  	br->count = count;
>  	*dev_br = br;
> +	if (pmax_level)
> +		*pmax_level = max_level;
>  
>  out:
>  	kfree(obj);
> @@ -869,7 +872,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)
>  	struct acpi_video_device_brightness *br = NULL;
>  	int result = -EINVAL;
>  
> -	result = acpi_video_get_levels(device->dev, &br);
> +	result = acpi_video_get_levels(device->dev, &br, &max_level);
>  	if (result)
>  		return result;
>  	device->brightness = br;
> @@ -1737,7 +1740,7 @@ static void acpi_video_run_bcl_for_osi(struct acpi_video_bus *video)
>  
>  	mutex_lock(&video->device_list_lock);
>  	list_for_each_entry(dev, &video->video_device_list, entry) {
> -		if (!acpi_video_device_lcd_query_levels(dev, &levels))
> +		if (!acpi_video_device_lcd_query_levels(dev->dev->handle, &levels))
>  			kfree(levels);
>  	}
>  	mutex_unlock(&video->device_list_lock);
> diff --git a/drivers/thermal/int340x_thermal/int3406_thermal.c b/drivers/thermal/int340x_thermal/int3406_thermal.c
> index 13d431cbd29e..a578cd257db4 100644
> --- a/drivers/thermal/int340x_thermal/int3406_thermal.c
> +++ b/drivers/thermal/int340x_thermal/int3406_thermal.c
> @@ -177,7 +177,7 @@ static int int3406_thermal_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	d->raw_bd = bd;
>  
> -	ret = acpi_video_get_levels(ACPI_COMPANION(&pdev->dev), &d->br);
> +	ret = acpi_video_get_levels(ACPI_COMPANION(&pdev->dev), &d->br, NULL);
>  	if (ret)
>  		return ret;
>  
> diff --git a/include/acpi/video.h b/include/acpi/video.h
> index 70a41f742037..5731ccb42585 100644
> --- a/include/acpi/video.h
> +++ b/include/acpi/video.h
> @@ -51,7 +51,8 @@ extern void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type);
>   */
>  extern bool acpi_video_handles_brightness_key_presses(void);
>  extern int acpi_video_get_levels(struct acpi_device *device,
> -				 struct acpi_video_device_brightness **dev_br);
> +				 struct acpi_video_device_brightness **dev_br,
> +				 int *pmax_level);
>  #else
>  static inline int acpi_video_register(void) { return 0; }
>  static inline void acpi_video_unregister(void) { return; }
> @@ -72,7 +73,8 @@ static inline bool acpi_video_handles_brightness_key_presses(void)
>  	return false;
>  }
>  static inline int acpi_video_get_levels(struct acpi_device *device,
> -			struct acpi_video_device_brightness **dev_br)
> +			struct acpi_video_device_brightness **dev_br,
> +			int *pmax_level)
>  {
>  	return -ENODEV;
>  }
> -- 
> 2.5.5
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Valdis Klētnieks May 26, 2016, 1:49 a.m. UTC | #2
On Wed, 25 May 2016 13:15:26 +0800, Aaron Lu said:
> Valdis, can you please give the patch a try? Thanks.

Sorry, had a few days where actual work commitments and other
things got in the way... I tested this patch against next-20160524,
and can report that the problem is fixed, so feel free to
stick a "Tested-By:" on it....

> > Reported-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
> > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> > ---
> >  drivers/acpi/acpi_video.c                         | 9 ++++++---
> >  drivers/thermal/int340x_thermal/int3406_thermal.c | 2 +-
> >  include/acpi/video.h                              | 6 ++++--
> >  3 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> > index 3d5b8a099351..c1d138e128cb 100644
> > --- a/drivers/acpi/acpi_video.c
> > +++ b/drivers/acpi/acpi_video.c
diff mbox

Patch

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 3d5b8a099351..c1d138e128cb 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -754,7 +754,8 @@  static int acpi_video_bqc_quirk(struct acpi_video_device *device,
 }
 
 int acpi_video_get_levels(struct acpi_device *device,
-			  struct acpi_video_device_brightness **dev_br)
+			  struct acpi_video_device_brightness **dev_br,
+			  int *pmax_level)
 {
 	union acpi_object *obj = NULL;
 	int i, max_level = 0, count = 0, level_ac_battery = 0;
@@ -841,6 +842,8 @@  int acpi_video_get_levels(struct acpi_device *device,
 
 	br->count = count;
 	*dev_br = br;
+	if (pmax_level)
+		*pmax_level = max_level;
 
 out:
 	kfree(obj);
@@ -869,7 +872,7 @@  acpi_video_init_brightness(struct acpi_video_device *device)
 	struct acpi_video_device_brightness *br = NULL;
 	int result = -EINVAL;
 
-	result = acpi_video_get_levels(device->dev, &br);
+	result = acpi_video_get_levels(device->dev, &br, &max_level);
 	if (result)
 		return result;
 	device->brightness = br;
@@ -1737,7 +1740,7 @@  static void acpi_video_run_bcl_for_osi(struct acpi_video_bus *video)
 
 	mutex_lock(&video->device_list_lock);
 	list_for_each_entry(dev, &video->video_device_list, entry) {
-		if (!acpi_video_device_lcd_query_levels(dev, &levels))
+		if (!acpi_video_device_lcd_query_levels(dev->dev->handle, &levels))
 			kfree(levels);
 	}
 	mutex_unlock(&video->device_list_lock);
diff --git a/drivers/thermal/int340x_thermal/int3406_thermal.c b/drivers/thermal/int340x_thermal/int3406_thermal.c
index 13d431cbd29e..a578cd257db4 100644
--- a/drivers/thermal/int340x_thermal/int3406_thermal.c
+++ b/drivers/thermal/int340x_thermal/int3406_thermal.c
@@ -177,7 +177,7 @@  static int int3406_thermal_probe(struct platform_device *pdev)
 		return -ENODEV;
 	d->raw_bd = bd;
 
-	ret = acpi_video_get_levels(ACPI_COMPANION(&pdev->dev), &d->br);
+	ret = acpi_video_get_levels(ACPI_COMPANION(&pdev->dev), &d->br, NULL);
 	if (ret)
 		return ret;
 
diff --git a/include/acpi/video.h b/include/acpi/video.h
index 70a41f742037..5731ccb42585 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -51,7 +51,8 @@  extern void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type);
  */
 extern bool acpi_video_handles_brightness_key_presses(void);
 extern int acpi_video_get_levels(struct acpi_device *device,
-				 struct acpi_video_device_brightness **dev_br);
+				 struct acpi_video_device_brightness **dev_br,
+				 int *pmax_level);
 #else
 static inline int acpi_video_register(void) { return 0; }
 static inline void acpi_video_unregister(void) { return; }
@@ -72,7 +73,8 @@  static inline bool acpi_video_handles_brightness_key_presses(void)
 	return false;
 }
 static inline int acpi_video_get_levels(struct acpi_device *device,
-			struct acpi_video_device_brightness **dev_br)
+			struct acpi_video_device_brightness **dev_br,
+			int *pmax_level)
 {
 	return -ENODEV;
 }