From: Joel Stanley <joel@jms.id.au>
To: Zev Weiss <zev@bewilderbeest.net>
Cc: Jason M Biils <jason.m.bills@linux.intel.com>,
Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>,
OpenBMC Maillist <openbmc@lists.ozlabs.org>,
James Feist <james.feist@linux.intel.com>
Subject: Re: [PATCH 2/2] peci-cputemp: label CPU cores from zero instead of one
Date: Tue, 29 Sep 2020 06:00:37 +0000 [thread overview]
Message-ID: <CACPK8XdzjEhxKHbajMXbMpktOAhm_xFqUW7rY67WdmQ4p8PXPg@mail.gmail.com> (raw)
In-Reply-To: <20200928220124.k47kocdvi2ahgtc6@hatter.bewilderbeest.net>
On Mon, 28 Sep 2020 at 22:02, Zev Weiss <zev@bewilderbeest.net> wrote:
>
> On Mon, Sep 28, 2020 at 04:32:31PM CDT, Jae Hyun Yoo wrote:
> >>Oh I see -- I had thought you were referring to other existing hwmon
> >>drivers in the kernel.
> >>
> >>As far as I can tell, all those instances appear to be numbering CPU
> >>*sockets* though -- which as Jason mentioned in a call earlier today
> >>I gather is done to line up with motherboard silkscreen labeling.
> >>But in the code in question here we're labeling *cores* within a
> >>given socket, which I don't see arising anywhere in any existing
> >>entity-manager configs. So I'm still unclear on why we want to use
> >>one-based indexing here instead of zero-based -- I'd think we'd want
> >>the PECI driver to match the PECI spec?
> >
> >PECI driver uses zero-based index for PECI command handling but label is
> >user facing stuff which shouldn't make confusion to users. We can modify
> >driver like you did in this patch and previous driver also used
> >zero-based indexing but I changed it to natural number based indexing
> >to avoid confusion between driver labels and dbus-sensors names.
> >Any specific reason for the zero-based indexing? Any benefit?
> >
>
> [Re-adding CCs...]
Thanks. Please keep the discussion on the list.
>
> Well, as I see it basically just consistency with a larger set of
> things. Most other related numbering schemes I'm aware of are
> zero-based -- userspace tools like 'taskset' and 'lscpu', system APIs
> like the <sched.h> CPU_SET() routines, and the kernel's own numbering
> (e.g. what's shown in /proc/cpuinfo) all number processors starting from
> zero, so dbus-sensors seems kind of like the odd one out there.
> (Personally I'd be fully in support of changing it to be zero-based as
> well, though I have no idea offhand about how distruptive a change that
> would be.)
>
> It also seems pretty OpenBMC-specific, whereas I'd expect we want to aim
> for greater generality in things going into mainline.
Agreed. The hwmon numbering varies; some attributes are zero indexed
and some start at 1. More commonly we start counting from zero in the
kernel, so I would expect PECI to do the same.
If there's some userspace that depends on the behaviour of these out
of tree PECI patches, then that userspace will need to change. This
reminds us why the project prefers patches exposing userspace ABI are
merged to mainline first.
Cheers,
Joel
next prev parent reply other threads:[~2020-09-29 6:02 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-26 21:27 [PATCH 0/2] PECI patchset tweaks Zev Weiss
2020-09-26 21:27 ` [PATCH 1/2] peci: fix error-handling in peci_dev_ioctl() Zev Weiss
2020-09-28 19:03 ` Jae Hyun Yoo
2020-09-28 19:09 ` Zev Weiss
2020-09-28 19:37 ` Jae Hyun Yoo
2020-09-29 5:55 ` Joel Stanley
2020-09-26 21:27 ` [PATCH 2/2] peci-cputemp: label CPU cores from zero instead of one Zev Weiss
2020-09-28 19:08 ` Jae Hyun Yoo
2020-09-28 19:54 ` Zev Weiss
2020-09-28 20:21 ` Jae Hyun Yoo
2020-09-28 21:09 ` Zev Weiss
2020-09-28 21:32 ` Jae Hyun Yoo
2020-09-28 22:02 ` Zev Weiss
2020-09-28 22:20 ` Jae Hyun Yoo
2020-09-29 6:00 ` Joel Stanley [this message]
2020-10-06 18:01 ` Jae Hyun Yoo
2020-10-07 5:39 ` Joel Stanley
2020-09-28 19:01 ` [PATCH 0/2] PECI patchset tweaks Jae Hyun Yoo
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=CACPK8XdzjEhxKHbajMXbMpktOAhm_xFqUW7rY67WdmQ4p8PXPg@mail.gmail.com \
--to=joel@jms.id.au \
--cc=jae.hyun.yoo@linux.intel.com \
--cc=james.feist@linux.intel.com \
--cc=jason.m.bills@linux.intel.com \
--cc=openbmc@lists.ozlabs.org \
--cc=zev@bewilderbeest.net \
/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;
as well as URLs for NNTP newsgroup(s).