[6/6] pci, acpi: Share ACPI PCI config space accessors.

Commit Message

Tomasz Nowicki Nov. 19, 2014, 4:04 p.m. UTC

MMCFG can be used perfectly for all architectures which support ACPI.
ACPI mandates MMCFG to describe PCI config space ranges which means
we should use MMCONFIG accessors by default.

Signed-off-by: Tomasz Nowicki <[e-mail]>
Tested-by: Hanjun Guo <[e-mail]>
---
 drivers/acpi/mmconfig.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Arnd Bergmann Nov. 19, 2014, 4:19 p.m. UTC | #1

On Wednesday 19 November 2014 17:04:51 Tomasz Nowicki wrote:
> +/*
> + * raw_pci_read/write - ACPI PCI config space accessors.
> + *
> + * ACPI spec defines MMCFG as the way we can access PCI config space,
> + * so let MMCFG be default (__weak).
> + *
> + * If platform needs more fancy stuff, should provides its own implementation.
> + */
> +int __weak raw_pci_read(unsigned int domain, unsigned int bus,
> +                       unsigned int devfn, int reg, int len, u32 *val)
> +{
> +       return pci_mmcfg_read(domain, bus, devfn, reg, len, val);
> +}
> +
> +int __weak raw_pci_write(unsigned int domain, unsigned int bus,
> +                        unsigned int devfn, int reg, int len, u32 val)
> +{
> +       return pci_mmcfg_write(domain, bus, devfn, reg, len, val);
> +}
> +
> 

I think it would be better to avoid __weak functions here, as they tend
to be hard to follow when trying to understand the code.

How about using a Kconfig symbol like this:

#ifdef CONFIG_ARCH_RAW_PCI_READWRITE
int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn,
                 int reg, int len, u32 *val);
int raw_pci_write(unsigned int domain, unsigned int bus, unsigned int devfn,
                  int reg, int len, u32 val);
#else
static inline int raw_pci_read(unsigned int domain, unsigned int bus,
	                       unsigned int devfn, int reg, int len, u32 *val)
{
	return pci_mmcfg_read(domain, bus, devfn, reg, len, val);
}

static inline int raw_pci_write(unsigned int domain, unsigned int bus,
             		                unsigned int devfn, int reg, int len, u32 val)
{
       return pci_mmcfg_write(domain, bus, devfn, reg, len, val);
}
#endif

Same thing for the weak symbols in patch 5.

	Arnd

Tomasz Nowicki Nov. 19, 2014, 4:24 p.m. UTC | #2

On 19.11.2014 17:19, Arnd Bergmann wrote:
> On Wednesday 19 November 2014 17:04:51 Tomasz Nowicki wrote:
>> +/*
>> + * raw_pci_read/write - ACPI PCI config space accessors.
>> + *
>> + * ACPI spec defines MMCFG as the way we can access PCI config space,
>> + * so let MMCFG be default (__weak).
>> + *
>> + * If platform needs more fancy stuff, should provides its own implementation.
>> + */
>> +int __weak raw_pci_read(unsigned int domain, unsigned int bus,
>> +                       unsigned int devfn, int reg, int len, u32 *val)
>> +{
>> +       return pci_mmcfg_read(domain, bus, devfn, reg, len, val);
>> +}
>> +
>> +int __weak raw_pci_write(unsigned int domain, unsigned int bus,
>> +                        unsigned int devfn, int reg, int len, u32 val)
>> +{
>> +       return pci_mmcfg_write(domain, bus, devfn, reg, len, val);
>> +}
>> +
>>
>
> I think it would be better to avoid __weak functions here, as they tend
> to be hard to follow when trying to understand the code.
>
> How about using a Kconfig symbol like this:
>
> #ifdef CONFIG_ARCH_RAW_PCI_READWRITE
> int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn,
>                   int reg, int len, u32 *val);
> int raw_pci_write(unsigned int domain, unsigned int bus, unsigned int devfn,
>                    int reg, int len, u32 val);
> #else
> static inline int raw_pci_read(unsigned int domain, unsigned int bus,
> 	                       unsigned int devfn, int reg, int len, u32 *val)
> {
> 	return pci_mmcfg_read(domain, bus, devfn, reg, len, val);
> }
>
> static inline int raw_pci_write(unsigned int domain, unsigned int bus,
>               		                unsigned int devfn, int reg, int len, u32 val)
> {
>         return pci_mmcfg_write(domain, bus, devfn, reg, len, val);
> }
> #endif
>
> Same thing for the weak symbols in patch 5.
>

