* Re: [PATCH] xhci: fix write to USB3_PSSEN and XUSB2PRM pci config registers [not found] <1379695553-5891-1-git-send-email-burzalodowa@gmail.com> @ 2013-09-23 16:45 ` Sarah Sharp 2013-09-23 18:06 ` Xenia Ragiadakou 0 siblings, 1 reply; 7+ messages in thread From: Sarah Sharp @ 2013-09-23 16:45 UTC (permalink / raw) To: Xenia Ragiadakou; +Cc: linux-usb, Alan Stern, Greg KH, linux-pci On Fri, Sep 20, 2013 at 07:45:53PM +0300, Xenia Ragiadakou wrote: > The function pci_write_config_dword() sets the appropriate byteordering > internally so the value argument should not be converted to little-endian. > This bug was found by sparse. Can you give the exact error or warning message that sparse gave? I ask because this description sounded odd to Greg and I when we met last week at LinuxCon North America. I've tried to track this down to see where the code might be converting the value from CPU format to little endian, and I don't see it. AFAICT, pci_write_config_dword() is defined in include/linux/pci.h, and calls pci_bus_write_config_dword(): static inline int pci_write_config_dword(const struct pci_dev *dev, int where, u32 val) { return pci_bus_write_config_dword(dev->bus, dev->devfn, where, val); } pci_bus_write_config_dword is defined as a macro in drivers/pci/access.h: #define PCI_OP_WRITE(size,type,len) \ int pci_bus_write_config_##size \ (struct pci_bus *bus, unsigned int devfn, int pos, type value) \ { \ int res; \ unsigned long flags; \ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \ raw_spin_lock_irqsave(&pci_lock, flags); \ res = bus->ops->write(bus, devfn, pos, len, value); \ raw_spin_unlock_irqrestore(&pci_lock, flags); \ return res; \ } That macro simply calls the write function for whatever PCI bus driver is installed. Note that bus driver can be different than the standard bus driver. I don't see any conversion to little endian here, so that means each bus driver would have to convert it. I can dig deeper into each .write function, but if the conversion isn't done at the upper layers, it's possible someone will create a .write function without the conversion to little endian. Am I missing something? > > Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> > --- > drivers/usb/host/pci-quirks.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c > index 2c76ef1..08ef282 100644 > --- a/drivers/usb/host/pci-quirks.c > +++ b/drivers/usb/host/pci-quirks.c > @@ -799,7 +799,7 @@ void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev) > * switchable ports. > */ > pci_write_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN, > - cpu_to_le32(ports_available)); > + ports_available); > > pci_read_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN, > &ports_available); > @@ -821,7 +821,7 @@ void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev) > * host. > */ > pci_write_config_dword(xhci_pdev, USB_INTEL_XUSB2PR, > - cpu_to_le32(ports_available)); > + ports_available); > > pci_read_config_dword(xhci_pdev, USB_INTEL_XUSB2PR, > &ports_available); > -- > 1.8.3.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xhci: fix write to USB3_PSSEN and XUSB2PRM pci config registers 2013-09-23 16:45 ` [PATCH] xhci: fix write to USB3_PSSEN and XUSB2PRM pci config registers Sarah Sharp @ 2013-09-23 18:06 ` Xenia Ragiadakou 2013-09-23 20:49 ` Greg KH 0 siblings, 1 reply; 7+ messages in thread From: Xenia Ragiadakou @ 2013-09-23 18:06 UTC (permalink / raw) To: Sarah Sharp; +Cc: linux-usb, Alan Stern, Greg KH, linux-pci On 09/23/2013 07:45 PM, Sarah Sharp wrote: > On Fri, Sep 20, 2013 at 07:45:53PM +0300, Xenia Ragiadakou wrote: >> The function pci_write_config_dword() sets the appropriate byteordering >> internally so the value argument should not be converted to little-endian. >> This bug was found by sparse. > Can you give the exact error or warning message that sparse gave? Yes, sure. drivers/usb/host/pci-quirks.c:802:25: warning: incorrect type in argument 3 (different base types) drivers/usb/host/pci-quirks.c:802:25: expected unsigned int [unsigned] [usertype] val drivers/usb/host/pci-quirks.c:802:25: got restricted __le32 [usertype] <noident> drivers/usb/host/pci-quirks.c:824:25: warning: incorrect type in argument 3 (different base types) drivers/usb/host/pci-quirks.c:824:25: expected unsigned int [unsigned] [usertype] val drivers/usb/host/pci-quirks.c:824:25: got restricted __le32 [usertype] <noident> > > I ask because this description sounded odd to Greg and I when we met > last week at LinuxCon North America. I've tried to track this down to > see where the code might be converting the value from CPU format to > little endian, and I don't see it. > > AFAICT, pci_write_config_dword() is defined in include/linux/pci.h, and > calls pci_bus_write_config_dword(): > > static inline int pci_write_config_dword(const struct pci_dev *dev, int where, > u32 val) > { > return pci_bus_write_config_dword(dev->bus, dev->devfn, where, val); > } > > pci_bus_write_config_dword is defined as a macro in drivers/pci/access.h: > > #define PCI_OP_WRITE(size,type,len) \ > int pci_bus_write_config_##size \ > (struct pci_bus *bus, unsigned int devfn, int pos, type value) \ > { \ > int res; \ > unsigned long flags; \ > if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \ > raw_spin_lock_irqsave(&pci_lock, flags); \ > res = bus->ops->write(bus, devfn, pos, len, value); \ > raw_spin_unlock_irqrestore(&pci_lock, flags); \ > return res; \ > } > > That macro simply calls the write function for whatever PCI bus driver > is installed. Note that bus driver can be different than the standard > bus driver. I don't see any conversion to little endian here, so that > means each bus driver would have to convert it. > > I can dig deeper into each .write function, but if the conversion isn't > done at the upper layers, it's possible someone will create a .write > function without the conversion to little endian. > > Am I missing something? I had in mind that the pci_ops .read and .write defined by the PCI driver will take care of consistent byteorder access to the configuration registers. At least, that was what i understood after reading the chapter on PCI of Linux Device Drivers (more specifically for pci_write_config_* functions, it states that "The word and dword functions convert the value to little-endian before writing to the peripheral device."). regards, ksenia >> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> >> --- >> drivers/usb/host/pci-quirks.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c >> index 2c76ef1..08ef282 100644 >> --- a/drivers/usb/host/pci-quirks.c >> +++ b/drivers/usb/host/pci-quirks.c >> @@ -799,7 +799,7 @@ void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev) >> * switchable ports. >> */ >> pci_write_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN, >> - cpu_to_le32(ports_available)); >> + ports_available); >> >> pci_read_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN, >> &ports_available); >> @@ -821,7 +821,7 @@ void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev) >> * host. >> */ >> pci_write_config_dword(xhci_pdev, USB_INTEL_XUSB2PR, >> - cpu_to_le32(ports_available)); >> + ports_available); >> >> pci_read_config_dword(xhci_pdev, USB_INTEL_XUSB2PR, >> &ports_available); >> -- >> 1.8.3.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-usb" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xhci: fix write to USB3_PSSEN and XUSB2PRM pci config registers 2013-09-23 18:06 ` Xenia Ragiadakou @ 2013-09-23 20:49 ` Greg KH 2013-09-23 21:38 ` Bjorn Helgaas 0 siblings, 1 reply; 7+ messages in thread From: Greg KH @ 2013-09-23 20:49 UTC (permalink / raw) To: Xenia Ragiadakou; +Cc: Sarah Sharp, linux-usb, Alan Stern, linux-pci On Mon, Sep 23, 2013 at 09:06:54PM +0300, Xenia Ragiadakou wrote: > On 09/23/2013 07:45 PM, Sarah Sharp wrote: > >On Fri, Sep 20, 2013 at 07:45:53PM +0300, Xenia Ragiadakou wrote: > >>The function pci_write_config_dword() sets the appropriate byteordering > >>internally so the value argument should not be converted to little-endian. > >>This bug was found by sparse. > >Can you give the exact error or warning message that sparse gave? > > Yes, sure. > > drivers/usb/host/pci-quirks.c:802:25: warning: incorrect type in > argument 3 (different base types) > drivers/usb/host/pci-quirks.c:802:25: expected unsigned int > [unsigned] [usertype] val > drivers/usb/host/pci-quirks.c:802:25: got restricted __le32 > [usertype] <noident> > drivers/usb/host/pci-quirks.c:824:25: warning: incorrect type in > argument 3 (different base types) > drivers/usb/host/pci-quirks.c:824:25: expected unsigned int > [unsigned] [usertype] val > drivers/usb/host/pci-quirks.c:824:25: got restricted __le32 > [usertype] <noident> > > > > >I ask because this description sounded odd to Greg and I when we met > >last week at LinuxCon North America. I've tried to track this down to > >see where the code might be converting the value from CPU format to > >little endian, and I don't see it. > > > >AFAICT, pci_write_config_dword() is defined in include/linux/pci.h, and > >calls pci_bus_write_config_dword(): > > > >static inline int pci_write_config_dword(const struct pci_dev *dev, int where, > > u32 val) > >{ > > return pci_bus_write_config_dword(dev->bus, dev->devfn, where, val); > >} > > > >pci_bus_write_config_dword is defined as a macro in drivers/pci/access.h: > > > >#define PCI_OP_WRITE(size,type,len) \ > >int pci_bus_write_config_##size \ > > (struct pci_bus *bus, unsigned int devfn, int pos, type value) \ > >{ \ > > int res; \ > > unsigned long flags; \ > > if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \ > > raw_spin_lock_irqsave(&pci_lock, flags); \ > > res = bus->ops->write(bus, devfn, pos, len, value); \ > > raw_spin_unlock_irqrestore(&pci_lock, flags); \ > > return res; \ > >} > > > >That macro simply calls the write function for whatever PCI bus driver > >is installed. Note that bus driver can be different than the standard > >bus driver. I don't see any conversion to little endian here, so that > >means each bus driver would have to convert it. > > > >I can dig deeper into each .write function, but if the conversion isn't > >done at the upper layers, it's possible someone will create a .write > >function without the conversion to little endian. > > > >Am I missing something? > > I had in mind that the pci_ops .read and .write defined by the PCI > driver will take care of consistent byteorder access to the > configuration registers. At least, that was what i understood after > reading the > chapter on PCI of Linux Device Drivers (more specifically for > pci_write_config_* functions, it states that "The word and dword > functions convert the value to little-endian before writing to the > peripheral device."). Hm, I wrote that paragraph (or at least I think I did), but I sure didn't remember this at all... Hm, wait, I do see this happening for the PowerPC cell PCI code, so it might happen somewhere burried in the platform-specific code for different arches. You will not see it happen on x86 as there's no need to swap any bytes around. thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xhci: fix write to USB3_PSSEN and XUSB2PRM pci config registers 2013-09-23 20:49 ` Greg KH @ 2013-09-23 21:38 ` Bjorn Helgaas 2013-09-23 21:49 ` Greg KH 0 siblings, 1 reply; 7+ messages in thread From: Bjorn Helgaas @ 2013-09-23 21:38 UTC (permalink / raw) To: Greg KH Cc: Xenia Ragiadakou, Sarah Sharp, USB list, Alan Stern, linux-pci@vger.kernel.org On Mon, Sep 23, 2013 at 3:49 PM, Greg KH <greg@kroah.com> wrote: > On Mon, Sep 23, 2013 at 09:06:54PM +0300, Xenia Ragiadakou wrote: >> On 09/23/2013 07:45 PM, Sarah Sharp wrote: >> >On Fri, Sep 20, 2013 at 07:45:53PM +0300, Xenia Ragiadakou wrote: >> >>The function pci_write_config_dword() sets the appropriate byteordering >> >>internally so the value argument should not be converted to little-endian. >> >>This bug was found by sparse. >> >Can you give the exact error or warning message that sparse gave? >> >> Yes, sure. >> >> drivers/usb/host/pci-quirks.c:802:25: warning: incorrect type in >> argument 3 (different base types) >> drivers/usb/host/pci-quirks.c:802:25: expected unsigned int >> [unsigned] [usertype] val >> drivers/usb/host/pci-quirks.c:802:25: got restricted __le32 >> [usertype] <noident> >> drivers/usb/host/pci-quirks.c:824:25: warning: incorrect type in >> argument 3 (different base types) >> drivers/usb/host/pci-quirks.c:824:25: expected unsigned int >> [unsigned] [usertype] val >> drivers/usb/host/pci-quirks.c:824:25: got restricted __le32 >> [usertype] <noident> >> >> > >> >I ask because this description sounded odd to Greg and I when we met >> >last week at LinuxCon North America. I've tried to track this down to >> >see where the code might be converting the value from CPU format to >> >little endian, and I don't see it. >> > >> >AFAICT, pci_write_config_dword() is defined in include/linux/pci.h, and >> >calls pci_bus_write_config_dword(): >> > >> >static inline int pci_write_config_dword(const struct pci_dev *dev, int where, >> > u32 val) >> >{ >> > return pci_bus_write_config_dword(dev->bus, dev->devfn, where, val); >> >} >> > >> >pci_bus_write_config_dword is defined as a macro in drivers/pci/access.h: >> > >> >#define PCI_OP_WRITE(size,type,len) \ >> >int pci_bus_write_config_##size \ >> > (struct pci_bus *bus, unsigned int devfn, int pos, type value) \ >> >{ \ >> > int res; \ >> > unsigned long flags; \ >> > if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \ >> > raw_spin_lock_irqsave(&pci_lock, flags); \ >> > res = bus->ops->write(bus, devfn, pos, len, value); \ >> > raw_spin_unlock_irqrestore(&pci_lock, flags); \ >> > return res; \ >> >} >> > >> >That macro simply calls the write function for whatever PCI bus driver >> >is installed. Note that bus driver can be different than the standard >> >bus driver. I don't see any conversion to little endian here, so that >> >means each bus driver would have to convert it. >> > >> >I can dig deeper into each .write function, but if the conversion isn't >> >done at the upper layers, it's possible someone will create a .write >> >function without the conversion to little endian. >> > >> >Am I missing something? >> >> I had in mind that the pci_ops .read and .write defined by the PCI >> driver will take care of consistent byteorder access to the >> configuration registers. At least, that was what i understood after >> reading the >> chapter on PCI of Linux Device Drivers (more specifically for >> pci_write_config_* functions, it states that "The word and dword >> functions convert the value to little-endian before writing to the >> peripheral device."). > > Hm, I wrote that paragraph (or at least I think I did), but I sure > didn't remember this at all... > > Hm, wait, I do see this happening for the PowerPC cell PCI code, so it > might happen somewhere burried in the platform-specific code for > different arches. You will not see it happen on x86 as there's no need > to swap any bytes around. Greg, with regard to Xenia's patch, is this an ack or a nack? Since you didn't include an "Acked-by" line, I assume you think Xenia's patch is unnecessary. In that case, is there any way to shut sparse up so it doesn't complain about this? Bjorn ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xhci: fix write to USB3_PSSEN and XUSB2PRM pci config registers 2013-09-23 21:38 ` Bjorn Helgaas @ 2013-09-23 21:49 ` Greg KH 2013-09-24 0:23 ` Sarah Sharp 0 siblings, 1 reply; 7+ messages in thread From: Greg KH @ 2013-09-23 21:49 UTC (permalink / raw) To: Bjorn Helgaas Cc: Xenia Ragiadakou, Sarah Sharp, USB list, Alan Stern, linux-pci@vger.kernel.org On Mon, Sep 23, 2013 at 04:38:22PM -0500, Bjorn Helgaas wrote: > On Mon, Sep 23, 2013 at 3:49 PM, Greg KH <greg@kroah.com> wrote: > > On Mon, Sep 23, 2013 at 09:06:54PM +0300, Xenia Ragiadakou wrote: > >> On 09/23/2013 07:45 PM, Sarah Sharp wrote: > >> >On Fri, Sep 20, 2013 at 07:45:53PM +0300, Xenia Ragiadakou wrote: > >> >>The function pci_write_config_dword() sets the appropriate byteordering > >> >>internally so the value argument should not be converted to little-endian. > >> >>This bug was found by sparse. > >> >Can you give the exact error or warning message that sparse gave? > >> > >> Yes, sure. > >> > >> drivers/usb/host/pci-quirks.c:802:25: warning: incorrect type in > >> argument 3 (different base types) > >> drivers/usb/host/pci-quirks.c:802:25: expected unsigned int > >> [unsigned] [usertype] val > >> drivers/usb/host/pci-quirks.c:802:25: got restricted __le32 > >> [usertype] <noident> > >> drivers/usb/host/pci-quirks.c:824:25: warning: incorrect type in > >> argument 3 (different base types) > >> drivers/usb/host/pci-quirks.c:824:25: expected unsigned int > >> [unsigned] [usertype] val > >> drivers/usb/host/pci-quirks.c:824:25: got restricted __le32 > >> [usertype] <noident> > >> > >> > > >> >I ask because this description sounded odd to Greg and I when we met > >> >last week at LinuxCon North America. I've tried to track this down to > >> >see where the code might be converting the value from CPU format to > >> >little endian, and I don't see it. > >> > > >> >AFAICT, pci_write_config_dword() is defined in include/linux/pci.h, and > >> >calls pci_bus_write_config_dword(): > >> > > >> >static inline int pci_write_config_dword(const struct pci_dev *dev, int where, > >> > u32 val) > >> >{ > >> > return pci_bus_write_config_dword(dev->bus, dev->devfn, where, val); > >> >} > >> > > >> >pci_bus_write_config_dword is defined as a macro in drivers/pci/access.h: > >> > > >> >#define PCI_OP_WRITE(size,type,len) \ > >> >int pci_bus_write_config_##size \ > >> > (struct pci_bus *bus, unsigned int devfn, int pos, type value) \ > >> >{ \ > >> > int res; \ > >> > unsigned long flags; \ > >> > if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \ > >> > raw_spin_lock_irqsave(&pci_lock, flags); \ > >> > res = bus->ops->write(bus, devfn, pos, len, value); \ > >> > raw_spin_unlock_irqrestore(&pci_lock, flags); \ > >> > return res; \ > >> >} > >> > > >> >That macro simply calls the write function for whatever PCI bus driver > >> >is installed. Note that bus driver can be different than the standard > >> >bus driver. I don't see any conversion to little endian here, so that > >> >means each bus driver would have to convert it. > >> > > >> >I can dig deeper into each .write function, but if the conversion isn't > >> >done at the upper layers, it's possible someone will create a .write > >> >function without the conversion to little endian. > >> > > >> >Am I missing something? > >> > >> I had in mind that the pci_ops .read and .write defined by the PCI > >> driver will take care of consistent byteorder access to the > >> configuration registers. At least, that was what i understood after > >> reading the > >> chapter on PCI of Linux Device Drivers (more specifically for > >> pci_write_config_* functions, it states that "The word and dword > >> functions convert the value to little-endian before writing to the > >> peripheral device."). > > > > Hm, I wrote that paragraph (or at least I think I did), but I sure > > didn't remember this at all... > > > > Hm, wait, I do see this happening for the PowerPC cell PCI code, so it > > might happen somewhere burried in the platform-specific code for > > different arches. You will not see it happen on x86 as there's no need > > to swap any bytes around. > > Greg, with regard to Xenia's patch, is this an ack or a nack? Since > you didn't include an "Acked-by" line, I assume you think Xenia's > patch is unnecessary. In that case, is there any way to shut sparse > up so it doesn't complain about this? At this point in time, I don't remember what the original patch looked like, and as it's an xhci patch, Sarah needs to ack it, not me :) thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xhci: fix write to USB3_PSSEN and XUSB2PRM pci config registers 2013-09-23 21:49 ` Greg KH @ 2013-09-24 0:23 ` Sarah Sharp 2013-09-24 3:01 ` Greg KH 0 siblings, 1 reply; 7+ messages in thread From: Sarah Sharp @ 2013-09-24 0:23 UTC (permalink / raw) To: Greg KH Cc: Bjorn Helgaas, Xenia Ragiadakou, USB list, Alan Stern, linux-pci@vger.kernel.org On Mon, Sep 23, 2013 at 02:49:07PM -0700, Greg KH wrote: > On Mon, Sep 23, 2013 at 04:38:22PM -0500, Bjorn Helgaas wrote: > > On Mon, Sep 23, 2013 at 3:49 PM, Greg KH <greg@kroah.com> wrote: > > > On Mon, Sep 23, 2013 at 09:06:54PM +0300, Xenia Ragiadakou wrote: > > >> I had in mind that the pci_ops .read and .write defined by the PCI > > >> driver will take care of consistent byteorder access to the > > >> configuration registers. At least, that was what i understood after > > >> reading the > > >> chapter on PCI of Linux Device Drivers (more specifically for > > >> pci_write_config_* functions, it states that "The word and dword > > >> functions convert the value to little-endian before writing to the > > >> peripheral device."). > > > > > > Hm, I wrote that paragraph (or at least I think I did), but I sure > > > didn't remember this at all... > > > > > > Hm, wait, I do see this happening for the PowerPC cell PCI code, so it > > > might happen somewhere burried in the platform-specific code for > > > different arches. You will not see it happen on x86 as there's no need > > > to swap any bytes around. > > > > Greg, with regard to Xenia's patch, is this an ack or a nack? Since > > you didn't include an "Acked-by" line, I assume you think Xenia's > > patch is unnecessary. In that case, is there any way to shut sparse > > up so it doesn't complain about this? > > At this point in time, I don't remember what the original patch looked > like, and as it's an xhci patch, Sarah needs to ack it, not me :) Greg: So you're saying that there isn't a need to convert values from CPU byte-ordering to little endian byte-ordering before passing them on to pci_write_config_*? If so, then yes, Xenia's patch is fine and I'll pull that into my xHCI tree. Sarah Sharp ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xhci: fix write to USB3_PSSEN and XUSB2PRM pci config registers 2013-09-24 0:23 ` Sarah Sharp @ 2013-09-24 3:01 ` Greg KH 0 siblings, 0 replies; 7+ messages in thread From: Greg KH @ 2013-09-24 3:01 UTC (permalink / raw) To: Sarah Sharp Cc: Bjorn Helgaas, Xenia Ragiadakou, USB list, Alan Stern, linux-pci@vger.kernel.org On Mon, Sep 23, 2013 at 05:23:07PM -0700, Sarah Sharp wrote: > On Mon, Sep 23, 2013 at 02:49:07PM -0700, Greg KH wrote: > > On Mon, Sep 23, 2013 at 04:38:22PM -0500, Bjorn Helgaas wrote: > > > On Mon, Sep 23, 2013 at 3:49 PM, Greg KH <greg@kroah.com> wrote: > > > > On Mon, Sep 23, 2013 at 09:06:54PM +0300, Xenia Ragiadakou wrote: > > > >> I had in mind that the pci_ops .read and .write defined by the PCI > > > >> driver will take care of consistent byteorder access to the > > > >> configuration registers. At least, that was what i understood after > > > >> reading the > > > >> chapter on PCI of Linux Device Drivers (more specifically for > > > >> pci_write_config_* functions, it states that "The word and dword > > > >> functions convert the value to little-endian before writing to the > > > >> peripheral device."). > > > > > > > > Hm, I wrote that paragraph (or at least I think I did), but I sure > > > > didn't remember this at all... > > > > > > > > Hm, wait, I do see this happening for the PowerPC cell PCI code, so it > > > > might happen somewhere burried in the platform-specific code for > > > > different arches. You will not see it happen on x86 as there's no need > > > > to swap any bytes around. > > > > > > Greg, with regard to Xenia's patch, is this an ack or a nack? Since > > > you didn't include an "Acked-by" line, I assume you think Xenia's > > > patch is unnecessary. In that case, is there any way to shut sparse > > > up so it doesn't complain about this? > > > > At this point in time, I don't remember what the original patch looked > > like, and as it's an xhci patch, Sarah needs to ack it, not me :) > > Greg: So you're saying that there isn't a need to convert values from > CPU byte-ordering to little endian byte-ordering before passing them on > to pci_write_config_*? That is correct. Xenia, thanks for figuring this out, nice job. greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-09-24 3:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1379695553-5891-1-git-send-email-burzalodowa@gmail.com>
2013-09-23 16:45 ` [PATCH] xhci: fix write to USB3_PSSEN and XUSB2PRM pci config registers Sarah Sharp
2013-09-23 18:06 ` Xenia Ragiadakou
2013-09-23 20:49 ` Greg KH
2013-09-23 21:38 ` Bjorn Helgaas
2013-09-23 21:49 ` Greg KH
2013-09-24 0:23 ` Sarah Sharp
2013-09-24 3:01 ` Greg KH
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.