Re: [PATCH 2/3] pci: Add a generic, weakly-linked pcibios_align_resource

2017-06-24 Thread Palmer Dabbelt 
On Sat, 24 Jun 2017 02:41:42 PDT (-0700), ge...@linux-m68k.org wrote:
> Hi Palmer,
>
> On Sat, Jun 24, 2017 at 3:50 AM, Palmer Dabbelt  wrote:
>> Multiple architectures define this as trivial function, and I'm adding
>> another one as part of the RISC-V port.  This adds a __weak version of
>> pcibios_align_resource and deletes the now obselete ones in a handful of
>> ports.
>>
>> The only functional change should be that a handful of ports used to
>> export pcibios_fixup_bus.  Only some architectures export this, so I
>> just dropped it.
>>
>> Signed-off-by: Palmer Dabbelt 
>
> This function is only ever used as a pointer passed to
> pci_bus_alloc_resource()?
>
> What about having
>
> #ifndef pcibios_fixup_bus
> #define pcibios_fixup_bus NULL
> #endif
>
> in asm-generic/pci.h, letting the architecture with a non-trivial
> implementation predefine the preprocessor symbol, and teaching
> pci_bus_alloc_resource() to handle NULL?
>
> [...]
>
> Oh, the latter eventually calls into allocate_resource(), which already falls
> back to simple_align_resource() if the alignment function is NULL, which
> does the same thing.
> So NULL should already work.

I'm OK with that, but like your comments on the last one I think it might fit
better as a __weak function.  I think there were also some questions as to
whether these should actually be empty functions or not.  I'm really happy with
either way.

Since this is all out of my wheelhouse, can one of the PCI maintainers chime in
as to what I should do here?

Re: [PATCH 2/3] pci: Add a generic, weakly-linked pcibios_align_resource

2017-06-24 Thread Palmer Dabbelt 
On Sat, 24 Jun 2017 02:41:42 PDT (-0700), ge...@linux-m68k.org wrote:
> Hi Palmer,
>
> On Sat, Jun 24, 2017 at 3:50 AM, Palmer Dabbelt  wrote:
>> Multiple architectures define this as trivial function, and I'm adding
>> another one as part of the RISC-V port.  This adds a __weak version of
>> pcibios_align_resource and deletes the now obselete ones in a handful of
>> ports.
>>
>> The only functional change should be that a handful of ports used to
>> export pcibios_fixup_bus.  Only some architectures export this, so I
>> just dropped it.
>>
>> Signed-off-by: Palmer Dabbelt 
>
> This function is only ever used as a pointer passed to
> pci_bus_alloc_resource()?
>
> What about having
>
> #ifndef pcibios_fixup_bus
> #define pcibios_fixup_bus NULL
> #endif
>
> in asm-generic/pci.h, letting the architecture with a non-trivial
> implementation predefine the preprocessor symbol, and teaching
> pci_bus_alloc_resource() to handle NULL?
>
> [...]
>
> Oh, the latter eventually calls into allocate_resource(), which already falls
> back to simple_align_resource() if the alignment function is NULL, which
> does the same thing.
> So NULL should already work.

I'm OK with that, but like your comments on the last one I think it might fit
better as a __weak function.  I think there were also some questions as to
whether these should actually be empty functions or not.  I'm really happy with
either way.

Since this is all out of my wheelhouse, can one of the PCI maintainers chime in
as to what I should do here?

Re: [PATCH 2/3] pci: Add a generic, weakly-linked pcibios_align_resource

2017-06-24 Thread Geert Uytterhoeven 
Hi Palmer,

On Sat, Jun 24, 2017 at 3:50 AM, Palmer Dabbelt  wrote:
> Multiple architectures define this as trivial function, and I'm adding
> another one as part of the RISC-V port.  This adds a __weak version of
> pcibios_align_resource and deletes the now obselete ones in a handful of
> ports.
>
> The only functional change should be that a handful of ports used to
> export pcibios_fixup_bus.  Only some architectures export this, so I
> just dropped it.
>
> Signed-off-by: Palmer Dabbelt 

This function is only ever used as a pointer passed to
pci_bus_alloc_resource()?

What about having

#ifndef pcibios_fixup_bus
#define pcibios_fixup_bus NULL
#endif

in asm-generic/pci.h, letting the architecture with a non-trivial
implementation predefine the preprocessor symbol, and teaching
pci_bus_alloc_resource() to handle NULL?

[...]

Oh, the latter eventually calls into allocate_resource(), which already falls
back to simple_align_resource() if the alignment function is NULL, which
does the same thing.
So NULL should already work.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 2/3] pci: Add a generic, weakly-linked pcibios_align_resource

