Re: [PATCH] PCI: designware: Remove unnecessary RC BAR setting

[Jingoo Han <jg1.han@samsung.com>]

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