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

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

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