2017-06-24 Thread Geert Uytterhoeven 
Hi Palmer,

On Sat, Jun 24, 2017 at 3:50 AM, Palmer Dabbelt  wrote:
> Multiple architectures define this as trivial function, and I'm adding
> another one as part of the RISC-V port.  This adds a __weak version of
> pcibios_align_resource and deletes the now obselete ones in a handful of
> ports.
>
> The only functional change should be that a handful of ports used to
> export pcibios_fixup_bus.  Only some architectures export this, so I
> just dropped it.
>
> Signed-off-by: Palmer Dabbelt 

This function is only ever used as a pointer passed to
pci_bus_alloc_resource()?

What about having

#ifndef pcibios_fixup_bus
#define pcibios_fixup_bus NULL
#endif

in asm-generic/pci.h, letting the architecture with a non-trivial
implementation predefine the preprocessor symbol, and teaching
pci_bus_alloc_resource() to handle NULL?

[...]

Oh, the latter eventually calls into allocate_resource(), which already falls
back to simple_align_resource() if the alignment function is NULL, which
does the same thing.
So NULL should already work.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

[PATCH 2/3] pci: Add a generic, weakly-linked pcibios_align_resource

2017-06-23 Thread Palmer Dabbelt 
Multiple architectures define this as trivial function, and I'm adding
another one as part of the RISC-V port.  This adds a __weak version of
pcibios_align_resource and deletes the now obselete ones in a handful of
ports.

The only functional change should be that a handful of ports used to
export pcibios_fixup_bus.  Only some architectures export this, so I
just dropped it.

Signed-off-by: Palmer Dabbelt 
---
 arch/arc/kernel/pcibios.c|  9 -
 arch/arm64/kernel/pci.c  |  9 -
 arch/ia64/pci/pci.c  |  7 ---
 arch/microblaze/pci/pci-common.c |  7 ---
 arch/sparc/kernel/leon_pci.c |  6 --
 arch/sparc/kernel/pci.c  |  6 --
 arch/sparc/kernel/pcic.c |  6 --
 arch/tile/kernel/pci.c   | 10 --
 arch/tile/kernel/pci_gx.c|  9 -
 drivers/pci/setup-res.c  | 12 
 10 files changed, 12 insertions(+), 69 deletions(-)

diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
index 1c8df8fd5fed..05aba5a7b5d2 100644
--- a/arch/arc/kernel/pcibios.c
+++ b/arch/arc/kernel/pcibios.c
@@ -7,12 +7,3 @@
  */
 
 #include 
