Re: [PATCH net-next v7 03/14] ax88179_178a: Split driver into library and device specific code
Birger Koblitz <mail-+5ewKuAgbOWcE/[email protected]>
| Newsgroups | gmane.linux.usb.general,gmane.linux.network,gmane.linux.kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi Jakub,
thanks for reviewing the code!
On 8/18/26 00:28, Jakub Kicinski wrote:
> On Mon, 10 Aug 2026 14:33:28 +0200 Birger Koblitz wrote:
>> +static int __ax88179_write_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
>> + u16 size, const void *data)
>
> Local netdev convention is to still wrap at 80 chars.
> Anything beyond that is inviting laziness.
>
>> +{
>> + int ret;
>> + int (*fn)(struct usbnet *dev, u8 cmd, u8 rtype, u16 val, u16 i, const void *d, u16 size);
>> + struct ax88179_data *ax179_data = dev->driver_priv;
>
> Local netdev convention - please sort the variable declaration lines
> longest to shortest
Both of the above and other occurrences will be fixed in v8. Sorry about those, I was
moving existing code and tried to minimally change it in order to allow blame to correctly
identify the original author of the code, while still making checkpatch happy.
I was not expecting a review while netdev was closed for patches, btw. Am I right
to assume that I should not send v8 during the merge window, or do you prefer the
next version to be submitted earlier while everyone still easily remembers what this
was about?
>
>> + if (!dev) {
>> + netdev_err(dev->net, "No net device.\n");
>> + return -ENODEV;
>> + }
>
> Is this a real check you need? It was a BUG() previously, so clearly
> it must never happen. Please don't add pointless defensive checks.
No, I did not see how BUG() was doing anything useful before, either, and just
wanted to preserve the logic of the existing code, while making checkpatch happy.
Will remove the 2 occurrences of this bit of code.
Birger