From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.13]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9A8D339BFE7; Wed, 29 Apr 2026 06:55:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.13 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777445716; cv=none; b=iCNcOo5r1JhTh9JhfmXjBcSbNMiIiNydzesToF6qZ1vKlr1PlYTTAS3LoB75+wGqXyAnnJqssvTxr+AiT2sGx5K77xbBPs17VL6idN28vz0FZvddLHAiLzI4VcSgyDgqcJoVsp/5Y3RukPV1i8R/HUsNbhLk73CMyvBBrc2y4iw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777445716; c=relaxed/simple; bh=kYYeufPkRyGcr0hiIW6bKGaXhz8Q+nnaSSo0fxSS6co=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=UWdJy73NlEUWD1wNquPzln6IAf1BxlgxtcCXILsqpY95sYe3/5GW/kE0JsNUNgU6v30G/L9/8w49Fv+EzvvAWmozlyMm/Q8FGg3RbU5L8t6CaZdbofQtmzpKrHX/PS6Xv9LZtO8h/Qk5m6d0Ewbimrh13p9H1ZFG1nNlSGR/XhI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=mRYNGpKk; arc=none smtp.client-ip=198.175.65.13 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="mRYNGpKk" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1777445715; x=1808981715; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=kYYeufPkRyGcr0hiIW6bKGaXhz8Q+nnaSSo0fxSS6co=; b=mRYNGpKkdPZn92Js4w44vvHbhSgXVXy1f8mAr+/9W1FABmA43BvjO7EP GVD0J2RI12BQWLSVn+4pSjmpK3tJLexJPN1TP7IkIGvcp1XQ3/d//xFrS CJp3WQFbkvgN16LA4psH1fQ+t2XLRziSgU91OeRpokNK6AaSQAq0gdaSG uvWDZk0FksjFnREON71yB0shU5ojBgYqTUbdKAnKCfQpfx76Fc62Dnd3u WaBINNRZ0vVuN8peK/uponY/3gyaThgUpnevzRqr5fFlkDu/NtPvnOYcD K8l9VlBFLlOf6aC4AUDY9hYTH3ZFoMnxoSpVTUBBdaTPfOlQyc0M39ger w==; X-CSE-ConnectionGUID: UupN5YIITw+bGaoBrMuS9g== X-CSE-MsgGUID: BFAAvPR8QBqruOecGbaejw== X-IronPort-AV: E=McAfee;i="6800,10657,11770"; a="89456642" X-IronPort-AV: E=Sophos;i="6.23,205,1770624000"; d="scan'208";a="89456642" Received: from fmviesa004.fm.intel.com ([10.60.135.144]) by orvoesa105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Apr 2026 23:55:14 -0700 X-CSE-ConnectionGUID: NWCIGupGRW+k5c69AnHXJQ== X-CSE-MsgGUID: 5oHPhrg7Q1iV2x0zLMtPhQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.23,205,1770624000"; d="scan'208";a="235958719" Received: from ettammin-mobl2.ger.corp.intel.com (HELO localhost) ([10.245.245.141]) by fmviesa004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Apr 2026 23:54:56 -0700 Date: Wed, 29 Apr 2026 09:54:54 +0300 From: Andy Shevchenko To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig_=28The_Capable_Hub=29?= Cc: Michael Grzeschik , Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Marc Kleine-Budde , Vincent Mailhol , Krzysztof Halasa , Johannes Berg , Markus Schneider-Pargmann , Steffen Klassert , David Dillow , Ion Badulescu , Mark Einon , Rasesh Mody , GR-Linux-NIC-Dev@marvell.com, Sudarsana Kalluru , Manish Chopra , Potnuri Bharat Teja , Denis Kirjanov , Jijie Shao , Jian Shen , Cai Huoqing , Fan Gong , Tony Nguyen , Przemek Kitszel , Tariq Toukan , Saeed Mahameed , Leon Romanovsky , Mark Bloch , Ido Schimmel , Petr Machata , Yibo Dong , Simon Horman , Heiner Kallweit , nic_swsd@realtek.com, Jiri Pirko , Francois Romieu , Daniele Venzano , Samuel Chessman , Jiawen Wu , Mengyuan Lou , Kevin Curtis , Arend van Spriel , Stanislav Yakovlev , Richard Cochran , Kees Cook , Thomas Gleixner , Thomas Fourier , Ingo Molnar , Kory Maincent , Zilin Guan , Marco Crivellari , Vadim Fedorenko , Jacob Keller , Philipp Stanner , Bjorn Helgaas , Yeounsu Moon , Denis Benato , Peiyang Wang , Yonglong Liu , Yicong Hui , Randy Dunlap , MD Danish Anwar , Nathan Chancellor , Sai Krishna , Ethan Nelson-Moore , Larysa Zaremba , Joe Damato , Double Lo , Chi-hsien Lin , Colin Ian King , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-can@vger.kernel.org, linux-parisc@vger.kernel.org, intel-wired-lan@lists.osuosl.org, linux-rdma@vger.kernel.org, oss-drivers@corigine.com, linux-wireless@vger.kernel.org, brcm80211@lists.linux.dev, brcm80211-dev-list.pdl@broadcom.com Subject: Re: [PATCH net-next] net: Consistently define pci_device_ids using named initializers Message-ID: References: <20260428171845.2288395-2-u.kleine-koenig@baylibre.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260428171845.2288395-2-u.kleine-koenig@baylibre.com> Organization: Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo On Tue, Apr 28, 2026 at 07:18:44PM +0200, Uwe Kleine-König (The Capable Hub) wrote: > ... and PCI device helpers. > > The various struct pci_device_id arrays were initialized mostly by one > the PCI_DEVICE macros and then list expressions. The latter isn't easily > readable if you're not into PCI. Using named initializers is more > explicit and thus easier to parse. > > Also use PCI_DEVICE* helper macros to assign .vendor, .device, > .subvendor and .subdevice where appropriate and skip explicit > assignments of 0 (which the compiler takes care of). > > The secret plan is to make struct pci_device_id::driver_data an > anonymous union (similar to > https://lore.kernel.org/all/cover.1776579304.git.u.kleine-koenig@baylibre.com/) > and that requires named initializers. But it's also a nice cleanup on > its own. > > This change doesn't introduce changes to the compiled pci_device_id > arrays. Tested on x86 and arm64. ... > - {0,} /* 0 terminated list. */ > + { } /* 0 terminated list. */ The comments like these are just noises. The rule of thumb is to play with a trailing comma: - always drop it in the terminator entry - always keep it in the normal initialisers when semantically it's not a terminator ... > static const struct pci_device_id liquidio_pci_tbl[] = { > { /* 68xx */ > - PCI_VENDOR_ID_CAVIUM, 0x91, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 > + PCI_VDEVICE(CAVIUM, 0x91) Use full fixed-width device id value(s). 0x0091 here and so on... > }, Also seems that you may decrease number of LoC here putting it as { PCI_VDEVICE(CAVIUM, 0x0091) }, /* 68xx */ and so on... > { /* 66xx */ > - PCI_VENDOR_ID_CAVIUM, 0x92, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 > + PCI_VDEVICE(CAVIUM, 0x92) > }, > { /* 23xx pf */ > - PCI_VENDOR_ID_CAVIUM, 0x9702, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 > + PCI_VDEVICE(CAVIUM, 0x9702) > }, > - { > - 0, 0, 0, 0, 0, 0, 0 > - } > + { } > }; ... > #define CH_PCI_DEVICE_ID_TABLE_DEFINE_END \ > - { 0, } \ > + { } \ > } Why do we have this macro at all? > -#define CH_PCI_DEVICE_ID_TABLE_DEFINE_END { 0, } } > +#define CH_PCI_DEVICE_ID_TABLE_DEFINE_END { } } Ditto. ... > static const struct pci_device_id de_pci_tbl[] = { > - { PCI_VENDOR_ID_DEC, PCI_DEVICE_ID_DEC_TULIP, > - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }, > - { PCI_VENDOR_ID_DEC, PCI_DEVICE_ID_DEC_TULIP_PLUS, > - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 1 }, > + { PCI_VDEVICE(DEC, PCI_DEVICE_ID_DEC_TULIP), .driver_data = 0 }, > + { PCI_VDEVICE(DEC, PCI_DEVICE_ID_DEC_TULIP_PLUS), .driver_data = 1 }, > { }, Drop comma. I.o.w. please make sure you also unify the style of the ID tables, including terminator entries. > }; ... > static const struct pci_device_id sis190_pci_tbl[] = { > - { PCI_DEVICE(PCI_VENDOR_ID_SI, 0x0190), 0, 0, 0 }, > - { PCI_DEVICE(PCI_VENDOR_ID_SI, 0x0191), 0, 0, 1 }, > - { 0, }, > + { PCI_VDEVICE(SI, 0x0190), .driver_data = 0 }, > + { PCI_VDEVICE(SI, 0x0191), .driver_data = 1 }, > + { }, Ditto and so on... > }; ... Also I somehow managed to remove, but I remember you had an inner comma in some cases after the .driver_data, when the full ID entry is located on a single line. I.o.w. do { PCI_...(), .driver_data = ... // no trailing comma here! }, When it's a single line trailing comma inside helps nothing and just makes lines longer and harder to read. -- With Best Regards, Andy Shevchenko