[PATCH] PCI: designware: Use IS_ENABLED()

[PATCH] PCI: designware: Use IS_ENABLED()

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

We can get rid of the 'ifdef' by using the IS_ENABLED() macro.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/pci/host/pcie-designware.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 1eaf4df3..dc842fd 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -497,9 +497,8 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
 
 	pci_common_init_dev(pp->dev, &dw_pci);
 	pci_assign_unassigned_resources();
-#ifdef CONFIG_PCI_DOMAINS
-	dw_pci.domain++;
-#endif
+	if (IS_ENABLED(CONFIG_PCI_DOMAINS))
+		dw_pci.domain++;
 
 	return 0;
 }
-- 
1.8.3.2

Re: [PATCH] PCI: designware: Use IS_ENABLED()

[Jingoo Han]

On Thursday, June 19, 2014 12:17 AM, Fabio Estevam wrote:
> 
> We can get rid of the 'ifdef' by using the IS_ENABLED() macro.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

(+cc Mohit Kumar)

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/pci/host/pcie-designware.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 1eaf4df3..dc842fd 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -497,9 +497,8 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> 
>  	pci_common_init_dev(pp->dev, &dw_pci);
>  	pci_assign_unassigned_resources();
> -#ifdef CONFIG_PCI_DOMAINS
> -	dw_pci.domain++;
> -#endif
> +	if (IS_ENABLED(CONFIG_PCI_DOMAINS))
> +		dw_pci.domain++;
> 
>  	return 0;
>  }
> --
> 1.8.3.2

RE: [PATCH] PCI: designware: Use IS_ENABLED()

[Mohit KUMAR DCG]

Hello Fabio,

> -----Original Message-----
> From: Jingoo Han [mailto:jg1.han@samsung.com]
> Sent: Thursday, June 19, 2014 5:41 AM
> To: 'Fabio Estevam'; bhelgaas@google.com
> Cc: linux-pci@vger.kernel.org; 'Fabio Estevam'; 'Jingoo Han'; Mohit KUMAR
> DCG
> Subject: Re: [PATCH] PCI: designware: Use IS_ENABLED()
> 
> On Thursday, June 19, 2014 12:17 AM, Fabio Estevam wrote:
> >
> > We can get rid of the 'ifdef' by using the IS_ENABLED() macro.
> >
> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> 
> (+cc Mohit Kumar)
> 

- yes, looks good...

Acked-by: Mohit Kumar <mohit.kumar@st.com>

Thanks
Mohit

> Acked-by: Jingoo Han <jg1.han@samsung.com>
> 
> Best regards,
> Jingoo Han
> 
> > ---
> >  drivers/pci/host/pcie-designware.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-
> designware.c
> > index 1eaf4df3..dc842fd 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -497,9 +497,8 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> >
> >  	pci_common_init_dev(pp->dev, &dw_pci);
> >  	pci_assign_unassigned_resources();
> > -#ifdef CONFIG_PCI_DOMAINS
> > -	dw_pci.domain++;
> > -#endif
> > +	if (IS_ENABLED(CONFIG_PCI_DOMAINS))
> > +		dw_pci.domain++;
> >
> >  	return 0;
> >  }
> > --
> > 1.8.3.2

Re: [PATCH] PCI: designware: Use IS_ENABLED()

[Bjorn Helgaas]

On Wed, Jun 18, 2014 at 12:16:53PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> We can get rid of the 'ifdef' by using the IS_ENABLED() macro.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

Applied with Jingoo and Mohit's acks to pci/host-designware for v3.17,
thanks!

> ---
>  drivers/pci/host/pcie-designware.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 1eaf4df3..dc842fd 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -497,9 +497,8 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>  
>  	pci_common_init_dev(pp->dev, &dw_pci);
>  	pci_assign_unassigned_resources();
> -#ifdef CONFIG_PCI_DOMAINS
> -	dw_pci.domain++;
> -#endif
> +	if (IS_ENABLED(CONFIG_PCI_DOMAINS))
> +		dw_pci.domain++;
>  
>  	return 0;
>  }
> -- 
> 1.8.3.2
> 

Re: PCI: designware: Use IS_ENABLED()

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]

Hi Fabio,

[I noticed this in the pci tree in linux-next this morning]

On Wed, 18 Jun 2014 12:16:53 -0300 Fabio Estevam <festevam@gmail.com> wrote:
>
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> We can get rid of the 'ifdef' by using the IS_ENABLED() macro.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> Acked-by: Jingoo Han <jg1.han@samsung.com>
> Acked-by: Mohit Kumar <mohit.kumar@st.com>
> 
> ---
> drivers/pci/host/pcie-designware.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 1eaf4df3..dc842fd 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -497,9 +497,8 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>  
>  	pci_common_init_dev(pp->dev, &dw_pci);
>  	pci_assign_unassigned_resources();
> -#ifdef CONFIG_PCI_DOMAINS
> -	dw_pci.domain++;
> -#endif
> +	if (IS_ENABLED(CONFIG_PCI_DOMAINS))
> +		dw_pci.domain++;
>  
>  	return 0;
>  }

