diff mbox

PCI: designware: Use IS_ENABLED()

Message ID 1403104613-9867-1-git-send-email-festevam@gmail.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Fabio Estevam June 18, 2014, 3:16 p.m. UTC 
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(-)

Comments

Jingoo Han June 19, 2014, 12:10 a.m. UTC | #1 
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

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mohit KUMAR DCG June 19, 2014, 3:49 a.m. UTC | #2 
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

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas July 2, 2014, 10:24 p.m. UTC | #3 
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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Rothwell July 9, 2014, 12:57 a.m. UTC | #4 
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?
Jingoo Han July 9, 2014, 1:34 a.m. UTC | #5 
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

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas July 9, 2014, 3:40 a.m. UTC | #6 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Fabio Estevam July 9, 2014, 6:04 a.m. UTC | #7 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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