Re: [PATCH v7 1/2] PCI: designware-ep: Fix DBI access before core init

[Niklas Cassel <Niklas.Cassel@wdc.com>]

On Thu, Nov 30, 2023 at 07:27:57PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Nov 30, 2023 at 11:22:30AM +0000, Niklas Cassel wrote:
> > On Thu, Nov 30, 2023 at 12:08:00PM +0530, Manivannan Sadhasivam wrote:
> > > 
> > > Looking at this issue again, I think your statement may not be correct. In the
> > > stop_link() callback, non-core_init_notifier platforms are just disabling the
> > > LTSSM and enabling it again in start_link(). So all the rest of the
> > > configurations (DBI etc...) should not be affected during EPF bind/unbind.
> > 
> > Sorry for confusing you.
> > 
> > When toggling PERST on a driver with a core_init_notifier, you will call
> > dw_pcie_ep_init_complete() - which will initialize DBI settings, and then
> > stop/start the link by deasserting/asserting LTSSM.
> > (perst_assert()/perst_deassert() is functionally the equivalent to start_link()/
> > stop_link() on non-core_init_notifier platforms.)
> > 
> > 
> > For drivers without a core_init_notifier, they currently don't listen to PERST
> > input GPIO.
> > Stopping and starting the link will not call dw_pcie_ep_init_complete(),
> > so it will not (re-)initialize DBI settings.
> > 
> > 
> > My problem is that a bunch of hardware registers gets reset when receiving
> > a link-down reset or hot reset. It would be nice to write all DBI settings
> > when starting the link. That way the link going down for a millisecond, and
> > gettting immediately reestablished, will not be so bad. A stop + start link
> > will rewrite the settings. (Just like toggling PERST rewrites the settings.)
> > 
> 
> I got slightly confused by this part. So you are saying that when a user stops
> the controller through configfs, EP receives the LINK_DOWN event and that causes
> the EP specific registers (like DBI) to be reset?

Sorry for taking time to reply.
(I wanted to make sure that I was 100% understanding what was going on.)

So to give you a clear example:
If you:
1) start the EP, start the pci-epf-test
2) start the RC
3) run pci-epf-test
4) reboot the RC

this will trigger a link-down reset IRQ on the EP side (link_req_rst_not).


> 
> I thought the LINK_DOWN event can only change LTSSM and some status registers.

link_req_rst_not will assert link_down_rst_n, which will assert
non_sticky_rst_n. So all non-sticky registers will be reset.

Some thing that has been biting me is that:
While advertized Resizable BAR sizes are sticky,
the currently used resizable BAR size is non-sticky.

So a reboot of the RC will reset the BAR sizes to reset values
(which on the SoC I'm using is 1 GB...)

So, there is an advantage of having a .core_init_notifier,
as this will allow you to reboot the RC without any problems.

I guess one solution, for DWC glue drivers that don't strictly
need a refclock is to just call dw_pcie_ep_init_complete() when
receiving the link-down IRQ (since when you get that, the core
has already reset non-sticky registers to reset values), and
then when you get a link-up again, you have already re-inited
the registers that got reset.


Kind regards,
Niklas