diff mbox

PCI: designware: Remove unnecessary RC BAR setting

Message ID 000801cf4d95$64322ef0$2c968cd0$%han@samsung.com
State Not Applicable
Headers show

Commit Message

Jingoo Han April 1, 2014, 10:30 a.m. UTC
According to the spec, the synopsys core does not implement the
optional BARs such as BAR0/1. This is based on the assumption
that the RC host probably has registers on some other internal
bus and has knowledge and setup access to these registers already.
So, remove unnecessary RC BAR setting.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
Tested on Exynos5440.

 drivers/pci/host/pcie-designware.c |    4 ----
 1 file changed, 4 deletions(-)

Comments

Mohit KUMAR DCG April 2, 2014, 4:57 a.m. UTC | #1
Hello Jingoo,

> -----Original Message-----
> From: Jingoo Han [mailto:jg1.han@samsung.com]
> Sent: Tuesday, April 01, 2014 4:00 PM
> To: 'Bjorn Helgaas'
> Cc: linux-pci@vger.kernel.org; Mohit KUMAR DCG; Pratyush ANAND; 'Marek
> Vasut'; 'Richard Zhu'; 'Kishon Vijay Abraham I'; 'Jingoo Han'
> Subject: [PATCH] PCI: designware: Remove unnecessary RC BAR setting
> 
> According to the spec, the synopsys core does not implement the optional
> BARs such as BAR0/1. This is based on the assumption that the RC host
> probably has registers on some other internal bus and has knowledge and
> setup access to these registers already.
> So, remove unnecessary RC BAR setting.
> 
- Normally BARs in RC are not used but somehow available in the design. One possible
BAR use can be if RC has some memory connected to the BAR that needs to be accessed through link.

Otherwise we can ignore BARs setup here.

Thanks
Mohit
 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
> Tested on Exynos5440.
> 
>  drivers/pci/host/pcie-designware.c |    4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-
> designware.c
> index 6d23d8c..7bee01f 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -798,10 +798,6 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>  	}
>  	dw_pcie_writel_rc(pp, val, PCIE_LINK_WIDTH_SPEED_CONTROL);
> 
> -	/* setup RC BARs */
> -	dw_pcie_writel_rc(pp, 0x00000004, PCI_BASE_ADDRESS_0);
> -	dw_pcie_writel_rc(pp, 0x00000000, PCI_BASE_ADDRESS_1);
> -
>  	/* setup interrupt pins */
>  	dw_pcie_readl_rc(pp, PCI_INTERRUPT_LINE, &val);
>  	val &= 0xffff00ff;
> --
> 1.7.10.4
> 

--
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
Jingoo Han April 2, 2014, 5:22 a.m. UTC | #2
On Wednesday, April 02, 2014 1:57 PM, Mohit KUMAR DCG wrote:
> On Tuesday, April 01, 2014 4:00 PM, Jingoo Han wrote:
> >
> > According to the spec, the synopsys core does not implement the optional
> > BARs such as BAR0/1. This is based on the assumption that the RC host
> > probably has registers on some other internal bus and has knowledge and
> > setup access to these registers already.
> > So, remove unnecessary RC BAR setting.
> >
> - Normally BARs in RC are not used but somehow available in the design. One possible
> BAR use can be if RC has some memory connected to the BAR that needs to be accessed through link.
> 
> Otherwise we can ignore BARs setup here.

Hi Mohit KUMAR DCG,

Thank you for your feedback.

I want to know whether or not other SoCs such as ST, Freescale, TI
support BAR0/BAR1. If no SoC supports BAR0/BAR1, the unnecessary
RC BAR setting code should  be removed.

Best regards,
Jingoo Han