Will this actually compile if CONFIG_PCI_DOMAINS is disabled, since the
"domains" field of dw_pci does not even exist in that case (it is
ifdeffed out in arch/arm/include/asm/mach/pci.h)?  Even if it works
with newer compilers, will it work with older ones i.e. how new does
the compiler have to be to elide the code before noticing that
"domains" does not exist?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: PCI: designware: Use IS_ENABLED()

[Jingoo Han]

On Wednesday, July 09, 2014 9:58 AM, Stephen Rothwell wrote:
> 
> Hi Fabio,
> 
> [I noticed this in the pci tree in linux-next this morning]
> 
> On Wed, 18 Jun 2014 12:16:53 -0300 Fabio Estevam <festevam@gmail.com> wrote:
> >
> > From: Fabio Estevam <fabio.estevam@freescale.com>
> >
> > We can get rid of the 'ifdef' by using the IS_ENABLED() macro.
> >
> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> > Acked-by: Jingoo Han <jg1.han@samsung.com>
> > Acked-by: Mohit Kumar <mohit.kumar@st.com>
> >
> > ---
> > drivers/pci/host/pcie-designware.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> > index 1eaf4df3..dc842fd 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -497,9 +497,8 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> >
> >  	pci_common_init_dev(pp->dev, &dw_pci);
> >  	pci_assign_unassigned_resources();
> > -#ifdef CONFIG_PCI_DOMAINS
> > -	dw_pci.domain++;
> > -#endif
> > +	if (IS_ENABLED(CONFIG_PCI_DOMAINS))
> > +		dw_pci.domain++;
> >
> >  	return 0;
> >  }
> 
> Will this actually compile if CONFIG_PCI_DOMAINS is disabled, since the
> "domains" field of dw_pci does not even exist in that case (it is
> ifdeffed out in arch/arm/include/asm/mach/pci.h)?  Even if it works
> with newer compilers, will it work with older ones i.e. how new does
> the compiler have to be to elide the code before noticing that
> "domains" does not exist?

Oops, you're right! It makes build errors when CONFIG_PCI_DOMAINS
is disabled.

The "domain" variable is guarded by "#ifdef CONFIG_PCI_DOMAINS";
thus, IS_ENABLED() should NOT be used.

./arch/arm/include/asm/mach/pci.h
struct hw_pci {
#ifdef CONFIG_PCI_DOMAINS
	int		domain;
#endif

Bjorn Helgaas,
Please revert this patch from your pci tree. Thank you.

Best regards,
Jingoo Han

> 
> --
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au

Re: PCI: designware: Use IS_ENABLED()

[Bjorn Helgaas]

On Tue, Jul 8, 2014 at 7:34 PM, Jingoo Han <jg1.han@samsung.com> wrote:
> On Wednesday, July 09, 2014 9:58 AM, Stephen Rothwell wrote:
>>
>> Hi Fabio,
>>
>> [I noticed this in the pci tree in linux-next this morning]
>>
>> On Wed, 18 Jun 2014 12:16:53 -0300 Fabio Estevam <festevam@gmail.com> wrote:
>> >
>> > From: Fabio Estevam <fabio.estevam@freescale.com>
>> >
>> > We can get rid of the 'ifdef' by using the IS_ENABLED() macro.
>> >
>> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>> > Acked-by: Jingoo Han <jg1.han@samsung.com>
>> > Acked-by: Mohit Kumar <mohit.kumar@st.com>
>> >
>> > ---
>> > drivers/pci/host/pcie-designware.c | 5 ++---
>> >  1 file changed, 2 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
>> > index 1eaf4df3..dc842fd 100644
>> > --- a/drivers/pci/host/pcie-designware.c
>> > +++ b/drivers/pci/host/pcie-designware.c
>> > @@ -497,9 +497,8 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> >
>> >     pci_common_init_dev(pp->dev, &dw_pci);
>> >     pci_assign_unassigned_resources();
>> > -#ifdef CONFIG_PCI_DOMAINS
>> > -   dw_pci.domain++;
>> > -#endif
>> > +   if (IS_ENABLED(CONFIG_PCI_DOMAINS))
>> > +           dw_pci.domain++;
>> >
>> >     return 0;
>> >  }
>>
>> Will this actually compile if CONFIG_PCI_DOMAINS is disabled, since the
>> "domains" field of dw_pci does not even exist in that case (it is
>> ifdeffed out in arch/arm/include/asm/mach/pci.h)?  Even if it works
>> with newer compilers, will it work with older ones i.e. how new does
>> the compiler have to be to elide the code before noticing that
>> "domains" does not exist?
>
> Oops, you're right! It makes build errors when CONFIG_PCI_DOMAINS
> is disabled.
>
> The "domain" variable is guarded by "#ifdef CONFIG_PCI_DOMAINS";
> thus, IS_ENABLED() should NOT be used.
>
> ./arch/arm/include/asm/mach/pci.h
> struct hw_pci {
> #ifdef CONFIG_PCI_DOMAINS
>         int             domain;
> #endif
>
> Bjorn Helgaas,
> Please revert this patch from your pci tree. Thank you.

I dropped it.  Thanks for noticing this, Stephen.

Bjorn

Re: PCI: designware: Use IS_ENABLED()

[Fabio Estevam]

On Wed, Jul 9, 2014 at 12:40 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:

> I dropped it.  Thanks for noticing this, Stephen.

Sorry for the trouble!

Thanks