On Wed, 2024-11-13 at 17:04 +0100, Thomas Gleixner wrote:

On Wed, Nov 13 2024 at 13:41, Philipp Stanner wrote:

quoted

+/**
+ * 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

quoted

+ * Enables/disables PCI INTx for device @pdev
+ *
+ * This function behavios identically to pci_intx(), but is never
managed with
+ * devres.

behavios?

-> behaves. Will fix.

quoted

+ */
+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@redhat.com/ (local)


Please also take a look at patch 11, then you'll see the full picture

Danke,
Philipp

Thanks,

        tglx