On Wed, 2024-11-13 at 17:04 +0100, Thomas Gleixner wrote: > On Wed, Nov 13 2024 at 13:41, Philipp Stanner wrote: > > +/** > > + * pci_intx_unmanaged - enables/disables PCI INTx for device dev, > > + * unmanaged version > > + * @pdev: the PCI device to operate on > > + * @enable: boolean: whether to enable or disable PCI INTx > > Except that the argument is of type int, which really should be type > bool. True, but this is a *temporary* copy of pci_intx(), a ~16 year old function. Older C programmers had the habit of for some reason using 32-bit integers for a true/false boolean all the time. We _could_ think of changing pci_intx()'s parameter to a boolean, but I think it wouldn't really improve things very much see also below > > > + * Enables/disables PCI INTx for device @pdev > > + * > > + * This function behavios identically to pci_intx(), but is never > > managed with > > + * devres. > > behavios? -> behaves. Will fix. > > > + */ > > +void pci_intx_unmanaged(struct pci_dev *pdev, int enable) > > I find this function name mildy confusing. This _unmanaged suffix is > not > really telling me anything. And the reference that this behaves > identically to pci_intx() makes it even worse. > > This function is about controlling the PCI INTX_DISABLE bit in the > PCI_COMMAND config word, right? > > So naming it pci_intx_control() would make it entirely clear what > this > is about, no? We had this conversation last week. I answered on that already, maybe you have overlooked it: https://lore.kernel.org/all/a8d9f32f60f55c58d79943c4409b8b94535ff853.camel@xxxxxxxxxx/ Please also take a look at patch 11, then you'll see the full picture Danke, Philipp > > Thanks, > > tglx >
