From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: "David E. Box" <david.e.box@linux.intel.com>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
LKML <linux-kernel@vger.kernel.org>,
platform-driver-x86@vger.kernel.org,
rajvi.jingar@linux.intel.com
Subject: Re: [PATCH V3 09/16] platform/x86/intel/pmc: Allow pmc_core_ssram_init to fail
Date: Fri, 13 Oct 2023 14:36:19 +0300 (EEST) [thread overview]
Message-ID: <92594f48-fa9-59d4-e2b7-f5f83ded0ea@linux.intel.com> (raw)
In-Reply-To: <7f2fd7a054912960c6599e4a62e2095d1567aab8.camel@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 1336 bytes --]
On Thu, 12 Oct 2023, David E. Box wrote:
> On Thu, 2023-10-12 at 18:01 +0300, Ilpo Järvinen wrote:
> > On Wed, 11 Oct 2023, David E. Box wrote:
> >
> > > Currently, if the PMC SSRAM initialization fails, no error is returned and
> > > the only indication is that a PMC device has not been created. Instead,
> > > allow an error to be returned and handled directly by the caller.
> >
> > You might have a good reason for it but why isn't the call into
> > pmc_core_pmc_add() changed in this patch to take the error value into
> > account?
>
> Good catch. The return value of pmc_core_pmc_add() is first used in the next
> patch but should be used here.
>
> >
> > (I vaguely remember this was probably discussed in the context of some
> > earlier patch touching this area that it was about the other code dealing
> > with NULLs or something like that).
Okay but please also take into consideration what I tried to imply above:
Since you are doing what looks a major logic change in the next patch, it
might be okay to _not use_ that return value until then if you e.g., need
to add lots of rollback that isn't there already to the code that is going
away anyway in the next patch. (After all, returning an error code from a
function that was void previously isn't going to magically break the old
calling code).
--
i.
next prev parent reply other threads:[~2023-10-13 11:36 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-12 2:38 [PATCH V3 00/16] intel_pmc: Add telemetry API to read counters David E. Box
2023-10-12 2:38 ` [PATCH V3 01/16] platform/x86/intel/vsec: Move structures to header David E. Box
2023-10-12 2:38 ` [PATCH V3 02/16] platform/x86/intel/vsec: remove platform_info from vsec device structure David E. Box
2023-10-12 15:31 ` Ilpo Järvinen
2023-10-12 16:55 ` David E. Box
2023-10-12 2:38 ` [PATCH V3 03/16] platform/x86/intel/vsec: Use cleanup.h David E. Box
2023-10-12 5:25 ` kernel test robot
2023-10-12 17:23 ` David E. Box
2023-10-13 10:39 ` Ilpo Järvinen
2023-10-13 18:14 ` Joe Perches
2023-10-24 5:15 ` Joe Perches
2023-10-12 5:48 ` kernel test robot
2023-10-12 14:46 ` Ilpo Järvinen
2023-10-12 17:13 ` David E. Box
2023-10-13 10:54 ` Ilpo Järvinen
2023-10-13 22:16 ` David E. Box
2023-10-16 12:02 ` Ilpo Järvinen
2023-10-12 2:38 ` [PATCH V3 04/16] platform/x86/intel/vsec: Add intel_vsec_register David E. Box
2023-10-12 2:38 ` [PATCH V3 05/16] platform/x86/intel/vsec: Add base address field David E. Box
2023-10-12 2:38 ` [PATCH V3 06/16] platform/x86/intel/pmt: Add header to struct intel_pmt_entry David E. Box
2023-10-12 2:38 ` [PATCH V3 07/16] platform/x86/intel/pmt: telemetry: Export API to read telemetry David E. Box
2023-10-12 2:38 ` [PATCH V3 08/16] platform/x86:intel/pmc: Call pmc_get_low_power_modes from platform init David E. Box
2023-10-12 2:38 ` [PATCH V3 09/16] platform/x86/intel/pmc: Allow pmc_core_ssram_init to fail David E. Box
2023-10-12 15:01 ` Ilpo Järvinen
2023-10-12 17:52 ` David E. Box
2023-10-13 11:36 ` Ilpo Järvinen [this message]
2023-10-12 2:38 ` [PATCH V3 10/16] platform/x86/intel/pmc: Split pmc_core_ssram_get_pmc() David E. Box
2023-10-12 15:14 ` Ilpo Järvinen
2023-10-12 17:28 ` David E. Box
2023-10-12 2:38 ` [PATCH V3 11/16] platform/x86/intel/pmc: Find and register PMC telemetry entries David E. Box
2023-10-12 15:17 ` Ilpo Järvinen
2023-10-12 2:38 ` [PATCH V3 12/16] platform/x86/intel/pmc: Display LPM requirements for multiple PMCs David E. Box
2023-10-12 2:38 ` [PATCH V3 13/16] platform/x86/intel/pmc: Retrieve LPM information using Intel PMT David E. Box
2023-10-12 2:38 ` [PATCH V3 14/16] platform/x86/intel/pmc: Read low power mode requirements for MTL-M and MTL-P David E. Box
2023-10-12 2:38 ` [PATCH V3 15/16] platform/x86/intel/pmc: Add debug attribute for Die C6 counter David E. Box
2023-10-12 2:38 ` [PATCH V3 16/16] platform/x86/intel/pmc: Show Die C6 counter on Meteor Lake David E. Box
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=92594f48-fa9-59d4-e2b7-f5f83ded0ea@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=david.e.box@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=rajvi.jingar@linux.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox