Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Date:   Wed, 4 Oct 2023 12:46:49 +0300 (EEST)
From:   =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>
To:     srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
cc:     =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>,
        Hans de Goede <hdegoede@redhat.com>, markgross@kernel.org,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        platform-driver-x86@vger.kernel.org,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [UPDATE][PATCH] platform/x86/intel-uncore-freq: Conditionally
 create attribute for read frequency
In-Reply-To: <ab0ab04980b07e2893d9672b96311230ac981e40.camel@linux.intel.com>
Message-ID: <b7fd5b52-9d91-cfe-e011-522160947115@linux.intel.com>
References: <20231002131817.1590966-1-srinivas.pandruvada@linux.intel.com>  <6c59be5-1ff1-d0b1-5960-3789fe10c692@linux.intel.com> <ab0ab04980b07e2893d9672b96311230ac981e40.camel@linux.intel.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323329-667317560-1696412813=:1931"
Precedence: bulk

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323329-667317560-1696412813=:1931
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT

On Tue, 3 Oct 2023, srinivas pandruvada wrote:
> On Tue, 2023-10-03 at 16:10 +0300, Ilpo Järvinen wrote:
> > On Mon, 2 Oct 2023, Srinivas Pandruvada wrote:
> > 
> > > When the current uncore frequency can't be read, don't create
> > > attribute
> > > "current_freq_khz" as any read will fail later. Some user space
> > > applications like turbostat fail to continue with the failure. So,
> > > check
> > > error during attribute creation.
> > > 
> > > Fixes: 8a54e2253e4c ("platform/x86/intel-uncore-freq: Uncore
> > > frequency control via TPMI")
> > 
> > Hi,
> > 
> > Thanks for the update but that commit id looks bogus, or where the
> > value 
> > is used w/o error check?
> 
> commit 8a54e2253e4c25e5b61c9a9bee157bb52da5d432
> Author: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Date:   Thu Apr 20 15:05:14 2023 -0700
> 
>     platform/x86/intel-uncore-freq: Uncore frequency control via TPMI
> 
> 
> This is the commit exposed the issue. This is not the commit which
> changed the code in question. 
> 
> 
> I can add also
> Fixes: dbce412a7733 ("platform/x86/intel-uncore-freq: Split common and
> enumeration part")
> 
> But the change even before that as this commit just reorganized code
> but because of change of folders, that will look like correct commit.

I never thought dbce412a7733 is being fixed here, it's just a refactor 
moving code around like you say.

But how about 414eef27283a ("platform/x86/intel/uncore-freq: Display 
uncore current frequency") which actually adds the code line you're now 
fixing. What was broken before it? All I see is the one call in
show_perf_status_freq_khz() but that's checking for errors.

-- 
 i.

--8323329-667317560-1696412813=:1931--