It makes sense to me, thanks!

Tomasz

Bjorn Helgaas Nov. 20, 2014, 10:26 p.m. UTC | #3

On Wed, Nov 19, 2014 at 05:19:20PM +0100, Arnd Bergmann wrote:
> On Wednesday 19 November 2014 17:04:51 Tomasz Nowicki wrote:
> > +/*
> > + * raw_pci_read/write - ACPI PCI config space accessors.
> > + *
> > + * ACPI spec defines MMCFG as the way we can access PCI config space,
> > + * so let MMCFG be default (__weak).
> > + *
> > + * If platform needs more fancy stuff, should provides its own implementation.
> > + */
> > +int __weak raw_pci_read(unsigned int domain, unsigned int bus,
> > +                       unsigned int devfn, int reg, int len, u32 *val)
> > +{
> > +       return pci_mmcfg_read(domain, bus, devfn, reg, len, val);
> > +}
> > +
> > +int __weak raw_pci_write(unsigned int domain, unsigned int bus,
> > +                        unsigned int devfn, int reg, int len, u32 val)
> > +{
> > +       return pci_mmcfg_write(domain, bus, devfn, reg, len, val);
> > +}
> > +
> > 
> 
> I think it would be better to avoid __weak functions here, as they tend
> to be hard to follow when trying to understand the code.

That's interesting.  I would have said exactly the opposite -- I think the
extra Kconfiggery is harder to follow than weak/strong functions :)

But consistency is better than my personal opinion.  Is there a consensus
that we should use the Kconfig strategy instead of __weak?

> How about using a Kconfig symbol like this:
> 
> #ifdef CONFIG_ARCH_RAW_PCI_READWRITE
> int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn,
>                  int reg, int len, u32 *val);
> int raw_pci_write(unsigned int domain, unsigned int bus, unsigned int devfn,
>                   int reg, int len, u32 val);
> #else
> static inline int raw_pci_read(unsigned int domain, unsigned int bus,
> 	                       unsigned int devfn, int reg, int len, u32 *val)
> {
> 	return pci_mmcfg_read(domain, bus, devfn, reg, len, val);
> }
> 
> static inline int raw_pci_write(unsigned int domain, unsigned int bus,
>              		                unsigned int devfn, int reg, int len, u32 val)
> {
>        return pci_mmcfg_write(domain, bus, devfn, reg, len, val);
> }
> #endif

Myron Stowe Nov. 21, 2014, 4 a.m. UTC | #4

On Thu, Nov 20, 2014 at 3:26 PM, Bjorn Helgaas <[e-mail]> wrote:
> On Wed, Nov 19, 2014 at 05:19:20PM +0100, Arnd Bergmann wrote:
>> On Wednesday 19 November 2014 17:04:51 Tomasz Nowicki wrote:
>> > +/*
>> > + * raw_pci_read/write - ACPI PCI config space accessors.
>> > + *
>> > + * ACPI spec defines MMCFG as the way we can access PCI config space,
>> > + * so let MMCFG be default (__weak).
>> > + *
>> > + * If platform needs more fancy stuff, should provides its own implementation.
>> > + */
>> > +int __weak raw_pci_read(unsigned int domain, unsigned int bus,
>> > +                       unsigned int devfn, int reg, int len, u32 *val)
>> > +{
>> > +       return pci_mmcfg_read(domain, bus, devfn, reg, len, val);
>> > +}
>> > +
>> > +int __weak raw_pci_write(unsigned int domain, unsigned int bus,
>> > +                        unsigned int devfn, int reg, int len, u32 val)
>> > +{
>> > +       return pci_mmcfg_write(domain, bus, devfn, reg, len, val);
>> > +}
>> > +
>> >
>>
>> I think it would be better to avoid __weak functions here, as they tend
>> to be hard to follow when trying to understand the code.
>
> That's interesting.  I would have said exactly the opposite -- I think the
> extra Kconfiggery is harder to follow than weak/strong functions :)
>
> But consistency is better than my personal opinion.  Is there a consensus
> that we should use the Kconfig strategy instead of __weak?

I too find weak/strong functions easier to follow than "Kconfiggery" (nice term
invention there).

