On Mon, Jan 23, 2012 at 8:13 AM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Sat, Jan 21, 2012 at 1:52 AM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
>> +
>> + retval = pciehp_readw(ctrl, PCI_EXP_LNKCTL, &lnk_ctrl);
>> + if (retval) {
>> + ctrl_err(ctrl, "Cannot read LNKCTRL register\n");
>> + return retval;
>> + }
>
> Is there really any point at all in checking the return value of that
> readw() function?
>
> Nobody actually ever sets it. We should stop even bothering to pretend
> to care. There is no return value in real life.
>
> If the device doesn't exist or the read fails, you will never ever get
> an error *anyway*. You'll likely get 0xffff, and possibly a machine
> check exception. And that is unlikely to ever change - even if the
> hardware were to do it, it's insane to do error checking on an
> individual IO level. It causes more problems than it could possibly
> ever solve - error checking has to be done at a different level than
> on some random individual access.
>
> We should make the return type of pci_read_config_word() be 'void',
> instead of having code like this that looks like it is careful, but in
> actual fact in reality is just totally pointless.
looks like
#define PCI_OP_READ(size,type,len) \
int pci_bus_read_config_##size \
(struct pci_bus *bus, unsigned int devfn, int pos, type *value) \
{ \
int res; \
unsigned long flags; \
u32 data = 0; \
if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
raw_spin_lock_irqsave(&pci_lock, flags); \
res = bus->ops->read(bus, devfn, pos, len, &data); \
*value = (type)data; \
raw_spin_unlock_irqrestore(&pci_lock, flags); \
return res; \
}
will check register number. like we can not pass 0x01 but want to read
word and dword.
also bus ops read/write like pci_mmcfg_read/write will double check if
passed bus/dev/fn/reg
ranges.
Thanks
Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
