On 03/15/2018 10:26 AM, Oliver Neukum wrote:
> Am Mittwoch, den 14.03.2018, 16:44 +0100 schrieb Richard Leitner:
>> On 03/14/2018 04:27 PM, Oliver Neukum wrote:
>>> Am Mittwoch, den 14.03.2018, 14:31 +0100 schrieb Richard Leitner:
>>>>
>>> Well, but it does not. Removing a redundant definition is a clear
>>> benefit. But you are not removing a definition. You are introducing
>>> a preprocessor constant. Why?
>>> What is its benefit?
>>
>> AFAIK pci_ids.h collects PCI vendor and device IDs in one single
>> point. As the PCI vendor ID of Netlogic is used in multiple files
>> IMHO it would be a good idea to add it to pci_ids.h and furthermore
>> remove it from arch/mips/include/asm/netlogic/xlp-hal/iomap.h (where
>> it's currently defined).
>>
>> Or am I getting things wrong?
>
> I think so, yes. We are giving names to constants as a form
> of comment or to change them at multiple places at once and
> consistently.
>
> So
>
> #define XYZ_NETDEV_RESET_RETRIES 2
>
> makes clearly sense. So does
>
> #define XYZ_MAGIC_VALUE1 0xab4e
>
> because it tells you that you have a magic value.
> But you will never redefine a PCI vendor ID. In fact you
> must not. And if you have a comparison like
>
> dev->vID == 0x1234
>
> if you change this to
>
> dev->vID == SOME_VENDOR_ID
>
> what good does this to you? You already knew it was a vendor ID.
> Now you can name it at a glance. So what? If you have a device
> you will have to check whether you have some OEM version. You
> will always go and check the raw number. And if you have a log
> and need to check whether the check will be true, you will have
> a number.
> Using a constant there is nothing but trouble. Yet one more grep.
Thank you for that explanation. But IMHO it was clearer with a
human-readable name in such comparisons...
For example in the following I see at the first glance which
device from which vendor is affected and I don't need any
additional comments or ID databases...
if (pdev->vendor == PCI_VENDOR_ID_TI &&
pdev->device == PCI_DEVICE_ID_TI_TUSB73X0)
...but if that's not the preferred way of doing things I'm
perfectly fine with that.
Furthermore to me it sounds you are saying that the complete
pci_ids.h should be thrown over-board?!?
regards;richard.l
