diff mbox

[05/13] input: convert sun4i-ts to use devm_thermal_zone_of_sensor_register

Message ID 1457559336-17652-6-git-send-email-edubezval@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eduardo Valentin March 9, 2016, 9:35 p.m. UTC
This changes the driver to use the devm_ version
of thermal_zone_of_sensor_register and cleans
up the  local points and unregister calls.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Jens Thiele <karme@karme.de>
Cc: linux-input@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
---
 drivers/input/touchscreen/sun4i-ts.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

Comments

Dmitry Torokhov March 9, 2016, 9:45 p.m. UTC | #1
On Wed, Mar 09, 2016 at 01:35:27PM -0800, Eduardo Valentin wrote:
> This changes the driver to use the devm_ version
> of thermal_zone_of_sensor_register and cleans
> up the  local points and unregister calls.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Jens Thiele <karme@karme.de>
> Cc: linux-input@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Eduardo Valentin <edubezval@gmail.com>

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

I guess it will make sense to merge through your tree unless you want to
hold off until after I merge winth mainline (around -rc3) assuming that
it even makes into 4.6.

Thanks.

> ---
>  drivers/input/touchscreen/sun4i-ts.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c
> index 4857943..d07dd29 100644
> --- a/drivers/input/touchscreen/sun4i-ts.c
> +++ b/drivers/input/touchscreen/sun4i-ts.c
> @@ -115,7 +115,6 @@
>  struct sun4i_ts_data {
>  	struct device *dev;
>  	struct input_dev *input;
> -	struct thermal_zone_device *tz;
>  	void __iomem *base;
>  	unsigned int irq;
>  	bool ignore_fifo_data;
> @@ -366,10 +365,7 @@ static int sun4i_ts_probe(struct platform_device *pdev)
>  	if (IS_ERR(hwmon))
>  		return PTR_ERR(hwmon);
>  
> -	ts->tz = thermal_zone_of_sensor_register(ts->dev, 0, ts,
> -						 &sun4i_ts_tz_ops);
> -	if (IS_ERR(ts->tz))
> -		ts->tz = NULL;
> +	devm_thermal_zone_of_sensor_register(ts->dev, 0, ts, &sun4i_ts_tz_ops);
>  
>  	writel(TEMP_IRQ_EN(1), ts->base + TP_INT_FIFOC);
>  
> @@ -377,7 +373,6 @@ static int sun4i_ts_probe(struct platform_device *pdev)
>  		error = input_register_device(ts->input);
>  		if (error) {
>  			writel(0, ts->base + TP_INT_FIFOC);
> -			thermal_zone_of_sensor_unregister(ts->dev, ts->tz);
>  			return error;
>  		}
>  	}
> @@ -394,8 +389,6 @@ static int sun4i_ts_remove(struct platform_device *pdev)
>  	if (ts->input)
>  		input_unregister_device(ts->input);
>  
> -	thermal_zone_of_sensor_unregister(ts->dev, ts->tz);
> -
>  	/* Deactivate all IRQs */
>  	writel(0, ts->base + TP_INT_FIFOC);
>  
> -- 
> 2.1.4
>
Sascha Hauer March 10, 2016, 9:34 a.m. UTC | #2
On Wed, Mar 09, 2016 at 01:35:27PM -0800, Eduardo Valentin wrote:
> This changes the driver to use the devm_ version
> of thermal_zone_of_sensor_register and cleans
> up the  local points and unregister calls.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Jens Thiele <karme@karme.de>
> Cc: linux-input@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
> ---
>  drivers/input/touchscreen/sun4i-ts.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c
> index 4857943..d07dd29 100644
> --- a/drivers/input/touchscreen/sun4i-ts.c
> +++ b/drivers/input/touchscreen/sun4i-ts.c
> @@ -115,7 +115,6 @@
>  struct sun4i_ts_data {
>  	struct device *dev;
>  	struct input_dev *input;
> -	struct thermal_zone_device *tz;
>  	void __iomem *base;
>  	unsigned int irq;
>  	bool ignore_fifo_data;
> @@ -366,10 +365,7 @@ static int sun4i_ts_probe(struct platform_device *pdev)
>  	if (IS_ERR(hwmon))
>  		return PTR_ERR(hwmon);
>  
> -	ts->tz = thermal_zone_of_sensor_register(ts->dev, 0, ts,
> -						 &sun4i_ts_tz_ops);
> -	if (IS_ERR(ts->tz))
> -		ts->tz = NULL;
> +	devm_thermal_zone_of_sensor_register(ts->dev, 0, ts, &sun4i_ts_tz_ops);

Shouldn't we check the return value? There are a few possibilities for
thermal_zone_of_sensor_register to fail.

Sascha
Hans de Goede March 10, 2016, 1:19 p.m. UTC | #3
Hi,

On 10-03-16 10:34, Sascha Hauer wrote:
> On Wed, Mar 09, 2016 at 01:35:27PM -0800, Eduardo Valentin wrote:
>> This changes the driver to use the devm_ version
>> of thermal_zone_of_sensor_register and cleans
>> up the  local points and unregister calls.
>>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
>> Cc: Chen-Yu Tsai <wens@csie.org>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Cc: Zhang Rui <rui.zhang@intel.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> Cc: Heiko Stuebner <heiko@sntech.de>
>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>> Cc: Jens Thiele <karme@karme.de>
>> Cc: linux-input@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
>> ---
>>   drivers/input/touchscreen/sun4i-ts.c | 9 +--------
>>   1 file changed, 1 insertion(+), 8 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c
>> index 4857943..d07dd29 100644
>> --- a/drivers/input/touchscreen/sun4i-ts.c
>> +++ b/drivers/input/touchscreen/sun4i-ts.c
>> @@ -115,7 +115,6 @@
>>   struct sun4i_ts_data {
>>   	struct device *dev;
>>   	struct input_dev *input;
>> -	struct thermal_zone_device *tz;
>>   	void __iomem *base;
>>   	unsigned int irq;
>>   	bool ignore_fifo_data;
>> @@ -366,10 +365,7 @@ static int sun4i_ts_probe(struct platform_device *pdev)
>>   	if (IS_ERR(hwmon))
>>   		return PTR_ERR(hwmon);
>>
>> -	ts->tz = thermal_zone_of_sensor_register(ts->dev, 0, ts,
>> -						 &sun4i_ts_tz_ops);
>> -	if (IS_ERR(ts->tz))
>> -		ts->tz = NULL;
>> +	devm_thermal_zone_of_sensor_register(ts->dev, 0, ts, &sun4i_ts_tz_ops);
>
> Shouldn't we check the return value? There are a few possibilities for
> thermal_zone_of_sensor_register to fail.

Note thee old code also was not checking this, it was simply continuing
without having registered a tz-sensor.

I guess we could log an error in that case, but that should be done in a
seperate follow-up patch.

The current patch looks good to me:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans
Eduardo Valentin March 14, 2016, 9:07 p.m. UTC | #4
On Wed, Mar 09, 2016 at 01:45:14PM -0800, Dmitry Torokhov wrote:
> On Wed, Mar 09, 2016 at 01:35:27PM -0800, Eduardo Valentin wrote:
> > This changes the driver to use the devm_ version
> > of thermal_zone_of_sensor_register and cleans
> > up the  local points and unregister calls.
> > 
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> > Cc: Chen-Yu Tsai <wens@csie.org>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Lukasz Majewski <l.majewski@samsung.com>
> > Cc: Heiko Stuebner <heiko@sntech.de>
> > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Jens Thiele <karme@karme.de>
> > Cc: linux-input@vger.kernel.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
> 
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> I guess it will make sense to merge through your tree unless you want to
> hold off until after I merge winth mainline (around -rc3) assuming that
> it even makes into 4.6.

No rush from my side. I would rather get the right review cycles.

The devm_ APIs were sent to 4.6-rc1 (well, right now in Rui's tree, but
I assume he is sending them).

Thanks for your review.
diff mbox

Patch

diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c
index 4857943..d07dd29 100644
--- a/drivers/input/touchscreen/sun4i-ts.c
+++ b/drivers/input/touchscreen/sun4i-ts.c
@@ -115,7 +115,6 @@ 
 struct sun4i_ts_data {
 	struct device *dev;
 	struct input_dev *input;
-	struct thermal_zone_device *tz;
 	void __iomem *base;
 	unsigned int irq;
 	bool ignore_fifo_data;
@@ -366,10 +365,7 @@  static int sun4i_ts_probe(struct platform_device *pdev)
 	if (IS_ERR(hwmon))
 		return PTR_ERR(hwmon);
 
-	ts->tz = thermal_zone_of_sensor_register(ts->dev, 0, ts,
-						 &sun4i_ts_tz_ops);
-	if (IS_ERR(ts->tz))
-		ts->tz = NULL;
+	devm_thermal_zone_of_sensor_register(ts->dev, 0, ts, &sun4i_ts_tz_ops);
 
 	writel(TEMP_IRQ_EN(1), ts->base + TP_INT_FIFOC);
 
@@ -377,7 +373,6 @@  static int sun4i_ts_probe(struct platform_device *pdev)
 		error = input_register_device(ts->input);
 		if (error) {
 			writel(0, ts->base + TP_INT_FIFOC);
-			thermal_zone_of_sensor_unregister(ts->dev, ts->tz);
 			return error;
 		}
 	}
@@ -394,8 +389,6 @@  static int sun4i_ts_remove(struct platform_device *pdev)
 	if (ts->input)
 		input_unregister_device(ts->input);
 
-	thermal_zone_of_sensor_unregister(ts->dev, ts->tz);
-
 	/* Deactivate all IRQs */
 	writel(0, ts->base + TP_INT_FIFOC);