From: Steve Twiss <stwiss.opensource@diasemi.com>
To: Eduardo Valentin <edubezval@gmail.com>
CC: LINUX-KERNEL <linux-kernel@vger.kernel.org>,
        LINUX-PM <linux-pm@vger.kernel.org>, Zhang Rui <rui.zhang@intel.com>,
        DEVICETREE <devicetree@vger.kernel.org>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Guenter Roeck <linux@roeck-us.net>,
        LINUX-INPUT <linux-input@vger.kernel.org>,
        LINUX-WATCHDOG <linux-watchdog@vger.kernel.org>,
        Lee Jones <lee.jones@linaro.org>,
        "Liam Girdwood" <lgirdwood@gmail.com>,
        Lukasz Luba <lukasz.luba@arm.com>, Mark Brown <broonie@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>, Rob Herring <robh+dt@kernel.org>,
        Support Opensource <Support.Opensource@diasemi.com>,
        Wim Van Sebroeck <wim@iguana.be>
Subject: RE: [RESEND PATCH V5 7/8] thermal: da9062/61: Thermal junction
 temperature monitoring driver
Thread-Topic: [RESEND PATCH V5 7/8] thermal: da9062/61: Thermal junction
 temperature monitoring driver
Thread-Index: AQHSfTUSlk5o+xhMLkaViGWDNqkcHKFvp8wAgCmlLVA=
Date: Mon, 27 Mar 2017 10:09:35 +0000
Message-ID: <6ED8E3B22081A4459DAC7699F3695FB7018CD69BD3@SW-EX-MBX02.diasemi.com>
References: <cover.1486026227.git.stwiss.opensource@diasemi.com>
 <6a1a651530fdaedd3e380aa33749ca5fce1a80a8.1486026227.git.stwiss.opensource@diasemi.com>
 <20170219014007.GA20845@localhost.localdomain>
In-Reply-To: <20170219014007.GA20845@localhost.localdomain>
Accept-Language: en-GB, en-US
Content-Language: en-US
x-kse-attachmentfiltering-interceptor-info: protection disabled
x-kse-serverinfo: sw-ex-cashub01.diasemi.com, 9
x-kse-antivirus-interceptor-info: scan successful
x-kse-antivirus-info: Clean, bases: 27/03/2017 08:32:00
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 4094
Lines: 141

On 19 February 2017 01:40, Eduardo Valentin wrote:

Hi Eduardo,

My apologies in taking so long to reply.
There were *no* problems with implementing your requests. See below.
I will have sent these changes as PATCH V6.

https://lkml.org/lkml/2017/3/27/253

Regards,
Steve

> To: Steve Twiss
> Subject: Re: [RESEND PATCH V5 7/8] thermal: da9062/61: Thermal junction
> temperature monitoring driver

[...]

> I see no reason why this driver cannot have the COMPILE_TEST flag.
> Tested myself here so:
> 
> +	depends on MFD_DA9062 || COMPILE_TEST

Added.

> please cleanup the minor issues checkpatch complains:
> /scripts/checkpatch.pl --strict <your patch>

I have fixed all of those for latest checkpatch.pl script, this time using "--strict".

[...]

> > +static void da9062_thermal_poll_on(struct work_struct *work)
> > +{
> > +	struct da9062_thermal *thermal = container_of(work,
> > +						struct da9062_thermal,
> > +						work.work);
> > +	unsigned int val;
> > +	int ret;
> > +
> > +	/* clear E_TEMP */
> > +	ret = regmap_write(thermal->hw->regmap,
> > +				DA9062AA_EVENT_B,
> > +				DA9062AA_E_TEMP_MASK);
> > +	if (ret < 0) {
> > +		dev_err(thermal->dev,
> > +			"Cannot clear the TJUNC temperature status\n");
> > +		goto err_enable_irq;
> > +	}
> > +
> > +	/* Now read E_TEMP again: it is acting like a status bit.
> > +	 * If over-temperature, then this status will be true.
> > +	 * If not over-temperature, this status will be false.
> > +	 */
> > +	ret = regmap_read(thermal->hw->regmap,
> > +			  DA9062AA_EVENT_B,
> > +			  &val);
> > +	if (ret < 0) {
> > +		dev_err(thermal->dev,
> > +			"Cannot check the TJUNC temperature status\n");
> > +		goto err_enable_irq;
> > +	} else {
> > +		if (val & DA9062AA_E_TEMP_MASK) {
> > +			mutex_lock(&thermal->lock);
> > +			thermal->temperature = DA9062_MILLI_CELSIUS(125);
> > +			mutex_unlock(&thermal->lock);
> > +			thermal_zone_device_update(thermal->zone,
> > +				THERMAL_EVENT_UNSPECIFIED);
> > +
> > +			schedule_delayed_work(&thermal->work,
> > +				msecs_to_jiffies(thermal->zone->passive_delay));
> > +			return;
> > +		} else {
> > +			mutex_lock(&thermal->lock);
> > +			thermal->temperature = DA9062_MILLI_CELSIUS(0);
> > +			mutex_unlock(&thermal->lock);
> > +			thermal_zone_device_update(thermal->zone,
> > +				THERMAL_EVENT_UNSPECIFIED);
> > +		}
> > +	}
> 
> The above code is a little confusing, can it be maybe, better read like
> this?
> 
> diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-
> thermal.c
> index 52a095d..6ac8574 100644
> --- a/drivers/thermal/da9062-thermal.c
> +++ b/drivers/thermal/da9062-thermal.c
> @@ -95,26 +95,26 @@ static void da9062_thermal_poll_on(struct work_struct
> *work)
>  		dev_err(thermal->dev,
>  			"Cannot check the TJUNC temperature status\n");
>  		goto err_enable_irq;
> -	} else {
> -		if (val & DA9062AA_E_TEMP_MASK) {
> -			mutex_lock(&thermal->lock);
> -			thermal->temperature = DA9062_MILLI_CELSIUS(125);
> -			mutex_unlock(&thermal->lock);
> -			thermal_zone_device_update(thermal->zone,
> -				THERMAL_EVENT_UNSPECIFIED);
> -
> -			schedule_delayed_work(&thermal->work,
> +	}
> +
> +	if (val & DA9062AA_E_TEMP_MASK) {
> +		mutex_lock(&thermal->lock);
> +		thermal->temperature = DA9062_MILLI_CELSIUS(125);
> +		mutex_unlock(&thermal->lock);
> +		thermal_zone_device_update(thermal->zone,
> +				THERMAL_EVENT_UNSPECIFIED);
> +
> +		schedule_delayed_work(&thermal->work,
>  				msecs_to_jiffies(thermal->zone->passive_delay));
> -			return;
> -		} else {
> -			mutex_lock(&thermal->lock);
> -			thermal->temperature = DA9062_MILLI_CELSIUS(0);
> -			mutex_unlock(&thermal->lock);
> -			thermal_zone_device_update(thermal->zone,
> -			THERMAL_EVENT_UNSPECIFIED);
> -		}
> +		return;
>  	}
> 
> +	mutex_lock(&thermal->lock);
> +	thermal->temperature = DA9062_MILLI_CELSIUS(0);
> +	mutex_unlock(&thermal->lock);
> +	thermal_zone_device_update(thermal->zone,
> +			THERMAL_EVENT_UNSPECIFIED);
> +
>  err_enable_irq:
>  	enable_irq(thermal->irq);
>  }

That makes more sense getting rid of those else clauses.
Applied that, thanks.

Regards,
Steve