>
>> How about using a Kconfig symbol like this:
>>
>> #ifdef CONFIG_ARCH_RAW_PCI_READWRITE
>> int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn,
>>                  int reg, int len, u32 *val);
>> int raw_pci_write(unsigned int domain, unsigned int bus, unsigned int devfn,
>>                   int reg, int len, u32 val);
>> #else
>> static inline int raw_pci_read(unsigned int domain, unsigned int bus,
>>                              unsigned int devfn, int reg, int len, u32 *val)
>> {
>>       return pci_mmcfg_read(domain, bus, devfn, reg, len, val);
>> }
>>
>> static inline int raw_pci_write(unsigned int domain, unsigned int bus,
>>                                       unsigned int devfn, int reg, int len, u32 val)
>> {
>>        return pci_mmcfg_write(domain, bus, devfn, reg, len, val);
>> }
>> #endif
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Arnd Bergmann Nov. 21, 2014, 12:24 p.m. UTC | #5

On Thursday 20 November 2014 21:00:17 Myron Stowe wrote:
> On Thu, Nov 20, 2014 at 3:26 PM, Bjorn Helgaas <[e-mail]> wrote:
> > On Wed, Nov 19, 2014 at 05:19:20PM +0100, Arnd Bergmann wrote:
> >> On Wednesday 19 November 2014 17:04:51 Tomasz Nowicki wrote:
> >> > +/*
> >> > + * raw_pci_read/write - ACPI PCI config space accessors.
> >> > + *
> >> > + * ACPI spec defines MMCFG as the way we can access PCI config space,
> >> > + * so let MMCFG be default (__weak).
> >> > + *
> >> > + * If platform needs more fancy stuff, should provides its own implementation.
> >> > + */
> >> > +int __weak raw_pci_read(unsigned int domain, unsigned int bus,
> >> > +                       unsigned int devfn, int reg, int len, u32 *val)
> >> > +{
> >> > +       return pci_mmcfg_read(domain, bus, devfn, reg, len, val);
> >> > +}
> >> > +
> >> > +int __weak raw_pci_write(unsigned int domain, unsigned int bus,
> >> > +                        unsigned int devfn, int reg, int len, u32 val)
> >> > +{
> >> > +       return pci_mmcfg_write(domain, bus, devfn, reg, len, val);
> >> > +}
> >> > +
> >> >
> >>
> >> I think it would be better to avoid __weak functions here, as they tend
> >> to be hard to follow when trying to understand the code.
> >
> > That's interesting.  I would have said exactly the opposite -- I think the
> > extra Kconfiggery is harder to follow than weak/strong functions 
> >
> > But consistency is better than my personal opinion.  Is there a consensus
> > that we should use the Kconfig strategy instead of __weak?
> 
> I too find weak/strong functions easier to follow than "Kconfiggery" (nice term
> invention there).

I don't think there is a universal consensus, but the majority of
maintainers seems to avoid them for the same reasons that I think
__weak is problematic.

We have some uses of __weak in the core kernel, but there is
basically none in drivers outside of PCI, and the most common
uses are all providing an empty __weak function that can be
overridden with a function that actually does something, unlike
the code above.

My pragmatic approach so far has been to advocate __weak for
drivers/pci patches but discourage it elsewhere when I review
patches, in order to maintain consistency. I also think it
would be nice to change the way that PCI handles architecture
specific overrides in the process of unifying the host bridge
handling.

I wouldn't use Kconfig symbols in most cases though. My preferred
choice would be to turn a lot of the __weak symbols into function
pointers within a per-hostbridge structure. As an example, we could
replace pcibios_add_device() with a pointer in pci_host_bridge->ops
that gets set by all the architectures and host drivers that currently
override it, and replace the one caller with

	if (pci_host_bridge->ops->add_device)
		pci_host_bridge->ops->add_device(dev);

	Arnd

Bjorn Helgaas Nov. 21, 2014, 6:08 p.m. UTC | #6