> 
> Thanks
> Mohit
> 
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> > Tested on Exynos5440.
> >
> >  drivers/pci/host/pcie-designware.c |    4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-
> > designware.c
> > index 6d23d8c..7bee01f 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -798,10 +798,6 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
> >  	}
> >  	dw_pcie_writel_rc(pp, val, PCIE_LINK_WIDTH_SPEED_CONTROL);
> >
> > -	/* setup RC BARs */
> > -	dw_pcie_writel_rc(pp, 0x00000004, PCI_BASE_ADDRESS_0);
> > -	dw_pcie_writel_rc(pp, 0x00000000, PCI_BASE_ADDRESS_1);
> > -
> >  	/* setup interrupt pins */
> >  	dw_pcie_readl_rc(pp, PCI_INTERRUPT_LINE, &val);
> >  	val &= 0xffff00ff;
> > --
> > 1.7.10.4

--
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 April 2, 2014, 5:33 a.m. UTC | #3
Hello Jingoo,

> -----Original Message-----
> From: Jingoo Han [mailto:jg1.han@samsung.com]
> Sent: Wednesday, April 02, 2014 10:53 AM
> To: Mohit KUMAR DCG
> Cc: 'Bjorn Helgaas'; linux-pci@vger.kernel.org; Pratyush ANAND; 'Marek
> Vasut'; 'Richard Zhu'; 'Kishon Vijay Abraham I'
> Subject: Re: [PATCH] PCI: designware: Remove unnecessary RC BAR setting
> 
> On Wednesday, April 02, 2014 1:57 PM, Mohit KUMAR DCG wrote:
> > On Tuesday, April 01, 2014 4:00 PM, Jingoo Han wrote:
> > >
> > > According to the spec, the synopsys core does not implement the
> > > optional BARs such as BAR0/1. This is based on the assumption that
> > > the RC host probably has registers on some other internal bus and
> > > has knowledge and setup access to these registers already.
> > > So, remove unnecessary RC BAR setting.
> > >
> > - Normally BARs in RC are not used but somehow available in the
> > design. One possible BAR use can be if RC has some memory connected to
> the BAR that needs to be accessed through link.
> >
> > Otherwise we can ignore BARs setup here.
> 
> Hi Mohit KUMAR DCG,
> 
> Thank you for your feedback.
> 
> I want to know whether or not other SoCs such as ST, Freescale, TI support
> BAR0/BAR1. If no SoC supports BAR0/BAR1, the unnecessary RC BAR setting
> code should  be removed.
> 

- We are not currently using RC's BAR0/1 in any application but no such restriction from HW as
ST SoCs support BARs in HW design. 

Regards
Mohit

> Best regards,
> Jingoo Han
> 
> >
> > Thanks
> > Mohit
> >
> > > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > > ---
> > > Tested on Exynos5440.
> > >
> > >  drivers/pci/host/pcie-designware.c |    4 ----
> > >  1 file changed, 4 deletions(-)
> > >
> > > diff --git a/drivers/pci/host/pcie-designware.c
> > > b/drivers/pci/host/pcie- designware.c index 6d23d8c..7bee01f 100644
> > > --- a/drivers/pci/host/pcie-designware.c
> > > +++ b/drivers/pci/host/pcie-designware.c
> > > @@ -798,10 +798,6 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
> > >  	}
> > >  	dw_pcie_writel_rc(pp, val, PCIE_LINK_WIDTH_SPEED_CONTROL);
> > >
> > > -	/* setup RC BARs */
> > > -	dw_pcie_writel_rc(pp, 0x00000004, PCI_BASE_ADDRESS_0);
> > > -	dw_pcie_writel_rc(pp, 0x00000000, PCI_BASE_ADDRESS_1);
> > > -
> > >  	/* setup interrupt pins */
> > >  	dw_pcie_readl_rc(pp, PCI_INTERRUPT_LINE, &val);
> > >  	val &= 0xffff00ff;
> > > --
> > > 1.7.10.4