-
-/*
- * We don't have to worry about legacy ISA devices, so nothing to do here
- */
-resource_size_t pcibios_align_resource(void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 4c7f451aca05..9753ca23cfa1 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -23,15 +23,6 @@
 #include 
 
 /*
- * We don't have to worry about legacy ISA devices, so nothing to do here
- */
-resource_size_t pcibios_align_resource(void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
-
-/*
  * Try to assign the IRQ number when probing a new device
  */
 int pcibios_alloc_irq(struct pci_dev *dev)
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 4068bde623dc..f5ec736100ee 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -411,13 +411,6 @@ pcibios_disable_device (struct pci_dev *dev)
acpi_pci_irq_disable(dev);
 }
 
-resource_size_t
-pcibios_align_resource (void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
-
 /**
  * ia64_pci_get_legacy_mem - generic legacy mem routine
  * @bus: bus to get legacy memory base address for
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 940f266e5d5c..5835c09c6e26 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -823,13 +823,6 @@ void pcibios_setup_bus_devices(struct pci_bus *bus)
  * but we want to try to avoid allocating at 0x2900-0x2bff
  * which might have be mirrored at 0x0100-0x03ff..
  */
-resource_size_t pcibios_align_resource(void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
-EXPORT_SYMBOL(pcibios_align_resource);
-
 int pcibios_add_device(struct pci_dev *dev)
 {
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
index 4371f72ff025..0eafdf3d036d 100644
--- a/arch/sparc/kernel/leon_pci.c
+++ b/arch/sparc/kernel/leon_pci.c
@@ -94,9 +94,3 @@ void pcibios_fixup_bus(struct pci_bus *pbus)
}
}
 }
-
-resource_size_t pcibios_align_resource(void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 78d3dc25e126..3f8670c92951 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -690,12 +690,6 @@ struct pci_bus *pci_scan_one_pbm(struct pci_pbm_info *pbm,
return bus;
 }
 
-resource_size_t pcibios_align_resource(void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
-
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
u16 cmd, oldcmd;
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index a38787b84322..e038e343f2c1 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -746,12 +746,6 @@ static void watchdog_reset() {
 }
 #endif
 
-resource_size_t pcibios_align_resource(void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
-
 int pcibios_enable_device(struct pci_dev *pdev, int mask)
 {
return 0;
diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
index 3113d4d5c329..8999a20ed9d1 100644
--- a/arch/tile/kernel/pci.c
+++ b/arch/tile/kernel/pci.c

[PATCH 2/3] pci: Add a generic, weakly-linked pcibios_align_resource

2017-06-23 Thread Palmer Dabbelt 
Multiple architectures define this as trivial function, and I'm adding
another one as part of the RISC-V port.  This adds a __weak version of
pcibios_align_resource and deletes the now obselete ones in a handful of
ports.

The only functional change should be that a handful of ports used to
export pcibios_fixup_bus.  Only some architectures export this, so I
just dropped it.

Signed-off-by: Palmer Dabbelt 
---
 arch/arc/kernel/pcibios.c|  9 -
 arch/arm64/kernel/pci.c  |  9 -
 arch/ia64/pci/pci.c  |  7 ---
 arch/microblaze/pci/pci-common.c |  7 ---
 arch/sparc/kernel/leon_pci.c |  6 --
 arch/sparc/kernel/pci.c  |  6 --
 arch/sparc/kernel/pcic.c |  6 --
 arch/tile/kernel/pci.c   | 10 --
 arch/tile/kernel/pci_gx.c|  9 -
 drivers/pci/setup-res.c  | 12 
 10 files changed, 12 insertions(+), 69 deletions(-)

diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
index 1c8df8fd5fed..05aba5a7b5d2 100644
--- a/arch/arc/kernel/pcibios.c
+++ b/arch/arc/kernel/pcibios.c
@@ -7,12 +7,3 @@
  */
 
 #include 
-
-/*
- * We don't have to worry about legacy ISA devices, so nothing to do here
- */
-resource_size_t pcibios_align_resource(void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 4c7f451aca05..9753ca23cfa1 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -23,15 +23,6 @@
 #include 
 
 /*
- * We don't have to worry about legacy ISA devices, so nothing to do here
- */
-resource_size_t pcibios_align_resource(void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
-
-/*
  * Try to assign the IRQ number when probing a new device
  */
 int pcibios_alloc_irq(struct pci_dev *dev)
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 4068bde623dc..f5ec736100ee 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -411,13 +411,6 @@ pcibios_disable_device (struct pci_dev *dev)
acpi_pci_irq_disable(dev);
 }
 
-resource_size_t
-pcibios_align_resource (void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
-
 /**
  * ia64_pci_get_legacy_mem - generic legacy mem routine
  * @bus: bus to get legacy memory base address for
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 940f266e5d5c..5835c09c6e26 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -823,13 +823,6 @@ void pcibios_setup_bus_devices(struct pci_bus *bus)
  * but we want to try to avoid allocating at 0x2900-0x2bff
  * which might have be mirrored at 0x0100-0x03ff..
  */
-resource_size_t pcibios_align_resource(void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
-EXPORT_SYMBOL(pcibios_align_resource);
-
 int pcibios_add_device(struct pci_dev *dev)
 {
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
index 4371f72ff025..0eafdf3d036d 100644
--- a/arch/sparc/kernel/leon_pci.c
+++ b/arch/sparc/kernel/leon_pci.c
@@ -94,9 +94,3 @@ void pcibios_fixup_bus(struct pci_bus *pbus)
}
}
 }
-
-resource_size_t pcibios_align_resource(void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 78d3dc25e126..3f8670c92951 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -690,12 +690,6 @@ struct pci_bus *pci_scan_one_pbm(struct pci_pbm_info *pbm,
return bus;
 }
 
-resource_size_t pcibios_align_resource(void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
-
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
u16 cmd, oldcmd;
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index a38787b84322..e038e343f2c1 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -746,12 +746,6 @@ static void watchdog_reset() {
 }
 #endif
 
-resource_size_t pcibios_align_resource(void *data, const struct resource *res,
-   resource_size_t size, resource_size_t align)
-{
-   return res->start;
-}
-
 int pcibios_enable_device(struct pci_dev *pdev, int mask)
 {
return 0;
diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
index 3113d4d5c329..8999a20ed9d1 100644
--- a/arch/tile/kernel/pci.c
+++ b/arch/tile/kernel/pci.c
@@ -67,16 +67,6 @@