On Fri, Nov 21, 2014 at 01:24:52PM +0100, Arnd Bergmann wrote:
> On Thursday 20 November 2014 21:00:17 Myron Stowe wrote:
> > On Thu, Nov 20, 2014 at 3:26 PM, Bjorn Helgaas <[e-mail]> wrote:
> > > On Wed, Nov 19, 2014 at 05:19:20PM +0100, Arnd Bergmann wrote:
> > >> On Wednesday 19 November 2014 17:04:51 Tomasz Nowicki wrote:
> > >> > +/*
> > >> > + * raw_pci_read/write - ACPI PCI config space accessors.
> > >> > + *
> > >> > + * ACPI spec defines MMCFG as the way we can access PCI config space,
> > >> > + * so let MMCFG be default (__weak).
> > >> > + *
> > >> > + * If platform needs more fancy stuff, should provides its own implementation.
> > >> > + */
> > >> > +int __weak raw_pci_read(unsigned int domain, unsigned int bus,
> > >> > +                       unsigned int devfn, int reg, int len, u32 *val)
> > >> > +{
> > >> > +       return pci_mmcfg_read(domain, bus, devfn, reg, len, val);
> > >> > +}
> > >> > +
> > >> > +int __weak raw_pci_write(unsigned int domain, unsigned int bus,
> > >> > +                        unsigned int devfn, int reg, int len, u32 val)
> > >> > +{
> > >> > +       return pci_mmcfg_write(domain, bus, devfn, reg, len, val);
> > >> > +}
> > >> > +
> > >> >
> > >>
> > >> I think it would be better to avoid __weak functions here, as they tend
> > >> to be hard to follow when trying to understand the code.
> > >
> > > That's interesting.  I would have said exactly the opposite -- I think the
> > > extra Kconfiggery is harder to follow than weak/strong functions 
> > >
> > > But consistency is better than my personal opinion.  Is there a consensus
> > > that we should use the Kconfig strategy instead of __weak?
> > 
> > I too find weak/strong functions easier to follow than "Kconfiggery" (nice term
> > invention there).
> 
> I don't think there is a universal consensus, but the majority of
> maintainers seems to avoid them for the same reasons that I think
> __weak is problematic.
> 
> We have some uses of __weak in the core kernel, but there is
> basically none in drivers outside of PCI, and the most common
> uses are all providing an empty __weak function that can be
> overridden with a function that actually does something, unlike
> the code above.

One thing I like better about __weak (when used correctly) is that you have
exactly one declaration, and the role of each definition (weak default
implementation or strong override) is obvious from looking at it.

In your #ifdef example, the extern declaration and the inline definition
are never compiled together, so you have to repeat the signature and the
compiler doesn't enforce that they match.  So you end up with the extern
and the inline in one file, a #define in an arch header file or Kconfig,
and an arch definition in a third file.

But it's certainly true that everybody knows how #ifdef works, and the fact
that __weak on a declaration affects all in-scope definitions is definitely
a land mine (multiple weak definitions with no strong one is a disaster).

> My pragmatic approach so far has been to advocate __weak for
> drivers/pci patches but discourage it elsewhere when I review
> patches, in order to maintain consistency. I also think it
> would be nice to change the way that PCI handles architecture
> specific overrides in the process of unifying the host bridge
> handling.
> 
> I wouldn't use Kconfig symbols in most cases though. My preferred
> choice would be to turn a lot of the __weak symbols into function
> pointers within a per-hostbridge structure. As an example, we could
> replace pcibios_add_device() with a pointer in pci_host_bridge->ops
> that gets set by all the architectures and host drivers that currently
> override it, and replace the one caller with
> 
> 	if (pci_host_bridge->ops->add_device)
> 		pci_host_bridge->ops->add_device(dev);

I definitely agree with this part, but I think it's orthogonal to the
__weak question.  In this case, we'd like to support multiple host bridges,
each with a different flavor of add_device().  We can't do that at all with
either __weak or #ifdef.

Bjorn

Arnd Bergmann Nov. 24, 2014, 10:41 a.m. UTC | #7

