> -----Original Message----- > From: Jisheng Zhang [mailto:Jisheng.Zhang@xxxxxxxxxxxxx] > Sent: 2019年4月22日 15:55 > To: Gustavo Pimentel <Gustavo.Pimentel@xxxxxxxxxxxx>; Z.q. Hou > <zhiqiang.hou@xxxxxxx> > Cc: Vidya Sagar <vidyas@xxxxxxxxxx>; bhelgaas@xxxxxxxxxx; > robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx; thierry.reding@xxxxxxxxx; > jonathanh@xxxxxxxxxx; kishon@xxxxxx; catalin.marinas@xxxxxxx; > will.deacon@xxxxxxx; lorenzo.pieralisi@xxxxxxx; jingoohan1@xxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; mmaddireddy@xxxxxxxxxx; kthota@xxxxxxxxxx; > linux-pci@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > mperttunen@xxxxxxxxxx; linux-tegra@xxxxxxxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; sagar.tv@xxxxxxxxx > Subject: [EXT] Re: [PATCH V3 04/16] PCI: dwc: Perform dbi regs write lock > towards the end > > WARNING: This email was created outside of NXP. DO NOT CLICK links or > attachments unless you recognize the sender and know the content is safe. > > > > On Wed, 17 Apr 2019 09:56:33 +0000 Gustavo Pimentel wrote: > > > > > On Tue, Apr 16, 2019 at 20:27:18, Vidya Sagar <vidyas@xxxxxxxxxx> wrote: > > > > > Remove multiple write enable and disable sequences of dbi registers > > > as > > > Tegra194 implements writes to BAR-0 register (offset: 0x10) > > > controlled by DBI write-lock enable bit thereby not allowing any > > > further writes to BAR-0 register in config space to take place. > > > Hence disabling write permission only towards the end. > > > > > > Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx> > > > --- > > > Changes since [v2]: > > > * None > > > > > > Changes since [v1]: > > > * None > > > > > > drivers/pci/controller/dwc/pcie-designware-host.c | 3 --- > > > 1 file changed, 3 deletions(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c > > > b/drivers/pci/controller/dwc/pcie-designware-host.c > > > index 2a5332e5ccfa..c0334c92c1a6 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > > @@ -683,7 +683,6 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > > > val &= 0xffff00ff; > > > val |= 0x00000100; > > > dw_pcie_writel_dbi(pci, PCI_INTERRUPT_LINE, val); > > > - dw_pcie_dbi_ro_wr_dis(pci); > > > > > > /* Setup bus numbers */ > > > val = dw_pcie_readl_dbi(pci, PCI_PRIMARY_BUS); @@ -723,8 > > > +722,6 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > > > > > > dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0); > > > > > > - /* Enable write permission for the DBI read-only register */ > > > - dw_pcie_dbi_ro_wr_en(pci); > > > /* Program correct class for RC */ > > > dw_pcie_wr_own_conf(pp, PCI_CLASS_DEVICE, 2, > PCI_CLASS_BRIDGE_PCI); > > > /* Better disable write permission right after the update */ > > > -- > > > 2.17.1 > > > > This setup sequence was written by Jingoo Han, let's check if he did > > this by some particular reason. > > Jingoo do you remember why you wrote the code like this? > > FWICT, enabling RO writeable in the setup sequence is introduced in commit > d91dfe5054d4 ("PCI: dwc: Enable write permission for Class Code, Interrupt > Pin updates"). The Reason why not towards the end maybe only enable the > RO writeable when necessary. > Yes, you get the point, I think it's not a good choice to unlock the write permission of the RO registers all through. Thanks, Zhiqiang
