* 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
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
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
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
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.