Linux-Renesas-SoC Archive mirror
 help / color / mirror / Atom feed
From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: "Jingoo Han" <jingoohan1@gmail.com>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Rob Herring" <robh@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Marek Vasut" <marek.vasut+renesas@gmail.com>,
	"Yoshihiro Shimoda" <yoshihiro.shimoda.uh@renesas.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Jonathan Hunter" <jonathanh@nvidia.com>,
	"Vidya Sagar" <vidyas@nvidia.com>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Richard Zhu" <hongxing.zhu@nxp.com>,
	"Lucas Stach" <l.stach@pengutronix.de>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	"Fabio Estevam" <festevam@gmail.com>,
	"NXP Linux Team" <linux-imx@nxp.com>,
	"Minghuan Lian" <minghuan.Lian@nxp.com>,
	"Mingkai Hu" <mingkai.hu@nxp.com>, "Roy Zang" <roy.zang@nxp.com>,
	"Kunihiko Hayashi" <hayashi.kunihiko@socionext.com>,
	"Masami Hiramatsu" <mhiramat@kernel.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Jesper Nilsson" <jesper.nilsson@axis.com>,
	"Srikanth Thokala" <srikanth.thokala@intel.com>,
	"Shawn Lin" <shawn.lin@rock-chips.com>,
	"Heiko Stuebner" <heiko@sntech.de>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-tegra@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org,
	"Niklas Cassel" <cassel@kernel.org>,
	linux-arm-kernel@axis.com, linux-rockchip@lists.infradead.org
Subject: Re: [PATCH v12 8/8] PCI: endpoint: Remove "core_init_notifier" flag
Date: Sun, 14 Apr 2024 16:21:48 +0530	[thread overview]
Message-ID: <20240414105148.GC2294@thinkpad> (raw)
In-Reply-To: <20240412202216.GA14590@bhelgaas>

On Fri, Apr 12, 2024 at 03:22:16PM -0500, Bjorn Helgaas wrote:
> On Wed, Mar 27, 2024 at 02:43:37PM +0530, Manivannan Sadhasivam wrote:
> > "core_init_notifier" flag is set by the glue drivers requiring refclk from
> > the host to complete the DWC core initialization. Also, those drivers will
> > send a notification to the EPF drivers once the initialization is fully
> > completed using the pci_epc_init_notify() API. Only then, the EPF drivers
> > will start functioning.
> > 
> > For the rest of the drivers generating refclk locally, EPF drivers will
> > start functioning post binding with them. EPF drivers rely on the
> > 'core_init_notifier' flag to differentiate between the drivers.
> > Unfortunately, this creates two different flows for the EPF drivers.
> > 
> > So to avoid that, let's get rid of the "core_init_notifier" flag and follow
> > a single initialization flow for the EPF drivers. This is done by calling
> > the dw_pcie_ep_init_notify() from all glue drivers after the completion of
> > dw_pcie_ep_init_registers() API. This will allow all the glue drivers to
> > send the notification to the EPF drivers once the initialization is fully
> > completed.
> 
> Thanks for doing this!  I think this is a significantly nicer
> solution than core_init_notifier was.
> 
> One question: both qcom and tegra194 call dw_pcie_ep_init_registers()
> from an interrupt handler, but they register that handler in a
> different order with respect to dw_pcie_ep_init().
> 
> I don't know what actually starts the process that leads to the
> interrupt, but if it's dw_pcie_ep_init(), then one of these (qcom, I
> think) must be racy:
> 

Your analysis is correct. But there is no race observed as of now since the IRQ
will only be enabled by configuring the endpoint using configfs interface and
right now I use an init script to do that. By that time, the driver would've
already probed completely.

But there is a slight chance that if the driver gets loaded as a module and the
userspace script starts configuring the endpoint interface using inotify watch
or something similar, then race could occur since the IRQ handler may not be
registered at that point.

>   qcom_pcie_ep_probe
>     dw_pcie_ep_init                                             <- A
>     qcom_pcie_ep_enable_irq_resources
>       devm_request_threaded_irq(qcom_pcie_ep_perst_irq_thread)  <- B
> 
>   qcom_pcie_ep_perst_irq_thread
>     qcom_pcie_perst_deassert
>       dw_pcie_ep_init_registers
> 
>   tegra_pcie_dw_probe
>     tegra_pcie_config_ep
>       devm_request_threaded_irq(tegra_pcie_ep_pex_rst_irq)      <- B
>       dw_pcie_ep_init                                           <- A
> 
>   tegra_pcie_ep_pex_rst_irq
>     pex_ep_event_pex_rst_deassert
>       dw_pcie_ep_init_registers
> 
> Whatever the right answer is, I think qcom and tegra194 should both
> order dw_pcie_ep_init() and the devm_request_threaded_irq() the same
> way.
> 

