On 2025/8/1 16:18, Manivannan Sadhasivam wrote:
EXTERNAL EMAIL
On Thu, Jul 31, 2025 at 09:01:17PM GMT, Arnd Bergmann wrote:
On Thu, Jul 31, 2025, at 20:39, Bjorn Helgaas wrote:
On Thu, Jul 31, 2025 at 07:38:58PM +0200, Gerd Bayer wrote:
- if (size == 1)
- return pci_bus_read_config_byte(bus, devfn, where, (u8 *)val);
- else if (size == 2)
- return pci_bus_read_config_word(bus, devfn, where, (u16 *)val);
- else if (size == 4)
- return pci_bus_read_config_dword(bus, devfn, where, val);
- else
- return PCIBIOS_BAD_REGISTER_NUMBER;
+ if (size == 1) {
+ rc = pci_bus_read_config_byte(bus, devfn, where, (u8 *)val);
+#if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+ *val = ((*val >> 24) & 0xff);
+#endif
Yeah, this is all pretty ugly. Obviously the previous code in
__pci_find_next_cap_ttl() didn't need this. My guess is that was
because the destination for the read data was always the correct type
(u8/u16/u32), but here we always use a u32 and cast it to the
appropriate type. Maybe we can use the correct types here instead of
the casts?
Agreed, the casts here just add more potential for bugs.
Ack. Missed the obvious issue during review.
The pci_bus_read_config() interface itself may have been a
mistake, can't the callers just use the underlying helpers
directly?
They can! Since the callers of this API is mostly the macros, we can easily
implement the logic to call relevant accessors based on the requested size.
Hans, could you please respin the series based the feedback since the series is
dropped for 6.17.
Dear all,
I am once again deeply sorry for the problems that occurred in this
series. I only test pulling the ARM platform.
Thank you very much, Gerd, for reporting the problem.
Thank you all for your discussions and suggestions for revision.
Hi Mani,
Geert provided a solution. My patch based on this is as follows. Please
check if there are any problems.
https://lore.kernel.org/linux-pci/CAMuHMdVwFeV46oCid_sMHjXfP+yyGTpBfs9t3uaa=wRxNcSOAQ@xxxxxxxxxxxxxx/
Also, please ask Gerd to help test whether it works properly. Thank you
very much.
If there are no issues, am I sending the new version? Can this series of
pacth 0001 be directly replaced?
The patch is as follows:
diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index ba66f55d2524..2bfd8fc1c0f5 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -89,15 +89,25 @@ int pci_bus_read_config(void *priv, unsigned int
devfn, int where, u32 size,
u32 *val)
{
struct pci_bus *bus = priv;
+ int rc;
+
+ if (size == 1) {
+ u8 byte;
+
+ rc = pci_bus_read_config_byte(bus, devfn, where, &byte);
+ *val = byte;
+ } else if (size == 2) {
+ u16 word;
+
+ rc = pci_bus_read_config_word(bus, devfn, where, &word);
+ *val = word;
+ } else if (size == 4) {
+ rc = pci_bus_read_config_dword(bus, devfn, where, val);
+ } else {
+ rc = PCIBIOS_BAD_REGISTER_NUMBER;
+ }
- if (size == 1)
- return pci_bus_read_config_byte(bus, devfn, where, (u8
*)val);
- else if (size == 2)
- return pci_bus_read_config_word(bus, devfn, where, (u16
*)val);
- else if (size == 4)
- return pci_bus_read_config_dword(bus, devfn, where, val);
- else
- return PCIBIOS_BAD_REGISTER_NUMBER;
+ return rc;
}
int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,
Best regards,
Hans
