Messages in this thread Patch in this message | | | Subject | Re: [PATCH v3 04/18] driver core: platform: Add driver dma ownership management | From | Lu Baolu <> | Date | Thu, 9 Dec 2021 09:20:10 +0800 |
| |
On 12/7/21 9:16 PM, Jason Gunthorpe wrote: > On Tue, Dec 07, 2021 at 10:57:25AM +0800, Lu Baolu wrote: >> On 12/6/21 11:06 PM, Jason Gunthorpe wrote: >>> On Mon, Dec 06, 2021 at 06:36:27AM -0800, Christoph Hellwig wrote: >>>> I really hate the amount of boilerplate code that having this in each >>>> bus type causes. >>> +1 >>> >>> I liked the first version of this series better with the code near >>> really_probe(). >>> >>> Can we go back to that with some device_configure_dma() wrapper >>> condtionally called by really_probe as we discussed? >>> >> >> Are you talking about below change? >> >> diff --git a/drivers/base/dd.c b/drivers/base/dd.c >> index 68ea1f949daa..368f9e530515 100644 >> +++ b/drivers/base/dd.c >> @@ -577,7 +577,13 @@ static int really_probe(struct device *dev, struct >> device_driver *drv) >> if (dev->bus->dma_configure) { >> ret = dev->bus->dma_configure(dev); >> if (ret) >> - goto probe_failed; >> + goto pinctrl_bind_failed; >> + >> + if (!drv->no_kernel_dma) { >> + ret = iommu_device_set_dma_owner(dev, DMA_OWNER_DMA_API, NULL); >> + if (ret) >> + goto pinctrl_bind_failed; >> + } >> } > > Yes, the suggestion was to put everything that 'if' inside a function > and then of course a matching undo function.
Followed your suggestion, I refactored the change like below:
diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 68ea1f949daa..68ca5a579eb1 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -538,6 +538,32 @@ static int call_driver_probe(struct device *dev, struct device_driver *drv) return ret; }
+static int device_dma_configure(struct device *dev, struct device_driver *drv) +{ + int ret; + + if (!dev->bus->dma_configure) + return 0; + + ret = dev->bus->dma_configure(dev); + if (ret) + return ret; + + if (!drv->suppress_auto_claim_dma_owner) + ret = iommu_device_set_dma_owner(dev, DMA_OWNER_DMA_API, NULL); + + return ret; +} + +static void device_dma_cleanup(struct device *dev, struct device_driver *drv) +{ + if (!dev->bus->dma_configure) + return; + + if (!drv->suppress_auto_claim_dma_owner) + iommu_device_release_dma_owner(dev, DMA_OWNER_DMA_API, NULL); +} + static int really_probe(struct device *dev, struct device_driver *drv) { bool test_remove = IS_ENABLED(CONFIG_DEBUG_TEST_DRIVER_REMOVE) && @@ -574,11 +600,8 @@ static int really_probe(struct device *dev, struct device_driver *drv) if (ret) goto pinctrl_bind_failed;
- if (dev->bus->dma_configure) { - ret = dev->bus->dma_configure(dev); - if (ret) - goto probe_failed; - } + if (device_dma_configure(dev, drv)) + goto pinctrl_bind_failed;
ret = driver_sysfs_add(dev); if (ret) { @@ -660,6 +683,8 @@ 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); + + device_dma_cleanup(dev, drv); pinctrl_bind_failed: device_links_no_driver(dev); devres_release_all(dev); @@ -1204,6 +1229,7 @@ static void __device_release_driver(struct device *dev, struct device *parent) else if (drv->remove) drv->remove(dev);
+ device_dma_cleanup(dev, drv); device_links_driver_cleanup(dev);
devres_release_all(dev); diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h index a498ebcf4993..374a3c2cc10d 100644 --- a/include/linux/device/driver.h +++ b/include/linux/device/driver.h @@ -100,6 +100,7 @@ struct device_driver { const char *mod_name; /* used for built-in modules */
bool suppress_bind_attrs; /* disables bind/unbind via sysfs */ + bool suppress_auto_claim_dma_owner; enum probe_type probe_type;
const struct of_device_id *of_match_table; Further suggestions?
Best regards, baolu
| |