--
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
Kishon Vijay Abraham I April 2, 2014, 5:34 a.m. UTC | #4
On Wednesday 02 April 2014 11:03 AM, Mohit KUMAR DCG wrote:
> Hello Jingoo,
> 
>> -----Original Message-----
>> From: Jingoo Han [mailto:jg1.han@samsung.com]
>> Sent: Wednesday, April 02, 2014 10:53 AM
>> To: Mohit KUMAR DCG
>> Cc: 'Bjorn Helgaas'; linux-pci@vger.kernel.org; Pratyush ANAND; 'Marek
>> Vasut'; 'Richard Zhu'; 'Kishon Vijay Abraham I'
>> Subject: Re: [PATCH] PCI: designware: Remove unnecessary RC BAR setting
>>
>> On Wednesday, April 02, 2014 1:57 PM, Mohit KUMAR DCG wrote:
>>> On Tuesday, April 01, 2014 4:00 PM, Jingoo Han wrote:
>>>>
>>>> According to the spec, the synopsys core does not implement the
>>>> optional BARs such as BAR0/1. This is based on the assumption that
>>>> the RC host probably has registers on some other internal bus and
>>>> has knowledge and setup access to these registers already.
>>>> So, remove unnecessary RC BAR setting.
>>>>
>>> - Normally BARs in RC are not used but somehow available in the
>>> design. One possible BAR use can be if RC has some memory connected to
>> the BAR that needs to be accessed through link.
>>>
>>> Otherwise we can ignore BARs setup here.
>>
>> Hi Mohit KUMAR DCG,
>>
>> Thank you for your feedback.
>>
>> I want to know whether or not other SoCs such as ST, Freescale, TI support
>> BAR0/BAR1. If no SoC supports BAR0/BAR1, the unnecessary RC BAR setting
>> code should  be removed.
>>
> 
> - We are not currently using RC's BAR0/1 in any application but no such restriction from HW as
> ST SoCs support BARs in HW design. 

Neither do we in DRA7xx.

Cheers
Kishon
> 
> Regards
> Mohit
> 
>> Best regards,
>> Jingoo Han
>>
>>>
>>> Thanks
>>> Mohit
>>>
>>>> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
>>>> ---
>>>> Tested on Exynos5440.
>>>>
>>>>  drivers/pci/host/pcie-designware.c |    4 ----
>>>>  1 file changed, 4 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/host/pcie-designware.c
>>>> b/drivers/pci/host/pcie- designware.c index 6d23d8c..7bee01f 100644
>>>> --- a/drivers/pci/host/pcie-designware.c
>>>> +++ b/drivers/pci/host/pcie-designware.c
>>>> @@ -798,10 +798,6 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>>>>  	}
>>>>  	dw_pcie_writel_rc(pp, val, PCIE_LINK_WIDTH_SPEED_CONTROL);
>>>>
>>>> -	/* setup RC BARs */
>>>> -	dw_pcie_writel_rc(pp, 0x00000004, PCI_BASE_ADDRESS_0);
>>>> -	dw_pcie_writel_rc(pp, 0x00000000, PCI_BASE_ADDRESS_1);
>>>> -
>>>>  	/* setup interrupt pins */
>>>>  	dw_pcie_readl_rc(pp, PCI_INTERRUPT_LINE, &val);
>>>>  	val &= 0xffff00ff;
>>>> --
>>>> 1.7.10.4
> 
--
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
Marek Vasut April 2, 2014, 10:34 a.m. UTC | #5
On Wednesday, April 02, 2014 at 07:34:15 AM, Kishon Vijay Abraham I wrote:
> On Wednesday 02 April 2014 11:03 AM, Mohit KUMAR DCG wrote:
> > Hello Jingoo,
> > 
> >> -----Original Message-----
> >> From: Jingoo Han [mailto:jg1.han@samsung.com]
> >> Sent: Wednesday, April 02, 2014 10:53 AM
> >> To: Mohit KUMAR DCG
> >> Cc: 'Bjorn Helgaas'; linux-pci@vger.kernel.org; Pratyush ANAND; 'Marek
> >> Vasut'; 'Richard Zhu'; 'Kishon Vijay Abraham I'
> >> Subject: Re: [PATCH] PCI: designware: Remove unnecessary RC BAR setting
> >> 
> >> On Wednesday, April 02, 2014 1:57 PM, Mohit KUMAR DCG wrote:
> >>> On Tuesday, April 01, 2014 4:00 PM, Jingoo Han wrote:
> >>>> According to the spec, the synopsys core does not implement the
> >>>> optional BARs such as BAR0/1. This is based on the assumption that
> >>>> the RC host probably has registers on some other internal bus and
> >>>> has knowledge and setup access to these registers already.
> >>>> So, remove unnecessary RC BAR setting.
> >>> 
> >>> - Normally BARs in RC are not used but somehow available in the
> >>> design. One possible BAR use can be if RC has some memory connected to
> >> 
> >> the BAR that needs to be accessed through link.
> >> 
> >>> Otherwise we can ignore BARs setup here.
> >> 
> >> Hi Mohit KUMAR DCG,
> >> 
> >> Thank you for your feedback.
> >> 
> >> I want to know whether or not other SoCs such as ST, Freescale, TI
> >> support BAR0/BAR1. If no SoC supports BAR0/BAR1, the unnecessary RC BAR
> >> setting code should  be removed.
> > 
> > - We are not currently using RC's BAR0/1 in any application but no such
> > restriction from HW as ST SoCs support BARs in HW design.
> 
> Neither do we in DRA7xx.

