[PATCH -next] crypto: inside-secure - Fix build error without CONFIG_PCI

[PATCH -next] crypto: inside-secure - Fix build error without CONFIG_PCI

[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

Re: [PATCH -next] crypto: inside-secure - Fix build error without CONFIG_PCI

[Ard Biesheuvel]

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
>
>

[PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI

[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

RE: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI

[Pascal Van Leeuwen]

> -----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

Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI

[Ard Biesheuvel]

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

RE: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI

[Pascal Van Leeuwen]

> -----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

PCI: Add stub pci_irq_vector and others

[Herbert Xu]

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

Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI

[Ard Biesheuvel]

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
>

Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI

[Herbert Xu]

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

RE: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI

[Pascal Van Leeuwen]

> -----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

RE: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI

[Pascal Van Leeuwen]

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

Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI

[Herbert Xu]

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

Re: PCI: Add stub pci_irq_vector and others

[Yuehaibing]

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);
> 

Re: PCI: Add stub pci_irq_vector and others

[Bjorn Helgaas]

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

Re: PCI: Add stub pci_irq_vector and others

[Herbert Xu]

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

Re: PCI: Add stub pci_irq_vector and others

[Bjorn Helgaas]

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

Re: PCI: Add stub pci_irq_vector and others

[Herbert Xu]

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