All of lore.kernel.org
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: gregkh@linuxfoundation.org, rafael@kernel.org,
	rafael.j.wysocki@intel.com, will@kernel.org,
	robin.murphy@arm.com, joro@8bytes.org,
	bjorn.andersson@linaro.org, ulf.hansson@linaro.org,
	adrian.hunter@intel.com, bhelgaas@google.com
Cc: robdclark@chromium.org, linux-arm-msm@vger.kernel.org,
	linux-pci@vger.kernel.org, quic_c_gdjako@quicinc.com,
	iommu@lists.linux-foundation.org, sonnyrao@chromium.org,
	saiprakash.ranjan@codeaurora.org, linux-mmc@vger.kernel.org,
	vbadigan@codeaurora.org, rajatja@google.com,
	saravanak@google.com, joel@joelfernandes.org,
	Douglas Anderson <dianders@chromium.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 1/6] drivers: base: Add the concept of "pre_probe" to drivers
Date: Mon, 21 Jun 2021 16:52:43 -0700	[thread overview]
Message-ID: <20210621165230.1.Id4ee5788c993294f66542721fca7719c00a5d8f3@changeid> (raw)
In-Reply-To: <20210621235248.2521620-1-dianders@chromium.org>

Right now things are a bit awkward if a driver would like a chance to
run before some of the more "automatic" things (pinctrl, DMA, IOMMUs,
...) happen to a device. This patch aims to fix that problem by
introducing the concept of a "pre_probe" function that drivers can
implement to run before the "automatic" stuff.

Why would you want to run before the "automatic" stuff? The incentive
in my case is that I want to be able to fill in some boolean flags in
the "struct device" before the IOMMU init runs. It appears that the
strictness vs. non-strictness of a device's iommu config is determined
once at init time and can't be changed afterwards. However, I would
like to avoid hardcoding the rules for strictness in the IOMMU
driver. Instead I'd like to let individual drivers be able to make
informed decisions about the appropriateness of strictness
vs. non-strictness.

The desire for running code pre_probe is likely not limited to my use
case. I believe that the list "qcom_smmu_client_of_match" is hacked
into the iommu driver specifically because there was no real good
framework for this. For the existing list it wasn't _quite_ as ugly as
my needs since the decision could be made solely on compatible string,
but it still feels like it would have been better for individual
drivers to run code and setup some state rather than coding up a big
list in the IOMMU driver.

Even without this patch, I believe it is possible for a driver to run
before the "automatic" things by registering for
"BUS_NOTIFY_BIND_DRIVER" in its init call, though I haven't personally
tested this. Using the notifier is a bit awkward, though, and I'd
rather avoid it. Also, using "BUS_NOTIFY_BIND_DRIVER" would require
drivers to stop using the convenience module_platform_driver() helper
and roll a bunch of boilerplate code.

NOTE: the pre_probe here is listed in the driver structure. As a side
effect of this it will be passed a "struct device *" rather than the
more specific device type (like the "struct platform_device *" that
most platform devices get passed to their probe). Presumably this
won't cause trouble and it's a lot less code to write but if we need
to make it more symmetric that's also possible by touching more files.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/base/dd.c             | 10 ++++++++--
 include/linux/device/driver.h |  9 +++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index ecd7cf848daf..9a13bff8dafa 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -549,10 +549,16 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 re_probe:
 	dev->driver = drv;
 
+	if (drv->pre_probe) {
+		ret = drv->pre_probe(dev);
+		if (ret)
+			goto probe_failed_pre_dma;
+	}
+
 	/* If using pinctrl, bind pins now before probing */
 	ret = pinctrl_bind_pins(dev);
 	if (ret)
