Re: [PATCH 1/3] cxgb3: Replace PCI related literals with defines & correct variable

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]

On Tue, 10 Jun 2025, Jakub Kicinski wrote:
> On Tue, 10 Jun 2025 13:32:03 +0300 Ilpo Järvinen wrote:
> > Replace literals 0, 2, 0x1425 with PCI_VENDOR_ID, PCI_DEVICE_ID,
> > PCI_VENDOR_ID_CHELSIO, respectively. Rename devid variable to vendor_id
> > to remove confusion.
> 
> This series is missing a cover letter. An explanation of why you're
> touching this very very old driver is in order, and please comment
> on whether you can test this on real HW, because we don't like

No, I don't have the HW available.

> refactoring of very old code:
> 
> Quoting documentation:
> 
>   Clean-up patches
>   ~~~~~~~~~~~~~~~~
>   
>   Netdev discourages patches which perform simple clean-ups, which are not in
>   the context of other work. For example:
>   
>   * Addressing ``checkpatch.pl`` warnings
>   * Addressing :ref:`Local variable ordering<rcs>` issues
>   * Conversions to device-managed APIs (``devm_`` helpers)
>   
>   This is because it is felt that the churn that such changes produce comes
>   at a greater cost than the value of such clean-ups.
>   
>   Conversely, spelling and grammar fixes are not discouraged.
>   
> See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#clean-up-patches

Fine, I don't _need_ to get these accepted. I'll keep this in mind in
future.

It probably came up during some other work when I had to look through use 
of all PCI accessor functions and understand what the callers are trying 
to do. When I took a look at the C file, I ended up noticing other things 
too.

As these are/were not direct requirement for the actual accessor work, I 
tend to send such series separately to avoid complicating review of one or 
the other, feature series tend to be complicated enough even without the 
cleanup patches.

It seems that in the unsent state, these patches predated adding that 
guidance.

-- 
 i.