I suspect that means we should keep the code to make sure the registers are 
configured correctly, no?

Richard, can you comment on MX6 please ?

Best regards,
Marek Vasut
--
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
Richard Zhu April 2, 2014, 11:58 a.m. UTC | #6
Hi Marek:

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-owner@vger.kernel.org]
> On Behalf Of Marek Vasut
> Sent: Wednesday, April 02, 2014 6:35 PM
> To: Kishon Vijay Abraham I
> Cc: Mohit KUMAR DCG; Jingoo Han; 'Bjorn Helgaas'; linux-pci@vger.kernel.org;
> Pratyush ANAND; Zhu Richard-R65037
> Subject: Re: [PATCH] PCI: designware: Remove unnecessary RC BAR setting
> 
> On Wednesday, April 02, 2014 at 07:34:15 AM, Kishon Vijay Abraham I wrote:
> > On Wednesday 02 April 2014 11:03 AM, Mohit KUMAR DCG wrote:
> > > Hello Jingoo,
> > >
> > >> -----Original Message-----
> > >> From: Jingoo Han [mailto:jg1.han@samsung.com]
> > >> Sent: Wednesday, April 02, 2014 10:53 AM
> > >> To: Mohit KUMAR DCG
> > >> Cc: 'Bjorn Helgaas'; linux-pci@vger.kernel.org; Pratyush ANAND;
> > >> 'Marek Vasut'; 'Richard Zhu'; 'Kishon Vijay Abraham I'
> > >> Subject: Re: [PATCH] PCI: designware: Remove unnecessary RC BAR
> > >> setting
> > >>
> > >> On Wednesday, April 02, 2014 1:57 PM, Mohit KUMAR DCG wrote:
> > >>> On Tuesday, April 01, 2014 4:00 PM, Jingoo Han wrote:
> > >>>> According to the spec, the synopsys core does not implement the
> > >>>> optional BARs such as BAR0/1. This is based on the assumption
> > >>>> that the RC host probably has registers on some other internal
> > >>>> bus and has knowledge and setup access to these registers already.
> > >>>> So, remove unnecessary RC BAR setting.
> > >>>
> > >>> - Normally BARs in RC are not used but somehow available in the
> > >>> design. One possible BAR use can be if RC has some memory
> > >>> connected to
> > >>
> > >> the BAR that needs to be accessed through link.
> > >>
> > >>> Otherwise we can ignore BARs setup here.
> > >>
> > >> Hi Mohit KUMAR DCG,
> > >>
> > >> Thank you for your feedback.
> > >>
> > >> I want to know whether or not other SoCs such as ST, Freescale, TI
> > >> support BAR0/BAR1. If no SoC supports BAR0/BAR1, the unnecessary RC
> > >> BAR setting code should  be removed.
> > >
> > > - We are not currently using RC's BAR0/1 in any application but no
> > > such restriction from HW as ST SoCs support BARs in HW design.
> >
> > Neither do we in DRA7xx.
> 
> I suspect that means we should keep the code to make sure the registers are
> configured correctly, no?
> 
> Richard, can you comment on MX6 please ?

 [Richard] Sorry to reply late. I'm engaged in another stuff in the past days.
