PCI: designware: Use IS_ENABLED()

PATCH In review Fabio Estevam 5 participants 8 messages Jun 18, 2014 – Jul 9, 2014 PCI

lore

Share & tools

lore

Fabio Estevam

author

2014-06-18 15:17 UTC #0 lore raw

PATCH 1/1 PCI: designware: Use IS_ENABLED()

From: Fabio Estevam <[email protected]>

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

Signed-off-by Fabio Estevam <[email protected]>

---
 drivers/pci/host/pcie-designware.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

drivers/pci/host/pcie-designware.c +2 -3

index 1eaf4df3..dc842fd 100644
@@ -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;
 }

Signature · 1 line

1.8.3.2

Mohit KUMAR DCG

Acked-by

2014-06-19 03:49 UTC #2 lore raw

Hello Fabio,

Quoted 8 lines

-----Original Message-----
From: Jingoo Han [mailto:[email protected]]
Sent: Thursday, June 19, 2014 5:41 AM
To: 'Fabio Estevam'; [email protected]
Cc: [email protected]; 'Fabio Estevam'; 'Jingoo Han'; Mohit KUMAR DCG
Subject: Re: [PATCH] PCI: designware: Use IS_ENABLED()

Quoted 8 lines

Quoted 4 lines · Fabio Estevam
We can get rid of the 'ifdef' by using the IS_ENABLED() macro.
Signed-off-by Fabio Estevam <[email protected]>
(+cc Mohit Kumar)

- yes, looks good...

Acked-by Mohit Kumar <[email protected]>

Thanks
Mohit

Quoted 28 lines

Acked-by Jingoo Han <[email protected]>
Best regards,
Jingoo Han
drivers/pci/host/pcie-
Quoted 5 lines
---
 drivers/pci/host/pcie-designware.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/pci/host/pcie- renamed from drivers/pci/host/pcie-designware.c +0 -0
designware.c
Quoted 17 lines
index 1eaf4df3..dc842fd 100644
drivers/pci/host/pcie-designware.c +2 -4
@@ -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

Bjorn Helgaas

maintainer

2014-07-02 22:24 UTC #3 lore raw

In reply to Fabio Estevam

Quoted 5 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>
We can get rid of the 'ifdef' by using the IS_ENABLED() macro.
Signed-off-by Fabio Estevam <[email protected]>

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

Quoted 23 lines

---
 drivers/pci/host/pcie-designware.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

drivers/pci/host/pcie-designware.c +2 -3

index 1eaf4df3..dc842fd 100644
@@ -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;
 }

Signature · 1 line

1.8.3.2

Stephen Rothwell

2014-07-09 00:57 UTC #4 lore raw

In reply to Fabio Estevam

Hi Fabio,

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

Quoted 19 lines · On Wed, 18 Jun 2014 12:16:53 -0300 Fabio Estevam <[email protected]> wrote:

From: Fabio Estevam <[email protected]>
We can get rid of the 'ifdef' by using the IS_ENABLED() macro.
Signed-off-by Fabio Estevam <[email protected]>
Acked-by Jingoo Han <[email protected]>
Acked-by Mohit Kumar <[email protected]>
---
drivers/pci/host/pcie-designware.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/pci/host/pcie-designware.c +0 -0
index 1eaf4df3..dc842fd 100644
@@ -497,9 +497,8 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
 

drivers/pci/host/pcie-designware.c int __init dw_pcie_host_init(struct pcie_port *pp) · line 504

Quoted 10 lines

drivers/pci/host/pcie-designware.c +2 -3

 	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?

Signature · 2 lines

Cheers,
Stephen Rothwell                    [email protected]

Jingoo Han

2014-07-09 01:34 UTC #5 lore raw

Quoted 33 lines · Stephen Rothwell

Hi Fabio,
[I noticed this in the pci tree in linux-next this morning]
Quoted 27 lines · On Wed, 18 Jun 2014 12:16:53 -0300 Fabio Estevam <[email protected]> wrote:
From: Fabio Estevam <[email protected]>
We can get rid of the 'ifdef' by using the IS_ENABLED() macro.
Signed-off-by Fabio Estevam <[email protected]>
Acked-by Jingoo Han <[email protected]>
Acked-by Mohit Kumar <[email protected]>
---
drivers/pci/host/pcie-designware.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/pci/host/pcie-designware.c +2 -3
index 1eaf4df3..dc842fd 100644
@@ -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++;
 

Quoted 9 lines

drivers/pci/host/pcie-designware.c int __init dw_pcie_host_init(struct pcie_port *pp) · line 504
Quoted 2 lines
drivers/pci/host/pcie-designware.c +0 -0
 	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

Quoted 4 lines

--
Cheers,
Stephen Rothwell [email protected]

Bjorn Helgaas

maintainer

2014-07-09 03:41 UTC #6 lore raw

Quoted 50 lines · Jingoo Han

Quoted 33 lines · Stephen Rothwell
Hi Fabio,
[I noticed this in the pci tree in linux-next this morning]
Quoted 27 lines · On Wed, 18 Jun 2014 12:16:53 -0300 Fabio Estevam <[email protected]> wrote:
From: Fabio Estevam <[email protected]>
We can get rid of the 'ifdef' by using the IS_ENABLED() macro.
Signed-off-by Fabio Estevam <[email protected]>
Acked-by Jingoo Han <[email protected]>
Acked-by Mohit Kumar <[email protected]>
---
drivers/pci/host/pcie-designware.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/pci/host/pcie-designware.c +2 -3
index 1eaf4df3..dc842fd 100644
@@ -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++;
 
Quoted 9 lines
drivers/pci/host/pcie-designware.c int __init dw_pcie_host_init(struct pcie_port *pp) · line 504
Quoted 2 lines
drivers/pci/host/pcie-designware.c +0 -0
    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.

Quoted 8 lines

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

Fabio Estevam

author

2014-07-09 06:04 UTC #7 lore raw

Quoted 1 line · Bjorn Helgaas

I dropped it. Thanks for noticing this, Stephen.

Sorry for the trouble!

Thanks