Re: [PATCH v5 2/2] hwmon: add AMD Promontory 21 xHCI temperature sensor support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using readb() on a 32-bit PCIe MMIO vendor data register may cause unsupported request faults.
--

commit 9f079250f98377be1d241f6f3c08a14ae44d8785
Author: Jihong Min <hurryman2212@gmail.com>

hwmon: add AMD Promontory 21 xHCI temperature sensor support

Add an auxiliary-bus hwmon driver for the temperature sensor exposed by
AMD Promontory 21 (PROM21) xHCI PCI functions.

> diff --git a/drivers/hwmon/prom21-xhci.c b/drivers/hwmon/prom21-xhci.c
> new file mode 100644
> index 0000000000000..7124824a1f5ed
> --- /dev/null
> +++ b/drivers/hwmon/prom21-xhci.c

[ ... ]

> +static int prom21_xhci_read_temp_raw_restore_index(struct prom21_xhci *hwmon,
> +						   u8 *raw)
> +{
> +	struct device *dev = &hwmon->pdev->dev;
> +	u32 index;
> +	u8 data;
> +	int ret;
> +
> +	ret = prom21_xhci_pm_get(hwmon);
> +	if (ret)
> +		return ret;
> +
> +	index = readl(hwmon->regs + PROM21_XHCI_INDEX_OFFSET);
> +	/* Select the PROM21 temperature register through the vendor index. */
> +	writel(PROM21_XHCI_TEMP_SELECTOR,
> +	       hwmon->regs + PROM21_XHCI_INDEX_OFFSET);
> +	data = readb(hwmon->regs + PROM21_XHCI_DATA_OFFSET);

Is it safe to use readb() on this 32-bit PCIe MMIO vendor data register?

Many PCIe controllers and peripheral MMIO implementations strictly require
32-bit transactions. Issuing an 8-bit read can result in a PCIe unsupported
request, which may return 0xff or trigger a fatal PCIe error.

Could this instead use readl() and mask out the lowest 8 bits?

> +	/* Restore the previous vendor index register value. */
> +	writel(index, hwmon->regs + PROM21_XHCI_INDEX_OFFSET);
> +	readl(hwmon->regs + PROM21_XHCI_INDEX_OFFSET);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512213910.871859-1-hurryman2212@gmail.com?part=2