i.MX6 pcie doesn't use RC's BAR0/1 in applications either.
> 
> Best regards,
> Marek Vasut
> --
> 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
> 
Best Regards
Richard Zhu

--
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
Jingoo Han April 4, 2014, 2:31 a.m. UTC | #7
On Wednesday, April 02, 2014 8:59 PM, Richard Zhu wrote:
> Wednesday, April 02, 2014 6:35 PM, Marek Vasut wrote:
> > On Wednesday, April 02, 2014 at 07:34:15 AM, Kishon Vijay Abraham I wrote:
> > > On Wednesday 02 April 2014 11:03 AM, Mohit KUMAR DCG wrote:
> > > > Wednesday, April 02, 2014 10:53 AM, Jingoo Han wrote:
> > > >> On Wednesday, April 02, 2014 1:57 PM, Mohit KUMAR DCG wrote:
> > > >>> On Tuesday, April 01, 2014 4:00 PM, Jingoo Han wrote:
> > > >>>> According to the spec, the synopsys core does not implement the
> > > >>>> optional BARs such as BAR0/1. This is based on the assumption
> > > >>>> that the RC host probably has registers on some other internal
> > > >>>> bus and has knowledge and setup access to these registers already.
> > > >>>> So, remove unnecessary RC BAR setting.
> > > >>>
> > > >>> - Normally BARs in RC are not used but somehow available in the
> > > >>> design. One possible BAR use can be if RC has some memory
> > > >>> connected to
> > > >>
> > > >> the BAR that needs to be accessed through link.
> > > >>
> > > >>> Otherwise we can ignore BARs setup here.
> > > >>
> > > >> Hi Mohit KUMAR DCG,
> > > >>
> > > >> Thank you for your feedback.
> > > >>
> > > >> I want to know whether or not other SoCs such as ST, Freescale, TI
> > > >> support BAR0/BAR1. If no SoC supports BAR0/BAR1, the unnecessary RC
> > > >> BAR setting code should  be removed.
> > > >
> > > > - We are not currently using RC's BAR0/1 in any application but no
> > > > such restriction from HW as ST SoCs support BARs in HW design.
> > >
> > > Neither do we in DRA7xx.
> >
> > I suspect that means we should keep the code to make sure the registers are
> > configured correctly, no?
> >
> > Richard, can you comment on MX6 please ?
> 
>  [Richard] Sorry to reply late. I'm engaged in another stuff in the past days.
> i.MX6 pcie doesn't use RC's BAR0/1 in applications either.
> >

Thank you all for your comments!

I noticed that Synopsys PCIe "Dual mode" can support BAR0/BAR1.
The bits[3:0] of BAR0 is RO(CS), which means Read-Only, but writable
from the local application through the DBI.

So, the BAR0/1 setting might be necessary, in order to ensure the
BAR0/1 setting is written properly.

But, when BAR0/1 are implemented, the bits[3:0] of BAR0 is decided
by hardware configuration parameters. So, this BAR0/1 setting looks
unnecessary.

How about others' opinions?

1. Necessary: in order to ensure the BAR0/BAR1 setting, even though
                   BAR0/BAR1 were already set as hardware default values

2. Unnecessary: the BAR0/BAR1 setting code is dummy, because BAR0/BAR1
                      were already set as hardware default values

For example, in the case of Exynos5440, BAR0/BAR1 are not implemented;
thus, even though BAR0 is written as 0x4, BAR0 can be always read as 0.
So, there is not side effect, but just unnecessary code is executed
during boot time.

Thank you. :-)

Best regards,
Jingoo Han

