* [PATCH -next] crypto: inside-secure - Fix build error without CONFIG_PCI @ 2019-09-02 14:19 YueHaibing 2019-09-03 1:32 ` Ard Biesheuvel 2019-09-03 1:45 ` [PATCH v2 " YueHaibing 0 siblings, 2 replies; 17+ messages in thread From: YueHaibing @ 2019-09-02 14:19 UTC (permalink / raw) To: antoine.tenart, herbert, davem, pvanleeuwen Cc: linux-crypto, linux-kernel, YueHaibing If CONFIG_PCI is not set, building fails: rivers/crypto/inside-secure/safexcel.c: In function safexcel_request_ring_irq: drivers/crypto/inside-secure/safexcel.c:944:9: error: implicit declaration of function pci_irq_vector; did you mean rcu_irq_enter? [-Werror=implicit-function-declaration] irq = pci_irq_vector(pci_pdev, irqid); ^~~~~~~~~~~~~~ Use #ifdef block to guard this. Fixes: 625f269a5a7a ("crypto: inside-secure - add support for PCI based FPGA development board") Signed-off-by: YueHaibing <yuehaibing@huawei.com> --- drivers/crypto/inside-secure/safexcel.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c index e12a2a3..c23fe34 100644 --- a/drivers/crypto/inside-secure/safexcel.c +++ b/drivers/crypto/inside-secure/safexcel.c @@ -937,7 +937,8 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, int ret, irq; struct device *dev; - if (IS_ENABLED(CONFIG_PCI) && is_pci_dev) { +#if IS_ENABLED(CONFIG_PCI) + if (is_pci_dev) { struct pci_dev *pci_pdev = pdev; dev = &pci_pdev->dev; @@ -947,7 +948,10 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, irqid, irq); return irq; } - } else if (IS_ENABLED(CONFIG_OF)) { + } else +#endif + { +#if IS_ENABLED(CONFIG_OF) struct platform_device *plf_pdev = pdev; char irq_name[6] = {0}; /* "ringX\0" */ @@ -960,6 +964,7 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, irq_name, irq); return irq; } +#endif } ret = devm_request_threaded_irq(dev, irq, handler, @@ -1137,7 +1142,8 @@ static int safexcel_probe_generic(void *pdev, safexcel_configure(priv); - if (IS_ENABLED(CONFIG_PCI) && priv->version == EIP197_DEVBRD) { +#if IS_ENABLED(CONFIG_PCI) + if (priv->version == EIP197_DEVBRD) { /* * Request MSI vectors for global + 1 per ring - * or just 1 for older dev images @@ -1153,6 +1159,7 @@ static int safexcel_probe_generic(void *pdev, return ret; } } +#endif /* Register the ring IRQ handlers and configure the rings */ priv->ring = devm_kcalloc(dev, priv->config.rings, -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH -next] crypto: inside-secure - Fix build error without CONFIG_PCI 2019-09-02 14:19 [PATCH -next] crypto: inside-secure - Fix build error without CONFIG_PCI YueHaibing @ 2019-09-03 1:32 ` Ard Biesheuvel 2019-09-03 1:45 ` [PATCH v2 " YueHaibing 1 sibling, 0 replies; 17+ messages in thread From: Ard Biesheuvel @ 2019-09-03 1:32 UTC (permalink / raw) To: YueHaibing Cc: Antoine Tenart, Herbert Xu, David S. Miller, Pascal Van Leeuwen, open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Linux Kernel Mailing List On Mon, 2 Sep 2019 at 07:19, YueHaibing <yuehaibing@huawei.com> wrote: > > If CONFIG_PCI is not set, building fails: > > rivers/crypto/inside-secure/safexcel.c: In function safexcel_request_ring_irq: > drivers/crypto/inside-secure/safexcel.c:944:9: error: implicit declaration of function pci_irq_vector; > did you mean rcu_irq_enter? [-Werror=implicit-function-declaration] > irq = pci_irq_vector(pci_pdev, irqid); > ^~~~~~~~~~~~~~ > > Use #ifdef block to guard this. > > Fixes: 625f269a5a7a ("crypto: inside-secure - add support for PCI based FPGA development board") > Signed-off-by: YueHaibing <yuehaibing@huawei.com> > --- > drivers/crypto/inside-secure/safexcel.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c > index e12a2a3..c23fe34 100644 > --- a/drivers/crypto/inside-secure/safexcel.c > +++ b/drivers/crypto/inside-secure/safexcel.c > @@ -937,7 +937,8 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > int ret, irq; > struct device *dev; > > - if (IS_ENABLED(CONFIG_PCI) && is_pci_dev) { > +#if IS_ENABLED(CONFIG_PCI) please don't use IS_ENABLED() with preprocessor conditionals - just use #ifdef CONFIG_PCI instead > + if (is_pci_dev) { > struct pci_dev *pci_pdev = pdev; > > dev = &pci_pdev->dev; > @@ -947,7 +948,10 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > irqid, irq); > return irq; > } > - } else if (IS_ENABLED(CONFIG_OF)) { > + } else > +#endif > + { > +#if IS_ENABLED(CONFIG_OF) > struct platform_device *plf_pdev = pdev; > char irq_name[6] = {0}; /* "ringX\0" */ > > @@ -960,6 +964,7 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > irq_name, irq); > return irq; > } > +#endif > } > > ret = devm_request_threaded_irq(dev, irq, handler, > @@ -1137,7 +1142,8 @@ static int safexcel_probe_generic(void *pdev, > > safexcel_configure(priv); > > - if (IS_ENABLED(CONFIG_PCI) && priv->version == EIP197_DEVBRD) { > +#if IS_ENABLED(CONFIG_PCI) > + if (priv->version == EIP197_DEVBRD) { > /* > * Request MSI vectors for global + 1 per ring - > * or just 1 for older dev images > @@ -1153,6 +1159,7 @@ static int safexcel_probe_generic(void *pdev, > return ret; > } > } > +#endif > > /* Register the ring IRQ handlers and configure the rings */ > priv->ring = devm_kcalloc(dev, priv->config.rings, > -- > 2.7.4 > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI 2019-09-02 14:19 [PATCH -next] crypto: inside-secure - Fix build error without CONFIG_PCI YueHaibing 2019-09-03 1:32 ` Ard Biesheuvel @ 2019-09-03 1:45 ` YueHaibing 2019-09-04 11:57 ` Pascal Van Leeuwen 1 sibling, 1 reply; 17+ messages in thread From: YueHaibing @ 2019-09-03 1:45 UTC (permalink / raw) To: antoine.tenart, herbert, davem, pvanleeuwen Cc: linux-crypto, linux-kernel, YueHaibing If CONFIG_PCI is not set, building fails: rivers/crypto/inside-secure/safexcel.c: In function safexcel_request_ring_irq: drivers/crypto/inside-secure/safexcel.c:944:9: error: implicit declaration of function pci_irq_vector; did you mean rcu_irq_enter? [-Werror=implicit-function-declaration] irq = pci_irq_vector(pci_pdev, irqid); ^~~~~~~~~~~~~~ Use #ifdef block to guard this. Fixes: 625f269a5a7a ("crypto: inside-secure - add support for PCI based FPGA development board") Signed-off-by: YueHaibing <yuehaibing@huawei.com> --- v2: use 'ifdef' instead of 'IS_ENABLED' --- drivers/crypto/inside-secure/safexcel.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c index e12a2a3..5253900 100644 --- a/drivers/crypto/inside-secure/safexcel.c +++ b/drivers/crypto/inside-secure/safexcel.c @@ -937,7 +937,8 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, int ret, irq; struct device *dev; - if (IS_ENABLED(CONFIG_PCI) && is_pci_dev) { +#ifdef CONFIG_PCI + if (is_pci_dev) { struct pci_dev *pci_pdev = pdev; dev = &pci_pdev->dev; @@ -947,7 +948,10 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, irqid, irq); return irq; } - } else if (IS_ENABLED(CONFIG_OF)) { + } else +#endif + { +#ifdef CONFIG_OF struct platform_device *plf_pdev = pdev; char irq_name[6] = {0}; /* "ringX\0" */ @@ -960,6 +964,7 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, irq_name, irq); return irq; } +#endif } ret = devm_request_threaded_irq(dev, irq, handler, @@ -1137,7 +1142,8 @@ static int safexcel_probe_generic(void *pdev, safexcel_configure(priv); - if (IS_ENABLED(CONFIG_PCI) && priv->version == EIP197_DEVBRD) { +#ifdef CONFIG_PCI + if (priv->version == EIP197_DEVBRD) { /* * Request MSI vectors for global + 1 per ring - * or just 1 for older dev images @@ -1153,6 +1159,7 @@ static int safexcel_probe_generic(void *pdev, return ret; } } +#endif /* Register the ring IRQ handlers and configure the rings */ priv->ring = devm_kcalloc(dev, priv->config.rings, -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI 2019-09-03 1:45 ` [PATCH v2 " YueHaibing @ 2019-09-04 11:57 ` Pascal Van Leeuwen 2019-09-04 12:10 ` Ard Biesheuvel 0 siblings, 1 reply; 17+ messages in thread From: Pascal Van Leeuwen @ 2019-09-04 11:57 UTC (permalink / raw) To: YueHaibing, antoine.tenart, herbert, davem, pvanleeuwen Cc: linux-crypto, linux-kernel > -----Original Message----- > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of > YueHaibing > Sent: Tuesday, September 3, 2019 3:45 AM > To: antoine.tenart@bootlin.com; herbert@gondor.apana.org.au; davem@davemloft.net; > pvanleeuwen@insidesecure.com > Cc: linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org; YueHaibing > <yuehaibing@huawei.com> > Subject: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI > > If CONFIG_PCI is not set, building fails: > > rivers/crypto/inside-secure/safexcel.c: In function safexcel_request_ring_irq: > drivers/crypto/inside-secure/safexcel.c:944:9: error: implicit declaration of function > pci_irq_vector; > did you mean rcu_irq_enter? [-Werror=implicit-function-declaration] > irq = pci_irq_vector(pci_pdev, irqid); > ^~~~~~~~~~~~~~ > > Use #ifdef block to guard this. > Actually, this is interesting. My *original* implementation was using straight #ifdefs, but then I got review feedback stating that I should not do that, as it's not compile testable, suggesting to use regular C if's instead. Then there was quite some back-and-forth on the actual implementation and I ended up with this. So now it turns out that doesn't work and I'm suggested to go full-circle back to straight #ifdef's? Or is there some other way to make this work? Because I don't know where to go from here ... > Fixes: 625f269a5a7a ("crypto: inside-secure - add support for PCI based FPGA development > board") > Signed-off-by: YueHaibing <yuehaibing@huawei.com> > --- > v2: use 'ifdef' instead of 'IS_ENABLED' > --- > drivers/crypto/inside-secure/safexcel.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside- > secure/safexcel.c > index e12a2a3..5253900 100644 > --- a/drivers/crypto/inside-secure/safexcel.c > +++ b/drivers/crypto/inside-secure/safexcel.c > @@ -937,7 +937,8 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > int ret, irq; > struct device *dev; > > - if (IS_ENABLED(CONFIG_PCI) && is_pci_dev) { > +#ifdef CONFIG_PCI > + if (is_pci_dev) { > struct pci_dev *pci_pdev = pdev; > > dev = &pci_pdev->dev; > @@ -947,7 +948,10 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > irqid, irq); > return irq; > } > - } else if (IS_ENABLED(CONFIG_OF)) { > + } else > +#endif > + { > +#ifdef CONFIG_OF > struct platform_device *plf_pdev = pdev; > char irq_name[6] = {0}; /* "ringX\0" */ > > @@ -960,6 +964,7 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > irq_name, irq); > return irq; > } > +#endif > } > > ret = devm_request_threaded_irq(dev, irq, handler, > @@ -1137,7 +1142,8 @@ static int safexcel_probe_generic(void *pdev, > > safexcel_configure(priv); > > - if (IS_ENABLED(CONFIG_PCI) && priv->version == EIP197_DEVBRD) { > +#ifdef CONFIG_PCI > + if (priv->version == EIP197_DEVBRD) { > /* > * Request MSI vectors for global + 1 per ring - > * or just 1 for older dev images > @@ -1153,6 +1159,7 @@ static int safexcel_probe_generic(void *pdev, > return ret; > } > } > +#endif > > /* Register the ring IRQ handlers and configure the rings */ > priv->ring = devm_kcalloc(dev, priv->config.rings, > -- > 2.7.4 > Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI 2019-09-04 11:57 ` Pascal Van Leeuwen @ 2019-09-04 12:10 ` Ard Biesheuvel 2019-09-04 12:25 ` Pascal Van Leeuwen 2019-09-04 12:26 ` PCI: Add stub pci_irq_vector and others Herbert Xu 0 siblings, 2 replies; 17+ messages in thread From: Ard Biesheuvel @ 2019-09-04 12:10 UTC (permalink / raw) To: Pascal Van Leeuwen Cc: YueHaibing, antoine.tenart, herbert, davem, pvanleeuwen, linux-crypto, linux-kernel On Wed, 4 Sep 2019 at 04:57, Pascal Van Leeuwen <pvanleeuwen@verimatrix.com> wrote: > > > > -----Original Message----- > > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of > > YueHaibing > > Sent: Tuesday, September 3, 2019 3:45 AM > > To: antoine.tenart@bootlin.com; herbert@gondor.apana.org.au; davem@davemloft.net; > > pvanleeuwen@insidesecure.com > > Cc: linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org; YueHaibing > > <yuehaibing@huawei.com> > > Subject: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI > > > > If CONFIG_PCI is not set, building fails: > > > > rivers/crypto/inside-secure/safexcel.c: In function safexcel_request_ring_irq: > > drivers/crypto/inside-secure/safexcel.c:944:9: error: implicit declaration of function > > pci_irq_vector; > > did you mean rcu_irq_enter? [-Werror=implicit-function-declaration] > > irq = pci_irq_vector(pci_pdev, irqid); > > ^~~~~~~~~~~~~~ > > > > Use #ifdef block to guard this. > > > Actually, this is interesting. My *original* implementation was using > straight #ifdefs, but then I got review feedback stating that I should not > do that, as it's not compile testable, suggesting to use regular C if's > instead. Then there was quite some back-and-forth on the actual > implementation and I ended up with this. > > So now it turns out that doesn't work and I'm suggested to go full-circle > back to straight #ifdef's? Or is there some other way to make this work? > Because I don't know where to go from here ... > C conditionals are preferred over preprocessor conditional, but if the conditional code refers to symbols that are not declared when the Kconfig symbol is not defined, preprocessor conditionals are the only option. This is the reason we have so many empty static inline functions in header files - it ensures that the symbols are declared even if the only invocations are from dead code. > > Fixes: 625f269a5a7a ("crypto: inside-secure - add support for PCI based FPGA development > > board") > > Signed-off-by: YueHaibing <yuehaibing@huawei.com> > > --- > > v2: use 'ifdef' instead of 'IS_ENABLED' > > --- > > drivers/crypto/inside-secure/safexcel.c | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside- > > secure/safexcel.c > > index e12a2a3..5253900 100644 > > --- a/drivers/crypto/inside-secure/safexcel.c > > +++ b/drivers/crypto/inside-secure/safexcel.c > > @@ -937,7 +937,8 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > > int ret, irq; > > struct device *dev; > > > > - if (IS_ENABLED(CONFIG_PCI) && is_pci_dev) { > > +#ifdef CONFIG_PCI > > + if (is_pci_dev) { > > struct pci_dev *pci_pdev = pdev; > > > > dev = &pci_pdev->dev; > > @@ -947,7 +948,10 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > > irqid, irq); > > return irq; > > } > > - } else if (IS_ENABLED(CONFIG_OF)) { > > + } else > > +#endif > > + { > > +#ifdef CONFIG_OF > > struct platform_device *plf_pdev = pdev; > > char irq_name[6] = {0}; /* "ringX\0" */ > > > > @@ -960,6 +964,7 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > > irq_name, irq); > > return irq; > > } > > +#endif > > } > > > > ret = devm_request_threaded_irq(dev, irq, handler, > > @@ -1137,7 +1142,8 @@ static int safexcel_probe_generic(void *pdev, > > > > safexcel_configure(priv); > > > > - if (IS_ENABLED(CONFIG_PCI) && priv->version == EIP197_DEVBRD) { > > +#ifdef CONFIG_PCI > > + if (priv->version == EIP197_DEVBRD) { > > /* > > * Request MSI vectors for global + 1 per ring - > > * or just 1 for older dev images > > @@ -1153,6 +1159,7 @@ static int safexcel_probe_generic(void *pdev, > > return ret; > > } > > } > > +#endif > > > > /* Register the ring IRQ handlers and configure the rings */ > > priv->ring = devm_kcalloc(dev, priv->config.rings, > > -- > > 2.7.4 > > > > Regards, > Pascal van Leeuwen > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix > www.insidesecure.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI 2019-09-04 12:10 ` Ard Biesheuvel @ 2019-09-04 12:25 ` Pascal Van Leeuwen 2019-09-04 12:27 ` Ard Biesheuvel 2019-09-04 12:26 ` PCI: Add stub pci_irq_vector and others Herbert Xu 1 sibling, 1 reply; 17+ messages in thread From: Pascal Van Leeuwen @ 2019-09-04 12:25 UTC (permalink / raw) To: Ard Biesheuvel Cc: YueHaibing, antoine.tenart, herbert, davem, pvanleeuwen, linux-crypto, linux-kernel > -----Original Message----- > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Sent: Wednesday, September 4, 2019 2:11 PM > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com> > Cc: YueHaibing <yuehaibing@huawei.com>; antoine.tenart@bootlin.com; > herbert@gondor.apana.org.au; davem@davemloft.net; pvanleeuwen@insidesecure.com; linux- > crypto@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI > > On Wed, 4 Sep 2019 at 04:57, Pascal Van Leeuwen > <pvanleeuwen@verimatrix.com> wrote: > > > > > > > -----Original Message----- > > > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On > Behalf Of > > > YueHaibing > > > Sent: Tuesday, September 3, 2019 3:45 AM > > > To: antoine.tenart@bootlin.com; herbert@gondor.apana.org.au; davem@davemloft.net; > > > pvanleeuwen@insidesecure.com > > > Cc: linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org; YueHaibing > > > <yuehaibing@huawei.com> > > > Subject: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI > > > > > > If CONFIG_PCI is not set, building fails: > > > > > > rivers/crypto/inside-secure/safexcel.c: In function safexcel_request_ring_irq: > > > drivers/crypto/inside-secure/safexcel.c:944:9: error: implicit declaration of function > > > pci_irq_vector; > > > did you mean rcu_irq_enter? [-Werror=implicit-function-declaration] > > > irq = pci_irq_vector(pci_pdev, irqid); > > > ^~~~~~~~~~~~~~ > > > > > > Use #ifdef block to guard this. > > > > > Actually, this is interesting. My *original* implementation was using > > straight #ifdefs, but then I got review feedback stating that I should not > > do that, as it's not compile testable, suggesting to use regular C if's > > instead. Then there was quite some back-and-forth on the actual > > implementation and I ended up with this. > > > > So now it turns out that doesn't work and I'm suggested to go full-circle > > back to straight #ifdef's? Or is there some other way to make this work? > > Because I don't know where to go from here ... > > > > > C conditionals are preferred over preprocessor conditional, but if the > conditional code refers to symbols that are not declared when the > Kconfig symbol is not defined, preprocessor conditionals are the only > option. > Sure, I get that. But I *had* the #ifdef's and then other people told me to get rid of them. How is one supposed to know when which symbols are declared exactly? Moreover, I feel that if #ifdef's are sometimes the only way, then you should be careful providing feedback on the subject. > This is the reason we have so many empty static inline functions in > header files - it ensures that the symbols are declared even if the > only invocations are from dead code. > This ties back into my previous question: how am I supposed to know whether stuff is nicely covered by these empty static inlines or not? If this happens to be a hit-and-miss affair. Note that I tested the code with the 2 platforms at my disposal - actually the only 2 relevant platforms for this driver, if you ask me - and they both compiled just fine, so I had no way of finding this "problem" myself. > > > > Fixes: 625f269a5a7a ("crypto: inside-secure - add support for PCI based FPGA > development > > > board") > > > Signed-off-by: YueHaibing <yuehaibing@huawei.com> > > > --- > > > v2: use 'ifdef' instead of 'IS_ENABLED' > > > --- > > > drivers/crypto/inside-secure/safexcel.c | 13 ++++++++++--- > > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside- > > > secure/safexcel.c > > > index e12a2a3..5253900 100644 > > > --- a/drivers/crypto/inside-secure/safexcel.c > > > +++ b/drivers/crypto/inside-secure/safexcel.c > > > @@ -937,7 +937,8 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > > > int ret, irq; > > > struct device *dev; > > > > > > - if (IS_ENABLED(CONFIG_PCI) && is_pci_dev) { > > > +#ifdef CONFIG_PCI > > > + if (is_pci_dev) { > > > struct pci_dev *pci_pdev = pdev; > > > > > > dev = &pci_pdev->dev; > > > @@ -947,7 +948,10 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > > > irqid, irq); > > > return irq; > > > } > > > - } else if (IS_ENABLED(CONFIG_OF)) { > > > + } else > > > +#endif > > > + { > > > +#ifdef CONFIG_OF > > > struct platform_device *plf_pdev = pdev; > > > char irq_name[6] = {0}; /* "ringX\0" */ > > > > > > @@ -960,6 +964,7 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > > > irq_name, irq); > > > return irq; > > > } > > > +#endif > > > } > > > > > > ret = devm_request_threaded_irq(dev, irq, handler, > > > @@ -1137,7 +1142,8 @@ static int safexcel_probe_generic(void *pdev, > > > > > > safexcel_configure(priv); > > > > > > - if (IS_ENABLED(CONFIG_PCI) && priv->version == EIP197_DEVBRD) { > > > +#ifdef CONFIG_PCI > > > + if (priv->version == EIP197_DEVBRD) { > > > /* > > > * Request MSI vectors for global + 1 per ring - > > > * or just 1 for older dev images > > > @@ -1153,6 +1159,7 @@ static int safexcel_probe_generic(void *pdev, > > > return ret; > > > } > > > } > > > +#endif > > > > > > /* Register the ring IRQ handlers and configure the rings */ > > > priv->ring = devm_kcalloc(dev, priv->config.rings, > > > -- > > > 2.7.4 > > > > > > > Regards, > > Pascal van Leeuwen > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix > > www.insidesecure.com Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI 2019-09-04 12:25 ` Pascal Van Leeuwen @ 2019-09-04 12:27 ` Ard Biesheuvel 2019-09-04 12:31 ` Herbert Xu 2019-09-04 12:43 ` Pascal Van Leeuwen 0 siblings, 2 replies; 17+ messages in thread From: Ard Biesheuvel @ 2019-09-04 12:27 UTC (permalink / raw) To: Pascal Van Leeuwen Cc: YueHaibing, antoine.tenart, herbert, davem, pvanleeuwen, linux-crypto, linux-kernel On Wed, 4 Sep 2019 at 05:25, Pascal Van Leeuwen <pvanleeuwen@verimatrix.com> wrote: > > > -----Original Message----- > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Sent: Wednesday, September 4, 2019 2:11 PM > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com> > > Cc: YueHaibing <yuehaibing@huawei.com>; antoine.tenart@bootlin.com; > > herbert@gondor.apana.org.au; davem@davemloft.net; pvanleeuwen@insidesecure.com; linux- > > crypto@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI > > > > On Wed, 4 Sep 2019 at 04:57, Pascal Van Leeuwen > > <pvanleeuwen@verimatrix.com> wrote: > > > > > > > > > > -----Original Message----- > > > > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On > > Behalf Of > > > > YueHaibing > > > > Sent: Tuesday, September 3, 2019 3:45 AM > > > > To: antoine.tenart@bootlin.com; herbert@gondor.apana.org.au; davem@davemloft.net; > > > > pvanleeuwen@insidesecure.com > > > > Cc: linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org; YueHaibing > > > > <yuehaibing@huawei.com> > > > > Subject: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI > > > > > > > > If CONFIG_PCI is not set, building fails: > > > > > > > > rivers/crypto/inside-secure/safexcel.c: In function safexcel_request_ring_irq: > > > > drivers/crypto/inside-secure/safexcel.c:944:9: error: implicit declaration of function > > > > pci_irq_vector; > > > > did you mean rcu_irq_enter? [-Werror=implicit-function-declaration] > > > > irq = pci_irq_vector(pci_pdev, irqid); > > > > ^~~~~~~~~~~~~~ > > > > > > > > Use #ifdef block to guard this. > > > > > > > Actually, this is interesting. My *original* implementation was using > > > straight #ifdefs, but then I got review feedback stating that I should not > > > do that, as it's not compile testable, suggesting to use regular C if's > > > instead. Then there was quite some back-and-forth on the actual > > > implementation and I ended up with this. > > > > > > So now it turns out that doesn't work and I'm suggested to go full-circle > > > back to straight #ifdef's? Or is there some other way to make this work? > > > Because I don't know where to go from here ... > > > > > > > > > C conditionals are preferred over preprocessor conditional, but if the > > conditional code refers to symbols that are not declared when the > > Kconfig symbol is not defined, preprocessor conditionals are the only > > option. > > > Sure, I get that. But I *had* the #ifdef's and then other people told me > to get rid of them. How is one supposed to know when which symbols are > declared exactly? Moreover, I feel that if #ifdef's are sometimes the > only way, then you should be careful providing feedback on the subject. > If you compile your code with and without the Kconfig symbol defined, the compiler will tell you if there is a problem or not. > > This is the reason we have so many empty static inline functions in > > header files - it ensures that the symbols are declared even if the > > only invocations are from dead code. > > > This ties back into my previous question: how am I supposed to know whether > stuff is nicely covered by these empty static inlines or not? If this > happens to be a hit-and-miss affair. > Indeed. > Note that I tested the code with the 2 platforms at my disposal - actually > the only 2 relevant platforms for this driver, if you ask me - and they > both compiled just fine, so I had no way of finding this "problem" myself. > Did you try disabling CONFIG_PCI? > > > > > > Fixes: 625f269a5a7a ("crypto: inside-secure - add support for PCI based FPGA > > development > > > > board") > > > > Signed-off-by: YueHaibing <yuehaibing@huawei.com> > > > > --- > > > > v2: use 'ifdef' instead of 'IS_ENABLED' > > > > --- > > > > drivers/crypto/inside-secure/safexcel.c | 13 ++++++++++--- > > > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside- > > > > secure/safexcel.c > > > > index e12a2a3..5253900 100644 > > > > --- a/drivers/crypto/inside-secure/safexcel.c > > > > +++ b/drivers/crypto/inside-secure/safexcel.c > > > > @@ -937,7 +937,8 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > > > > int ret, irq; > > > > struct device *dev; > > > > > > > > - if (IS_ENABLED(CONFIG_PCI) && is_pci_dev) { > > > > +#ifdef CONFIG_PCI > > > > + if (is_pci_dev) { > > > > struct pci_dev *pci_pdev = pdev; > > > > > > > > dev = &pci_pdev->dev; > > > > @@ -947,7 +948,10 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > > > > irqid, irq); > > > > return irq; > > > > } > > > > - } else if (IS_ENABLED(CONFIG_OF)) { > > > > + } else > > > > +#endif > > > > + { > > > > +#ifdef CONFIG_OF > > > > struct platform_device *plf_pdev = pdev; > > > > char irq_name[6] = {0}; /* "ringX\0" */ > > > > > > > > @@ -960,6 +964,7 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > > > > irq_name, irq); > > > > return irq; > > > > } > > > > +#endif > > > > } > > > > > > > > ret = devm_request_threaded_irq(dev, irq, handler, > > > > @@ -1137,7 +1142,8 @@ static int safexcel_probe_generic(void *pdev, > > > > > > > > safexcel_configure(priv); > > > > > > > > - if (IS_ENABLED(CONFIG_PCI) && priv->version == EIP197_DEVBRD) { > > > > +#ifdef CONFIG_PCI > > > > + if (priv->version == EIP197_DEVBRD) { > > > > /* > > > > * Request MSI vectors for global + 1 per ring - > > > > * or just 1 for older dev images > > > > @@ -1153,6 +1159,7 @@ static int safexcel_probe_generic(void *pdev, > > > > return ret; > > > > } > > > > } > > > > +#endif > > > > > > > > /* Register the ring IRQ handlers and configure the rings */ > > > > priv->ring = devm_kcalloc(dev, priv->config.rings, > > > > -- > > > > 2.7.4 > > > > > > > > > > Regards, > > > Pascal van Leeuwen > > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix > > > www.insidesecure.com > > Regards, > Pascal van Leeuwen > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix > www.insidesecure.com > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI 2019-09-04 12:27 ` Ard Biesheuvel @ 2019-09-04 12:31 ` Herbert Xu 2019-09-04 12:45 ` Pascal Van Leeuwen 2019-09-04 12:43 ` Pascal Van Leeuwen 1 sibling, 1 reply; 17+ messages in thread From: Herbert Xu @ 2019-09-04 12:31 UTC (permalink / raw) To: Ard Biesheuvel Cc: Pascal Van Leeuwen, YueHaibing, antoine.tenart, davem, pvanleeuwen, linux-crypto, linux-kernel On Wed, Sep 04, 2019 at 05:27:19AM -0700, Ard Biesheuvel wrote: > > Did you try disabling CONFIG_PCI? Indeed. Even with my patch if you compile with CONFIG_PCI you still get a warning: CC [M] drivers/crypto/inside-secure/safexcel.o ../drivers/crypto/inside-secure/safexcel.c: In function \u2018safexcel_init\u2019: ../drivers/crypto/inside-secure/safexcel.c:1506:6: warning: unused variable \u2018rc\u2019 [-Wunused-variable] int rc; ^~ We should fix that in inside-secure. Thanks, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI 2019-09-04 12:31 ` Herbert Xu @ 2019-09-04 12:45 ` Pascal Van Leeuwen 2019-09-04 13:08 ` Herbert Xu 0 siblings, 1 reply; 17+ messages in thread From: Pascal Van Leeuwen @ 2019-09-04 12:45 UTC (permalink / raw) To: Herbert Xu, Ard Biesheuvel Cc: YueHaibing, antoine.tenart, davem, pvanleeuwen, linux-crypto, linux-kernel So, with that patch you ONLY get a warning on that unused int rc, right? I do understand that one, that should have been inside an #ifdef as well. Everybody happy if I just fix that and leave the rest as is? Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com > -----Original Message----- > From: Herbert Xu <herbert@gondor.apana.org.au> > Sent: Wednesday, September 4, 2019 2:32 PM > To: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>; YueHaibing <yuehaibing@huawei.com>; > antoine.tenart@bootlin.com; davem@davemloft.net; pvanleeuwen@insidesecure.com; linux- > crypto@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI > > On Wed, Sep 04, 2019 at 05:27:19AM -0700, Ard Biesheuvel wrote: > > > > Did you try disabling CONFIG_PCI? > > Indeed. Even with my patch if you compile with CONFIG_PCI you still > get a warning: > > CC [M] drivers/crypto/inside-secure/safexcel.o > ../drivers/crypto/inside-secure/safexcel.c: In function \u2018safexcel_init\u2019: > ../drivers/crypto/inside-secure/safexcel.c:1506:6: warning: unused variable \u2018rc\u2019 > [-Wunused-variable] > int rc; > ^~ > > We should fix that in inside-secure. > > Thanks, > -- > Email: Herbert Xu <herbert@gondor.apana.org.au> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI 2019-09-04 12:45 ` Pascal Van Leeuwen @ 2019-09-04 13:08 ` Herbert Xu 0 siblings, 0 replies; 17+ messages in thread From: Herbert Xu @ 2019-09-04 13:08 UTC (permalink / raw) To: Pascal Van Leeuwen Cc: Ard Biesheuvel, YueHaibing, antoine.tenart, davem, pvanleeuwen, linux-crypto, linux-kernel On Wed, Sep 04, 2019 at 12:45:00PM +0000, Pascal Van Leeuwen wrote: > So, with that patch you ONLY get a warning on that unused int rc, right? > > I do understand that one, that should have been inside an #ifdef as well. > Everybody happy if I just fix that and leave the rest as is? Yes please send a patch to fix the unused variable warning. Thanks, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI 2019-09-04 12:27 ` Ard Biesheuvel 2019-09-04 12:31 ` Herbert Xu @ 2019-09-04 12:43 ` Pascal Van Leeuwen 1 sibling, 0 replies; 17+ messages in thread From: Pascal Van Leeuwen @ 2019-09-04 12:43 UTC (permalink / raw) To: Ard Biesheuvel Cc: YueHaibing, antoine.tenart, herbert, davem, pvanleeuwen, linux-crypto, linux-kernel > -----Original Message----- > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Sent: Wednesday, September 4, 2019 2:27 PM > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com> > Cc: YueHaibing <yuehaibing@huawei.com>; antoine.tenart@bootlin.com; > herbert@gondor.apana.org.au; davem@davemloft.net; pvanleeuwen@insidesecure.com; linux- > crypto@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI > > On Wed, 4 Sep 2019 at 05:25, Pascal Van Leeuwen > <pvanleeuwen@verimatrix.com> wrote: > > > > > -----Original Message----- > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > Sent: Wednesday, September 4, 2019 2:11 PM > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com> > > > Cc: YueHaibing <yuehaibing@huawei.com>; antoine.tenart@bootlin.com; > > > herbert@gondor.apana.org.au; davem@davemloft.net; pvanleeuwen@insidesecure.com; linux- > > > crypto@vger.kernel.org; linux-kernel@vger.kernel.org > > > Subject: Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without > CONFIG_PCI > > > > > > On Wed, 4 Sep 2019 at 04:57, Pascal Van Leeuwen > > > <pvanleeuwen@verimatrix.com> wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On > > > Behalf Of > > > > > YueHaibing > > > > > Sent: Tuesday, September 3, 2019 3:45 AM > > > > > To: antoine.tenart@bootlin.com; herbert@gondor.apana.org.au; davem@davemloft.net; > > > > > pvanleeuwen@insidesecure.com > > > > > Cc: linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org; YueHaibing > > > > > <yuehaibing@huawei.com> > > > > > Subject: [PATCH v2 -next] crypto: inside-secure - Fix build error without > CONFIG_PCI > > > > > > > > > > If CONFIG_PCI is not set, building fails: > > > > > > > > > > rivers/crypto/inside-secure/safexcel.c: In function safexcel_request_ring_irq: > > > > > drivers/crypto/inside-secure/safexcel.c:944:9: error: implicit declaration of > function > > > > > pci_irq_vector; > > > > > did you mean rcu_irq_enter? [-Werror=implicit-function-declaration] > > > > > irq = pci_irq_vector(pci_pdev, irqid); > > > > > ^~~~~~~~~~~~~~ > > > > > > > > > > Use #ifdef block to guard this. > > > > > > > > > Actually, this is interesting. My *original* implementation was using > > > > straight #ifdefs, but then I got review feedback stating that I should not > > > > do that, as it's not compile testable, suggesting to use regular C if's > > > > instead. Then there was quite some back-and-forth on the actual > > > > implementation and I ended up with this. > > > > > > > > So now it turns out that doesn't work and I'm suggested to go full-circle > > > > back to straight #ifdef's? Or is there some other way to make this work? > > > > Because I don't know where to go from here ... > > > > > > > > > > > > > C conditionals are preferred over preprocessor conditional, but if the > > > conditional code refers to symbols that are not declared when the > > > Kconfig symbol is not defined, preprocessor conditionals are the only > > > option. > > > > > Sure, I get that. But I *had* the #ifdef's and then other people told me > > to get rid of them. How is one supposed to know when which symbols are > > declared exactly? Moreover, I feel that if #ifdef's are sometimes the > > only way, then you should be careful providing feedback on the subject. > > > > If you compile your code with and without the Kconfig symbol defined, > the compiler will tell you if there is a problem or not. > Probably. Maybe. I don't know much about configuring Linux kernels (I'm just happy with working configs provided to me) so I don't know what problems that might give (beyond those in my own code). Actually, I assumed the Macchiatobin config did not have PCI (as Antoine asked me to ifdef that stuff out IIRC), so there was no incentive for me to try explictly. But it turns out the Macchiatobin config has PCI after all. Which makes me wonder what the point of #ifdef'ing out the PCI stuff is in the first place, considering there is no use case for this driver that I know of that does not have PCI support in the kernel. But I guess in that case I would make it depend on PCI in the Kconfig instead. And Antoine may still have had a good reason for his request. > > > This is the reason we have so many empty static inline functions in > > > header files - it ensures that the symbols are declared even if the > > > only invocations are from dead code. > > > > > This ties back into my previous question: how am I supposed to know whether > > stuff is nicely covered by these empty static inlines or not? If this > > happens to be a hit-and-miss affair. > > > > Indeed. > > > Note that I tested the code with the 2 platforms at my disposal - actually > > the only 2 relevant platforms for this driver, if you ask me - and they > > both compiled just fine, so I had no way of finding this "problem" myself. > > > > Did you try disabling CONFIG_PCI? > No, I'm afraid I assumed the Macchiatobin config covered that already. (yeah ... I know ... assumptions ...) > > > > > > > > Fixes: 625f269a5a7a ("crypto: inside-secure - add support for PCI based FPGA > > > development > > > > > board") > > > > > Signed-off-by: YueHaibing <yuehaibing@huawei.com> > > > > > --- > > > > > v2: use 'ifdef' instead of 'IS_ENABLED' > > > > > --- > > > > > drivers/crypto/inside-secure/safexcel.c | 13 ++++++++++--- > > > > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside- > > > > > secure/safexcel.c > > > > > index e12a2a3..5253900 100644 > > > > > --- a/drivers/crypto/inside-secure/safexcel.c > > > > > +++ b/drivers/crypto/inside-secure/safexcel.c > > > > > @@ -937,7 +937,8 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > > > > > int ret, irq; > > > > > struct device *dev; > > > > > > > > > > - if (IS_ENABLED(CONFIG_PCI) && is_pci_dev) { > > > > > +#ifdef CONFIG_PCI > > > > > + if (is_pci_dev) { > > > > > struct pci_dev *pci_pdev = pdev; > > > > > > > > > > dev = &pci_pdev->dev; > > > > > @@ -947,7 +948,10 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > > > > > irqid, irq); > > > > > return irq; > > > > > } > > > > > - } else if (IS_ENABLED(CONFIG_OF)) { > > > > > + } else > > > > > +#endif > > > > > + { > > > > > +#ifdef CONFIG_OF > > > > > struct platform_device *plf_pdev = pdev; > > > > > char irq_name[6] = {0}; /* "ringX\0" */ > > > > > > > > > > @@ -960,6 +964,7 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, > > > > > irq_name, irq); > > > > > return irq; > > > > > } > > > > > +#endif > > > > > } > > > > > > > > > > ret = devm_request_threaded_irq(dev, irq, handler, > > > > > @@ -1137,7 +1142,8 @@ static int safexcel_probe_generic(void *pdev, > > > > > > > > > > safexcel_configure(priv); > > > > > > > > > > - if (IS_ENABLED(CONFIG_PCI) && priv->version == EIP197_DEVBRD) { > > > > > +#ifdef CONFIG_PCI > > > > > + if (priv->version == EIP197_DEVBRD) { > > > > > /* > > > > > * Request MSI vectors for global + 1 per ring - > > > > > * or just 1 for older dev images > > > > > @@ -1153,6 +1159,7 @@ static int safexcel_probe_generic(void *pdev, > > > > > return ret; > > > > > } > > > > > } > > > > > +#endif > > > > > > > > > > /* Register the ring IRQ handlers and configure the rings */ > > > > > priv->ring = devm_kcalloc(dev, priv->config.rings, > > > > > -- > > > > > 2.7.4 > > > > > > > > > > > > > Regards, > > > > Pascal van Leeuwen > > > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix > > > > www.insidesecure.com > > > > Regards, > > Pascal van Leeuwen > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix > > www.insidesecure.com > > Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* PCI: Add stub pci_irq_vector and others 2019-09-04 12:10 ` Ard Biesheuvel 2019-09-04 12:25 ` Pascal Van Leeuwen @ 2019-09-04 12:26 ` Herbert Xu 2019-09-04 14:33 ` Yuehaibing ` (2 more replies) 1 sibling, 3 replies; 17+ messages in thread From: Herbert Xu @ 2019-09-04 12:26 UTC (permalink / raw) To: Ard Biesheuvel, Bjorn Helgaas, linux-pci Cc: Pascal Van Leeuwen, YueHaibing, antoine.tenart, davem, pvanleeuwen, linux-crypto, linux-kernel On Wed, Sep 04, 2019 at 05:10:34AM -0700, Ard Biesheuvel wrote: > > This is the reason we have so many empty static inline functions in > header files - it ensures that the symbols are declared even if the > only invocations are from dead code. Does this patch work? ---8<--- This patch adds stub functions pci_alloc_irq_vectors_affinity and pci_irq_vector when CONFIG_PCI is off so that drivers can use them without resorting to ifdefs. It also moves the PCI_IRQ_* macros outside of the ifdefs so that they are always available. Fixes: 625f269a5a7a ("crypto: inside-secure - add support for...") Reported-by: kbuild test robot <lkp@intel.com> Reported-by: YueHaibing <yuehaibing@huawei.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> diff --git a/include/linux/pci.h b/include/linux/pci.h index 9e700d9f9f28..74415ee62211 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -925,6 +925,11 @@ enum { PCI_SCAN_ALL_PCIE_DEVS = 0x00000040, /* Scan all, not just dev 0 */ }; +#define PCI_IRQ_LEGACY (1 << 0) /* Allow legacy interrupts */ +#define PCI_IRQ_MSI (1 << 1) /* Allow MSI interrupts */ +#define PCI_IRQ_MSIX (1 << 2) /* Allow MSI-X interrupts */ +#define PCI_IRQ_AFFINITY (1 << 3) /* Auto-assign affinity */ + /* These external functions are only available when PCI support is enabled */ #ifdef CONFIG_PCI @@ -1408,11 +1413,6 @@ resource_size_t pcibios_window_alignment(struct pci_bus *bus, int pci_set_vga_state(struct pci_dev *pdev, bool decode, unsigned int command_bits, u32 flags); -#define PCI_IRQ_LEGACY (1 << 0) /* Allow legacy interrupts */ -#define PCI_IRQ_MSI (1 << 1) /* Allow MSI interrupts */ -#define PCI_IRQ_MSIX (1 << 2) /* Allow MSI-X interrupts */ -#define PCI_IRQ_AFFINITY (1 << 3) /* Auto-assign affinity */ - /* * Virtual interrupts allow for more interrupts to be allocated * than the device has interrupts for. These are not programmed @@ -1517,14 +1517,6 @@ static inline int pci_irq_get_node(struct pci_dev *pdev, int vec) } #endif -static inline int -pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, - unsigned int max_vecs, unsigned int flags) -{ - return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs, flags, - NULL); -} - /** * pci_irqd_intx_xlate() - Translate PCI INTx value to an IRQ domain hwirq * @d: the INTx IRQ domain @@ -1780,8 +1772,29 @@ static inline const struct pci_device_id *pci_match_id(const struct pci_device_i struct pci_dev *dev) { return NULL; } static inline bool pci_ats_disabled(void) { return true; } + +static inline int pci_irq_vector(struct pci_dev *dev, unsigned int nr) +{ + return -EINVAL; +} + +static inline int +pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs, + unsigned int max_vecs, unsigned int flags, + struct irq_affinity *aff_desc) +{ + return -ENOSPC; +} #endif /* CONFIG_PCI */ +static inline int +pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, + unsigned int max_vecs, unsigned int flags) +{ + return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs, flags, + NULL); +} + #ifdef CONFIG_PCI_ATS /* Address Translation Service */ void pci_ats_init(struct pci_dev *dev); -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: PCI: Add stub pci_irq_vector and others 2019-09-04 12:26 ` PCI: Add stub pci_irq_vector and others Herbert Xu @ 2019-09-04 14:33 ` Yuehaibing 2019-09-05 21:07 ` Bjorn Helgaas 2019-09-20 19:42 ` Bjorn Helgaas 2 siblings, 0 replies; 17+ messages in thread From: Yuehaibing @ 2019-09-04 14:33 UTC (permalink / raw) To: Herbert Xu, Ard Biesheuvel, Bjorn Helgaas, linux-pci Cc: Pascal Van Leeuwen, antoine.tenart, davem, pvanleeuwen, linux-crypto, linux-kernel On 2019/9/4 20:26, Herbert Xu wrote: > On Wed, Sep 04, 2019 at 05:10:34AM -0700, Ard Biesheuvel wrote: >> >> This is the reason we have so many empty static inline functions in >> header files - it ensures that the symbols are declared even if the >> only invocations are from dead code. > > Does this patch work? It works, Thanks. Tested-by: YueHaibing <yuehaibing@huawei.com> > > ---8<--- > This patch adds stub functions pci_alloc_irq_vectors_affinity and > pci_irq_vector when CONFIG_PCI is off so that drivers can use them > without resorting to ifdefs. > > It also moves the PCI_IRQ_* macros outside of the ifdefs so that > they are always available. > > Fixes: 625f269a5a7a ("crypto: inside-secure - add support for...") > Reported-by: kbuild test robot <lkp@intel.com> > Reported-by: YueHaibing <yuehaibing@huawei.com> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 9e700d9f9f28..74415ee62211 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -925,6 +925,11 @@ enum { > PCI_SCAN_ALL_PCIE_DEVS = 0x00000040, /* Scan all, not just dev 0 */ > }; > > +#define PCI_IRQ_LEGACY (1 << 0) /* Allow legacy interrupts */ > +#define PCI_IRQ_MSI (1 << 1) /* Allow MSI interrupts */ > +#define PCI_IRQ_MSIX (1 << 2) /* Allow MSI-X interrupts */ > +#define PCI_IRQ_AFFINITY (1 << 3) /* Auto-assign affinity */ > + > /* These external functions are only available when PCI support is enabled */ > #ifdef CONFIG_PCI > > @@ -1408,11 +1413,6 @@ resource_size_t pcibios_window_alignment(struct pci_bus *bus, > int pci_set_vga_state(struct pci_dev *pdev, bool decode, > unsigned int command_bits, u32 flags); > > -#define PCI_IRQ_LEGACY (1 << 0) /* Allow legacy interrupts */ > -#define PCI_IRQ_MSI (1 << 1) /* Allow MSI interrupts */ > -#define PCI_IRQ_MSIX (1 << 2) /* Allow MSI-X interrupts */ > -#define PCI_IRQ_AFFINITY (1 << 3) /* Auto-assign affinity */ > - > /* > * Virtual interrupts allow for more interrupts to be allocated > * than the device has interrupts for. These are not programmed > @@ -1517,14 +1517,6 @@ static inline int pci_irq_get_node(struct pci_dev *pdev, int vec) > } > #endif > > -static inline int > -pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > - unsigned int max_vecs, unsigned int flags) > -{ > - return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs, flags, > - NULL); > -} > - > /** > * pci_irqd_intx_xlate() - Translate PCI INTx value to an IRQ domain hwirq > * @d: the INTx IRQ domain > @@ -1780,8 +1772,29 @@ static inline const struct pci_device_id *pci_match_id(const struct pci_device_i > struct pci_dev *dev) > { return NULL; } > static inline bool pci_ats_disabled(void) { return true; } > + > +static inline int pci_irq_vector(struct pci_dev *dev, unsigned int nr) > +{ > + return -EINVAL; > +} > + > +static inline int > +pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs, > + unsigned int max_vecs, unsigned int flags, > + struct irq_affinity *aff_desc) > +{ > + return -ENOSPC; > +} > #endif /* CONFIG_PCI */ > > +static inline int > +pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > + unsigned int max_vecs, unsigned int flags) > +{ > + return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs, flags, > + NULL); > +} > + > #ifdef CONFIG_PCI_ATS > /* Address Translation Service */ > void pci_ats_init(struct pci_dev *dev); > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PCI: Add stub pci_irq_vector and others 2019-09-04 12:26 ` PCI: Add stub pci_irq_vector and others Herbert Xu 2019-09-04 14:33 ` Yuehaibing @ 2019-09-05 21:07 ` Bjorn Helgaas 2019-09-05 22:53 ` Herbert Xu 2019-09-20 19:42 ` Bjorn Helgaas 2 siblings, 1 reply; 17+ messages in thread From: Bjorn Helgaas @ 2019-09-05 21:07 UTC (permalink / raw) To: Herbert Xu Cc: Ard Biesheuvel, linux-pci, Pascal Van Leeuwen, YueHaibing, antoine.tenart, davem, pvanleeuwen, linux-crypto, linux-kernel On Wed, Sep 04, 2019 at 10:26:00PM +1000, Herbert Xu wrote: > On Wed, Sep 04, 2019 at 05:10:34AM -0700, Ard Biesheuvel wrote: > > > > This is the reason we have so many empty static inline functions in > > header files - it ensures that the symbols are declared even if the > > only invocations are from dead code. > > Does this patch work? > > ---8<--- > This patch adds stub functions pci_alloc_irq_vectors_affinity and > pci_irq_vector when CONFIG_PCI is off so that drivers can use them > without resorting to ifdefs. > > It also moves the PCI_IRQ_* macros outside of the ifdefs so that > they are always available. > > Fixes: 625f269a5a7a ("crypto: inside-secure - add support for...") I don't see this commit in Linus' tree yet. I'd like to include the actual reason for this patch in the commit log. I assume it's fixing a build issue, but I'd like to be a little more specific about it. > Reported-by: kbuild test robot <lkp@intel.com> > Reported-by: YueHaibing <yuehaibing@huawei.com> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 9e700d9f9f28..74415ee62211 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -925,6 +925,11 @@ enum { > PCI_SCAN_ALL_PCIE_DEVS = 0x00000040, /* Scan all, not just dev 0 */ > }; > > +#define PCI_IRQ_LEGACY (1 << 0) /* Allow legacy interrupts */ > +#define PCI_IRQ_MSI (1 << 1) /* Allow MSI interrupts */ > +#define PCI_IRQ_MSIX (1 << 2) /* Allow MSI-X interrupts */ > +#define PCI_IRQ_AFFINITY (1 << 3) /* Auto-assign affinity */ > + > /* These external functions are only available when PCI support is enabled */ > #ifdef CONFIG_PCI > > @@ -1408,11 +1413,6 @@ resource_size_t pcibios_window_alignment(struct pci_bus *bus, > int pci_set_vga_state(struct pci_dev *pdev, bool decode, > unsigned int command_bits, u32 flags); > > -#define PCI_IRQ_LEGACY (1 << 0) /* Allow legacy interrupts */ > -#define PCI_IRQ_MSI (1 << 1) /* Allow MSI interrupts */ > -#define PCI_IRQ_MSIX (1 << 2) /* Allow MSI-X interrupts */ > -#define PCI_IRQ_AFFINITY (1 << 3) /* Auto-assign affinity */ > - > /* > * Virtual interrupts allow for more interrupts to be allocated > * than the device has interrupts for. These are not programmed > @@ -1517,14 +1517,6 @@ static inline int pci_irq_get_node(struct pci_dev *pdev, int vec) > } > #endif > > -static inline int > -pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > - unsigned int max_vecs, unsigned int flags) > -{ > - return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs, flags, > - NULL); > -} > - > /** > * pci_irqd_intx_xlate() - Translate PCI INTx value to an IRQ domain hwirq > * @d: the INTx IRQ domain > @@ -1780,8 +1772,29 @@ static inline const struct pci_device_id *pci_match_id(const struct pci_device_i > struct pci_dev *dev) > { return NULL; } > static inline bool pci_ats_disabled(void) { return true; } > + > +static inline int pci_irq_vector(struct pci_dev *dev, unsigned int nr) > +{ > + return -EINVAL; > +} > + > +static inline int > +pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs, > + unsigned int max_vecs, unsigned int flags, > + struct irq_affinity *aff_desc) > +{ > + return -ENOSPC; > +} > #endif /* CONFIG_PCI */ > > +static inline int > +pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > + unsigned int max_vecs, unsigned int flags) > +{ > + return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs, flags, > + NULL); > +} > + > #ifdef CONFIG_PCI_ATS > /* Address Translation Service */ > void pci_ats_init(struct pci_dev *dev); > -- > Email: Herbert Xu <herbert@gondor.apana.org.au> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PCI: Add stub pci_irq_vector and others 2019-09-05 21:07 ` Bjorn Helgaas @ 2019-09-05 22:53 ` Herbert Xu 0 siblings, 0 replies; 17+ messages in thread From: Herbert Xu @ 2019-09-05 22:53 UTC (permalink / raw) To: Bjorn Helgaas Cc: Ard Biesheuvel, linux-pci, Pascal Van Leeuwen, YueHaibing, antoine.tenart, davem, pvanleeuwen, linux-crypto, linux-kernel On Thu, Sep 05, 2019 at 04:07:22PM -0500, Bjorn Helgaas wrote: > > > This patch adds stub functions pci_alloc_irq_vectors_affinity and > > pci_irq_vector when CONFIG_PCI is off so that drivers can use them > > without resorting to ifdefs. > > > > It also moves the PCI_IRQ_* macros outside of the ifdefs so that > > they are always available. > > > > Fixes: 625f269a5a7a ("crypto: inside-secure - add support for...") > > I don't see this commit in Linus' tree yet. > > I'd like to include the actual reason for this patch in the commit > log. I assume it's fixing a build issue, but I'd like to be a little > more specific about it. The patch in question is https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git/commit/?id=625f269a5a7a3643771320387e474bd0a61d9654 Thanks, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PCI: Add stub pci_irq_vector and others 2019-09-04 12:26 ` PCI: Add stub pci_irq_vector and others Herbert Xu 2019-09-04 14:33 ` Yuehaibing 2019-09-05 21:07 ` Bjorn Helgaas @ 2019-09-20 19:42 ` Bjorn Helgaas 2019-09-20 23:16 ` Herbert Xu 2 siblings, 1 reply; 17+ messages in thread From: Bjorn Helgaas @ 2019-09-20 19:42 UTC (permalink / raw) To: Herbert Xu Cc: Ard Biesheuvel, linux-pci, Pascal Van Leeuwen, YueHaibing, antoine.tenart, davem, pvanleeuwen, linux-crypto, linux-kernel On Wed, Sep 04, 2019 at 10:26:00PM +1000, Herbert Xu wrote: > On Wed, Sep 04, 2019 at 05:10:34AM -0700, Ard Biesheuvel wrote: > > > > This is the reason we have so many empty static inline functions in > > header files - it ensures that the symbols are declared even if the > > only invocations are from dead code. > > Does this patch work? > > ---8<--- > This patch adds stub functions pci_alloc_irq_vectors_affinity and > pci_irq_vector when CONFIG_PCI is off so that drivers can use them > without resorting to ifdefs. > > It also moves the PCI_IRQ_* macros outside of the ifdefs so that > they are always available. > > Fixes: 625f269a5a7a ("crypto: inside-secure - add support for...") > Reported-by: kbuild test robot <lkp@intel.com> > Reported-by: YueHaibing <yuehaibing@huawei.com> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> Since you've already sent your crypto pull request for v5.4, would you like me to include this in mine? If you'd rather send it yourself, I'd prefer: PCI: Add pci_irq_vector() and other stubs when !CONFIG_PCI Add stub functions pci_alloc_irq_vectors_affinity() and pci_irq_vector() when CONFIG_PCI is off so drivers can use them without resorting to ifdefs. Also move the PCI_IRQ_* macros outside of the ifdefs so they are always available. Acked-by: Bjorn Helgaas <bhelgaas@google.com> > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 9e700d9f9f28..74415ee62211 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -925,6 +925,11 @@ enum { > PCI_SCAN_ALL_PCIE_DEVS = 0x00000040, /* Scan all, not just dev 0 */ > }; > > +#define PCI_IRQ_LEGACY (1 << 0) /* Allow legacy interrupts */ > +#define PCI_IRQ_MSI (1 << 1) /* Allow MSI interrupts */ > +#define PCI_IRQ_MSIX (1 << 2) /* Allow MSI-X interrupts */ > +#define PCI_IRQ_AFFINITY (1 << 3) /* Auto-assign affinity */ > + > /* These external functions are only available when PCI support is enabled */ > #ifdef CONFIG_PCI > > @@ -1408,11 +1413,6 @@ resource_size_t pcibios_window_alignment(struct pci_bus *bus, > int pci_set_vga_state(struct pci_dev *pdev, bool decode, > unsigned int command_bits, u32 flags); > > -#define PCI_IRQ_LEGACY (1 << 0) /* Allow legacy interrupts */ > -#define PCI_IRQ_MSI (1 << 1) /* Allow MSI interrupts */ > -#define PCI_IRQ_MSIX (1 << 2) /* Allow MSI-X interrupts */ > -#define PCI_IRQ_AFFINITY (1 << 3) /* Auto-assign affinity */ > - > /* > * Virtual interrupts allow for more interrupts to be allocated > * than the device has interrupts for. These are not programmed > @@ -1517,14 +1517,6 @@ static inline int pci_irq_get_node(struct pci_dev *pdev, int vec) > } > #endif > > -static inline int > -pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > - unsigned int max_vecs, unsigned int flags) > -{ > - return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs, flags, > - NULL); > -} > - > /** > * pci_irqd_intx_xlate() - Translate PCI INTx value to an IRQ domain hwirq > * @d: the INTx IRQ domain > @@ -1780,8 +1772,29 @@ static inline const struct pci_device_id *pci_match_id(const struct pci_device_i > struct pci_dev *dev) > { return NULL; } > static inline bool pci_ats_disabled(void) { return true; } > + > +static inline int pci_irq_vector(struct pci_dev *dev, unsigned int nr) > +{ > + return -EINVAL; > +} > + > +static inline int > +pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs, > + unsigned int max_vecs, unsigned int flags, > + struct irq_affinity *aff_desc) > +{ > + return -ENOSPC; > +} > #endif /* CONFIG_PCI */ > > +static inline int > +pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > + unsigned int max_vecs, unsigned int flags) > +{ > + return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs, flags, > + NULL); > +} > + > #ifdef CONFIG_PCI_ATS > /* Address Translation Service */ > void pci_ats_init(struct pci_dev *dev); > -- > Email: Herbert Xu <herbert@gondor.apana.org.au> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PCI: Add stub pci_irq_vector and others 2019-09-20 19:42 ` Bjorn Helgaas @ 2019-09-20 23:16 ` Herbert Xu 0 siblings, 0 replies; 17+ messages in thread From: Herbert Xu @ 2019-09-20 23:16 UTC (permalink / raw) To: Bjorn Helgaas Cc: Ard Biesheuvel, linux-pci, Pascal Van Leeuwen, YueHaibing, antoine.tenart, davem, pvanleeuwen, linux-crypto, linux-kernel On Fri, Sep 20, 2019 at 02:42:16PM -0500, Bjorn Helgaas wrote: > On Wed, Sep 04, 2019 at 10:26:00PM +1000, Herbert Xu wrote: > > On Wed, Sep 04, 2019 at 05:10:34AM -0700, Ard Biesheuvel wrote: > > > > > > This is the reason we have so many empty static inline functions in > > > header files - it ensures that the symbols are declared even if the > > > only invocations are from dead code. > > > > Does this patch work? > > > > ---8<--- > > This patch adds stub functions pci_alloc_irq_vectors_affinity and > > pci_irq_vector when CONFIG_PCI is off so that drivers can use them > > without resorting to ifdefs. > > > > It also moves the PCI_IRQ_* macros outside of the ifdefs so that > > they are always available. > > > > Fixes: 625f269a5a7a ("crypto: inside-secure - add support for...") > > Reported-by: kbuild test robot <lkp@intel.com> > > Reported-by: YueHaibing <yuehaibing@huawei.com> > > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > > Since you've already sent your crypto pull request for v5.4, would you > like me to include this in mine? That would be great. Thanks! -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-09-20 23:17 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-02 14:19 [PATCH -next] crypto: inside-secure - Fix build error without CONFIG_PCI YueHaibing 2019-09-03 1:32 ` Ard Biesheuvel 2019-09-03 1:45 ` [PATCH v2 " YueHaibing 2019-09-04 11:57 ` Pascal Van Leeuwen 2019-09-04 12:10 ` Ard Biesheuvel 2019-09-04 12:25 ` Pascal Van Leeuwen 2019-09-04 12:27 ` Ard Biesheuvel 2019-09-04 12:31 ` Herbert Xu 2019-09-04 12:45 ` Pascal Van Leeuwen 2019-09-04 13:08 ` Herbert Xu 2019-09-04 12:43 ` Pascal Van Leeuwen 2019-09-04 12:26 ` PCI: Add stub pci_irq_vector and others Herbert Xu 2019-09-04 14:33 ` Yuehaibing 2019-09-05 21:07 ` Bjorn Helgaas 2019-09-05 22:53 ` Herbert Xu 2019-09-20 19:42 ` Bjorn Helgaas 2019-09-20 23:16 ` Herbert Xu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.