Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Michal Pecio <michal.pecio@gmail.com>
To: Jihong Min <hurryman2212@icloud.com>
Cc: Jihong Min <hurryman2212@gmail.com>,
	Mario Limonciello <mario.limonciello@amd.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Mathias Nyman <mathias.nyman@intel.com>,
	Guenter Roeck <linux@roeck-us.net>,
	Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Basavaraj Natikar <Basavaraj.Natikar@amd.com>,
	linux-usb@vger.kernel.org, linux-hwmon@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor
Date: Thu, 7 May 2026 10:51:16 +0200	[thread overview]
Message-ID: <20260507105116.2c1607d7.michal.pecio@gmail.com> (raw)
In-Reply-To: <c7d0e6c4-1c6c-432e-90ed-f0604401d691@icloud.com>

On Thu, 7 May 2026 07:41:28 +0900, Jihong Min wrote:
> > I think this commit message and certainly the Kconfig help text
> > should include full name of the chip and perhaps its official
> > marketing names too, so that people better understand what hardware
> > is supported.
> >
> > So: "AMD Promontory 21 chipset" and "AM5 6xx/8xx series chipsets",
> > or whatever they are called by AMD and motherboard vendors.  
> 
> Addressed locally for v3. I changed the commit message, Kconfig
> prompt/help text, and hwmon documentation to use the full name, "AMD
> Promontory 21 (PROM21)".
> 
> I avoided putting chipset marketing names directly into the Kconfig
> text and commit subject because some AMD 6xx/8xx series chipsets can
> be built from more than one PROM21 IP in a daisy-chained
> configuration, including more than one PROM21 xHCI controller.

Not sure how is this a problem?
The driver is still applicable to those daisy-chained chipsets.

Configuring the kernel is tedious enough already, the help text should
ideally tell me whether I need this driver or not without searching
for obscure code names.

I would even go as far as to mention that it's for AM5 chipsets,
because AMD recycles chipset numbers every few generations.

>  > Is there any documentation of the index/data registers themselves?
> 
> I am not aware of public AMD documentation for the PROM21 vendor
> index/data registers or the temperature selector.
> 
> For my initial validation on an X870E system with two decade or
> so.PROM21 xHCI controllers, I passed one PROM21 xHCI controller
> through to a Windows VM, left USB traffic idle, and let a Windows
> monitoring utility poll the controller temperature. From the Linux
> host I monitored that controller's PCI MMIO BAR with read-only
> accesses.
>
> The vendor index register repeatedly held the same selector value,
> and the adjacent data register exposed a stable low 8-bit value.
> [...]
>
> After identifying the register pair, I used the same validation setup
> to derive the conversion formula by comparing the observed raw MMIO
> register value against HWiNFO64's reported PROM21 xHCI temperature on
> Windows.

Looks good, unless there are gotchas like the actual formula being
dependent on other factors you haven't noticed HWiNFO reading.

It would be good idea to record this methodology in the commit message
or docs so that others can repeat your experiment in case of issues.

Regards,
Michal

  reply	other threads:[~2026-05-07  8:51 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260506032939.92351-1-hurryman2212@gmail.com>
2026-05-06 13:37 ` [PATCH] usb: xhci: add AMD PROM21 xHCI hwmon support for temperature monitoring Mathias Nyman
2026-05-06 14:26   ` Guenter Roeck
2026-05-06 20:28     ` Jihong Min
2026-05-06 20:40 ` [PATCH v2 0/2] AMD PROM21 xHCI temperature hwmon support Jihong Min
2026-05-06 20:40   ` [PATCH v2 1/2] usb: xhci-pci: add generic auxiliary device interface Jihong Min
2026-05-06 20:53     ` Mario Limonciello
2026-05-06 21:09       ` Jihong Min
2026-05-06 21:15     ` Michal Pecio
2026-05-06 21:42       ` Jihong Min
2026-05-06 20:40   ` [PATCH v2 2/2] hwmon: add initial support for AMD PROM21 xHCI temperature sensor Jihong Min
2026-05-06 21:33     ` Michal Pecio
2026-05-06 21:36       ` Mario Limonciello
2026-05-07  9:08         ` Michal Pecio
2026-05-07 13:24           ` Mario Limonciello
2026-05-06 22:41       ` Jihong Min
2026-05-07  8:51         ` Michal Pecio [this message]
2026-05-06 22:17     ` Randy Dunlap
2026-05-06 22:52       ` Jihong Min
2026-05-07  3:29         ` Jihong Min

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=20260507105116.2c1607d7.michal.pecio@gmail.com \
    --to=michal.pecio@gmail.com \
    --cc=Basavaraj.Natikar@amd.com \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=hurryman2212@gmail.com \
    --cc=hurryman2212@icloud.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=mario.limonciello@amd.com \
    --cc=mathias.nyman@intel.com \
    --cc=skhan@linuxfoundation.org \
    /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