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