On Friday 21 November 2014 11:08:25 Bjorn Helgaas wrote:
> On Fri, Nov 21, 2014 at 01:24:52PM +0100, Arnd Bergmann wrote:
> > On Thursday 20 November 2014 21:00:17 Myron Stowe wrote:
> > > On Thu, Nov 20, 2014 at 3:26 PM, Bjorn Helgaas <[e-mail]> wrote:
>
> > > > That's interesting.  I would have said exactly the opposite -- I think the
> > > > extra Kconfiggery is harder to follow than weak/strong functions 
> > > >
> > > > But consistency is better than my personal opinion.  Is there a consensus
> > > > that we should use the Kconfig strategy instead of __weak?
> > > 
> > > I too find weak/strong functions easier to follow than "Kconfiggery" (nice term
> > > invention there).
> > 
> > I don't think there is a universal consensus, but the majority of
> > maintainers seems to avoid them for the same reasons that I think
> > __weak is problematic.
> > 
> > We have some uses of __weak in the core kernel, but there is
> > basically none in drivers outside of PCI, and the most common
> > uses are all providing an empty __weak function that can be
> > overridden with a function that actually does something, unlike
> > the code above.
> 
> One thing I like better about __weak (when used correctly) is that you have
> exactly one declaration, and the role of each definition (weak default
> implementation or strong override) is obvious from looking at it.

Right.

> In your #ifdef example, the extern declaration and the inline definition
> are never compiled together, so you have to repeat the signature and the
> compiler doesn't enforce that they match.  So you end up with the extern
> and the inline in one file, a #define in an arch header file or Kconfig,
> and an arch definition in a third file.
> 
> But it's certainly true that everybody knows how #ifdef works, and the fact
> that __weak on a declaration affects all in-scope definitions is definitely
> a land mine (multiple weak definitions with no strong one is a disaster).
> 
> > My pragmatic approach so far has been to advocate __weak for
> > drivers/pci patches but discourage it elsewhere when I review
> > patches, in order to maintain consistency. I also think it
> > would be nice to change the way that PCI handles architecture
> > specific overrides in the process of unifying the host bridge
> > handling.
> > 
> > I wouldn't use Kconfig symbols in most cases though. My preferred
> > choice would be to turn a lot of the __weak symbols into function
> > pointers within a per-hostbridge structure. As an example, we could
> > replace pcibios_add_device() with a pointer in pci_host_bridge->ops
> > that gets set by all the architectures and host drivers that currently
> > override it, and replace the one caller with
> > 
> > 	if (pci_host_bridge->ops->add_device)
> > 		pci_host_bridge->ops->add_device(dev);
> 
> I definitely agree with this part, but I think it's orthogonal to the
> __weak question.  In this case, we'd like to support multiple host bridges,
> each with a different flavor of add_device().  We can't do that at all with
> either __weak or #ifdef.

What we currently have though is a a __weak definition of add_device,
which some architectures override, and some of them (ARM in particular)
by implementing their own abstraction. I suspect for the majority of
what we currently define as __weak functions, we could use a similar
approach and kill off the global symbols entirely.

	Arnd

Tomasz Nowicki Dec. 8, 2014, 7:13 a.m. UTC | #8

W dniu 24.11.2014 o 11:41, Arnd Bergmann pisze:
> On Friday 21 November 2014 11:08:25 Bjorn Helgaas wrote:
>> On Fri, Nov 21, 2014 at 01:24:52PM +0100, Arnd Bergmann wrote:
>>> On Thursday 20 November 2014 21:00:17 Myron Stowe wrote:
>>>> On Thu, Nov 20, 2014 at 3:26 PM, Bjorn Helgaas <[e-mail]> wrote:
>>
>>>>> That's interesting.  I would have said exactly the opposite -- I think the
>>>>> extra Kconfiggery is harder to follow than weak/strong functions
>>>>>
>>>>> But consistency is better than my personal opinion.  Is there a consensus
>>>>> that we should use the Kconfig strategy instead of __weak?
>>>>
>>>> I too find weak/strong functions easier to follow than "Kconfiggery" (nice term
>>>> invention there).
>>>
>>> I don't think there is a universal consensus, but the majority of
>>> maintainers seems to avoid them for the same reasons that I think
>>> __weak is problematic.
>>>
>>> We have some uses of __weak in the core kernel, but there is
>>> basically none in drivers outside of PCI, and the most common
>>> uses are all providing an empty __weak function that can be
>>> overridden with a function that actually does something, unlike
>>> the code above.
>>
>> One thing I like better about __weak (when used correctly) is that you have
>> exactly one declaration, and the role of each definition (weak default
>> implementation or strong override) is obvious from looking at it.
>
> Right.
>
>> In your #ifdef example, the extern declaration and the inline definition
>> are never compiled together, so you have to repeat the signature and the
>> compiler doesn't enforce that they match.  So you end up with the extern
>> and the inline in one file, a #define in an arch header file or Kconfig,
>> and an arch definition in a third file.
>>
>> But it's certainly true that everybody knows how #ifdef works, and the fact
>> that __weak on a declaration affects all in-scope definitions is definitely
>> a land mine (multiple weak definitions with no strong one is a disaster).
>>
>>> My pragmatic approach so far has been to advocate __weak for
>>> drivers/pci patches but discourage it elsewhere when I review
>>> patches, in order to maintain consistency. I also think it
>>> would be nice to change the way that PCI handles architecture
>>> specific overrides in the process of unifying the host bridge
>>> handling.
>>>
>>> I wouldn't use Kconfig symbols in most cases though. My preferred
>>> choice would be to turn a lot of the __weak symbols into function
>>> pointers within a per-hostbridge structure. As an example, we could
>>> replace pcibios_add_device() with a pointer in pci_host_bridge->ops
>>> that gets set by all the architectures and host drivers that currently
>>> override it, and replace the one caller with
>>>
>>> 	if (pci_host_bridge->ops->add_device)
>>> 		pci_host_bridge->ops->add_device(dev);
>>
>> I definitely agree with this part, but I think it's orthogonal to the
>> __weak question.  In this case, we'd like to support multiple host bridges,
>> each with a different flavor of add_device().  We can't do that at all with
>> either __weak or #ifdef.
>
> What we currently have though is a a __weak definition of add_device,
> which some architectures override, and some of them (ARM in particular)
> by implementing their own abstraction. I suspect for the majority of
> what we currently define as __weak functions, we could use a similar
> approach and kill off the global symbols entirely.