Agree. The right way is to register the IRQ handler first and then do
dw_pcie_ep_init(). I will fix it in the qcom driver.

Thanks for spotting!

- Mani

-- 
மணிவண்ணன் சதாசிவம்

  reply	other threads:[~2024-04-14 10:52 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-27  9:13 [PATCH v12 0/8] PCI: dwc: ep: Fix DBI access failure for drivers requiring refclk from host Manivannan Sadhasivam
2024-03-27  9:13 ` [PATCH v12 1/8] " Manivannan Sadhasivam
2024-03-27  9:13 ` [PATCH v12 2/8] PCI: dwc: ep: Add Kernel-doc comments for APIs Manivannan Sadhasivam
2024-04-12 19:58   ` Bjorn Helgaas
2024-04-15 14:00     ` Manivannan Sadhasivam
2024-04-15 14:34       ` Bjorn Helgaas
2024-03-27  9:13 ` [PATCH v12 3/8] PCI: dwc: ep: Remove deinit() callback from struct dw_pcie_ep_ops Manivannan Sadhasivam
2024-03-27  9:13 ` [PATCH v12 4/8] PCI: dwc: ep: Rename dw_pcie_ep_exit() to dw_pcie_ep_deinit() Manivannan Sadhasivam
2024-03-27  9:13 ` [PATCH v12 5/8] PCI: dwc: ep: Introduce dw_pcie_ep_cleanup() API for drivers supporting PERST# Manivannan Sadhasivam
2024-03-27  9:13 ` [PATCH v12 6/8] PCI: dwc: ep: Rename dw_pcie_ep_init_complete() to dw_pcie_ep_init_registers() Manivannan Sadhasivam
2024-03-27  9:13 ` [PATCH v12 7/8] PCI: dwc: ep: Call dw_pcie_ep_init_registers() API directly from all glue drivers Manivannan Sadhasivam
2024-03-29 19:16   ` Frank Li
2024-03-27  9:13 ` [PATCH v12 8/8] PCI: endpoint: Remove "core_init_notifier" flag Manivannan Sadhasivam
2024-03-27  9:41   ` Niklas Cassel
2024-03-29 19:17   ` Frank Li
2024-04-12 20:22   ` Bjorn Helgaas
2024-04-14 10:51     ` Manivannan Sadhasivam [this message]
2024-05-17 17:04 ` [PATCH v12 0/8] PCI: dwc: ep: Fix DBI access failure for drivers requiring refclk from host Krzysztof Wilczyński

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240414105148.GC2294@thinkpad \
    --to=manivannan.sadhasivam@linaro.org \
    --cc=bhelgaas@google.com \
    --cc=cassel@kernel.org \
    --cc=festevam@gmail.com \
    --cc=hayashi.kunihiko@socionext.com \
    --cc=heiko@sntech.de \
    --cc=helgaas@kernel.org \
    --cc=hongxing.zhu@nxp.com \
    --cc=jesper.nilsson@axis.com \
    --cc=jingoohan1@gmail.com \
    --cc=jonathanh@nvidia.com \
    --cc=kernel@pengutronix.de \
    --cc=kishon@kernel.org \
    --cc=kw@linux.com \
    --cc=l.stach@pengutronix.de \
    --cc=linux-arm-kernel@axis.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=lpieralisi@kernel.org \
    --cc=marek.vasut+renesas@gmail.com \
    --cc=mhiramat@kernel.org \
    --cc=minghuan.Lian@nxp.com \
    --cc=mingkai.hu@nxp.com \
    --cc=robh@kernel.org \
    --cc=roy.zang@nxp.com \
    --cc=s.hauer@pengutronix.de \
    --cc=shawn.lin@rock-chips.com \
    --cc=shawnguo@kernel.org \
    --cc=srikanth.thokala@intel.com \
    --cc=thierry.reding@gmail.com \
    --cc=vidyas@nvidia.com \
    --cc=vigneshr@ti.com \
    --cc=yoshihiro.shimoda.uh@renesas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).