-		goto pinctrl_bind_failed;
+		goto probe_failed_pre_dma;
 
 	if (dev->bus->dma_configure) {
 		ret = dev->bus->dma_configure(dev);
@@ -639,7 +645,7 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 	if (dev->bus)
 		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
 					     BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
-pinctrl_bind_failed:
+probe_failed_pre_dma:
 	device_links_no_driver(dev);
 	devres_release_all(dev);
 	arch_teardown_dma_ops(dev);
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index a498ebcf4993..f7305dd6ceb1 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -57,6 +57,14 @@ enum probe_type {
  * @probe_type:	Type of the probe (synchronous or asynchronous) to use.
  * @of_match_table: The open firmware table.
  * @acpi_match_table: The ACPI match table.
+ * @pre_probe:	Called after a device has been bound to a driver but before
+ *		anything "automatic" (pinctrl, DMA, IOMMUs, ...) has been
+ *		setup. This is mostly a chance for the driver to do things
+ *		that might need to be run before any of those automatic
+ *		processes. The vast majority of devices don't need to
+ *		implement this. Note that there is no "post_remove" at the
+ *		moment. If you need to undo something that you did in
+ *		pre_probe() you can use devres.
  * @probe:	Called to query the existence of a specific device,
  *		whether this driver can work with it, and bind the driver
  *		to a specific device.
@@ -105,6 +113,7 @@ struct device_driver {
 	const struct of_device_id	*of_match_table;
 	const struct acpi_device_id	*acpi_match_table;
 
+	int (*pre_probe) (struct device *dev);
 	int (*probe) (struct device *dev);
 	void (*sync_state)(struct device *dev);
 	int (*remove) (struct device *dev);
-- 
2.32.0.288.g62a8d224e6-goog


WARNING: multiple messages have this Message-ID (diff)
From: Douglas Anderson <dianders@chromium.org>
To: gregkh@linuxfoundation.org, rafael@kernel.org,
	rafael.j.wysocki@intel.com, will@kernel.org,
	robin.murphy@arm.com, joro@8bytes.org,
	bjorn.andersson@linaro.org, ulf.hansson@linaro.org,
	adrian.hunter@intel.com, bhelgaas@google.com
Cc: robdclark@chromium.org, linux-kernel@vger.kernel.org,
	saravanak@google.com, linux-arm-msm@vger.kernel.org,
	linux-mmc@vger.kernel.org, quic_c_gdjako@quicinc.com,
	Douglas Anderson <dianders@chromium.org>,
	iommu@lists.linux-foundation.org,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	linux-pci@vger.kernel.org, joel@joelfernandes.org,
	rajatja@google.com, sonnyrao@chromium.org,
	vbadigan@codeaurora.org
Subject: [PATCH 1/6] drivers: base: Add the concept of "pre_probe" to drivers
Date: Mon, 21 Jun 2021 16:52:43 -0700	[thread overview]
Message-ID: <20210621165230.1.Id4ee5788c993294f66542721fca7719c00a5d8f3@changeid> (raw)
In-Reply-To: <20210621235248.2521620-1-dianders@chromium.org>

Right now things are a bit awkward if a driver would like a chance to
run before some of the more "automatic" things (pinctrl, DMA, IOMMUs,
...) happen to a device. This patch aims to fix that problem by
introducing the concept of a "pre_probe" function that drivers can
implement to run before the "automatic" stuff.

Why would you want to run before the "automatic" stuff? The incentive
in my case is that I want to be able to fill in some boolean flags in
the "struct device" before the IOMMU init runs. It appears that the
strictness vs. non-strictness of a device's iommu config is determined
once at init time and can't be changed afterwards. However, I would
like to avoid hardcoding the rules for strictness in the IOMMU
driver. Instead I'd like to let individual drivers be able to make
informed decisions about the appropriateness of strictness
vs. non-strictness.

The desire for running code pre_probe is likely not limited to my use
case. I believe that the list "qcom_smmu_client_of_match" is hacked
into the iommu driver specifically because there was no real good
framework for this. For the existing list it wasn't _quite_ as ugly as
my needs since the decision could be made solely on compatible string,
but it still feels like it would have been better for individual
drivers to run code and setup some state rather than coding up a big
list in the IOMMU driver.

Even without this patch, I believe it is possible for a driver to run
before the "automatic" things by registering for
"BUS_NOTIFY_BIND_DRIVER" in its init call, though I haven't personally
tested this. Using the notifier is a bit awkward, though, and I'd
rather avoid it. Also, using "BUS_NOTIFY_BIND_DRIVER" would require
drivers to stop using the convenience module_platform_driver() helper
and roll a bunch of boilerplate code.

NOTE: the pre_probe here is listed in the driver structure. As a side
effect of this it will be passed a "struct device *" rather than the
more specific device type (like the "struct platform_device *" that
most platform devices get passed to their probe). Presumably this
won't cause trouble and it's a lot less code to write but if we need
to make it more symmetric that's also possible by touching more files.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/base/dd.c             | 10 ++++++++--
 include/linux/device/driver.h |  9 +++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index ecd7cf848daf..9a13bff8dafa 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -549,10 +549,16 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 re_probe:
 	dev->driver = drv;
 
+	if (drv->pre_probe) {
+		ret = drv->pre_probe(dev);
+		if (ret)
+			goto probe_failed_pre_dma;
+	}
+
 	/* If using pinctrl, bind pins now before probing */
 	ret = pinctrl_bind_pins(dev);
 	if (ret)
-		goto pinctrl_bind_failed;
+		goto probe_failed_pre_dma;
 
 	if (dev->bus->dma_configure) {
 		ret = dev->bus->dma_configure(dev);
@@ -639,7 +645,7 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 	if (dev->bus)
 		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
 					     BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
-pinctrl_bind_failed:
+probe_failed_pre_dma:
 	device_links_no_driver(dev);
 	devres_release_all(dev);
 	arch_teardown_dma_ops(dev);
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index a498ebcf4993..f7305dd6ceb1 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -57,6 +57,14 @@ enum probe_type {
  * @probe_type:	Type of the probe (synchronous or asynchronous) to use.
  * @of_match_table: The open firmware table.
  * @acpi_match_table: The ACPI match table.
+ * @pre_probe:	Called after a device has been bound to a driver but before
+ *		anything "automatic" (pinctrl, DMA, IOMMUs, ...) has been
+ *		setup. This is mostly a chance for the driver to do things
+ *		that might need to be run before any of those automatic
+ *		processes. The vast majority of devices don't need to
+ *		implement this. Note that there is no "post_remove" at the
+ *		moment. If you need to undo something that you did in
+ *		pre_probe() you can use devres.
  * @probe:	Called to query the existence of a specific device,
  *		whether this driver can work with it, and bind the driver
  *		to a specific device.
@@ -105,6 +113,7 @@ struct device_driver {
 	const struct of_device_id	*of_match_table;
 	const struct acpi_device_id	*acpi_match_table;
 
+	int (*pre_probe) (struct device *dev);
 	int (*probe) (struct device *dev);
 	void (*sync_state)(struct device *dev);
 	int (*remove) (struct device *dev);
-- 
2.32.0.288.g62a8d224e6-goog

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  reply	other threads:[~2021-06-21 23:53 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-21 23:52 [PATCH 0/6] iommu: Enable devices to request non-strict DMA, starting with QCom SD/MMC Douglas Anderson
2021-06-21 23:52 ` Douglas Anderson
2021-06-21 23:52 ` Douglas Anderson [this message]
2021-06-21 23:52   ` [PATCH 1/6] drivers: base: Add the concept of "pre_probe" to drivers Douglas Anderson
2021-06-24 13:35   ` Greg KH
2021-06-24 13:35     ` Greg KH
2021-06-21 23:52 ` [PATCH 2/6] drivers: base: Add bits to struct device to control iommu strictness Douglas Anderson
2021-06-21 23:52   ` Douglas Anderson
2021-06-24 13:36   ` Greg KH
2021-06-24 13:36     ` Greg KH
2021-06-24 13:42     ` Doug Anderson
2021-06-24 13:42       ` Doug Anderson
2021-06-21 23:52 ` [PATCH 3/6] PCI: Indicate that we want to force strict DMA for untrusted devices Douglas Anderson
2021-06-21 23:52   ` Douglas Anderson
2021-06-24 13:38   ` Greg KH
2021-06-24 13:38     ` Greg KH
2021-06-24 13:46     ` Doug Anderson
2021-06-24 13:46       ` Doug Anderson
2021-06-21 23:52 ` [PATCH 4/6] iommu: Combine device strictness requests with the global default Douglas Anderson
2021-06-21 23:52   ` Douglas Anderson
2021-06-22  2:03   ` Lu Baolu
2021-06-22  2:03     ` Lu Baolu
2021-06-22 16:53     ` Doug Anderson
2021-06-22 16:53       ` Doug Anderson
2021-06-22 17:01       ` Doug Anderson
2021-06-22 17:01         ` Doug Anderson
2021-06-22  2:55   ` Saravana Kannan
2021-06-22  2:55     ` Saravana Kannan via iommu
2021-06-22 16:40     ` Doug Anderson
2021-06-22 16:40       ` Doug Anderson
2021-06-22 19:50       ` Saravana Kannan
2021-06-22 19:50         ` Saravana Kannan via iommu
2021-06-22 11:49   ` Robin Murphy
2021-06-22 11:49     ` Robin Murphy
2021-06-22 18:45   ` Rajat Jain
2021-06-22 18:45     ` Rajat Jain via iommu
2021-06-22 19:35     ` Doug Anderson
2021-06-22 19:35       ` Doug Anderson
2021-06-21 23:52 ` [PATCH 5/6] iommu: Stop reaching into PCIe devices to decide strict vs. non-strict Douglas Anderson
2021-06-21 23:52   ` Douglas Anderson
2021-06-21 23:52 ` [PATCH 6/6] mmc: sdhci-msm: Request non-strict IOMMU mode Douglas Anderson
2021-06-21 23:52   ` Douglas Anderson
2021-06-24 13:43   ` Greg KH
2021-06-24 13:43     ` Greg KH
2021-06-24 14:00     ` Doug Anderson
2021-06-24 14:00       ` Doug Anderson
2021-06-22 11:35 ` [PATCH 0/6] iommu: Enable devices to request non-strict DMA, starting with QCom SD/MMC Robin Murphy
2021-06-22 11:35   ` Robin Murphy
2021-06-22 16:06   ` Doug Anderson
2021-06-22 16:06     ` Doug Anderson
2021-06-22 20:02     ` Rob Herring
2021-06-22 20:02       ` Rob Herring
2021-06-22 20:05       ` Saravana Kannan
2021-06-22 20:05         ` Saravana Kannan via iommu
2021-06-22 20:10         ` Doug Anderson
2021-06-22 20:10           ` Doug Anderson
2021-06-23 13:54           ` Rob Herring
2021-06-23 13:54             ` Rob Herring
2021-06-22 22:10     ` Robin Murphy
2021-06-22 22:10       ` Robin Murphy
2021-06-23 17:29       ` Doug Anderson
2021-06-23 17:29         ` Doug Anderson
2021-06-24 17:23         ` Doug Anderson
2021-06-24 17:23           ` Doug Anderson
2021-06-22 17:39 ` John Garry
2021-06-22 17:39   ` John Garry
2021-06-22 19:50   ` Doug Anderson
2021-06-22 19:50     ` Doug Anderson

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=20210621165230.1.Id4ee5788c993294f66542721fca7719c00a5d8f3@changeid \
    --to=dianders@chromium.org \
    --cc=adrian.hunter@intel.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joel@joelfernandes.org \
    --cc=joro@8bytes.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=quic_c_gdjako@quicinc.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rafael@kernel.org \
    --cc=rajatja@google.com \
    --cc=robdclark@chromium.org \
    --cc=robin.murphy@arm.com \
    --cc=saiprakash.ranjan@codeaurora.org \
    --cc=saravanak@google.com \
    --cc=sonnyrao@chromium.org \
    --cc=ulf.hansson@linaro.org \
    --cc=vbadigan@codeaurora.org \
    --cc=will@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.