What would be next steps regarding this patch set? I am not sure we have 
reached a consensus on weak vs #ifdef choice.

Regards,
Tomasz

Bjorn Helgaas Dec. 9, 2014, 9:50 p.m. UTC | #9

On Mon, Dec 8, 2014 at 12:13 AM, Tomasz Nowicki
<[e-mail]> wrote:

> What would be next steps regarding this patch set? I am not sure we have
> reached a consensus on weak vs #ifdef choice.

I work through the list at
https://patchwork.ozlabs.org/project/linux-pci/list/ in FIFO order, so
you don't need to do anything else except poke me to move faster :)

The weak/ifdef question is relatively minor and easily changed, so I
wouldn't worry about it yet.

Bjorn

Tomasz Nowicki Dec. 10, 2014, 6:16 a.m. UTC | #10

W dniu 09.12.2014 o 22:50, Bjorn Helgaas pisze:
> On Mon, Dec 8, 2014 at 12:13 AM, Tomasz Nowicki
> <[e-mail]> wrote:
>
>> What would be next steps regarding this patch set? I am not sure we have
>> reached a consensus on weak vs #ifdef choice.
>
> I work through the list at
> https://patchwork.ozlabs.org/project/linux-pci/list/ in FIFO order, so
> you don't need to do anything else except poke me to move faster :)

Awesome! I guess I am good at poke people so you can count on me :)

Regards,
Tomasz

Patch

diff --git a/drivers/acpi/mmconfig.c b/drivers/acpi/mmconfig.c
index c0ad05f..c9c6e05 100644
--- a/drivers/acpi/mmconfig.c
+++ b/drivers/acpi/mmconfig.c
@@ -23,6 +23,26 @@  static DEFINE_MUTEX(pci_mmcfg_lock);
 
 LIST_HEAD(pci_mmcfg_list);
 
+/*
+ * raw_pci_read/write - ACPI PCI config space accessors.
+ *
+ * ACPI spec defines MMCFG as the way we can access PCI config space,
+ * so let MMCFG be default (__weak).
+ *
+ * If platform needs more fancy stuff, should provides its own implementation.
+ */
+int __weak raw_pci_read(unsigned int domain, unsigned int bus,
+			unsigned int devfn, int reg, int len, u32 *val)
+{
+	return pci_mmcfg_read(domain, bus, devfn, reg, len, val);
+}
+
+int __weak raw_pci_write(unsigned int domain, unsigned int bus,
+			 unsigned int devfn, int reg, int len, u32 val)
+{
+	return pci_mmcfg_write(domain, bus, devfn, reg, len, val);
+}
+
 static char __iomem *pci_dev_base(unsigned int seg, unsigned int bus,
 				  unsigned int devfn)
 {