| Message ID | 20211206015903.88687-4-baolu.lu@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Fix BUG_ON in vfio_iommu_group_notifier() | expand |
On Mon, Dec 06, 2021 at 09:58:48AM +0800, Lu Baolu wrote: > The platform_dma_configure() is shared between platform and amba bus > drivers. Rename the common helper to firmware_dma_configure() so that > both platform and amba bus drivers could customize their dma_configure > callbacks. Please, if you are going to call these functions "firmware_" then move them to the drivers/firmware/ location, they do not belong in drivers/base/platform.c anymore, right? thanks, greg k-h
On Mon, Dec 06, 2021 at 08:53:07AM +0100, Greg Kroah-Hartman wrote: > On Mon, Dec 06, 2021 at 09:58:48AM +0800, Lu Baolu wrote: > > The platform_dma_configure() is shared between platform and amba bus > > drivers. Rename the common helper to firmware_dma_configure() so that > > both platform and amba bus drivers could customize their dma_configure > > callbacks. > > Please, if you are going to call these functions "firmware_" then move > them to the drivers/firmware/ location, they do not belong in > drivers/base/platform.c anymore, right? firmware seems rather misnamed anyway, amba doesn't reall have anything to do with "firmware".
On Mon, Dec 06, 2021 at 06:13:01AM -0800, Christoph Hellwig wrote: > On Mon, Dec 06, 2021 at 08:53:07AM +0100, Greg Kroah-Hartman wrote: > > On Mon, Dec 06, 2021 at 09:58:48AM +0800, Lu Baolu wrote: > > > The platform_dma_configure() is shared between platform and amba bus > > > drivers. Rename the common helper to firmware_dma_configure() so that > > > both platform and amba bus drivers could customize their dma_configure > > > callbacks. > > > > Please, if you are going to call these functions "firmware_" then move > > them to the drivers/firmware/ location, they do not belong in > > drivers/base/platform.c anymore, right? > > firmware seems rather misnamed anyway, amba doesn't reall have anything > to do with "firmware". Then the name is not a good one and should be called something else :)
On Mon, Dec 06, 2021 at 06:13:01AM -0800, Christoph Hellwig wrote: > On Mon, Dec 06, 2021 at 08:53:07AM +0100, Greg Kroah-Hartman wrote: > > On Mon, Dec 06, 2021 at 09:58:48AM +0800, Lu Baolu wrote: > > > The platform_dma_configure() is shared between platform and amba bus > > > drivers. Rename the common helper to firmware_dma_configure() so that > > > both platform and amba bus drivers could customize their dma_configure > > > callbacks. > > > > Please, if you are going to call these functions "firmware_" then move > > them to the drivers/firmware/ location, they do not belong in > > drivers/base/platform.c anymore, right? > > firmware seems rather misnamed anyway, amba doesn't reall have anything > to do with "firmware". IIRC the only thing this function does is touch ACPI and OF stuff? Isn't that firmware? AFAICT amba uses this because AMBA devices might be linked to DT descriptions? Jason
On Mon, Dec 06, 2021 at 10:45:35AM -0400, Jason Gunthorpe via iommu wrote: > IIRC the only thing this function does is touch ACPI and OF stuff? > Isn't that firmware? > > AFAICT amba uses this because AMBA devices might be linked to DT > descriptions? But DT descriptions aren't firmware. They are usually either passed onb the bootloader or in some deeply embedded setups embedded into the kernel image.
On Mon, Dec 06, 2021 at 06:47:45AM -0800, Christoph Hellwig wrote: > On Mon, Dec 06, 2021 at 10:45:35AM -0400, Jason Gunthorpe via iommu wrote: > > IIRC the only thing this function does is touch ACPI and OF stuff? > > Isn't that firmware? > > > > AFAICT amba uses this because AMBA devices might be linked to DT > > descriptions? > > But DT descriptions aren't firmware. They are usually either passed onb > the bootloader or in some deeply embedded setups embedded into the > kernel image. Pedenatically yes, but do you know of a common word to refer to both OF and ACPI that is better than firmware? :) AFAICT we already use firwmare for this in a few places, eg fwnode_handle and so on. Jason
On 12/6/21 11:04 PM, Jason Gunthorpe wrote: > On Mon, Dec 06, 2021 at 06:47:45AM -0800, Christoph Hellwig wrote: >> On Mon, Dec 06, 2021 at 10:45:35AM -0400, Jason Gunthorpe via iommu wrote: >>> IIRC the only thing this function does is touch ACPI and OF stuff? >>> Isn't that firmware? >>> >>> AFAICT amba uses this because AMBA devices might be linked to DT >>> descriptions? >> But DT descriptions aren't firmware. They are usually either passed onb >> the bootloader or in some deeply embedded setups embedded into the >> kernel image. > Pedenatically yes, but do you know of a common word to refer to both > OF and ACPI that is better than firmware?:) If the firmware_ name is confusing, how about common_dma_configure()? Or, copy the 6 lines of code to amba bus driver? Best regards, baolu
On Mon, Dec 6, 2021 at 7:04 AM Jason Gunthorpe <jgg@nvidia.com> wrote: > > On Mon, Dec 06, 2021 at 06:47:45AM -0800, Christoph Hellwig wrote: > > On Mon, Dec 06, 2021 at 10:45:35AM -0400, Jason Gunthorpe via iommu wrote: > > > IIRC the only thing this function does is touch ACPI and OF stuff? > > > Isn't that firmware? > > > > > > AFAICT amba uses this because AMBA devices might be linked to DT > > > descriptions? > > > > But DT descriptions aren't firmware. They are usually either passed onb > > the bootloader or in some deeply embedded setups embedded into the > > kernel image. > > Pedenatically yes, but do you know of a common word to refer to both > OF and ACPI that is better than firmware? :) > > AFAICT we already use firwmare for this in a few places, eg > fwnode_handle and so on. I've always thought 'platform' was the generic name for otherwise non-enumerable platform-firmware/data things enumerated by ACPI / OF.
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 7c96f169d274..4381c34af7e0 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -328,7 +328,7 @@ extern int platform_pm_restore(struct device *dev); #define platform_pm_restore NULL #endif -extern int platform_dma_configure(struct device *dev); +extern int firmware_dma_configure(struct device *dev); #ifdef CONFIG_PM_SLEEP #define USE_PLATFORM_PM_SLEEP_OPS \ diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c index 720aa6cdd402..08c094124c0e 100644 --- a/drivers/amba/bus.c +++ b/drivers/amba/bus.c @@ -319,7 +319,7 @@ struct bus_type amba_bustype = { .probe = amba_probe, .remove = amba_remove, .shutdown = amba_shutdown, - .dma_configure = platform_dma_configure, + .dma_configure = firmware_dma_configure, .pm = &amba_pm, }; EXPORT_SYMBOL_GPL(amba_bustype); diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 598acf93a360..125d708c0eb3 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -1449,8 +1449,7 @@ static void platform_shutdown(struct device *_dev) drv->shutdown(dev); } - -int platform_dma_configure(struct device *dev) +int firmware_dma_configure(struct device *dev) { enum dev_dma_attr attr; int ret = 0; @@ -1478,7 +1477,7 @@ struct bus_type platform_bus_type = { .probe = platform_probe, .remove = platform_remove, .shutdown = platform_shutdown, - .dma_configure = platform_dma_configure, + .dma_configure = firmware_dma_configure, .pm = &platform_dev_pm_ops, }; EXPORT_SYMBOL_GPL(platform_bus_type);
The platform_dma_configure() is shared between platform and amba bus drivers. Rename the common helper to firmware_dma_configure() so that both platform and amba bus drivers could customize their dma_configure callbacks. Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> --- include/linux/platform_device.h | 2 +- drivers/amba/bus.c | 2 +- drivers/base/platform.c | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-)