On 9/30/2022 5:19 AM, Greg Kroah-Hartman wrote:
On Wed, Sep 28, 2022 at 12:56:32PM -0700, Elliot Berman wrote:
Gunyah resource manager exposes a concrete functionalities which
complicate a single resource manager driver.
I am sorry, but I do not understand this sentance. What is so
complicated about individual devices being created? Where are they
created? What bus?
There's no complexity here with using individual devices, that's why I
wanted to create secondary (auxiliary devices).
IOW -- "I have a platform device that does a lot of different things.
Split up the different functionalities of that device into sub devices
using the auxiliary bus."
Use auxiliary bus
to help split high level functions for the resource manager and keep the
primary resource manager driver focused on the RPC with RM itself.
Delegate Resource Manager's console functionality to the auxiliary bus.
Signed-off-by: Elliot Berman <quic_eberman@xxxxxxxxxxx>
---
drivers/virt/gunyah/Kconfig | 1 +
drivers/virt/gunyah/rsc_mgr.c | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/drivers/virt/gunyah/Kconfig b/drivers/virt/gunyah/Kconfig
index 78deed3c4562..610c8586005b 100644
--- a/drivers/virt/gunyah/Kconfig
+++ b/drivers/virt/gunyah/Kconfig
@@ -17,6 +17,7 @@ config GUNYAH_RESORUCE_MANAGER
tristate "Gunyah Resource Manager"
select MAILBOX
select GUNYAH_MESSAGE_QUEUES
+ select AUXILIARY_BUS
default y
help
The resource manager (RM) is a privileged application VM supporting
diff --git a/drivers/virt/gunyah/rsc_mgr.c b/drivers/virt/gunyah/rsc_mgr.c
index 7f7e89a6436b..435fe0149915 100644
--- a/drivers/virt/gunyah/rsc_mgr.c
+++ b/drivers/virt/gunyah/rsc_mgr.c
@@ -16,6 +16,7 @@
#include <linux/notifier.h>
#include <linux/workqueue.h>
#include <linux/completion.h>
+#include <linux/auxiliary_bus.h>
#include <linux/gunyah_rsc_mgr.h>
#include <linux/platform_device.h>
@@ -98,6 +99,8 @@ struct gh_rsc_mgr {
struct mutex send_lock;
struct work_struct recv_work;
+
+ struct auxiliary_device console_adev;
};
static struct gh_rsc_mgr *__rsc_mgr;
@@ -573,13 +576,31 @@ static int gh_rm_drv_probe(struct platform_device *pdev)
__rsc_mgr = rsc_mgr;
+ rsc_mgr->console_adev.dev.parent = &pdev->dev;
+ rsc_mgr->console_adev.name = "console";
+ ret = auxiliary_device_init(&rsc_mgr->console_adev);
+ if (ret)
+ goto err_msgq;
+ ret = auxiliary_device_add(&rsc_mgr->console_adev);
+ if (ret)
+ goto err_console_adev_uninit;
+
return 0;
+
+err_console_adev_uninit:
+ auxiliary_device_uninit(&rsc_mgr->console_adev);
+err_msgq:
+ gunyah_msgq_remove(&rsc_mgr->msgq);
+ return ret;
}
Why can't you just have individual platform devices for the individual
devices your hypervisor exposes?
You control the platform devices, why force them to be shared like this?
The resource manager exposes quite a bit of functionality, and I think
it makes sense to expose them as auxiliary devices. From
Documentation/driver-api/auxiliary_bus.rst:
A key requirement for utilizing the auxiliary bus is that there is no
dependency on a physical bus, device, register accesses or regmap support.
These individual devices split from the core cannot live on the platform
bus as
they are not physical devices that are controlled by DT/ACPI.
thanks,
greg k-h