--
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 April 24, 2014, 9:35 p.m. UTC | #8
On Fri, Apr 04, 2014 at 11:31:34AM +0900, Jingoo Han wrote:
> On Wednesday, April 02, 2014 8:59 PM, Richard Zhu wrote:
> > Wednesday, April 02, 2014 6:35 PM, Marek Vasut wrote:
> > > On Wednesday, April 02, 2014 at 07:34:15 AM, Kishon Vijay Abraham I wrote:
> > > > On Wednesday 02 April 2014 11:03 AM, Mohit KUMAR DCG wrote:
> > > > > Wednesday, April 02, 2014 10:53 AM, Jingoo Han wrote:
> > > > >> On Wednesday, April 02, 2014 1:57 PM, Mohit KUMAR DCG wrote:
> > > > >>> On Tuesday, April 01, 2014 4:00 PM, Jingoo Han wrote:
> > > > >>>> According to the spec, the synopsys core does not implement the
> > > > >>>> optional BARs such as BAR0/1. This is based on the assumption
> > > > >>>> that the RC host probably has registers on some other internal
> > > > >>>> bus and has knowledge and setup access to these registers already.
> > > > >>>> So, remove unnecessary RC BAR setting.
> > > > >>>
> > > > >>> - Normally BARs in RC are not used but somehow available in the
> > > > >>> design. One possible BAR use can be if RC has some memory
> > > > >>> connected to
> > > > >>
> > > > >> the BAR that needs to be accessed through link.
> > > > >>
> > > > >>> Otherwise we can ignore BARs setup here.
> > > > >>
> > > > >> Hi Mohit KUMAR DCG,
> > > > >>
> > > > >> Thank you for your feedback.
> > > > >>
> > > > >> I want to know whether or not other SoCs such as ST, Freescale, TI
> > > > >> support BAR0/BAR1. If no SoC supports BAR0/BAR1, the unnecessary RC
> > > > >> BAR setting code should  be removed.
> > > > >
> > > > > - We are not currently using RC's BAR0/1 in any application but no
> > > > > such restriction from HW as ST SoCs support BARs in HW design.
> > > >
> > > > Neither do we in DRA7xx.
> > >
> > > I suspect that means we should keep the code to make sure the registers are
> > > configured correctly, no?
> > >
> > > Richard, can you comment on MX6 please ?
> > 
> >  [Richard] Sorry to reply late. I'm engaged in another stuff in the past days.
> > i.MX6 pcie doesn't use RC's BAR0/1 in applications either.
> > >
> 
> Thank you all for your comments!
> 
> I noticed that Synopsys PCIe "Dual mode" can support BAR0/BAR1.
> The bits[3:0] of BAR0 is RO(CS), which means Read-Only, but writable
> from the local application through the DBI.
> 
> So, the BAR0/1 setting might be necessary, in order to ensure the
> BAR0/1 setting is written properly.
> 
> But, when BAR0/1 are implemented, the bits[3:0] of BAR0 is decided
> by hardware configuration parameters. So, this BAR0/1 setting looks
> unnecessary.
> 
> How about others' opinions?
> 
> 1. Necessary: in order to ensure the BAR0/BAR1 setting, even though
>                    BAR0/BAR1 were already set as hardware default values
> 
> 2. Unnecessary: the BAR0/BAR1 setting code is dummy, because BAR0/BAR1
>                       were already set as hardware default values
> 
> For example, in the case of Exynos5440, BAR0/BAR1 are not implemented;
> thus, even though BAR0 is written as 0x4, BAR0 can be always read as 0.
> So, there is not side effect, but just unnecessary code is executed
> during boot time.

