All of lore.kernel.org
 help / color / mirror / Atom feed
From: Colin Ian King <colin.king@canonical.com>
To: "Hans de Goede <hdegoede"@redhat.com,
	Mark Gross <mgross@linux.intel.com>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: platform-driver-x86@vger.kernel.org,
	intel-gfx@lists.freedesktop.org,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Computation of return value being discarded in get_cpu_power() in drivers/platform/x86/intel_ips.c
Date: Wed, 9 Jun 2021 12:50:07 +0100	[thread overview]
Message-ID: <548dd463-3942-00a1-85c3-232897dea1a3@canonical.com> (raw)

Hi,

I was reviewing some old unassigned variable warnings from static
analysis by Coverity and found an issue introduced with the following
commit:

commit aa7ffc01d254c91a36bf854d57a14049c6134c72
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Fri May 14 15:41:14 2010 -0700

    x86 platform driver: intelligent power sharing driver

The analysis is as follows:

drivers/platform/x86/intel_ips.c

 871 static u32 get_cpu_power(struct ips_driver *ips, u32 *last, int period)
 872 {
 873        u32 val;
 874        u32 ret;
 875
 876        /*
 877         * CEC is in joules/65535.  Take difference over time to
 878         * get watts.
 879         */
 880        val = thm_readl(THM_CEC);
 881
 882        /* period is in ms and we want mW */
 883        ret = (((val - *last) * 1000) / period);

Unused value (UNUSED_VALUE)
assigned_value:  Assigning value from ret * 1000U / 65535U to ret here,
but that stored value is not used.

 884        ret = (ret * 1000) / 65535;
 885        *last = val;
 886
 887        return 0;
 888 }

I'm really not sure why ret is being calculated on lines 883,884 and not
being used. Should that be *last = ret on line 885? Looks suspect anyhow.

Colin



WARNING: multiple messages have this Message-ID (diff)
From: Colin Ian King <colin.king@canonical.com>
To: "Hans de Goede <hdegoede"@redhat.com,
	Mark Gross <mgross@linux.intel.com>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	platform-driver-x86@vger.kernel.org
Subject: Computation of return value being discarded in get_cpu_power() in drivers/platform/x86/intel_ips.c
Date: Wed, 9 Jun 2021 12:50:07 +0100	[thread overview]
Message-ID: <548dd463-3942-00a1-85c3-232897dea1a3@canonical.com> (raw)

Hi,

I was reviewing some old unassigned variable warnings from static
analysis by Coverity and found an issue introduced with the following
commit:

commit aa7ffc01d254c91a36bf854d57a14049c6134c72
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Fri May 14 15:41:14 2010 -0700

    x86 platform driver: intelligent power sharing driver

The analysis is as follows:

drivers/platform/x86/intel_ips.c

 871 static u32 get_cpu_power(struct ips_driver *ips, u32 *last, int period)
 872 {
 873        u32 val;
 874        u32 ret;
 875
 876        /*
 877         * CEC is in joules/65535.  Take difference over time to
 878         * get watts.
 879         */
 880        val = thm_readl(THM_CEC);
 881
 882        /* period is in ms and we want mW */
 883        ret = (((val - *last) * 1000) / period);

Unused value (UNUSED_VALUE)
assigned_value:  Assigning value from ret * 1000U / 65535U to ret here,
but that stored value is not used.

 884        ret = (ret * 1000) / 65535;
 885        *last = val;
 886
 887        return 0;
 888 }

I'm really not sure why ret is being calculated on lines 883,884 and not
being used. Should that be *last = ret on line 885? Looks suspect anyhow.

Colin



WARNING: multiple messages have this Message-ID (diff)
From: Colin Ian King <colin.king@canonical.com>
To: "Hans de Goede <hdegoede"@redhat.com,
	Mark Gross <mgross@linux.intel.com>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	platform-driver-x86@vger.kernel.org
Subject: [Intel-gfx] Computation of return value being discarded in get_cpu_power() in drivers/platform/x86/intel_ips.c
Date: Wed, 9 Jun 2021 12:50:07 +0100	[thread overview]
Message-ID: <548dd463-3942-00a1-85c3-232897dea1a3@canonical.com> (raw)

Hi,

I was reviewing some old unassigned variable warnings from static
analysis by Coverity and found an issue introduced with the following
commit:

commit aa7ffc01d254c91a36bf854d57a14049c6134c72
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Fri May 14 15:41:14 2010 -0700

    x86 platform driver: intelligent power sharing driver

The analysis is as follows:

drivers/platform/x86/intel_ips.c

 871 static u32 get_cpu_power(struct ips_driver *ips, u32 *last, int period)
 872 {
 873        u32 val;
 874        u32 ret;
 875
 876        /*
 877         * CEC is in joules/65535.  Take difference over time to
 878         * get watts.
 879         */
 880        val = thm_readl(THM_CEC);
 881
 882        /* period is in ms and we want mW */
 883        ret = (((val - *last) * 1000) / period);

Unused value (UNUSED_VALUE)
assigned_value:  Assigning value from ret * 1000U / 65535U to ret here,
but that stored value is not used.

 884        ret = (ret * 1000) / 65535;
 885        *last = val;
 886
 887        return 0;
 888 }

I'm really not sure why ret is being calculated on lines 883,884 and not
being used. Should that be *last = ret on line 885? Looks suspect anyhow.

Colin


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

             reply	other threads:[~2021-06-09 11:50 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-09 11:50 Colin Ian King [this message]
2021-06-09 11:50 ` [Intel-gfx] Computation of return value being discarded in get_cpu_power() in drivers/platform/x86/intel_ips.c Colin Ian King
2021-06-09 11:50 ` Colin Ian King
2021-06-10 11:51 ` joonas.lahtinen
2021-06-10 11:51   ` [Intel-gfx] " joonas.lahtinen
2021-06-10 11:51   ` joonas.lahtinen
2021-06-10 11:55 ` Joonas Lahtinen
2021-06-10 11:55   ` [Intel-gfx] " Joonas Lahtinen
2021-06-10 11:55   ` Joonas Lahtinen
2021-06-10 15:40   ` Jesse Barnes
2021-06-10 15:40     ` [Intel-gfx] " Jesse Barnes
2021-06-10 15:43   ` Jesse Barnes
2021-06-10 15:43     ` [Intel-gfx] " Jesse Barnes
2021-06-10 15:43     ` Jesse Barnes
2021-06-10 15:55   ` Hans de Goede
2021-06-10 15:55     ` [Intel-gfx] " Hans de Goede
2021-06-10 15:55     ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=548dd463-3942-00a1-85c3-232897dea1a3@canonical.com \
    --to=colin.king@canonical.com \
    --cc="Hans de Goede <hdegoede"@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgross@linux.intel.com \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.