I don't sense a clear consensus that we should apply this, so I'll drop
it for now.  Please repost with appropriate acks if we do need it.

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
Jingoo Han April 25, 2014, 1:04 a.m. UTC | #9
On Friday, April 25, 2014 6:36 AM, Bjorn Helgaas wrote:
> On Fri, Apr 04, 2014 at 11:31:34AM +0900, Jingoo Han wrote:
> > On Wednesday, April 02, 2014 8:59 PM, Richard Zhu wrote:
> > > Wednesday, April 02, 2014 6:35 PM, Marek Vasut wrote:
> > > > On Wednesday, April 02, 2014 at 07:34:15 AM, Kishon Vijay Abraham I wrote:
> > > > > On Wednesday 02 April 2014 11:03 AM, Mohit KUMAR DCG wrote:
> > > > > > Wednesday, April 02, 2014 10:53 AM, Jingoo Han wrote:
> > > > > >> On Wednesday, April 02, 2014 1:57 PM, Mohit KUMAR DCG wrote:
> > > > > >>> On Tuesday, April 01, 2014 4:00 PM, Jingoo Han wrote:
> > > > > >>>> According to the spec, the synopsys core does not implement the
> > > > > >>>> optional BARs such as BAR0/1. This is based on the assumption
> > > > > >>>> that the RC host probably has registers on some other internal
> > > > > >>>> bus and has knowledge and setup access to these registers already.
> > > > > >>>> So, remove unnecessary RC BAR setting.
> > > > > >>>
> > > > > >>> - Normally BARs in RC are not used but somehow available in the
> > > > > >>> design. One possible BAR use can be if RC has some memory
> > > > > >>> connected to
> > > > > >>
> > > > > >> the BAR that needs to be accessed through link.
> > > > > >>
> > > > > >>> Otherwise we can ignore BARs setup here.
> > > > > >>
> > > > > >> Hi Mohit KUMAR DCG,
> > > > > >>
> > > > > >> Thank you for your feedback.
> > > > > >>
> > > > > >> I want to know whether or not other SoCs such as ST, Freescale, TI
> > > > > >> support BAR0/BAR1. If no SoC supports BAR0/BAR1, the unnecessary RC
> > > > > >> BAR setting code should  be removed.
> > > > > >
> > > > > > - We are not currently using RC's BAR0/1 in any application but no
> > > > > > such restriction from HW as ST SoCs support BARs in HW design.
> > > > >
> > > > > Neither do we in DRA7xx.
> > > >
> > > > I suspect that means we should keep the code to make sure the registers are
> > > > configured correctly, no?
> > > >
> > > > Richard, can you comment on MX6 please ?
> > >
> > >  [Richard] Sorry to reply late. I'm engaged in another stuff in the past days.
> > > i.MX6 pcie doesn't use RC's BAR0/1 in applications either.
> > > >
> >
> > Thank you all for your comments!
> >
> > I noticed that Synopsys PCIe "Dual mode" can support BAR0/BAR1.
> > The bits[3:0] of BAR0 is RO(CS), which means Read-Only, but writable
> > from the local application through the DBI.
> >
> > So, the BAR0/1 setting might be necessary, in order to ensure the
> > BAR0/1 setting is written properly.
> >
> > But, when BAR0/1 are implemented, the bits[3:0] of BAR0 is decided
> > by hardware configuration parameters. So, this BAR0/1 setting looks
> > unnecessary.
> >
> > How about others' opinions?
> >
> > 1. Necessary: in order to ensure the BAR0/BAR1 setting, even though
> >                    BAR0/BAR1 were already set as hardware default values
> >
> > 2. Unnecessary: the BAR0/BAR1 setting code is dummy, because BAR0/BAR1
> >                       were already set as hardware default values
> >
> > For example, in the case of Exynos5440, BAR0/BAR1 are not implemented;
> > thus, even though BAR0 is written as 0x4, BAR0 can be always read as 0.
> > So, there is not side effect, but just unnecessary code is executed
> > during boot time.
> 
> I don't sense a clear consensus that we should apply this, so I'll drop
> it for now.  Please repost with appropriate acks if we do need it.

OK, I agree with your opinion.
Thank you.

Best regards,
Jingoo Han


--
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 6d23d8c..7bee01f 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -798,10 +798,6 @@  void dw_pcie_setup_rc(struct pcie_port *pp)
 	}
 	dw_pcie_writel_rc(pp, val, PCIE_LINK_WIDTH_SPEED_CONTROL);
 
-	/* setup RC BARs */
-	dw_pcie_writel_rc(pp, 0x00000004, PCI_BASE_ADDRESS_0);
-	dw_pcie_writel_rc(pp, 0x00000000, PCI_BASE_ADDRESS_1);
-
 	/* setup interrupt pins */
 	dw_pcie_readl_rc(pp, PCI_INTERRUPT_LINE, &val);
 	val &= 0xffff00ff;