* [PATCH 0/9] Rework SCMI initialization and probing sequence
@ 2022-12-22 18:50 ` Cristian Marussi
0 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Hi,
under some configurations the SCMI core stack, which is now initialized
as a whole at the subsys_initcall level, can be dependent on some other
Kernel subsystems (like TEE) when some SCMI transport backend like optee
is used.
This has been reported to lead to some awkward probe loop which, even
though successful at the end, leaves a track of errors in the logs coming
directly from the core Linux driver model facilities.
In order to solve this issue and cleaning up a bit the SCMI stack startup
sequence, this small series reviews and reworks the SCMI core stack
initialization and probe logic.
Basically the SCMI Bus is split into its own module (scmi-core.ko) which is
initialized at subsys_initcall, while the SCMI core stack, including its
various transport backends (like optee, mailbox, virtio, smc), is kept into
a distinct module (scmi-module.ko) which get initialized at module_init.
The SCMI driver users initlevel, instead, remains unchanged at module_init.
No change is made to the Kconfig: the main ARM_SCMI_PROTOCOL option will
now cause both modules to be built.
This allows the other possibly needed subsystems to be up and running
well before the core SCMI stack and its dependent transport backends, so
solving the reported issue.
Tested with SCMI transports mailbox/virtio and, in a previous draft, optee,
in a number of different load/unload/bind/unbind combinations both as
builtin and as LKMs.
Applies on v6.1.
Any feedback, testing welcome.
Thanks,
Cristian
Cristian Marussi (9):
firmware: arm_scmi: Simplify chan_available transport operation
firmware: arm_scmi: Use dedicated devices to initialize channels
firmware: arm_scmi: Move protocol registration helpers
firmware: arm_scmi: Add common notifier helpers
firmware: arm_scmi: Refactor protocol device creation
firmware: arm_scmi: Move handle get/set helpers
firmware: arm_scmi: Refactor device create/destroy helpers
firmware: arm_scmi: Introduce a new lifecycle for protocol devices
firmware: arm_scmi: Split bus and driver into distinct modules
drivers/firmware/arm_scmi/Makefile | 8 +-
drivers/firmware/arm_scmi/bus.c | 388 ++++++++++++-----
drivers/firmware/arm_scmi/common.h | 25 +-
drivers/firmware/arm_scmi/driver.c | 623 ++++++++++++++--------------
drivers/firmware/arm_scmi/mailbox.c | 6 +-
drivers/firmware/arm_scmi/optee.c | 6 +-
drivers/firmware/arm_scmi/smc.c | 6 +-
drivers/firmware/arm_scmi/virtio.c | 4 +-
include/linux/scmi_protocol.h | 5 -
9 files changed, 622 insertions(+), 449 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 0/9] Rework SCMI initialization and probing sequence
@ 2022-12-22 18:50 ` Cristian Marussi
0 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Hi,
under some configurations the SCMI core stack, which is now initialized
as a whole at the subsys_initcall level, can be dependent on some other
Kernel subsystems (like TEE) when some SCMI transport backend like optee
is used.
This has been reported to lead to some awkward probe loop which, even
though successful at the end, leaves a track of errors in the logs coming
directly from the core Linux driver model facilities.
In order to solve this issue and cleaning up a bit the SCMI stack startup
sequence, this small series reviews and reworks the SCMI core stack
initialization and probe logic.
Basically the SCMI Bus is split into its own module (scmi-core.ko) which is
initialized at subsys_initcall, while the SCMI core stack, including its
various transport backends (like optee, mailbox, virtio, smc), is kept into
a distinct module (scmi-module.ko) which get initialized at module_init.
The SCMI driver users initlevel, instead, remains unchanged at module_init.
No change is made to the Kconfig: the main ARM_SCMI_PROTOCOL option will
now cause both modules to be built.
This allows the other possibly needed subsystems to be up and running
well before the core SCMI stack and its dependent transport backends, so
solving the reported issue.
Tested with SCMI transports mailbox/virtio and, in a previous draft, optee,
in a number of different load/unload/bind/unbind combinations both as
builtin and as LKMs.
Applies on v6.1.
Any feedback, testing welcome.
Thanks,
Cristian
Cristian Marussi (9):
firmware: arm_scmi: Simplify chan_available transport operation
firmware: arm_scmi: Use dedicated devices to initialize channels
firmware: arm_scmi: Move protocol registration helpers
firmware: arm_scmi: Add common notifier helpers
firmware: arm_scmi: Refactor protocol device creation
firmware: arm_scmi: Move handle get/set helpers
firmware: arm_scmi: Refactor device create/destroy helpers
firmware: arm_scmi: Introduce a new lifecycle for protocol devices
firmware: arm_scmi: Split bus and driver into distinct modules
drivers/firmware/arm_scmi/Makefile | 8 +-
drivers/firmware/arm_scmi/bus.c | 388 ++++++++++++-----
drivers/firmware/arm_scmi/common.h | 25 +-
drivers/firmware/arm_scmi/driver.c | 623 ++++++++++++++--------------
drivers/firmware/arm_scmi/mailbox.c | 6 +-
drivers/firmware/arm_scmi/optee.c | 6 +-
drivers/firmware/arm_scmi/smc.c | 6 +-
drivers/firmware/arm_scmi/virtio.c | 4 +-
include/linux/scmi_protocol.h | 5 -
9 files changed, 622 insertions(+), 449 deletions(-)
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/9] firmware: arm_scmi: Simplify chan_available transport operation
2022-12-22 18:50 ` Cristian Marussi
@ 2022-12-22 18:50 ` Cristian Marussi
-1 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
SCMI transport operation .chan_available determines in a transport-specific
way if an SCMI channel is still available and to be configured: such
information is derived analyzing bits of DT in a transport specific way;
all it needs is a DT node to operate upon, not necessarily a full blown
device.
Simplify the helper to receive in input a reference to a device_node
instead of a device carrying a pointer to such device_node.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/common.h | 2 +-
drivers/firmware/arm_scmi/driver.c | 2 +-
drivers/firmware/arm_scmi/mailbox.c | 4 ++--
drivers/firmware/arm_scmi/optee.c | 4 ++--
drivers/firmware/arm_scmi/smc.c | 4 ++--
drivers/firmware/arm_scmi/virtio.c | 2 +-
6 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index a1c0154c31c6..16db1e177123 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -153,7 +153,7 @@ struct scmi_chan_info {
*/
struct scmi_transport_ops {
int (*link_supplier)(struct device *dev);
- bool (*chan_available)(struct device *dev, int idx);
+ bool (*chan_available)(struct device_node *of_node, int idx);
int (*chan_setup)(struct scmi_chan_info *cinfo, struct device *dev,
bool tx);
int (*chan_free)(int id, void *p, void *data);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index ffdad59ec81f..f1ebadffdfe4 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2003,7 +2003,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev,
if (cinfo)
return 0;
- if (!info->desc->ops->chan_available(dev, idx)) {
+ if (!info->desc->ops->chan_available(dev->of_node, idx)) {
cinfo = idr_find(idr, SCMI_PROTOCOL_BASE);
if (unlikely(!cinfo)) /* Possible only if platform has no Rx */
return -EINVAL;
diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
index 1e40cb035044..c33dcadafea8 100644
--- a/drivers/firmware/arm_scmi/mailbox.c
+++ b/drivers/firmware/arm_scmi/mailbox.c
@@ -46,9 +46,9 @@ static void rx_callback(struct mbox_client *cl, void *m)
scmi_rx_callback(smbox->cinfo, shmem_read_header(smbox->shmem), NULL);
}
-static bool mailbox_chan_available(struct device *dev, int idx)
+static bool mailbox_chan_available(struct device_node *of_node, int idx)
{
- return !of_parse_phandle_with_args(dev->of_node, "mboxes",
+ return !of_parse_phandle_with_args(of_node, "mboxes",
"#mbox-cells", idx, NULL);
}
diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c
index 2a7aeab40e54..5a11091c72da 100644
--- a/drivers/firmware/arm_scmi/optee.c
+++ b/drivers/firmware/arm_scmi/optee.c
@@ -328,11 +328,11 @@ static int scmi_optee_link_supplier(struct device *dev)
return 0;
}
-static bool scmi_optee_chan_available(struct device *dev, int idx)
+static bool scmi_optee_chan_available(struct device_node *of_node, int idx)
{
u32 channel_id;
- return !of_property_read_u32_index(dev->of_node, "linaro,optee-channel-id",
+ return !of_property_read_u32_index(of_node, "linaro,optee-channel-id",
idx, &channel_id);
}
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
index 87a7b13cf868..122128d56d2f 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -52,9 +52,9 @@ static irqreturn_t smc_msg_done_isr(int irq, void *data)
return IRQ_HANDLED;
}
-static bool smc_chan_available(struct device *dev, int idx)
+static bool smc_chan_available(struct device_node *of_node, int idx)
{
- struct device_node *np = of_parse_phandle(dev->of_node, "shmem", 0);
+ struct device_node *np = of_parse_phandle(of_node, "shmem", 0);
if (!np)
return false;
diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c
index 1db975c08896..a917eca7d902 100644
--- a/drivers/firmware/arm_scmi/virtio.c
+++ b/drivers/firmware/arm_scmi/virtio.c
@@ -385,7 +385,7 @@ static int virtio_link_supplier(struct device *dev)
return 0;
}
-static bool virtio_chan_available(struct device *dev, int idx)
+static bool virtio_chan_available(struct device_node *of_node, int idx)
{
struct scmi_vio_channel *channels, *vioch = NULL;
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 1/9] firmware: arm_scmi: Simplify chan_available transport operation
@ 2022-12-22 18:50 ` Cristian Marussi
0 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
SCMI transport operation .chan_available determines in a transport-specific
way if an SCMI channel is still available and to be configured: such
information is derived analyzing bits of DT in a transport specific way;
all it needs is a DT node to operate upon, not necessarily a full blown
device.
Simplify the helper to receive in input a reference to a device_node
instead of a device carrying a pointer to such device_node.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/common.h | 2 +-
drivers/firmware/arm_scmi/driver.c | 2 +-
drivers/firmware/arm_scmi/mailbox.c | 4 ++--
drivers/firmware/arm_scmi/optee.c | 4 ++--
drivers/firmware/arm_scmi/smc.c | 4 ++--
drivers/firmware/arm_scmi/virtio.c | 2 +-
6 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index a1c0154c31c6..16db1e177123 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -153,7 +153,7 @@ struct scmi_chan_info {
*/
struct scmi_transport_ops {
int (*link_supplier)(struct device *dev);
- bool (*chan_available)(struct device *dev, int idx);
+ bool (*chan_available)(struct device_node *of_node, int idx);
int (*chan_setup)(struct scmi_chan_info *cinfo, struct device *dev,
bool tx);
int (*chan_free)(int id, void *p, void *data);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index ffdad59ec81f..f1ebadffdfe4 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2003,7 +2003,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev,
if (cinfo)
return 0;
- if (!info->desc->ops->chan_available(dev, idx)) {
+ if (!info->desc->ops->chan_available(dev->of_node, idx)) {
cinfo = idr_find(idr, SCMI_PROTOCOL_BASE);
if (unlikely(!cinfo)) /* Possible only if platform has no Rx */
return -EINVAL;
diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
index 1e40cb035044..c33dcadafea8 100644
--- a/drivers/firmware/arm_scmi/mailbox.c
+++ b/drivers/firmware/arm_scmi/mailbox.c
@@ -46,9 +46,9 @@ static void rx_callback(struct mbox_client *cl, void *m)
scmi_rx_callback(smbox->cinfo, shmem_read_header(smbox->shmem), NULL);
}
-static bool mailbox_chan_available(struct device *dev, int idx)
+static bool mailbox_chan_available(struct device_node *of_node, int idx)
{
- return !of_parse_phandle_with_args(dev->of_node, "mboxes",
+ return !of_parse_phandle_with_args(of_node, "mboxes",
"#mbox-cells", idx, NULL);
}
diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c
index 2a7aeab40e54..5a11091c72da 100644
--- a/drivers/firmware/arm_scmi/optee.c
+++ b/drivers/firmware/arm_scmi/optee.c
@@ -328,11 +328,11 @@ static int scmi_optee_link_supplier(struct device *dev)
return 0;
}
-static bool scmi_optee_chan_available(struct device *dev, int idx)
+static bool scmi_optee_chan_available(struct device_node *of_node, int idx)
{
u32 channel_id;
- return !of_property_read_u32_index(dev->of_node, "linaro,optee-channel-id",
+ return !of_property_read_u32_index(of_node, "linaro,optee-channel-id",
idx, &channel_id);
}
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
index 87a7b13cf868..122128d56d2f 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -52,9 +52,9 @@ static irqreturn_t smc_msg_done_isr(int irq, void *data)
return IRQ_HANDLED;
}
-static bool smc_chan_available(struct device *dev, int idx)
+static bool smc_chan_available(struct device_node *of_node, int idx)
{
- struct device_node *np = of_parse_phandle(dev->of_node, "shmem", 0);
+ struct device_node *np = of_parse_phandle(of_node, "shmem", 0);
if (!np)
return false;
diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c
index 1db975c08896..a917eca7d902 100644
--- a/drivers/firmware/arm_scmi/virtio.c
+++ b/drivers/firmware/arm_scmi/virtio.c
@@ -385,7 +385,7 @@ static int virtio_link_supplier(struct device *dev)
return 0;
}
-static bool virtio_chan_available(struct device *dev, int idx)
+static bool virtio_chan_available(struct device_node *of_node, int idx)
{
struct scmi_vio_channel *channels, *vioch = NULL;
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/9] firmware: arm_scmi: Use dedicated devices to initialize channels
2022-12-22 18:50 ` Cristian Marussi
@ 2022-12-22 18:50 ` Cristian Marussi
-1 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Refactor channels initialization to use dedicated transport devices instead
of using devices borrowed from the SCMI drivers.
Initialize all channels, as described in the DT, upfront during SCMI core
stack probe phase and free all of them, including the underlying devices,
when the SCMI core is removed.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/common.h | 1 -
drivers/firmware/arm_scmi/driver.c | 158 ++++++++++++++++++++--------
drivers/firmware/arm_scmi/mailbox.c | 2 -
drivers/firmware/arm_scmi/optee.c | 2 -
drivers/firmware/arm_scmi/smc.c | 2 -
drivers/firmware/arm_scmi/virtio.c | 2 -
6 files changed, 116 insertions(+), 51 deletions(-)
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 16db1e177123..136bfd30f99c 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -229,7 +229,6 @@ extern const struct scmi_desc scmi_optee_desc;
#endif
void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv);
-void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, int id);
/* shmem related declarations */
struct scmi_shared_mem;
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index f1ebadffdfe4..f85b34e69d01 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -1987,23 +1987,20 @@ static int scmi_xfer_info_init(struct scmi_info *sinfo)
return ret;
}
-static int scmi_chan_setup(struct scmi_info *info, struct device *dev,
+static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
int prot_id, bool tx)
{
int ret, idx;
+ char name[32];
struct scmi_chan_info *cinfo;
struct idr *idr;
+ struct scmi_device *tdev = NULL;
/* Transmit channel is first entry i.e. index 0 */
idx = tx ? 0 : 1;
idr = tx ? &info->tx_idr : &info->rx_idr;
- /* check if already allocated, used for multiple device per protocol */
- cinfo = idr_find(idr, prot_id);
- if (cinfo)
- return 0;
-
- if (!info->desc->ops->chan_available(dev->of_node, idx)) {
+ if (!info->desc->ops->chan_available(of_node, idx)) {
cinfo = idr_find(idr, SCMI_PROTOCOL_BASE);
if (unlikely(!cinfo)) /* Possible only if platform has no Rx */
return -EINVAL;
@@ -2014,27 +2011,49 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev,
if (!cinfo)
return -ENOMEM;
- cinfo->dev = dev;
cinfo->rx_timeout_ms = info->desc->max_rx_timeout_ms;
+ /* Create a unique name for this transport device */
+ snprintf(name, 32, "__scmi_transport_device_%s_%02X",
+ idx ? "rx" : "tx", prot_id);
+ /* Create a uniquely named, dedicated transport device for this chan */
+ tdev = scmi_device_create(of_node, info->dev, prot_id, name);
+ if (!tdev) {
+ devm_kfree(info->dev, cinfo);
+ return -EINVAL;
+ }
+ of_node_get(of_node);
+
+ cinfo->dev = &tdev->dev;
ret = info->desc->ops->chan_setup(cinfo, info->dev, tx);
- if (ret)
+ if (ret) {
+ of_node_put(of_node);
+ scmi_device_destroy(tdev);
+ devm_kfree(info->dev, cinfo);
return ret;
+ }
if (tx && is_polling_required(cinfo, info)) {
if (is_transport_polling_capable(info))
- dev_info(dev,
+ dev_info(&tdev->dev,
"Enabled polling mode TX channel - prot_id:%d\n",
prot_id);
else
- dev_warn(dev,
+ dev_warn(&tdev->dev,
"Polling mode NOT supported by transport.\n");
}
idr_alloc:
ret = idr_alloc(idr, cinfo, prot_id, prot_id + 1, GFP_KERNEL);
if (ret != prot_id) {
- dev_err(dev, "unable to allocate SCMI idr slot err %d\n", ret);
+ dev_err(info->dev,
+ "unable to allocate SCMI idr slot err %d\n", ret);
+ /* Destroy channel and device only if created by this call. */
+ if (tdev) {
+ of_node_put(of_node);
+ scmi_device_destroy(tdev);
+ devm_kfree(info->dev, cinfo);
+ }
return ret;
}
@@ -2043,13 +2062,14 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev,
}
static inline int
-scmi_txrx_setup(struct scmi_info *info, struct device *dev, int prot_id)
+scmi_txrx_setup(struct scmi_info *info, struct device_node *of_node,
+ int prot_id)
{
- int ret = scmi_chan_setup(info, dev, prot_id, true);
+ int ret = scmi_chan_setup(info, of_node, prot_id, true);
if (!ret) {
/* Rx is optional, report only memory errors */
- ret = scmi_chan_setup(info, dev, prot_id, false);
+ ret = scmi_chan_setup(info, of_node, prot_id, false);
if (ret && ret != -ENOMEM)
ret = 0;
}
@@ -2057,6 +2077,54 @@ scmi_txrx_setup(struct scmi_info *info, struct device *dev, int prot_id)
return ret;
}
+/**
+ * scmi_channels_setup - Helper to initialize all required channels
+ *
+ * @info: The SCMI instance descriptor.
+ *
+ * Initialize all the channels found described in the DT against the underlying
+ * configured transport using custom defined dedicated devices instead of
+ * borrowing devices from the SCMI drivers; this way channels are initialized
+ * upfront during core SCMI stack probing and are no more coupled with SCMI
+ * devices used by SCMI drivers.
+ *
+ * Note that, even though a pair of TX/RX channels is associated to each
+ * protocol defined in the DT, a distinct freshly initialized channel is
+ * created only if the DT node for the protocol at hand describes a dedicated
+ * channel: in all the other cases the common BASE protocol channel is reused.
+ *
+ * Return: 0 on Success
+ */
+static int scmi_channels_setup(struct scmi_info *info)
+{
+ int ret;
+ struct device_node *child, *top_np = info->dev->of_node;
+
+ /* Initialize a common generic channel at first */
+ ret = scmi_txrx_setup(info, top_np, SCMI_PROTOCOL_BASE);
+ if (ret)
+ return ret;
+
+ for_each_available_child_of_node(top_np, child) {
+ u32 prot_id;
+
+ if (of_property_read_u32(child, "reg", &prot_id))
+ continue;
+
+ if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id))
+ dev_err(info->dev,
+ "Out of range protocol %d\n", prot_id);
+
+ ret = scmi_txrx_setup(info, child, prot_id);
+ if (ret) {
+ of_node_put(child);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
/**
* scmi_get_protocol_device - Helper to get/create an SCMI device.
*
@@ -2106,14 +2174,6 @@ scmi_get_protocol_device(struct device_node *np, struct scmi_info *info,
return NULL;
}
- if (scmi_txrx_setup(info, &sdev->dev, prot_id)) {
- dev_err(&sdev->dev, "failed to setup transport\n");
- scmi_device_destroy(sdev);
- mutex_unlock(&scmi_syspower_mtx);
-
- return NULL;
- }
-
if (prot_id == SCMI_PROTOCOL_SYSTEM)
scmi_syspower_registered = true;
@@ -2347,19 +2407,39 @@ void scmi_protocol_device_unrequest(const struct scmi_device_id *id_table)
mutex_unlock(&scmi_requested_devices_mtx);
}
-static int scmi_cleanup_txrx_channels(struct scmi_info *info)
+static int scmi_chan_destroy(int id, void *p, void *idr)
{
- int ret;
- struct idr *idr = &info->tx_idr;
+ struct scmi_chan_info *cinfo = p;
+
+ if (cinfo->dev) {
+ struct scmi_device *sdev = to_scmi_dev(cinfo->dev);
- ret = idr_for_each(idr, info->desc->ops->chan_free, idr);
- idr_destroy(&info->tx_idr);
+ of_node_put(cinfo->dev->of_node);
+ scmi_device_destroy(sdev);
+ cinfo->dev = NULL;
+ }
- idr = &info->rx_idr;
- ret = idr_for_each(idr, info->desc->ops->chan_free, idr);
- idr_destroy(&info->rx_idr);
+ idr_remove(idr, id);
- return ret;
+ return 0;
+}
+
+static void scmi_cleanup_channels(struct scmi_info *info, struct idr *idr)
+{
+ /* At first free all channels at the transport layer ... */
+ idr_for_each(idr, info->desc->ops->chan_free, idr);
+
+ /* ...then destroy all underlying devices */
+ idr_for_each(idr, scmi_chan_destroy, idr);
+
+ idr_destroy(idr);
+}
+
+static void scmi_cleanup_txrx_channels(struct scmi_info *info)
+{
+ scmi_cleanup_channels(info, &info->tx_idr);
+
+ scmi_cleanup_channels(info, &info->rx_idr);
}
static int scmi_probe(struct platform_device *pdev)
@@ -2411,7 +2491,8 @@ static int scmi_probe(struct platform_device *pdev)
return ret;
}
- ret = scmi_txrx_setup(info, dev, SCMI_PROTOCOL_BASE);
+ /* Setup all channels described in the DT at first */
+ ret = scmi_channels_setup(info);
if (ret)
return ret;
@@ -2481,14 +2562,9 @@ static int scmi_probe(struct platform_device *pdev)
return ret;
}
-void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, int id)
-{
- idr_remove(idr, id);
-}
-
static int scmi_remove(struct platform_device *pdev)
{
- int ret, id;
+ int id;
struct scmi_info *info = platform_get_drvdata(pdev);
struct device_node *child;
@@ -2510,9 +2586,7 @@ static int scmi_remove(struct platform_device *pdev)
idr_destroy(&info->active_protocols);
/* Safe to free channels since no more users */
- ret = scmi_cleanup_txrx_channels(info);
- if (ret)
- dev_warn(&pdev->dev, "Failed to cleanup SCMI channels.\n");
+ scmi_cleanup_txrx_channels(info);
return 0;
}
diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
index c33dcadafea8..0d9c9538b7f4 100644
--- a/drivers/firmware/arm_scmi/mailbox.c
+++ b/drivers/firmware/arm_scmi/mailbox.c
@@ -120,8 +120,6 @@ static int mailbox_chan_free(int id, void *p, void *data)
smbox->cinfo = NULL;
}
- scmi_free_channel(cinfo, data, id);
-
return 0;
}
diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c
index 5a11091c72da..929720387102 100644
--- a/drivers/firmware/arm_scmi/optee.c
+++ b/drivers/firmware/arm_scmi/optee.c
@@ -481,8 +481,6 @@ static int scmi_optee_chan_free(int id, void *p, void *data)
cinfo->transport_info = NULL;
channel->cinfo = NULL;
- scmi_free_channel(cinfo, data, id);
-
return 0;
}
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
index 122128d56d2f..93272e4bbd12 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -171,8 +171,6 @@ static int smc_chan_free(int id, void *p, void *data)
cinfo->transport_info = NULL;
scmi_info->cinfo = NULL;
- scmi_free_channel(cinfo, data, id);
-
return 0;
}
diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c
index a917eca7d902..d68c01cb7aa0 100644
--- a/drivers/firmware/arm_scmi/virtio.c
+++ b/drivers/firmware/arm_scmi/virtio.c
@@ -489,8 +489,6 @@ static int virtio_chan_free(int id, void *p, void *data)
virtio_break_device(vioch->vqueue->vdev);
scmi_vio_channel_cleanup_sync(vioch);
- scmi_free_channel(cinfo, data, id);
-
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/9] firmware: arm_scmi: Use dedicated devices to initialize channels
@ 2022-12-22 18:50 ` Cristian Marussi
0 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Refactor channels initialization to use dedicated transport devices instead
of using devices borrowed from the SCMI drivers.
Initialize all channels, as described in the DT, upfront during SCMI core
stack probe phase and free all of them, including the underlying devices,
when the SCMI core is removed.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/common.h | 1 -
drivers/firmware/arm_scmi/driver.c | 158 ++++++++++++++++++++--------
drivers/firmware/arm_scmi/mailbox.c | 2 -
drivers/firmware/arm_scmi/optee.c | 2 -
drivers/firmware/arm_scmi/smc.c | 2 -
drivers/firmware/arm_scmi/virtio.c | 2 -
6 files changed, 116 insertions(+), 51 deletions(-)
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 16db1e177123..136bfd30f99c 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -229,7 +229,6 @@ extern const struct scmi_desc scmi_optee_desc;
#endif
void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv);
-void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, int id);
/* shmem related declarations */
struct scmi_shared_mem;
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index f1ebadffdfe4..f85b34e69d01 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -1987,23 +1987,20 @@ static int scmi_xfer_info_init(struct scmi_info *sinfo)
return ret;
}
-static int scmi_chan_setup(struct scmi_info *info, struct device *dev,
+static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
int prot_id, bool tx)
{
int ret, idx;
+ char name[32];
struct scmi_chan_info *cinfo;
struct idr *idr;
+ struct scmi_device *tdev = NULL;
/* Transmit channel is first entry i.e. index 0 */
idx = tx ? 0 : 1;
idr = tx ? &info->tx_idr : &info->rx_idr;
- /* check if already allocated, used for multiple device per protocol */
- cinfo = idr_find(idr, prot_id);
- if (cinfo)
- return 0;
-
- if (!info->desc->ops->chan_available(dev->of_node, idx)) {
+ if (!info->desc->ops->chan_available(of_node, idx)) {
cinfo = idr_find(idr, SCMI_PROTOCOL_BASE);
if (unlikely(!cinfo)) /* Possible only if platform has no Rx */
return -EINVAL;
@@ -2014,27 +2011,49 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev,
if (!cinfo)
return -ENOMEM;
- cinfo->dev = dev;
cinfo->rx_timeout_ms = info->desc->max_rx_timeout_ms;
+ /* Create a unique name for this transport device */
+ snprintf(name, 32, "__scmi_transport_device_%s_%02X",
+ idx ? "rx" : "tx", prot_id);
+ /* Create a uniquely named, dedicated transport device for this chan */
+ tdev = scmi_device_create(of_node, info->dev, prot_id, name);
+ if (!tdev) {
+ devm_kfree(info->dev, cinfo);
+ return -EINVAL;
+ }
+ of_node_get(of_node);
+
+ cinfo->dev = &tdev->dev;
ret = info->desc->ops->chan_setup(cinfo, info->dev, tx);
- if (ret)
+ if (ret) {
+ of_node_put(of_node);
+ scmi_device_destroy(tdev);
+ devm_kfree(info->dev, cinfo);
return ret;
+ }
if (tx && is_polling_required(cinfo, info)) {
if (is_transport_polling_capable(info))
- dev_info(dev,
+ dev_info(&tdev->dev,
"Enabled polling mode TX channel - prot_id:%d\n",
prot_id);
else
- dev_warn(dev,
+ dev_warn(&tdev->dev,
"Polling mode NOT supported by transport.\n");
}
idr_alloc:
ret = idr_alloc(idr, cinfo, prot_id, prot_id + 1, GFP_KERNEL);
if (ret != prot_id) {
- dev_err(dev, "unable to allocate SCMI idr slot err %d\n", ret);
+ dev_err(info->dev,
+ "unable to allocate SCMI idr slot err %d\n", ret);
+ /* Destroy channel and device only if created by this call. */
+ if (tdev) {
+ of_node_put(of_node);
+ scmi_device_destroy(tdev);
+ devm_kfree(info->dev, cinfo);
+ }
return ret;
}
@@ -2043,13 +2062,14 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev,
}
static inline int
-scmi_txrx_setup(struct scmi_info *info, struct device *dev, int prot_id)
+scmi_txrx_setup(struct scmi_info *info, struct device_node *of_node,
+ int prot_id)
{
- int ret = scmi_chan_setup(info, dev, prot_id, true);
+ int ret = scmi_chan_setup(info, of_node, prot_id, true);
if (!ret) {
/* Rx is optional, report only memory errors */
- ret = scmi_chan_setup(info, dev, prot_id, false);
+ ret = scmi_chan_setup(info, of_node, prot_id, false);
if (ret && ret != -ENOMEM)
ret = 0;
}
@@ -2057,6 +2077,54 @@ scmi_txrx_setup(struct scmi_info *info, struct device *dev, int prot_id)
return ret;
}
+/**
+ * scmi_channels_setup - Helper to initialize all required channels
+ *
+ * @info: The SCMI instance descriptor.
+ *
+ * Initialize all the channels found described in the DT against the underlying
+ * configured transport using custom defined dedicated devices instead of
+ * borrowing devices from the SCMI drivers; this way channels are initialized
+ * upfront during core SCMI stack probing and are no more coupled with SCMI
+ * devices used by SCMI drivers.
+ *
+ * Note that, even though a pair of TX/RX channels is associated to each
+ * protocol defined in the DT, a distinct freshly initialized channel is
+ * created only if the DT node for the protocol at hand describes a dedicated
+ * channel: in all the other cases the common BASE protocol channel is reused.
+ *
+ * Return: 0 on Success
+ */
+static int scmi_channels_setup(struct scmi_info *info)
+{
+ int ret;
+ struct device_node *child, *top_np = info->dev->of_node;
+
+ /* Initialize a common generic channel at first */
+ ret = scmi_txrx_setup(info, top_np, SCMI_PROTOCOL_BASE);
+ if (ret)
+ return ret;
+
+ for_each_available_child_of_node(top_np, child) {
+ u32 prot_id;
+
+ if (of_property_read_u32(child, "reg", &prot_id))
+ continue;
+
+ if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id))
+ dev_err(info->dev,
+ "Out of range protocol %d\n", prot_id);
+
+ ret = scmi_txrx_setup(info, child, prot_id);
+ if (ret) {
+ of_node_put(child);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
/**
* scmi_get_protocol_device - Helper to get/create an SCMI device.
*
@@ -2106,14 +2174,6 @@ scmi_get_protocol_device(struct device_node *np, struct scmi_info *info,
return NULL;
}
- if (scmi_txrx_setup(info, &sdev->dev, prot_id)) {
- dev_err(&sdev->dev, "failed to setup transport\n");
- scmi_device_destroy(sdev);
- mutex_unlock(&scmi_syspower_mtx);
-
- return NULL;
- }
-
if (prot_id == SCMI_PROTOCOL_SYSTEM)
scmi_syspower_registered = true;
@@ -2347,19 +2407,39 @@ void scmi_protocol_device_unrequest(const struct scmi_device_id *id_table)
mutex_unlock(&scmi_requested_devices_mtx);
}
-static int scmi_cleanup_txrx_channels(struct scmi_info *info)
+static int scmi_chan_destroy(int id, void *p, void *idr)
{
- int ret;
- struct idr *idr = &info->tx_idr;
+ struct scmi_chan_info *cinfo = p;
+
+ if (cinfo->dev) {
+ struct scmi_device *sdev = to_scmi_dev(cinfo->dev);
- ret = idr_for_each(idr, info->desc->ops->chan_free, idr);
- idr_destroy(&info->tx_idr);
+ of_node_put(cinfo->dev->of_node);
+ scmi_device_destroy(sdev);
+ cinfo->dev = NULL;
+ }
- idr = &info->rx_idr;
- ret = idr_for_each(idr, info->desc->ops->chan_free, idr);
- idr_destroy(&info->rx_idr);
+ idr_remove(idr, id);
- return ret;
+ return 0;
+}
+
+static void scmi_cleanup_channels(struct scmi_info *info, struct idr *idr)
+{
+ /* At first free all channels at the transport layer ... */
+ idr_for_each(idr, info->desc->ops->chan_free, idr);
+
+ /* ...then destroy all underlying devices */
+ idr_for_each(idr, scmi_chan_destroy, idr);
+
+ idr_destroy(idr);
+}
+
+static void scmi_cleanup_txrx_channels(struct scmi_info *info)
+{
+ scmi_cleanup_channels(info, &info->tx_idr);
+
+ scmi_cleanup_channels(info, &info->rx_idr);
}
static int scmi_probe(struct platform_device *pdev)
@@ -2411,7 +2491,8 @@ static int scmi_probe(struct platform_device *pdev)
return ret;
}
- ret = scmi_txrx_setup(info, dev, SCMI_PROTOCOL_BASE);
+ /* Setup all channels described in the DT at first */
+ ret = scmi_channels_setup(info);
if (ret)
return ret;
@@ -2481,14 +2562,9 @@ static int scmi_probe(struct platform_device *pdev)
return ret;
}
-void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, int id)
-{
- idr_remove(idr, id);
-}
-
static int scmi_remove(struct platform_device *pdev)
{
- int ret, id;
+ int id;
struct scmi_info *info = platform_get_drvdata(pdev);
struct device_node *child;
@@ -2510,9 +2586,7 @@ static int scmi_remove(struct platform_device *pdev)
idr_destroy(&info->active_protocols);
/* Safe to free channels since no more users */
- ret = scmi_cleanup_txrx_channels(info);
- if (ret)
- dev_warn(&pdev->dev, "Failed to cleanup SCMI channels.\n");
+ scmi_cleanup_txrx_channels(info);
return 0;
}
diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
index c33dcadafea8..0d9c9538b7f4 100644
--- a/drivers/firmware/arm_scmi/mailbox.c
+++ b/drivers/firmware/arm_scmi/mailbox.c
@@ -120,8 +120,6 @@ static int mailbox_chan_free(int id, void *p, void *data)
smbox->cinfo = NULL;
}
- scmi_free_channel(cinfo, data, id);
-
return 0;
}
diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c
index 5a11091c72da..929720387102 100644
--- a/drivers/firmware/arm_scmi/optee.c
+++ b/drivers/firmware/arm_scmi/optee.c
@@ -481,8 +481,6 @@ static int scmi_optee_chan_free(int id, void *p, void *data)
cinfo->transport_info = NULL;
channel->cinfo = NULL;
- scmi_free_channel(cinfo, data, id);
-
return 0;
}
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
index 122128d56d2f..93272e4bbd12 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -171,8 +171,6 @@ static int smc_chan_free(int id, void *p, void *data)
cinfo->transport_info = NULL;
scmi_info->cinfo = NULL;
- scmi_free_channel(cinfo, data, id);
-
return 0;
}
diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c
index a917eca7d902..d68c01cb7aa0 100644
--- a/drivers/firmware/arm_scmi/virtio.c
+++ b/drivers/firmware/arm_scmi/virtio.c
@@ -489,8 +489,6 @@ static int virtio_chan_free(int id, void *p, void *data)
virtio_break_device(vioch->vqueue->vdev);
scmi_vio_channel_cleanup_sync(vioch);
- scmi_free_channel(cinfo, data, id);
-
return 0;
}
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/9] firmware: arm_scmi: Move protocol registration helpers
2022-12-22 18:50 ` Cristian Marussi
@ 2022-12-22 18:50 ` Cristian Marussi
-1 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Move protocol registration helpers and logic out of bus.c compilation
unit into driver.c.
No functional change.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/bus.c | 68 ------------------------------
drivers/firmware/arm_scmi/common.h | 3 --
drivers/firmware/arm_scmi/driver.c | 67 +++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 71 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 35bb70724d44..089957f5fb9b 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -16,8 +16,6 @@
#include "common.h"
static DEFINE_IDA(scmi_bus_id);
-static DEFINE_IDR(scmi_protocols);
-static DEFINE_SPINLOCK(protocol_lock);
static const struct scmi_device_id *
scmi_dev_match_id(struct scmi_device *scmi_dev, struct scmi_driver *scmi_drv)
@@ -76,30 +74,6 @@ struct scmi_device *scmi_child_dev_find(struct device *parent,
return to_scmi_dev(dev);
}
-const struct scmi_protocol *scmi_protocol_get(int protocol_id)
-{
- const struct scmi_protocol *proto;
-
- proto = idr_find(&scmi_protocols, protocol_id);
- if (!proto || !try_module_get(proto->owner)) {
- pr_warn("SCMI Protocol 0x%x not found!\n", protocol_id);
- return NULL;
- }
-
- pr_debug("Found SCMI Protocol 0x%x\n", protocol_id);
-
- return proto;
-}
-
-void scmi_protocol_put(int protocol_id)
-{
- const struct scmi_protocol *proto;
-
- proto = idr_find(&scmi_protocols, protocol_id);
- if (proto)
- module_put(proto->owner);
-}
-
static int scmi_dev_probe(struct device *dev)
{
struct scmi_driver *scmi_drv = to_scmi_driver(dev->driver);
@@ -232,48 +206,6 @@ void scmi_set_handle(struct scmi_device *scmi_dev)
scmi_device_link_add(&scmi_dev->dev, scmi_dev->handle->dev);
}
-int scmi_protocol_register(const struct scmi_protocol *proto)
-{
- int ret;
-
- if (!proto) {
- pr_err("invalid protocol\n");
- return -EINVAL;
- }
-
- if (!proto->instance_init) {
- pr_err("missing init for protocol 0x%x\n", proto->id);
- return -EINVAL;
- }
-
- spin_lock(&protocol_lock);
- ret = idr_alloc(&scmi_protocols, (void *)proto,
- proto->id, proto->id + 1, GFP_ATOMIC);
- spin_unlock(&protocol_lock);
- if (ret != proto->id) {
- pr_err("unable to allocate SCMI idr slot for 0x%x - err %d\n",
- proto->id, ret);
- return ret;
- }
-
- pr_debug("Registered SCMI Protocol 0x%x\n", proto->id);
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(scmi_protocol_register);
-
-void scmi_protocol_unregister(const struct scmi_protocol *proto)
-{
- spin_lock(&protocol_lock);
- idr_remove(&scmi_protocols, proto->id);
- spin_unlock(&protocol_lock);
-
- pr_debug("Unregistered SCMI Protocol 0x%x\n", proto->id);
-
- return;
-}
-EXPORT_SYMBOL_GPL(scmi_protocol_unregister);
-
static int __scmi_devices_unregister(struct device *dev, void *data)
{
struct scmi_device *scmi_dev = to_scmi_dev(dev);
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 136bfd30f99c..6a38244494fd 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -106,9 +106,6 @@ void scmi_setup_protocol_implemented(const struct scmi_protocol_handle *ph,
int __init scmi_bus_init(void);
void __exit scmi_bus_exit(void);
-const struct scmi_protocol *scmi_protocol_get(int protocol_id);
-void scmi_protocol_put(int protocol_id);
-
int scmi_protocol_acquire(const struct scmi_handle *handle, u8 protocol_id);
void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index f85b34e69d01..d1e32ea6d90a 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -51,6 +51,9 @@ enum scmi_error_codes {
SCMI_ERR_PROTOCOL = -10,/* Protocol Error */
};
+static DEFINE_IDR(scmi_protocols);
+static DEFINE_SPINLOCK(protocol_lock);
+
/* List of all SCMI devices active in system */
static LIST_HEAD(scmi_list);
/* Protection for the entire list */
@@ -194,6 +197,70 @@ static inline int scmi_to_linux_errno(int errno)
return -EIO;
}
+static const struct scmi_protocol *scmi_protocol_get(int protocol_id)
+{
+ const struct scmi_protocol *proto;
+
+ proto = idr_find(&scmi_protocols, protocol_id);
+ if (!proto || !try_module_get(proto->owner)) {
+ pr_warn("SCMI Protocol 0x%x not found!\n", protocol_id);
+ return NULL;
+ }
+
+ pr_debug("Found SCMI Protocol 0x%x\n", protocol_id);
+
+ return proto;
+}
+
+static void scmi_protocol_put(int protocol_id)
+{
+ const struct scmi_protocol *proto;
+
+ proto = idr_find(&scmi_protocols, protocol_id);
+ if (proto)
+ module_put(proto->owner);
+}
+
+int scmi_protocol_register(const struct scmi_protocol *proto)
+{
+ int ret;
+
+ if (!proto) {
+ pr_err("invalid protocol\n");
+ return -EINVAL;
+ }
+
+ if (!proto->instance_init) {
+ pr_err("missing init for protocol 0x%x\n", proto->id);
+ return -EINVAL;
+ }
+
+ spin_lock(&protocol_lock);
+ ret = idr_alloc(&scmi_protocols, (void *)proto,
+ proto->id, proto->id + 1, GFP_ATOMIC);
+ spin_unlock(&protocol_lock);
+ if (ret != proto->id) {
+ pr_err("unable to allocate SCMI idr slot for 0x%x - err %d\n",
+ proto->id, ret);
+ return ret;
+ }
+
+ pr_debug("Registered SCMI Protocol 0x%x\n", proto->id);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(scmi_protocol_register);
+
+void scmi_protocol_unregister(const struct scmi_protocol *proto)
+{
+ spin_lock(&protocol_lock);
+ idr_remove(&scmi_protocols, proto->id);
+ spin_unlock(&protocol_lock);
+
+ pr_debug("Unregistered SCMI Protocol 0x%x\n", proto->id);
+}
+EXPORT_SYMBOL_GPL(scmi_protocol_unregister);
+
void scmi_notification_instance_data_set(const struct scmi_handle *handle,
void *priv)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/9] firmware: arm_scmi: Move protocol registration helpers
@ 2022-12-22 18:50 ` Cristian Marussi
0 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Move protocol registration helpers and logic out of bus.c compilation
unit into driver.c.
No functional change.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/bus.c | 68 ------------------------------
drivers/firmware/arm_scmi/common.h | 3 --
drivers/firmware/arm_scmi/driver.c | 67 +++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 71 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 35bb70724d44..089957f5fb9b 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -16,8 +16,6 @@
#include "common.h"
static DEFINE_IDA(scmi_bus_id);
-static DEFINE_IDR(scmi_protocols);
-static DEFINE_SPINLOCK(protocol_lock);
static const struct scmi_device_id *
scmi_dev_match_id(struct scmi_device *scmi_dev, struct scmi_driver *scmi_drv)
@@ -76,30 +74,6 @@ struct scmi_device *scmi_child_dev_find(struct device *parent,
return to_scmi_dev(dev);
}
-const struct scmi_protocol *scmi_protocol_get(int protocol_id)
-{
- const struct scmi_protocol *proto;
-
- proto = idr_find(&scmi_protocols, protocol_id);
- if (!proto || !try_module_get(proto->owner)) {
- pr_warn("SCMI Protocol 0x%x not found!\n", protocol_id);
- return NULL;
- }
-
- pr_debug("Found SCMI Protocol 0x%x\n", protocol_id);
-
- return proto;
-}
-
-void scmi_protocol_put(int protocol_id)
-{
- const struct scmi_protocol *proto;
-
- proto = idr_find(&scmi_protocols, protocol_id);
- if (proto)
- module_put(proto->owner);
-}
-
static int scmi_dev_probe(struct device *dev)
{
struct scmi_driver *scmi_drv = to_scmi_driver(dev->driver);
@@ -232,48 +206,6 @@ void scmi_set_handle(struct scmi_device *scmi_dev)
scmi_device_link_add(&scmi_dev->dev, scmi_dev->handle->dev);
}
-int scmi_protocol_register(const struct scmi_protocol *proto)
-{
- int ret;
-
- if (!proto) {
- pr_err("invalid protocol\n");
- return -EINVAL;
- }
-
- if (!proto->instance_init) {
- pr_err("missing init for protocol 0x%x\n", proto->id);
- return -EINVAL;
- }
-
- spin_lock(&protocol_lock);
- ret = idr_alloc(&scmi_protocols, (void *)proto,
- proto->id, proto->id + 1, GFP_ATOMIC);
- spin_unlock(&protocol_lock);
- if (ret != proto->id) {
- pr_err("unable to allocate SCMI idr slot for 0x%x - err %d\n",
- proto->id, ret);
- return ret;
- }
-
- pr_debug("Registered SCMI Protocol 0x%x\n", proto->id);
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(scmi_protocol_register);
-
-void scmi_protocol_unregister(const struct scmi_protocol *proto)
-{
- spin_lock(&protocol_lock);
- idr_remove(&scmi_protocols, proto->id);
- spin_unlock(&protocol_lock);
-
- pr_debug("Unregistered SCMI Protocol 0x%x\n", proto->id);
-
- return;
-}
-EXPORT_SYMBOL_GPL(scmi_protocol_unregister);
-
static int __scmi_devices_unregister(struct device *dev, void *data)
{
struct scmi_device *scmi_dev = to_scmi_dev(dev);
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 136bfd30f99c..6a38244494fd 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -106,9 +106,6 @@ void scmi_setup_protocol_implemented(const struct scmi_protocol_handle *ph,
int __init scmi_bus_init(void);
void __exit scmi_bus_exit(void);
-const struct scmi_protocol *scmi_protocol_get(int protocol_id);
-void scmi_protocol_put(int protocol_id);
-
int scmi_protocol_acquire(const struct scmi_handle *handle, u8 protocol_id);
void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index f85b34e69d01..d1e32ea6d90a 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -51,6 +51,9 @@ enum scmi_error_codes {
SCMI_ERR_PROTOCOL = -10,/* Protocol Error */
};
+static DEFINE_IDR(scmi_protocols);
+static DEFINE_SPINLOCK(protocol_lock);
+
/* List of all SCMI devices active in system */
static LIST_HEAD(scmi_list);
/* Protection for the entire list */
@@ -194,6 +197,70 @@ static inline int scmi_to_linux_errno(int errno)
return -EIO;
}
+static const struct scmi_protocol *scmi_protocol_get(int protocol_id)
+{
+ const struct scmi_protocol *proto;
+
+ proto = idr_find(&scmi_protocols, protocol_id);
+ if (!proto || !try_module_get(proto->owner)) {
+ pr_warn("SCMI Protocol 0x%x not found!\n", protocol_id);
+ return NULL;
+ }
+
+ pr_debug("Found SCMI Protocol 0x%x\n", protocol_id);
+
+ return proto;
+}
+
+static void scmi_protocol_put(int protocol_id)
+{
+ const struct scmi_protocol *proto;
+
+ proto = idr_find(&scmi_protocols, protocol_id);
+ if (proto)
+ module_put(proto->owner);
+}
+
+int scmi_protocol_register(const struct scmi_protocol *proto)
+{
+ int ret;
+
+ if (!proto) {
+ pr_err("invalid protocol\n");
+ return -EINVAL;
+ }
+
+ if (!proto->instance_init) {
+ pr_err("missing init for protocol 0x%x\n", proto->id);
+ return -EINVAL;
+ }
+
+ spin_lock(&protocol_lock);
+ ret = idr_alloc(&scmi_protocols, (void *)proto,
+ proto->id, proto->id + 1, GFP_ATOMIC);
+ spin_unlock(&protocol_lock);
+ if (ret != proto->id) {
+ pr_err("unable to allocate SCMI idr slot for 0x%x - err %d\n",
+ proto->id, ret);
+ return ret;
+ }
+
+ pr_debug("Registered SCMI Protocol 0x%x\n", proto->id);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(scmi_protocol_register);
+
+void scmi_protocol_unregister(const struct scmi_protocol *proto)
+{
+ spin_lock(&protocol_lock);
+ idr_remove(&scmi_protocols, proto->id);
+ spin_unlock(&protocol_lock);
+
+ pr_debug("Unregistered SCMI Protocol 0x%x\n", proto->id);
+}
+EXPORT_SYMBOL_GPL(scmi_protocol_unregister);
+
void scmi_notification_instance_data_set(const struct scmi_handle *handle,
void *priv)
{
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 4/9] firmware: arm_scmi: Add common notifier helpers
2022-12-22 18:50 ` Cristian Marussi
@ 2022-12-22 18:50 ` Cristian Marussi
-1 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Add a pair of notifier chains and generic empty notifier callbacks: still
currently unused but they will be used to act properly on device request
and creation events.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/bus.c | 6 ++-
drivers/firmware/arm_scmi/common.h | 5 ++
drivers/firmware/arm_scmi/driver.c | 83 +++++++++++++++++++++++++++++-
3 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 089957f5fb9b..bed566f58029 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -15,6 +15,9 @@
#include "common.h"
+BLOCKING_NOTIFIER_HEAD(scmi_requested_devices_nh);
+EXPORT_SYMBOL_GPL(scmi_requested_devices_nh);
+
static DEFINE_IDA(scmi_bus_id);
static const struct scmi_device_id *
@@ -94,12 +97,13 @@ static void scmi_dev_remove(struct device *dev)
scmi_drv->remove(scmi_dev);
}
-static struct bus_type scmi_bus_type = {
+struct bus_type scmi_bus_type = {
.name = "scmi_protocol",
.match = scmi_dev_match,
.probe = scmi_dev_probe,
.remove = scmi_dev_remove,
};
+EXPORT_SYMBOL_GPL(scmi_bus_type);
int scmi_driver_register(struct scmi_driver *driver, struct module *owner,
const char *mod_name)
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 6a38244494fd..7ddae90eb945 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -105,6 +105,11 @@ void scmi_setup_protocol_implemented(const struct scmi_protocol_handle *ph,
int __init scmi_bus_init(void);
void __exit scmi_bus_exit(void);
+extern struct bus_type scmi_bus_type;
+
+#define SCMI_BUS_NOTIFY_DEVICE_REQUEST 0
+#define SCMI_BUS_NOTIFY_DEVICE_UNREQUEST 1
+extern struct blocking_notifier_head scmi_requested_devices_nh;
int scmi_protocol_acquire(const struct scmi_handle *handle, u8 protocol_id);
void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index d1e32ea6d90a..62760bed1645 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -150,6 +150,9 @@ struct scmi_protocol_instance {
* @notify_priv: Pointer to private data structure specific to notifications.
* @node: List head
* @users: Number of users of this instance
+ * @bus_nb: A notifier to listen for device bind/unbind on the scmi bus
+ * @dev_req_nb: A notifier to listen for device request/unrequest on the scmi
+ * bus
*/
struct scmi_info {
struct device *dev;
@@ -169,9 +172,13 @@ struct scmi_info {
void *notify_priv;
struct list_head node;
int users;
+ struct notifier_block bus_nb;
+ struct notifier_block dev_req_nb;
};
#define handle_to_scmi_info(h) container_of(h, struct scmi_info, handle)
+#define bus_nb_to_scmi_info(nb) container_of(nb, struct scmi_info, bus_nb)
+#define req_nb_to_scmi_info(nb) container_of(nb, struct scmi_info, dev_req_nb)
static const int scmi_linux_errmap[] = {
/* better than switch case as long as return value is continuous */
@@ -2509,6 +2516,60 @@ static void scmi_cleanup_txrx_channels(struct scmi_info *info)
scmi_cleanup_channels(info, &info->rx_idr);
}
+static int scmi_bus_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct scmi_info *info = bus_nb_to_scmi_info(nb);
+ struct scmi_device *sdev = to_scmi_dev(data);
+
+ /* Skip transport devices and devices of different SCMI instances */
+ if (!strncmp(sdev->name, "__scmi_transport_device", 23) ||
+ sdev->dev.parent != info->dev)
+ return NOTIFY_DONE;
+
+ switch (action) {
+ case BUS_NOTIFY_BIND_DRIVER:
+ break;
+ case BUS_NOTIFY_UNBOUND_DRIVER:
+ break;
+ default:
+ return NOTIFY_DONE;
+ }
+
+ dev_dbg(info->dev, "Device %s (%s) is now %s\n", dev_name(&sdev->dev),
+ sdev->name, action == BUS_NOTIFY_BIND_DRIVER ?
+ "about to be BOUND." : "UNBOUND.");
+
+ return NOTIFY_OK;
+}
+
+static int scmi_device_request_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct device_node *np;
+ struct scmi_device_id *id_table = data;
+ struct scmi_info *info = req_nb_to_scmi_info(nb);
+
+ np = idr_find(&info->active_protocols, id_table->protocol_id);
+ if (!np)
+ return NOTIFY_DONE;
+
+ dev_dbg(info->dev, "%sRequested device (%s) for protocol 0x%x\n",
+ action == SCMI_BUS_NOTIFY_DEVICE_REQUEST ? "" : "UN-",
+ id_table->name, id_table->protocol_id);
+
+ switch (action) {
+ case SCMI_BUS_NOTIFY_DEVICE_REQUEST:
+ break;
+ case SCMI_BUS_NOTIFY_DEVICE_UNREQUEST:
+ break;
+ default:
+ return NOTIFY_DONE;
+ }
+
+ return NOTIFY_OK;
+}
+
static int scmi_probe(struct platform_device *pdev)
{
int ret;
@@ -2528,6 +2589,8 @@ static int scmi_probe(struct platform_device *pdev)
info->dev = dev;
info->desc = desc;
+ info->bus_nb.notifier_call = scmi_bus_notifier;
+ info->dev_req_nb.notifier_call = scmi_device_request_notifier;
INIT_LIST_HEAD(&info->node);
idr_init(&info->protocols);
mutex_init(&info->protocols_mtx);
@@ -2563,10 +2626,19 @@ static int scmi_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = scmi_xfer_info_init(info);
+ ret = bus_register_notifier(&scmi_bus_type, &info->bus_nb);
if (ret)
goto clear_txrx_setup;
+ ret = blocking_notifier_chain_register(&scmi_requested_devices_nh,
+ &info->dev_req_nb);
+ if (ret)
+ goto clear_bus_notifier;
+
+ ret = scmi_xfer_info_init(info);
+ if (ret)
+ goto clear_dev_req_notifier;
+
if (scmi_notification_init(handle))
dev_err(dev, "SCMI Notifications NOT available.\n");
@@ -2624,6 +2696,11 @@ static int scmi_probe(struct platform_device *pdev)
notification_exit:
scmi_notification_exit(&info->handle);
+clear_dev_req_notifier:
+ blocking_notifier_chain_unregister(&scmi_requested_devices_nh,
+ &info->dev_req_nb);
+clear_bus_notifier:
+ bus_unregister_notifier(&scmi_bus_type, &info->bus_nb);
clear_txrx_setup:
scmi_cleanup_txrx_channels(info);
return ret;
@@ -2652,6 +2729,10 @@ static int scmi_remove(struct platform_device *pdev)
of_node_put(child);
idr_destroy(&info->active_protocols);
+ blocking_notifier_chain_unregister(&scmi_requested_devices_nh,
+ &info->dev_req_nb);
+ bus_unregister_notifier(&scmi_bus_type, &info->bus_nb);
+
/* Safe to free channels since no more users */
scmi_cleanup_txrx_channels(info);
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 4/9] firmware: arm_scmi: Add common notifier helpers
@ 2022-12-22 18:50 ` Cristian Marussi
0 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Add a pair of notifier chains and generic empty notifier callbacks: still
currently unused but they will be used to act properly on device request
and creation events.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/bus.c | 6 ++-
drivers/firmware/arm_scmi/common.h | 5 ++
drivers/firmware/arm_scmi/driver.c | 83 +++++++++++++++++++++++++++++-
3 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 089957f5fb9b..bed566f58029 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -15,6 +15,9 @@
#include "common.h"
+BLOCKING_NOTIFIER_HEAD(scmi_requested_devices_nh);
+EXPORT_SYMBOL_GPL(scmi_requested_devices_nh);
+
static DEFINE_IDA(scmi_bus_id);
static const struct scmi_device_id *
@@ -94,12 +97,13 @@ static void scmi_dev_remove(struct device *dev)
scmi_drv->remove(scmi_dev);
}
-static struct bus_type scmi_bus_type = {
+struct bus_type scmi_bus_type = {
.name = "scmi_protocol",
.match = scmi_dev_match,
.probe = scmi_dev_probe,
.remove = scmi_dev_remove,
};
+EXPORT_SYMBOL_GPL(scmi_bus_type);
int scmi_driver_register(struct scmi_driver *driver, struct module *owner,
const char *mod_name)
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 6a38244494fd..7ddae90eb945 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -105,6 +105,11 @@ void scmi_setup_protocol_implemented(const struct scmi_protocol_handle *ph,
int __init scmi_bus_init(void);
void __exit scmi_bus_exit(void);
+extern struct bus_type scmi_bus_type;
+
+#define SCMI_BUS_NOTIFY_DEVICE_REQUEST 0
+#define SCMI_BUS_NOTIFY_DEVICE_UNREQUEST 1
+extern struct blocking_notifier_head scmi_requested_devices_nh;
int scmi_protocol_acquire(const struct scmi_handle *handle, u8 protocol_id);
void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index d1e32ea6d90a..62760bed1645 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -150,6 +150,9 @@ struct scmi_protocol_instance {
* @notify_priv: Pointer to private data structure specific to notifications.
* @node: List head
* @users: Number of users of this instance
+ * @bus_nb: A notifier to listen for device bind/unbind on the scmi bus
+ * @dev_req_nb: A notifier to listen for device request/unrequest on the scmi
+ * bus
*/
struct scmi_info {
struct device *dev;
@@ -169,9 +172,13 @@ struct scmi_info {
void *notify_priv;
struct list_head node;
int users;
+ struct notifier_block bus_nb;
+ struct notifier_block dev_req_nb;
};
#define handle_to_scmi_info(h) container_of(h, struct scmi_info, handle)
+#define bus_nb_to_scmi_info(nb) container_of(nb, struct scmi_info, bus_nb)
+#define req_nb_to_scmi_info(nb) container_of(nb, struct scmi_info, dev_req_nb)
static const int scmi_linux_errmap[] = {
/* better than switch case as long as return value is continuous */
@@ -2509,6 +2516,60 @@ static void scmi_cleanup_txrx_channels(struct scmi_info *info)
scmi_cleanup_channels(info, &info->rx_idr);
}
+static int scmi_bus_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct scmi_info *info = bus_nb_to_scmi_info(nb);
+ struct scmi_device *sdev = to_scmi_dev(data);
+
+ /* Skip transport devices and devices of different SCMI instances */
+ if (!strncmp(sdev->name, "__scmi_transport_device", 23) ||
+ sdev->dev.parent != info->dev)
+ return NOTIFY_DONE;
+
+ switch (action) {
+ case BUS_NOTIFY_BIND_DRIVER:
+ break;
+ case BUS_NOTIFY_UNBOUND_DRIVER:
+ break;
+ default:
+ return NOTIFY_DONE;
+ }
+
+ dev_dbg(info->dev, "Device %s (%s) is now %s\n", dev_name(&sdev->dev),
+ sdev->name, action == BUS_NOTIFY_BIND_DRIVER ?
+ "about to be BOUND." : "UNBOUND.");
+
+ return NOTIFY_OK;
+}
+
+static int scmi_device_request_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct device_node *np;
+ struct scmi_device_id *id_table = data;
+ struct scmi_info *info = req_nb_to_scmi_info(nb);
+
+ np = idr_find(&info->active_protocols, id_table->protocol_id);
+ if (!np)
+ return NOTIFY_DONE;
+
+ dev_dbg(info->dev, "%sRequested device (%s) for protocol 0x%x\n",
+ action == SCMI_BUS_NOTIFY_DEVICE_REQUEST ? "" : "UN-",
+ id_table->name, id_table->protocol_id);
+
+ switch (action) {
+ case SCMI_BUS_NOTIFY_DEVICE_REQUEST:
+ break;
+ case SCMI_BUS_NOTIFY_DEVICE_UNREQUEST:
+ break;
+ default:
+ return NOTIFY_DONE;
+ }
+
+ return NOTIFY_OK;
+}
+
static int scmi_probe(struct platform_device *pdev)
{
int ret;
@@ -2528,6 +2589,8 @@ static int scmi_probe(struct platform_device *pdev)
info->dev = dev;
info->desc = desc;
+ info->bus_nb.notifier_call = scmi_bus_notifier;
+ info->dev_req_nb.notifier_call = scmi_device_request_notifier;
INIT_LIST_HEAD(&info->node);
idr_init(&info->protocols);
mutex_init(&info->protocols_mtx);
@@ -2563,10 +2626,19 @@ static int scmi_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = scmi_xfer_info_init(info);
+ ret = bus_register_notifier(&scmi_bus_type, &info->bus_nb);
if (ret)
goto clear_txrx_setup;
+ ret = blocking_notifier_chain_register(&scmi_requested_devices_nh,
+ &info->dev_req_nb);
+ if (ret)
+ goto clear_bus_notifier;
+
+ ret = scmi_xfer_info_init(info);
+ if (ret)
+ goto clear_dev_req_notifier;
+
if (scmi_notification_init(handle))
dev_err(dev, "SCMI Notifications NOT available.\n");
@@ -2624,6 +2696,11 @@ static int scmi_probe(struct platform_device *pdev)
notification_exit:
scmi_notification_exit(&info->handle);
+clear_dev_req_notifier:
+ blocking_notifier_chain_unregister(&scmi_requested_devices_nh,
+ &info->dev_req_nb);
+clear_bus_notifier:
+ bus_unregister_notifier(&scmi_bus_type, &info->bus_nb);
clear_txrx_setup:
scmi_cleanup_txrx_channels(info);
return ret;
@@ -2652,6 +2729,10 @@ static int scmi_remove(struct platform_device *pdev)
of_node_put(child);
idr_destroy(&info->active_protocols);
+ blocking_notifier_chain_unregister(&scmi_requested_devices_nh,
+ &info->dev_req_nb);
+ bus_unregister_notifier(&scmi_bus_type, &info->bus_nb);
+
/* Safe to free channels since no more users */
scmi_cleanup_txrx_channels(info);
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 5/9] firmware: arm_scmi: Refactor protocol device creation
2022-12-22 18:50 ` Cristian Marussi
@ 2022-12-22 18:50 ` Cristian Marussi
-1 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Move protocol device request helpers from driver.c compilation unit to
bus.c, so reducing the cross interactions between driver.c and bus.c.
Get rid of old protocol device creation process as a whole from driver.c
and remove also stale SCMI system power unicity checks.
While at that make such helpers call into scmi_requested_devices_nh
notification chain.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
Note that this patch disables SCMI device creation, it will be
restored later in the series, together with SCMI syspower checks,
using a new approach.
---
drivers/firmware/arm_scmi/bus.c | 157 ++++++++++++++-
drivers/firmware/arm_scmi/common.h | 2 -
drivers/firmware/arm_scmi/driver.c | 296 -----------------------------
3 files changed, 156 insertions(+), 299 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index bed566f58029..e63cc1194d43 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -9,6 +9,7 @@
#include <linux/types.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/device.h>
@@ -20,6 +21,160 @@ EXPORT_SYMBOL_GPL(scmi_requested_devices_nh);
static DEFINE_IDA(scmi_bus_id);
+static DEFINE_IDR(scmi_requested_devices);
+/* Protect access to scmi_requested_devices */
+static DEFINE_MUTEX(scmi_requested_devices_mtx);
+
+struct scmi_requested_dev {
+ const struct scmi_device_id *id_table;
+ struct list_head node;
+};
+
+/**
+ * scmi_protocol_device_request - Helper to request a device
+ *
+ * @id_table: A protocol/name pair descriptor for the device to be created.
+ *
+ * This helper let an SCMI driver request specific devices identified by the
+ * @id_table to be created for each active SCMI instance.
+ *
+ * The requested device name MUST NOT be already existent for any protocol;
+ * at first the freshly requested @id_table is annotated in the IDR table
+ * @scmi_requested_devices and then the requested device is advertised to any
+ * registered party via the @scmi_requested_devices_nh notification chain.
+ *
+ * Return: 0 on Success
+ */
+static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
+{
+ int ret = 0;
+ unsigned int id = 0;
+ struct list_head *head, *phead = NULL;
+ struct scmi_requested_dev *rdev;
+
+ pr_debug("Requesting SCMI device (%s) for protocol %x\n",
+ id_table->name, id_table->protocol_id);
+
+ /*
+ * Search for the matching protocol rdev list and then search
+ * of any existent equally named device...fails if any duplicate found.
+ */
+ mutex_lock(&scmi_requested_devices_mtx);
+ idr_for_each_entry(&scmi_requested_devices, head, id) {
+ if (!phead) {
+ /* A list found registered in the IDR is never empty */
+ rdev = list_first_entry(head, struct scmi_requested_dev,
+ node);
+ if (rdev->id_table->protocol_id ==
+ id_table->protocol_id)
+ phead = head;
+ }
+ list_for_each_entry(rdev, head, node) {
+ if (!strcmp(rdev->id_table->name, id_table->name)) {
+ pr_err("Ignoring duplicate request [%d] %s\n",
+ rdev->id_table->protocol_id,
+ rdev->id_table->name);
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+ }
+
+ /*
+ * No duplicate found for requested id_table, so let's create a new
+ * requested device entry for this new valid request.
+ */
+ rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
+ if (!rdev) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ rdev->id_table = id_table;
+
+ /*
+ * Append the new requested device table descriptor to the head of the
+ * related protocol list, eventually creating such head if not already
+ * there.
+ */
+ if (!phead) {
+ phead = kzalloc(sizeof(*phead), GFP_KERNEL);
+ if (!phead) {
+ kfree(rdev);
+ ret = -ENOMEM;
+ goto out;
+ }
+ INIT_LIST_HEAD(phead);
+
+ ret = idr_alloc(&scmi_requested_devices, (void *)phead,
+ id_table->protocol_id,
+ id_table->protocol_id + 1, GFP_KERNEL);
+ if (ret != id_table->protocol_id) {
+ pr_err("Failed to save SCMI device - ret:%d\n", ret);
+ kfree(rdev);
+ kfree(phead);
+ ret = -EINVAL;
+ goto out;
+ }
+ ret = 0;
+ }
+ list_add(&rdev->node, phead);
+
+out:
+ mutex_unlock(&scmi_requested_devices_mtx);
+
+ if (!ret)
+ blocking_notifier_call_chain(&scmi_requested_devices_nh,
+ SCMI_BUS_NOTIFY_DEVICE_REQUEST,
+ (void *)rdev->id_table);
+
+ return ret;
+}
+
+/**
+ * scmi_protocol_device_unrequest - Helper to unrequest a device
+ *
+ * @id_table: A protocol/name pair descriptor for the device to be unrequested.
+ *
+ * The unrequested device, described by the provided id_table, is at first
+ * removed from the IDR @scmi_requested_devices and then the removal is
+ * advertised to any registered party via the @scmi_requested_devices_nh
+ * notification chain.
+ */
+static void scmi_protocol_device_unrequest(const struct scmi_device_id *id_table)
+{
+ struct list_head *phead;
+
+ pr_debug("Unrequesting SCMI device (%s) for protocol %x\n",
+ id_table->name, id_table->protocol_id);
+
+ mutex_lock(&scmi_requested_devices_mtx);
+ phead = idr_find(&scmi_requested_devices, id_table->protocol_id);
+ if (phead) {
+ struct scmi_requested_dev *victim, *tmp;
+
+ list_for_each_entry_safe(victim, tmp, phead, node) {
+ if (!strcmp(victim->id_table->name, id_table->name)) {
+ list_del(&victim->node);
+
+ mutex_unlock(&scmi_requested_devices_mtx);
+ blocking_notifier_call_chain(&scmi_requested_devices_nh,
+ SCMI_BUS_NOTIFY_DEVICE_UNREQUEST,
+ (void *)victim->id_table);
+ kfree(victim);
+ mutex_lock(&scmi_requested_devices_mtx);
+ break;
+ }
+ }
+
+ if (list_empty(phead)) {
+ idr_remove(&scmi_requested_devices,
+ id_table->protocol_id);
+ kfree(phead);
+ }
+ }
+ mutex_unlock(&scmi_requested_devices_mtx);
+}
+
static const struct scmi_device_id *
scmi_dev_match_id(struct scmi_device *scmi_dev, struct scmi_driver *scmi_drv)
{
@@ -124,7 +279,7 @@ int scmi_driver_register(struct scmi_driver *driver, struct module *owner,
retval = driver_register(&driver->driver);
if (!retval)
- pr_debug("registered new scmi driver %s\n", driver->name);
+ pr_debug("Registered new scmi driver %s\n", driver->name);
return retval;
}
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 7ddae90eb945..0f411679df7e 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -172,8 +172,6 @@ struct scmi_transport_ops {
bool (*poll_done)(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer);
};
-int scmi_protocol_device_request(const struct scmi_device_id *id_table);
-void scmi_protocol_device_unrequest(const struct scmi_device_id *id_table);
struct scmi_device *scmi_child_dev_find(struct device *parent,
int prot_id, const char *name);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 62760bed1645..73f640e9448f 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -61,19 +61,6 @@ static DEFINE_MUTEX(scmi_list_mutex);
/* Track the unique id for the transfers for debug & profiling purpose */
static atomic_t transfer_last_id;
-static DEFINE_IDR(scmi_requested_devices);
-static DEFINE_MUTEX(scmi_requested_devices_mtx);
-
-/* Track globally the creation of SCMI SystemPower related devices */
-static bool scmi_syspower_registered;
-/* Protect access to scmi_syspower_registered */
-static DEFINE_MUTEX(scmi_syspower_mtx);
-
-struct scmi_requested_dev {
- const struct scmi_device_id *id_table;
- struct list_head node;
-};
-
/**
* struct scmi_xfers_info - Structure to manage transfer information
*
@@ -2199,288 +2186,6 @@ static int scmi_channels_setup(struct scmi_info *info)
return 0;
}
-/**
- * scmi_get_protocol_device - Helper to get/create an SCMI device.
- *
- * @np: A device node representing a valid active protocols for the referred
- * SCMI instance.
- * @info: The referred SCMI instance for which we are getting/creating this
- * device.
- * @prot_id: The protocol ID.
- * @name: The device name.
- *
- * Referring to the specific SCMI instance identified by @info, this helper
- * takes care to return a properly initialized device matching the requested
- * @proto_id and @name: if device was still not existent it is created as a
- * child of the specified SCMI instance @info and its transport properly
- * initialized as usual.
- *
- * Return: A properly initialized scmi device, NULL otherwise.
- */
-static inline struct scmi_device *
-scmi_get_protocol_device(struct device_node *np, struct scmi_info *info,
- int prot_id, const char *name)
-{
- struct scmi_device *sdev;
-
- /* Already created for this parent SCMI instance ? */
- sdev = scmi_child_dev_find(info->dev, prot_id, name);
- if (sdev)
- return sdev;
-
- mutex_lock(&scmi_syspower_mtx);
- if (prot_id == SCMI_PROTOCOL_SYSTEM && scmi_syspower_registered) {
- dev_warn(info->dev,
- "SCMI SystemPower protocol device must be unique !\n");
- mutex_unlock(&scmi_syspower_mtx);
-
- return NULL;
- }
-
- pr_debug("Creating SCMI device (%s) for protocol %x\n", name, prot_id);
-
- sdev = scmi_device_create(np, info->dev, prot_id, name);
- if (!sdev) {
- dev_err(info->dev, "failed to create %d protocol device\n",
- prot_id);
- mutex_unlock(&scmi_syspower_mtx);
-
- return NULL;
- }
-
- if (prot_id == SCMI_PROTOCOL_SYSTEM)
- scmi_syspower_registered = true;
-
- mutex_unlock(&scmi_syspower_mtx);
-
- return sdev;
-}
-
-static inline void
-scmi_create_protocol_device(struct device_node *np, struct scmi_info *info,
- int prot_id, const char *name)
-{
- struct scmi_device *sdev;
-
- sdev = scmi_get_protocol_device(np, info, prot_id, name);
- if (!sdev)
- return;
-
- /* setup handle now as the transport is ready */
- scmi_set_handle(sdev);
-}
-
-/**
- * scmi_create_protocol_devices - Create devices for all pending requests for
- * this SCMI instance.
- *
- * @np: The device node describing the protocol
- * @info: The SCMI instance descriptor
- * @prot_id: The protocol ID
- *
- * All devices previously requested for this instance (if any) are found and
- * created by scanning the proper @&scmi_requested_devices entry.
- */
-static void scmi_create_protocol_devices(struct device_node *np,
- struct scmi_info *info, int prot_id)
-{
- struct list_head *phead;
-
- mutex_lock(&scmi_requested_devices_mtx);
- phead = idr_find(&scmi_requested_devices, prot_id);
- if (phead) {
- struct scmi_requested_dev *rdev;
-
- list_for_each_entry(rdev, phead, node)
- scmi_create_protocol_device(np, info, prot_id,
- rdev->id_table->name);
- }
- mutex_unlock(&scmi_requested_devices_mtx);
-}
-
-/**
- * scmi_protocol_device_request - Helper to request a device
- *
- * @id_table: A protocol/name pair descriptor for the device to be created.
- *
- * This helper let an SCMI driver request specific devices identified by the
- * @id_table to be created for each active SCMI instance.
- *
- * The requested device name MUST NOT be already existent for any protocol;
- * at first the freshly requested @id_table is annotated in the IDR table
- * @scmi_requested_devices, then a matching device is created for each already
- * active SCMI instance. (if any)
- *
- * This way the requested device is created straight-away for all the already
- * initialized(probed) SCMI instances (handles) and it remains also annotated
- * as pending creation if the requesting SCMI driver was loaded before some
- * SCMI instance and related transports were available: when such late instance
- * is probed, its probe will take care to scan the list of pending requested
- * devices and create those on its own (see @scmi_create_protocol_devices and
- * its enclosing loop)
- *
- * Return: 0 on Success
- */
-int scmi_protocol_device_request(const struct scmi_device_id *id_table)
-{
- int ret = 0;
- unsigned int id = 0;
- struct list_head *head, *phead = NULL;
- struct scmi_requested_dev *rdev;
- struct scmi_info *info;
-
- pr_debug("Requesting SCMI device (%s) for protocol %x\n",
- id_table->name, id_table->protocol_id);
-
- /*
- * Search for the matching protocol rdev list and then search
- * of any existent equally named device...fails if any duplicate found.
- */
- mutex_lock(&scmi_requested_devices_mtx);
- idr_for_each_entry(&scmi_requested_devices, head, id) {
- if (!phead) {
- /* A list found registered in the IDR is never empty */
- rdev = list_first_entry(head, struct scmi_requested_dev,
- node);
- if (rdev->id_table->protocol_id ==
- id_table->protocol_id)
- phead = head;
- }
- list_for_each_entry(rdev, head, node) {
- if (!strcmp(rdev->id_table->name, id_table->name)) {
- pr_err("Ignoring duplicate request [%d] %s\n",
- rdev->id_table->protocol_id,
- rdev->id_table->name);
- ret = -EINVAL;
- goto out;
- }
- }
- }
-
- /*
- * No duplicate found for requested id_table, so let's create a new
- * requested device entry for this new valid request.
- */
- rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
- if (!rdev) {
- ret = -ENOMEM;
- goto out;
- }
- rdev->id_table = id_table;
-
- /*
- * Append the new requested device table descriptor to the head of the
- * related protocol list, eventually creating such head if not already
- * there.
- */
- if (!phead) {
- phead = kzalloc(sizeof(*phead), GFP_KERNEL);
- if (!phead) {
- kfree(rdev);
- ret = -ENOMEM;
- goto out;
- }
- INIT_LIST_HEAD(phead);
-
- ret = idr_alloc(&scmi_requested_devices, (void *)phead,
- id_table->protocol_id,
- id_table->protocol_id + 1, GFP_KERNEL);
- if (ret != id_table->protocol_id) {
- pr_err("Failed to save SCMI device - ret:%d\n", ret);
- kfree(rdev);
- kfree(phead);
- ret = -EINVAL;
- goto out;
- }
- ret = 0;
- }
- list_add(&rdev->node, phead);
-
- /*
- * Now effectively create and initialize the requested device for every
- * already initialized SCMI instance which has registered the requested
- * protocol as a valid active one: i.e. defined in DT and supported by
- * current platform FW.
- */
- mutex_lock(&scmi_list_mutex);
- list_for_each_entry(info, &scmi_list, node) {
- struct device_node *child;
-
- child = idr_find(&info->active_protocols,
- id_table->protocol_id);
- if (child) {
- struct scmi_device *sdev;
-
- sdev = scmi_get_protocol_device(child, info,
- id_table->protocol_id,
- id_table->name);
- if (sdev) {
- /* Set handle if not already set: device existed */
- if (!sdev->handle)
- sdev->handle =
- scmi_handle_get_from_info_unlocked(info);
- /* Relink consumer and suppliers */
- if (sdev->handle)
- scmi_device_link_add(&sdev->dev,
- sdev->handle->dev);
- }
- } else {
- dev_err(info->dev,
- "Failed. SCMI protocol %d not active.\n",
- id_table->protocol_id);
- }
- }
- mutex_unlock(&scmi_list_mutex);
-
-out:
- mutex_unlock(&scmi_requested_devices_mtx);
-
- return ret;
-}
-
-/**
- * scmi_protocol_device_unrequest - Helper to unrequest a device
- *
- * @id_table: A protocol/name pair descriptor for the device to be unrequested.
- *
- * An helper to let an SCMI driver release its request about devices; note that
- * devices are created and initialized once the first SCMI driver request them
- * but they destroyed only on SCMI core unloading/unbinding.
- *
- * The current SCMI transport layer uses such devices as internal references and
- * as such they could be shared as same transport between multiple drivers so
- * that cannot be safely destroyed till the whole SCMI stack is removed.
- * (unless adding further burden of refcounting.)
- */
-void scmi_protocol_device_unrequest(const struct scmi_device_id *id_table)
-{
- struct list_head *phead;
-
- pr_debug("Unrequesting SCMI device (%s) for protocol %x\n",
- id_table->name, id_table->protocol_id);
-
- mutex_lock(&scmi_requested_devices_mtx);
- phead = idr_find(&scmi_requested_devices, id_table->protocol_id);
- if (phead) {
- struct scmi_requested_dev *victim, *tmp;
-
- list_for_each_entry_safe(victim, tmp, phead, node) {
- if (!strcmp(victim->id_table->name, id_table->name)) {
- list_del(&victim->node);
- kfree(victim);
- break;
- }
- }
-
- if (list_empty(phead)) {
- idr_remove(&scmi_requested_devices,
- id_table->protocol_id);
- kfree(phead);
- }
- }
- mutex_unlock(&scmi_requested_devices_mtx);
-}
-
static int scmi_chan_destroy(int id, void *p, void *idr)
{
struct scmi_chan_info *cinfo = p;
@@ -2689,7 +2394,6 @@ static int scmi_probe(struct platform_device *pdev)
}
of_node_get(child);
- scmi_create_protocol_devices(child, info, prot_id);
}
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 5/9] firmware: arm_scmi: Refactor protocol device creation
@ 2022-12-22 18:50 ` Cristian Marussi
0 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Move protocol device request helpers from driver.c compilation unit to
bus.c, so reducing the cross interactions between driver.c and bus.c.
Get rid of old protocol device creation process as a whole from driver.c
and remove also stale SCMI system power unicity checks.
While at that make such helpers call into scmi_requested_devices_nh
notification chain.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
Note that this patch disables SCMI device creation, it will be
restored later in the series, together with SCMI syspower checks,
using a new approach.
---
drivers/firmware/arm_scmi/bus.c | 157 ++++++++++++++-
drivers/firmware/arm_scmi/common.h | 2 -
drivers/firmware/arm_scmi/driver.c | 296 -----------------------------
3 files changed, 156 insertions(+), 299 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index bed566f58029..e63cc1194d43 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -9,6 +9,7 @@
#include <linux/types.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/device.h>
@@ -20,6 +21,160 @@ EXPORT_SYMBOL_GPL(scmi_requested_devices_nh);
static DEFINE_IDA(scmi_bus_id);
+static DEFINE_IDR(scmi_requested_devices);
+/* Protect access to scmi_requested_devices */
+static DEFINE_MUTEX(scmi_requested_devices_mtx);
+
+struct scmi_requested_dev {
+ const struct scmi_device_id *id_table;
+ struct list_head node;
+};
+
+/**
+ * scmi_protocol_device_request - Helper to request a device
+ *
+ * @id_table: A protocol/name pair descriptor for the device to be created.
+ *
+ * This helper let an SCMI driver request specific devices identified by the
+ * @id_table to be created for each active SCMI instance.
+ *
+ * The requested device name MUST NOT be already existent for any protocol;
+ * at first the freshly requested @id_table is annotated in the IDR table
+ * @scmi_requested_devices and then the requested device is advertised to any
+ * registered party via the @scmi_requested_devices_nh notification chain.
+ *
+ * Return: 0 on Success
+ */
+static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
+{
+ int ret = 0;
+ unsigned int id = 0;
+ struct list_head *head, *phead = NULL;
+ struct scmi_requested_dev *rdev;
+
+ pr_debug("Requesting SCMI device (%s) for protocol %x\n",
+ id_table->name, id_table->protocol_id);
+
+ /*
+ * Search for the matching protocol rdev list and then search
+ * of any existent equally named device...fails if any duplicate found.
+ */
+ mutex_lock(&scmi_requested_devices_mtx);
+ idr_for_each_entry(&scmi_requested_devices, head, id) {
+ if (!phead) {
+ /* A list found registered in the IDR is never empty */
+ rdev = list_first_entry(head, struct scmi_requested_dev,
+ node);
+ if (rdev->id_table->protocol_id ==
+ id_table->protocol_id)
+ phead = head;
+ }
+ list_for_each_entry(rdev, head, node) {
+ if (!strcmp(rdev->id_table->name, id_table->name)) {
+ pr_err("Ignoring duplicate request [%d] %s\n",
+ rdev->id_table->protocol_id,
+ rdev->id_table->name);
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+ }
+
+ /*
+ * No duplicate found for requested id_table, so let's create a new
+ * requested device entry for this new valid request.
+ */
+ rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
+ if (!rdev) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ rdev->id_table = id_table;
+
+ /*
+ * Append the new requested device table descriptor to the head of the
+ * related protocol list, eventually creating such head if not already
+ * there.
+ */
+ if (!phead) {
+ phead = kzalloc(sizeof(*phead), GFP_KERNEL);
+ if (!phead) {
+ kfree(rdev);
+ ret = -ENOMEM;
+ goto out;
+ }
+ INIT_LIST_HEAD(phead);
+
+ ret = idr_alloc(&scmi_requested_devices, (void *)phead,
+ id_table->protocol_id,
+ id_table->protocol_id + 1, GFP_KERNEL);
+ if (ret != id_table->protocol_id) {
+ pr_err("Failed to save SCMI device - ret:%d\n", ret);
+ kfree(rdev);
+ kfree(phead);
+ ret = -EINVAL;
+ goto out;
+ }
+ ret = 0;
+ }
+ list_add(&rdev->node, phead);
+
+out:
+ mutex_unlock(&scmi_requested_devices_mtx);
+
+ if (!ret)
+ blocking_notifier_call_chain(&scmi_requested_devices_nh,
+ SCMI_BUS_NOTIFY_DEVICE_REQUEST,
+ (void *)rdev->id_table);
+
+ return ret;
+}
+
+/**
+ * scmi_protocol_device_unrequest - Helper to unrequest a device
+ *
+ * @id_table: A protocol/name pair descriptor for the device to be unrequested.
+ *
+ * The unrequested device, described by the provided id_table, is at first
+ * removed from the IDR @scmi_requested_devices and then the removal is
+ * advertised to any registered party via the @scmi_requested_devices_nh
+ * notification chain.
+ */
+static void scmi_protocol_device_unrequest(const struct scmi_device_id *id_table)
+{
+ struct list_head *phead;
+
+ pr_debug("Unrequesting SCMI device (%s) for protocol %x\n",
+ id_table->name, id_table->protocol_id);
+
+ mutex_lock(&scmi_requested_devices_mtx);
+ phead = idr_find(&scmi_requested_devices, id_table->protocol_id);
+ if (phead) {
+ struct scmi_requested_dev *victim, *tmp;
+
+ list_for_each_entry_safe(victim, tmp, phead, node) {
+ if (!strcmp(victim->id_table->name, id_table->name)) {
+ list_del(&victim->node);
+
+ mutex_unlock(&scmi_requested_devices_mtx);
+ blocking_notifier_call_chain(&scmi_requested_devices_nh,
+ SCMI_BUS_NOTIFY_DEVICE_UNREQUEST,
+ (void *)victim->id_table);
+ kfree(victim);
+ mutex_lock(&scmi_requested_devices_mtx);
+ break;
+ }
+ }
+
+ if (list_empty(phead)) {
+ idr_remove(&scmi_requested_devices,
+ id_table->protocol_id);
+ kfree(phead);
+ }
+ }
+ mutex_unlock(&scmi_requested_devices_mtx);
+}
+
static const struct scmi_device_id *
scmi_dev_match_id(struct scmi_device *scmi_dev, struct scmi_driver *scmi_drv)
{
@@ -124,7 +279,7 @@ int scmi_driver_register(struct scmi_driver *driver, struct module *owner,
retval = driver_register(&driver->driver);
if (!retval)
- pr_debug("registered new scmi driver %s\n", driver->name);
+ pr_debug("Registered new scmi driver %s\n", driver->name);
return retval;
}
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 7ddae90eb945..0f411679df7e 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -172,8 +172,6 @@ struct scmi_transport_ops {
bool (*poll_done)(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer);
};
-int scmi_protocol_device_request(const struct scmi_device_id *id_table);
-void scmi_protocol_device_unrequest(const struct scmi_device_id *id_table);
struct scmi_device *scmi_child_dev_find(struct device *parent,
int prot_id, const char *name);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 62760bed1645..73f640e9448f 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -61,19 +61,6 @@ static DEFINE_MUTEX(scmi_list_mutex);
/* Track the unique id for the transfers for debug & profiling purpose */
static atomic_t transfer_last_id;
-static DEFINE_IDR(scmi_requested_devices);
-static DEFINE_MUTEX(scmi_requested_devices_mtx);
-
-/* Track globally the creation of SCMI SystemPower related devices */
-static bool scmi_syspower_registered;
-/* Protect access to scmi_syspower_registered */
-static DEFINE_MUTEX(scmi_syspower_mtx);
-
-struct scmi_requested_dev {
- const struct scmi_device_id *id_table;
- struct list_head node;
-};
-
/**
* struct scmi_xfers_info - Structure to manage transfer information
*
@@ -2199,288 +2186,6 @@ static int scmi_channels_setup(struct scmi_info *info)
return 0;
}
-/**
- * scmi_get_protocol_device - Helper to get/create an SCMI device.
- *
- * @np: A device node representing a valid active protocols for the referred
- * SCMI instance.
- * @info: The referred SCMI instance for which we are getting/creating this
- * device.
- * @prot_id: The protocol ID.
- * @name: The device name.
- *
- * Referring to the specific SCMI instance identified by @info, this helper
- * takes care to return a properly initialized device matching the requested
- * @proto_id and @name: if device was still not existent it is created as a
- * child of the specified SCMI instance @info and its transport properly
- * initialized as usual.
- *
- * Return: A properly initialized scmi device, NULL otherwise.
- */
-static inline struct scmi_device *
-scmi_get_protocol_device(struct device_node *np, struct scmi_info *info,
- int prot_id, const char *name)
-{
- struct scmi_device *sdev;
-
- /* Already created for this parent SCMI instance ? */
- sdev = scmi_child_dev_find(info->dev, prot_id, name);
- if (sdev)
- return sdev;
-
- mutex_lock(&scmi_syspower_mtx);
- if (prot_id == SCMI_PROTOCOL_SYSTEM && scmi_syspower_registered) {
- dev_warn(info->dev,
- "SCMI SystemPower protocol device must be unique !\n");
- mutex_unlock(&scmi_syspower_mtx);
-
- return NULL;
- }
-
- pr_debug("Creating SCMI device (%s) for protocol %x\n", name, prot_id);
-
- sdev = scmi_device_create(np, info->dev, prot_id, name);
- if (!sdev) {
- dev_err(info->dev, "failed to create %d protocol device\n",
- prot_id);
- mutex_unlock(&scmi_syspower_mtx);
-
- return NULL;
- }
-
- if (prot_id == SCMI_PROTOCOL_SYSTEM)
- scmi_syspower_registered = true;
-
- mutex_unlock(&scmi_syspower_mtx);
-
- return sdev;
-}
-
-static inline void
-scmi_create_protocol_device(struct device_node *np, struct scmi_info *info,
- int prot_id, const char *name)
-{
- struct scmi_device *sdev;
-
- sdev = scmi_get_protocol_device(np, info, prot_id, name);
- if (!sdev)
- return;
-
- /* setup handle now as the transport is ready */
- scmi_set_handle(sdev);
-}
-
-/**
- * scmi_create_protocol_devices - Create devices for all pending requests for
- * this SCMI instance.
- *
- * @np: The device node describing the protocol
- * @info: The SCMI instance descriptor
- * @prot_id: The protocol ID
- *
- * All devices previously requested for this instance (if any) are found and
- * created by scanning the proper @&scmi_requested_devices entry.
- */
-static void scmi_create_protocol_devices(struct device_node *np,
- struct scmi_info *info, int prot_id)
-{
- struct list_head *phead;
-
- mutex_lock(&scmi_requested_devices_mtx);
- phead = idr_find(&scmi_requested_devices, prot_id);
- if (phead) {
- struct scmi_requested_dev *rdev;
-
- list_for_each_entry(rdev, phead, node)
- scmi_create_protocol_device(np, info, prot_id,
- rdev->id_table->name);
- }
- mutex_unlock(&scmi_requested_devices_mtx);
-}
-
-/**
- * scmi_protocol_device_request - Helper to request a device
- *
- * @id_table: A protocol/name pair descriptor for the device to be created.
- *
- * This helper let an SCMI driver request specific devices identified by the
- * @id_table to be created for each active SCMI instance.
- *
- * The requested device name MUST NOT be already existent for any protocol;
- * at first the freshly requested @id_table is annotated in the IDR table
- * @scmi_requested_devices, then a matching device is created for each already
- * active SCMI instance. (if any)
- *
- * This way the requested device is created straight-away for all the already
- * initialized(probed) SCMI instances (handles) and it remains also annotated
- * as pending creation if the requesting SCMI driver was loaded before some
- * SCMI instance and related transports were available: when such late instance
- * is probed, its probe will take care to scan the list of pending requested
- * devices and create those on its own (see @scmi_create_protocol_devices and
- * its enclosing loop)
- *
- * Return: 0 on Success
- */
-int scmi_protocol_device_request(const struct scmi_device_id *id_table)
-{
- int ret = 0;
- unsigned int id = 0;
- struct list_head *head, *phead = NULL;
- struct scmi_requested_dev *rdev;
- struct scmi_info *info;
-
- pr_debug("Requesting SCMI device (%s) for protocol %x\n",
- id_table->name, id_table->protocol_id);
-
- /*
- * Search for the matching protocol rdev list and then search
- * of any existent equally named device...fails if any duplicate found.
- */
- mutex_lock(&scmi_requested_devices_mtx);
- idr_for_each_entry(&scmi_requested_devices, head, id) {
- if (!phead) {
- /* A list found registered in the IDR is never empty */
- rdev = list_first_entry(head, struct scmi_requested_dev,
- node);
- if (rdev->id_table->protocol_id ==
- id_table->protocol_id)
- phead = head;
- }
- list_for_each_entry(rdev, head, node) {
- if (!strcmp(rdev->id_table->name, id_table->name)) {
- pr_err("Ignoring duplicate request [%d] %s\n",
- rdev->id_table->protocol_id,
- rdev->id_table->name);
- ret = -EINVAL;
- goto out;
- }
- }
- }
-
- /*
- * No duplicate found for requested id_table, so let's create a new
- * requested device entry for this new valid request.
- */
- rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
- if (!rdev) {
- ret = -ENOMEM;
- goto out;
- }
- rdev->id_table = id_table;
-
- /*
- * Append the new requested device table descriptor to the head of the
- * related protocol list, eventually creating such head if not already
- * there.
- */
- if (!phead) {
- phead = kzalloc(sizeof(*phead), GFP_KERNEL);
- if (!phead) {
- kfree(rdev);
- ret = -ENOMEM;
- goto out;
- }
- INIT_LIST_HEAD(phead);
-
- ret = idr_alloc(&scmi_requested_devices, (void *)phead,
- id_table->protocol_id,
- id_table->protocol_id + 1, GFP_KERNEL);
- if (ret != id_table->protocol_id) {
- pr_err("Failed to save SCMI device - ret:%d\n", ret);
- kfree(rdev);
- kfree(phead);
- ret = -EINVAL;
- goto out;
- }
- ret = 0;
- }
- list_add(&rdev->node, phead);
-
- /*
- * Now effectively create and initialize the requested device for every
- * already initialized SCMI instance which has registered the requested
- * protocol as a valid active one: i.e. defined in DT and supported by
- * current platform FW.
- */
- mutex_lock(&scmi_list_mutex);
- list_for_each_entry(info, &scmi_list, node) {
- struct device_node *child;
-
- child = idr_find(&info->active_protocols,
- id_table->protocol_id);
- if (child) {
- struct scmi_device *sdev;
-
- sdev = scmi_get_protocol_device(child, info,
- id_table->protocol_id,
- id_table->name);
- if (sdev) {
- /* Set handle if not already set: device existed */
- if (!sdev->handle)
- sdev->handle =
- scmi_handle_get_from_info_unlocked(info);
- /* Relink consumer and suppliers */
- if (sdev->handle)
- scmi_device_link_add(&sdev->dev,
- sdev->handle->dev);
- }
- } else {
- dev_err(info->dev,
- "Failed. SCMI protocol %d not active.\n",
- id_table->protocol_id);
- }
- }
- mutex_unlock(&scmi_list_mutex);
-
-out:
- mutex_unlock(&scmi_requested_devices_mtx);
-
- return ret;
-}
-
-/**
- * scmi_protocol_device_unrequest - Helper to unrequest a device
- *
- * @id_table: A protocol/name pair descriptor for the device to be unrequested.
- *
- * An helper to let an SCMI driver release its request about devices; note that
- * devices are created and initialized once the first SCMI driver request them
- * but they destroyed only on SCMI core unloading/unbinding.
- *
- * The current SCMI transport layer uses such devices as internal references and
- * as such they could be shared as same transport between multiple drivers so
- * that cannot be safely destroyed till the whole SCMI stack is removed.
- * (unless adding further burden of refcounting.)
- */
-void scmi_protocol_device_unrequest(const struct scmi_device_id *id_table)
-{
- struct list_head *phead;
-
- pr_debug("Unrequesting SCMI device (%s) for protocol %x\n",
- id_table->name, id_table->protocol_id);
-
- mutex_lock(&scmi_requested_devices_mtx);
- phead = idr_find(&scmi_requested_devices, id_table->protocol_id);
- if (phead) {
- struct scmi_requested_dev *victim, *tmp;
-
- list_for_each_entry_safe(victim, tmp, phead, node) {
- if (!strcmp(victim->id_table->name, id_table->name)) {
- list_del(&victim->node);
- kfree(victim);
- break;
- }
- }
-
- if (list_empty(phead)) {
- idr_remove(&scmi_requested_devices,
- id_table->protocol_id);
- kfree(phead);
- }
- }
- mutex_unlock(&scmi_requested_devices_mtx);
-}
-
static int scmi_chan_destroy(int id, void *p, void *idr)
{
struct scmi_chan_info *cinfo = p;
@@ -2689,7 +2394,6 @@ static int scmi_probe(struct platform_device *pdev)
}
of_node_get(child);
- scmi_create_protocol_devices(child, info, prot_id);
}
return 0;
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 6/9] firmware: arm_scmi: Move handle get/set helpers
2022-12-22 18:50 ` Cristian Marussi
@ 2022-12-22 18:50 ` Cristian Marussi
-1 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Move handle get/set helpers definitions into driver.c and invoke them
through the bus notifier helper.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/bus.c | 17 ---------------
drivers/firmware/arm_scmi/common.h | 4 ----
drivers/firmware/arm_scmi/driver.c | 35 +++++++++++++++++++++---------
3 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index e63cc1194d43..61113def5a9a 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -344,27 +344,10 @@ scmi_device_create(struct device_node *np, struct device *parent, int protocol,
void scmi_device_destroy(struct scmi_device *scmi_dev)
{
kfree_const(scmi_dev->name);
- scmi_handle_put(scmi_dev->handle);
ida_free(&scmi_bus_id, scmi_dev->id);
device_unregister(&scmi_dev->dev);
}
-void scmi_device_link_add(struct device *consumer, struct device *supplier)
-{
- struct device_link *link;
-
- link = device_link_add(consumer, supplier, DL_FLAG_AUTOREMOVE_CONSUMER);
-
- WARN_ON(!link);
-}
-
-void scmi_set_handle(struct scmi_device *scmi_dev)
-{
- scmi_dev->handle = scmi_handle_get(&scmi_dev->dev);
- if (scmi_dev->handle)
- scmi_device_link_add(&scmi_dev->dev, scmi_dev->handle->dev);
-}
-
static int __scmi_devices_unregister(struct device *dev, void *data)
{
struct scmi_device *scmi_dev = to_scmi_dev(dev);
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 0f411679df7e..07d34954c060 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -96,10 +96,6 @@ static inline void unpack_scmi_header(u32 msg_hdr, struct scmi_msg_hdr *hdr)
struct scmi_revision_info *
scmi_revision_area_get(const struct scmi_protocol_handle *ph);
-int scmi_handle_put(const struct scmi_handle *handle);
-void scmi_device_link_add(struct device *consumer, struct device *supplier);
-struct scmi_handle *scmi_handle_get(struct device *dev);
-void scmi_set_handle(struct scmi_device *scmi_dev);
void scmi_setup_protocol_implemented(const struct scmi_protocol_handle *ph,
u8 *prot_imp);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 73f640e9448f..8591b2c740c6 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -1890,13 +1890,6 @@ static bool scmi_is_transport_atomic(const struct scmi_handle *handle,
return ret;
}
-static inline
-struct scmi_handle *scmi_handle_get_from_info_unlocked(struct scmi_info *info)
-{
- info->users++;
- return &info->handle;
-}
-
/**
* scmi_handle_get() - Get the SCMI handle for a device
*
@@ -1908,7 +1901,7 @@ struct scmi_handle *scmi_handle_get_from_info_unlocked(struct scmi_info *info)
*
* Return: pointer to handle if successful, NULL on error
*/
-struct scmi_handle *scmi_handle_get(struct device *dev)
+static struct scmi_handle *scmi_handle_get(struct device *dev)
{
struct list_head *p;
struct scmi_info *info;
@@ -1918,7 +1911,8 @@ struct scmi_handle *scmi_handle_get(struct device *dev)
list_for_each(p, &scmi_list) {
info = list_entry(p, struct scmi_info, node);
if (dev->parent == info->dev) {
- handle = scmi_handle_get_from_info_unlocked(info);
+ info->users++;
+ handle = &info->handle;
break;
}
}
@@ -1939,7 +1933,7 @@ struct scmi_handle *scmi_handle_get(struct device *dev)
* Return: 0 is successfully released
* if null was passed, it returns -EINVAL;
*/
-int scmi_handle_put(const struct scmi_handle *handle)
+static int scmi_handle_put(const struct scmi_handle *handle)
{
struct scmi_info *info;
@@ -1955,6 +1949,23 @@ int scmi_handle_put(const struct scmi_handle *handle)
return 0;
}
+static void scmi_device_link_add(struct device *consumer,
+ struct device *supplier)
+{
+ struct device_link *link;
+
+ link = device_link_add(consumer, supplier, DL_FLAG_AUTOREMOVE_CONSUMER);
+
+ WARN_ON(!link);
+}
+
+static void scmi_set_handle(struct scmi_device *scmi_dev)
+{
+ scmi_dev->handle = scmi_handle_get(&scmi_dev->dev);
+ if (scmi_dev->handle)
+ scmi_device_link_add(&scmi_dev->dev, scmi_dev->handle->dev);
+}
+
static int __scmi_xfer_info_init(struct scmi_info *sinfo,
struct scmi_xfers_info *info)
{
@@ -2234,8 +2245,12 @@ static int scmi_bus_notifier(struct notifier_block *nb,
switch (action) {
case BUS_NOTIFY_BIND_DRIVER:
+ /* setup handle now as the transport is ready */
+ scmi_set_handle(sdev);
break;
case BUS_NOTIFY_UNBOUND_DRIVER:
+ scmi_handle_put(sdev->handle);
+ sdev->handle = NULL;
break;
default:
return NOTIFY_DONE;
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 6/9] firmware: arm_scmi: Move handle get/set helpers
@ 2022-12-22 18:50 ` Cristian Marussi
0 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Move handle get/set helpers definitions into driver.c and invoke them
through the bus notifier helper.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/bus.c | 17 ---------------
drivers/firmware/arm_scmi/common.h | 4 ----
drivers/firmware/arm_scmi/driver.c | 35 +++++++++++++++++++++---------
3 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index e63cc1194d43..61113def5a9a 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -344,27 +344,10 @@ scmi_device_create(struct device_node *np, struct device *parent, int protocol,
void scmi_device_destroy(struct scmi_device *scmi_dev)
{
kfree_const(scmi_dev->name);
- scmi_handle_put(scmi_dev->handle);
ida_free(&scmi_bus_id, scmi_dev->id);
device_unregister(&scmi_dev->dev);
}
-void scmi_device_link_add(struct device *consumer, struct device *supplier)
-{
- struct device_link *link;
-
- link = device_link_add(consumer, supplier, DL_FLAG_AUTOREMOVE_CONSUMER);
-
- WARN_ON(!link);
-}
-
-void scmi_set_handle(struct scmi_device *scmi_dev)
-{
- scmi_dev->handle = scmi_handle_get(&scmi_dev->dev);
- if (scmi_dev->handle)
- scmi_device_link_add(&scmi_dev->dev, scmi_dev->handle->dev);
-}
-
static int __scmi_devices_unregister(struct device *dev, void *data)
{
struct scmi_device *scmi_dev = to_scmi_dev(dev);
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 0f411679df7e..07d34954c060 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -96,10 +96,6 @@ static inline void unpack_scmi_header(u32 msg_hdr, struct scmi_msg_hdr *hdr)
struct scmi_revision_info *
scmi_revision_area_get(const struct scmi_protocol_handle *ph);
-int scmi_handle_put(const struct scmi_handle *handle);
-void scmi_device_link_add(struct device *consumer, struct device *supplier);
-struct scmi_handle *scmi_handle_get(struct device *dev);
-void scmi_set_handle(struct scmi_device *scmi_dev);
void scmi_setup_protocol_implemented(const struct scmi_protocol_handle *ph,
u8 *prot_imp);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 73f640e9448f..8591b2c740c6 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -1890,13 +1890,6 @@ static bool scmi_is_transport_atomic(const struct scmi_handle *handle,
return ret;
}
-static inline
-struct scmi_handle *scmi_handle_get_from_info_unlocked(struct scmi_info *info)
-{
- info->users++;
- return &info->handle;
-}
-
/**
* scmi_handle_get() - Get the SCMI handle for a device
*
@@ -1908,7 +1901,7 @@ struct scmi_handle *scmi_handle_get_from_info_unlocked(struct scmi_info *info)
*
* Return: pointer to handle if successful, NULL on error
*/
-struct scmi_handle *scmi_handle_get(struct device *dev)
+static struct scmi_handle *scmi_handle_get(struct device *dev)
{
struct list_head *p;
struct scmi_info *info;
@@ -1918,7 +1911,8 @@ struct scmi_handle *scmi_handle_get(struct device *dev)
list_for_each(p, &scmi_list) {
info = list_entry(p, struct scmi_info, node);
if (dev->parent == info->dev) {
- handle = scmi_handle_get_from_info_unlocked(info);
+ info->users++;
+ handle = &info->handle;
break;
}
}
@@ -1939,7 +1933,7 @@ struct scmi_handle *scmi_handle_get(struct device *dev)
* Return: 0 is successfully released
* if null was passed, it returns -EINVAL;
*/
-int scmi_handle_put(const struct scmi_handle *handle)
+static int scmi_handle_put(const struct scmi_handle *handle)
{
struct scmi_info *info;
@@ -1955,6 +1949,23 @@ int scmi_handle_put(const struct scmi_handle *handle)
return 0;
}
+static void scmi_device_link_add(struct device *consumer,
+ struct device *supplier)
+{
+ struct device_link *link;
+
+ link = device_link_add(consumer, supplier, DL_FLAG_AUTOREMOVE_CONSUMER);
+
+ WARN_ON(!link);
+}
+
+static void scmi_set_handle(struct scmi_device *scmi_dev)
+{
+ scmi_dev->handle = scmi_handle_get(&scmi_dev->dev);
+ if (scmi_dev->handle)
+ scmi_device_link_add(&scmi_dev->dev, scmi_dev->handle->dev);
+}
+
static int __scmi_xfer_info_init(struct scmi_info *sinfo,
struct scmi_xfers_info *info)
{
@@ -2234,8 +2245,12 @@ static int scmi_bus_notifier(struct notifier_block *nb,
switch (action) {
case BUS_NOTIFY_BIND_DRIVER:
+ /* setup handle now as the transport is ready */
+ scmi_set_handle(sdev);
break;
case BUS_NOTIFY_UNBOUND_DRIVER:
+ scmi_handle_put(sdev->handle);
+ sdev->handle = NULL;
break;
default:
return NOTIFY_DONE;
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 7/9] firmware: arm_scmi: Refactor device create/destroy helpers
2022-12-22 18:50 ` Cristian Marussi
@ 2022-12-22 18:50 ` Cristian Marussi
-1 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Refactor SCMI device create/destroy helpers: it is now possible to ask
for the creation of all the currently requested devices for a whole
protocol, not only for the creation of a single well-defined device.
While at that, re-instate uniqueness checks on the creation of SCMI
SystemPower devices.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/bus.c | 141 ++++++++++++++++++++++++++---
drivers/firmware/arm_scmi/common.h | 8 +-
drivers/firmware/arm_scmi/driver.c | 9 +-
include/linux/scmi_protocol.h | 5 -
4 files changed, 141 insertions(+), 22 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 61113def5a9a..190999a658b2 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -7,6 +7,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/atomic.h>
#include <linux/types.h>
#include <linux/module.h>
#include <linux/of.h>
@@ -30,6 +31,9 @@ struct scmi_requested_dev {
struct list_head node;
};
+/* Track globally the creation of SCMI SystemPower related devices */
+static atomic_t scmi_syspower_registered = ATOMIC_INIT(0);
+
/**
* scmi_protocol_device_request - Helper to request a device
*
@@ -213,11 +217,11 @@ static int scmi_match_by_id_table(struct device *dev, void *data)
struct scmi_device_id *id_table = data;
return sdev->protocol_id == id_table->protocol_id &&
- !strcmp(sdev->name, id_table->name);
+ (id_table->name && !strcmp(sdev->name, id_table->name));
}
-struct scmi_device *scmi_child_dev_find(struct device *parent,
- int prot_id, const char *name)
+static struct scmi_device *scmi_child_dev_find(struct device *parent,
+ int prot_id, const char *name)
{
struct scmi_device_id id_table;
struct device *dev;
@@ -297,13 +301,53 @@ static void scmi_device_release(struct device *dev)
kfree(to_scmi_dev(dev));
}
-struct scmi_device *
-scmi_device_create(struct device_node *np, struct device *parent, int protocol,
- const char *name)
+static void __scmi_device_destroy(struct scmi_device *scmi_dev)
+{
+ pr_debug("(%s) Destroying SCMI device '%s' for protocol 0x%x (%s)\n",
+ of_node_full_name(scmi_dev->dev.parent->of_node),
+ dev_name(&scmi_dev->dev), scmi_dev->protocol_id,
+ scmi_dev->name);
+
+ if (scmi_dev->protocol_id == SCMI_PROTOCOL_SYSTEM)
+ atomic_set(&scmi_syspower_registered, 0);
+
+ kfree_const(scmi_dev->name);
+ ida_free(&scmi_bus_id, scmi_dev->id);
+ device_unregister(&scmi_dev->dev);
+}
+
+static struct scmi_device *
+__scmi_device_create(struct device_node *np, struct device *parent,
+ int protocol, const char *name)
{
int id, retval;
struct scmi_device *scmi_dev;
+ /*
+ * If the same protocol/name device already exist under the same parent
+ * (i.e. SCMI instance) just return the existent device.
+ * This avoids any race between the SCMI driver, creating devices for
+ * each DT defined protocol at probe time, and the concurrent
+ * registration of SCMI drivers.
+ */
+ scmi_dev = scmi_child_dev_find(parent, protocol, name);
+ if (scmi_dev)
+ return scmi_dev;
+
+ /*
+ * Ignore any possible subsequent failures while creating the device
+ * since we are doomed anyway at that point; not using a mutex which
+ * spans across this whole function to keep things simple and to avoid
+ * to serialize all the __scmi_device_create calls across possibly
+ * different SCMI server instances (parent)
+ */
+ if (protocol == SCMI_PROTOCOL_SYSTEM &&
+ atomic_cmpxchg(&scmi_syspower_registered, 0, 1)) {
+ dev_warn(parent,
+ "SCMI SystemPower protocol device must be unique !\n");
+ return NULL;
+ }
+
scmi_dev = kzalloc(sizeof(*scmi_dev), GFP_KERNEL);
if (!scmi_dev)
return NULL;
@@ -333,6 +377,10 @@ scmi_device_create(struct device_node *np, struct device *parent, int protocol,
if (retval)
goto put_dev;
+ pr_debug("(%s) Created SCMI device '%s' for protocol 0x%x (%s)\n",
+ of_node_full_name(parent->of_node),
+ dev_name(&scmi_dev->dev), protocol, name);
+
return scmi_dev;
put_dev:
kfree_const(scmi_dev->name);
@@ -341,18 +389,85 @@ scmi_device_create(struct device_node *np, struct device *parent, int protocol,
return NULL;
}
-void scmi_device_destroy(struct scmi_device *scmi_dev)
+/**
+ * scmi_device_create - A method to create one or more SCMI devices
+ *
+ * @np: A reference to the device node to use for the new device(s)
+ * @parent: The parent device to use identifying a specific SCMI instance
+ * @protocol: The SCMI protocol to be associated with this device
+ * @name: The requested-name of the device to be created; this is optional
+ * and if no @name is provided, all the devices currently known to
+ * be requested on the SCMI bus for @protocol will be created.
+ *
+ * This method can be invoked to create a single well-defined device (like
+ * a transport device or a device requested by an SCMI driver loaded after
+ * the core SCMI stack has been probed), or to create all the devices currently
+ * known to have been requested by the loaded SCMI drivers for a specific
+ * protocol (typically during SCMI core protocol enumeration at probe time).
+ *
+ * Return: The created device (or one of them if @name was NOT provided and
+ * multiple devices were created) or NULL if no device was created;
+ * note that NULL indicates an error ONLY in case a specific @name
+ * was provided: when @name param was not provided, a number of devices
+ * could have been potentially created for a whole protocol, unless no
+ * device was found to have been requested for that specific protocol.
+ */
+struct scmi_device *scmi_device_create(struct device_node *np,
+ struct device *parent, int protocol,
+ const char *name)
{
- kfree_const(scmi_dev->name);
- ida_free(&scmi_bus_id, scmi_dev->id);
- device_unregister(&scmi_dev->dev);
+ struct list_head *phead;
+ struct scmi_requested_dev *rdev;
+ struct scmi_device *scmi_dev = NULL;
+
+ if (name)
+ return __scmi_device_create(np, parent, protocol, name);
+
+ mutex_lock(&scmi_requested_devices_mtx);
+ phead = idr_find(&scmi_requested_devices, protocol);
+ /* Nothing to do. */
+ if (!phead) {
+ mutex_unlock(&scmi_requested_devices_mtx);
+ return scmi_dev;
+ }
+
+ /* Walk the list of requested devices for protocol and create them */
+ list_for_each_entry(rdev, phead, node) {
+ struct scmi_device *sdev;
+
+ sdev = __scmi_device_create(np, parent,
+ rdev->id_table->protocol_id,
+ rdev->id_table->name);
+ /* Report errors and carry on... */
+ if (sdev)
+ scmi_dev = sdev;
+ else
+ pr_err("(%s) Failed to create device for protocol 0x%x (%s)\n",
+ of_node_full_name(parent->of_node),
+ rdev->id_table->protocol_id,
+ rdev->id_table->name);
+ }
+ mutex_unlock(&scmi_requested_devices_mtx);
+
+ return scmi_dev;
}
+EXPORT_SYMBOL_GPL(scmi_device_create);
+
+void scmi_device_destroy(struct device *parent, int protocol, const char *name)
+{
+ struct scmi_device *scmi_dev;
+
+ scmi_dev = scmi_child_dev_find(parent, protocol, name);
+ if (scmi_dev)
+ __scmi_device_destroy(scmi_dev);
+}
+EXPORT_SYMBOL_GPL(scmi_device_destroy);
static int __scmi_devices_unregister(struct device *dev, void *data)
{
struct scmi_device *scmi_dev = to_scmi_dev(dev);
- scmi_device_destroy(scmi_dev);
+ __scmi_device_destroy(scmi_dev);
return 0;
}
@@ -374,6 +489,10 @@ int __init scmi_bus_init(void)
void __exit scmi_bus_exit(void)
{
+ /*
+ * Destroy all remaining devices: just in case the drivers were
+ * manually unbound and at first and then the modules unloaded.
+ */
scmi_devices_unregister();
bus_unregister(&scmi_bus_type);
ida_destroy(&scmi_bus_id);
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 07d34954c060..19726a037991 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -107,6 +107,11 @@ extern struct bus_type scmi_bus_type;
#define SCMI_BUS_NOTIFY_DEVICE_UNREQUEST 1
extern struct blocking_notifier_head scmi_requested_devices_nh;
+struct scmi_device *scmi_device_create(struct device_node *np,
+ struct device *parent, int protocol,
+ const char *name);
+void scmi_device_destroy(struct device *parent, int protocol, const char *name);
+
int scmi_protocol_acquire(const struct scmi_handle *handle, u8 protocol_id);
void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id);
@@ -168,9 +173,6 @@ struct scmi_transport_ops {
bool (*poll_done)(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer);
};
-struct scmi_device *scmi_child_dev_find(struct device *parent,
- int prot_id, const char *name);
-
/**
* struct scmi_desc - Description of SoC integration
*
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 8591b2c740c6..83a43a5f7bb4 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2091,6 +2091,8 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
/* Create a uniquely named, dedicated transport device for this chan */
tdev = scmi_device_create(of_node, info->dev, prot_id, name);
if (!tdev) {
+ dev_err(info->dev,
+ "failed to create transport device (%s)\n", name);
devm_kfree(info->dev, cinfo);
return -EINVAL;
}
@@ -2100,7 +2102,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
ret = info->desc->ops->chan_setup(cinfo, info->dev, tx);
if (ret) {
of_node_put(of_node);
- scmi_device_destroy(tdev);
+ scmi_device_destroy(info->dev, prot_id, name);
devm_kfree(info->dev, cinfo);
return ret;
}
@@ -2123,7 +2125,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
/* Destroy channel and device only if created by this call. */
if (tdev) {
of_node_put(of_node);
- scmi_device_destroy(tdev);
+ scmi_device_destroy(info->dev, prot_id, name);
devm_kfree(info->dev, cinfo);
}
return ret;
@@ -2202,10 +2204,11 @@ static int scmi_chan_destroy(int id, void *p, void *idr)
struct scmi_chan_info *cinfo = p;
if (cinfo->dev) {
+ struct scmi_info *info = handle_to_scmi_info(cinfo->handle);
struct scmi_device *sdev = to_scmi_dev(cinfo->dev);
of_node_put(cinfo->dev->of_node);
- scmi_device_destroy(sdev);
+ scmi_device_destroy(info->dev, id, sdev->name);
cinfo->dev = NULL;
}
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 4f765bc788ff..0ce5746a4470 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -804,11 +804,6 @@ struct scmi_device {
#define to_scmi_dev(d) container_of(d, struct scmi_device, dev)
-struct scmi_device *
-scmi_device_create(struct device_node *np, struct device *parent, int protocol,
- const char *name);
-void scmi_device_destroy(struct scmi_device *scmi_dev);
-
struct scmi_device_id {
u8 protocol_id;
const char *name;
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 7/9] firmware: arm_scmi: Refactor device create/destroy helpers
@ 2022-12-22 18:50 ` Cristian Marussi
0 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Refactor SCMI device create/destroy helpers: it is now possible to ask
for the creation of all the currently requested devices for a whole
protocol, not only for the creation of a single well-defined device.
While at that, re-instate uniqueness checks on the creation of SCMI
SystemPower devices.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/bus.c | 141 ++++++++++++++++++++++++++---
drivers/firmware/arm_scmi/common.h | 8 +-
drivers/firmware/arm_scmi/driver.c | 9 +-
include/linux/scmi_protocol.h | 5 -
4 files changed, 141 insertions(+), 22 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 61113def5a9a..190999a658b2 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -7,6 +7,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/atomic.h>
#include <linux/types.h>
#include <linux/module.h>
#include <linux/of.h>
@@ -30,6 +31,9 @@ struct scmi_requested_dev {
struct list_head node;
};
+/* Track globally the creation of SCMI SystemPower related devices */
+static atomic_t scmi_syspower_registered = ATOMIC_INIT(0);
+
/**
* scmi_protocol_device_request - Helper to request a device
*
@@ -213,11 +217,11 @@ static int scmi_match_by_id_table(struct device *dev, void *data)
struct scmi_device_id *id_table = data;
return sdev->protocol_id == id_table->protocol_id &&
- !strcmp(sdev->name, id_table->name);
+ (id_table->name && !strcmp(sdev->name, id_table->name));
}
-struct scmi_device *scmi_child_dev_find(struct device *parent,
- int prot_id, const char *name)
+static struct scmi_device *scmi_child_dev_find(struct device *parent,
+ int prot_id, const char *name)
{
struct scmi_device_id id_table;
struct device *dev;
@@ -297,13 +301,53 @@ static void scmi_device_release(struct device *dev)
kfree(to_scmi_dev(dev));
}
-struct scmi_device *
-scmi_device_create(struct device_node *np, struct device *parent, int protocol,
- const char *name)
+static void __scmi_device_destroy(struct scmi_device *scmi_dev)
+{
+ pr_debug("(%s) Destroying SCMI device '%s' for protocol 0x%x (%s)\n",
+ of_node_full_name(scmi_dev->dev.parent->of_node),
+ dev_name(&scmi_dev->dev), scmi_dev->protocol_id,
+ scmi_dev->name);
+
+ if (scmi_dev->protocol_id == SCMI_PROTOCOL_SYSTEM)
+ atomic_set(&scmi_syspower_registered, 0);
+
+ kfree_const(scmi_dev->name);
+ ida_free(&scmi_bus_id, scmi_dev->id);
+ device_unregister(&scmi_dev->dev);
+}
+
+static struct scmi_device *
+__scmi_device_create(struct device_node *np, struct device *parent,
+ int protocol, const char *name)
{
int id, retval;
struct scmi_device *scmi_dev;
+ /*
+ * If the same protocol/name device already exist under the same parent
+ * (i.e. SCMI instance) just return the existent device.
+ * This avoids any race between the SCMI driver, creating devices for
+ * each DT defined protocol at probe time, and the concurrent
+ * registration of SCMI drivers.
+ */
+ scmi_dev = scmi_child_dev_find(parent, protocol, name);
+ if (scmi_dev)
+ return scmi_dev;
+
+ /*
+ * Ignore any possible subsequent failures while creating the device
+ * since we are doomed anyway at that point; not using a mutex which
+ * spans across this whole function to keep things simple and to avoid
+ * to serialize all the __scmi_device_create calls across possibly
+ * different SCMI server instances (parent)
+ */
+ if (protocol == SCMI_PROTOCOL_SYSTEM &&
+ atomic_cmpxchg(&scmi_syspower_registered, 0, 1)) {
+ dev_warn(parent,
+ "SCMI SystemPower protocol device must be unique !\n");
+ return NULL;
+ }
+
scmi_dev = kzalloc(sizeof(*scmi_dev), GFP_KERNEL);
if (!scmi_dev)
return NULL;
@@ -333,6 +377,10 @@ scmi_device_create(struct device_node *np, struct device *parent, int protocol,
if (retval)
goto put_dev;
+ pr_debug("(%s) Created SCMI device '%s' for protocol 0x%x (%s)\n",
+ of_node_full_name(parent->of_node),
+ dev_name(&scmi_dev->dev), protocol, name);
+
return scmi_dev;
put_dev:
kfree_const(scmi_dev->name);
@@ -341,18 +389,85 @@ scmi_device_create(struct device_node *np, struct device *parent, int protocol,
return NULL;
}
-void scmi_device_destroy(struct scmi_device *scmi_dev)
+/**
+ * scmi_device_create - A method to create one or more SCMI devices
+ *
+ * @np: A reference to the device node to use for the new device(s)
+ * @parent: The parent device to use identifying a specific SCMI instance
+ * @protocol: The SCMI protocol to be associated with this device
+ * @name: The requested-name of the device to be created; this is optional
+ * and if no @name is provided, all the devices currently known to
+ * be requested on the SCMI bus for @protocol will be created.
+ *
+ * This method can be invoked to create a single well-defined device (like
+ * a transport device or a device requested by an SCMI driver loaded after
+ * the core SCMI stack has been probed), or to create all the devices currently
+ * known to have been requested by the loaded SCMI drivers for a specific
+ * protocol (typically during SCMI core protocol enumeration at probe time).
+ *
+ * Return: The created device (or one of them if @name was NOT provided and
+ * multiple devices were created) or NULL if no device was created;
+ * note that NULL indicates an error ONLY in case a specific @name
+ * was provided: when @name param was not provided, a number of devices
+ * could have been potentially created for a whole protocol, unless no
+ * device was found to have been requested for that specific protocol.
+ */
+struct scmi_device *scmi_device_create(struct device_node *np,
+ struct device *parent, int protocol,
+ const char *name)
{
- kfree_const(scmi_dev->name);
- ida_free(&scmi_bus_id, scmi_dev->id);
- device_unregister(&scmi_dev->dev);
+ struct list_head *phead;
+ struct scmi_requested_dev *rdev;
+ struct scmi_device *scmi_dev = NULL;
+
+ if (name)
+ return __scmi_device_create(np, parent, protocol, name);
+
+ mutex_lock(&scmi_requested_devices_mtx);
+ phead = idr_find(&scmi_requested_devices, protocol);
+ /* Nothing to do. */
+ if (!phead) {
+ mutex_unlock(&scmi_requested_devices_mtx);
+ return scmi_dev;
+ }
+
+ /* Walk the list of requested devices for protocol and create them */
+ list_for_each_entry(rdev, phead, node) {
+ struct scmi_device *sdev;
+
+ sdev = __scmi_device_create(np, parent,
+ rdev->id_table->protocol_id,
+ rdev->id_table->name);
+ /* Report errors and carry on... */
+ if (sdev)
+ scmi_dev = sdev;
+ else
+ pr_err("(%s) Failed to create device for protocol 0x%x (%s)\n",
+ of_node_full_name(parent->of_node),
+ rdev->id_table->protocol_id,
+ rdev->id_table->name);
+ }
+ mutex_unlock(&scmi_requested_devices_mtx);
+
+ return scmi_dev;
}
+EXPORT_SYMBOL_GPL(scmi_device_create);
+
+void scmi_device_destroy(struct device *parent, int protocol, const char *name)
+{
+ struct scmi_device *scmi_dev;
+
+ scmi_dev = scmi_child_dev_find(parent, protocol, name);
+ if (scmi_dev)
+ __scmi_device_destroy(scmi_dev);
+}
+EXPORT_SYMBOL_GPL(scmi_device_destroy);
static int __scmi_devices_unregister(struct device *dev, void *data)
{
struct scmi_device *scmi_dev = to_scmi_dev(dev);
- scmi_device_destroy(scmi_dev);
+ __scmi_device_destroy(scmi_dev);
return 0;
}
@@ -374,6 +489,10 @@ int __init scmi_bus_init(void)
void __exit scmi_bus_exit(void)
{
+ /*
+ * Destroy all remaining devices: just in case the drivers were
+ * manually unbound and at first and then the modules unloaded.
+ */
scmi_devices_unregister();
bus_unregister(&scmi_bus_type);
ida_destroy(&scmi_bus_id);
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 07d34954c060..19726a037991 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -107,6 +107,11 @@ extern struct bus_type scmi_bus_type;
#define SCMI_BUS_NOTIFY_DEVICE_UNREQUEST 1
extern struct blocking_notifier_head scmi_requested_devices_nh;
+struct scmi_device *scmi_device_create(struct device_node *np,
+ struct device *parent, int protocol,
+ const char *name);
+void scmi_device_destroy(struct device *parent, int protocol, const char *name);
+
int scmi_protocol_acquire(const struct scmi_handle *handle, u8 protocol_id);
void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id);
@@ -168,9 +173,6 @@ struct scmi_transport_ops {
bool (*poll_done)(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer);
};
-struct scmi_device *scmi_child_dev_find(struct device *parent,
- int prot_id, const char *name);
-
/**
* struct scmi_desc - Description of SoC integration
*
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 8591b2c740c6..83a43a5f7bb4 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2091,6 +2091,8 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
/* Create a uniquely named, dedicated transport device for this chan */
tdev = scmi_device_create(of_node, info->dev, prot_id, name);
if (!tdev) {
+ dev_err(info->dev,
+ "failed to create transport device (%s)\n", name);
devm_kfree(info->dev, cinfo);
return -EINVAL;
}
@@ -2100,7 +2102,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
ret = info->desc->ops->chan_setup(cinfo, info->dev, tx);
if (ret) {
of_node_put(of_node);
- scmi_device_destroy(tdev);
+ scmi_device_destroy(info->dev, prot_id, name);
devm_kfree(info->dev, cinfo);
return ret;
}
@@ -2123,7 +2125,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
/* Destroy channel and device only if created by this call. */
if (tdev) {
of_node_put(of_node);
- scmi_device_destroy(tdev);
+ scmi_device_destroy(info->dev, prot_id, name);
devm_kfree(info->dev, cinfo);
}
return ret;
@@ -2202,10 +2204,11 @@ static int scmi_chan_destroy(int id, void *p, void *idr)
struct scmi_chan_info *cinfo = p;
if (cinfo->dev) {
+ struct scmi_info *info = handle_to_scmi_info(cinfo->handle);
struct scmi_device *sdev = to_scmi_dev(cinfo->dev);
of_node_put(cinfo->dev->of_node);
- scmi_device_destroy(sdev);
+ scmi_device_destroy(info->dev, id, sdev->name);
cinfo->dev = NULL;
}
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 4f765bc788ff..0ce5746a4470 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -804,11 +804,6 @@ struct scmi_device {
#define to_scmi_dev(d) container_of(d, struct scmi_device, dev)
-struct scmi_device *
-scmi_device_create(struct device_node *np, struct device *parent, int protocol,
- const char *name);
-void scmi_device_destroy(struct scmi_device *scmi_dev);
-
struct scmi_device_id {
u8 protocol_id;
const char *name;
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 8/9] firmware: arm_scmi: Introduce a new lifecycle for protocol devices
2022-12-22 18:50 ` Cristian Marussi
@ 2022-12-22 18:50 ` Cristian Marussi
-1 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Protocol devices are created or destroyed depending on the related device
request/unrequest events emitted on the scmi_requested_devices_nh
notification chain by the SCMI bus and served in the driver by the
scmi_device_request_notifier.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/driver.c | 45 ++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 83a43a5f7bb4..115baaa4aca9 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -14,6 +14,8 @@
* Copyright (C) 2018-2021 ARM Ltd.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/bitmap.h>
#include <linux/device.h>
#include <linux/export.h>
@@ -140,6 +142,7 @@ struct scmi_protocol_instance {
* @bus_nb: A notifier to listen for device bind/unbind on the scmi bus
* @dev_req_nb: A notifier to listen for device request/unrequest on the scmi
* bus
+ * @devreq_mtx: A mutex to serialize device creation for this SCMI instance
*/
struct scmi_info {
struct device *dev;
@@ -161,6 +164,8 @@ struct scmi_info {
int users;
struct notifier_block bus_nb;
struct notifier_block dev_req_nb;
+ /* Serialize device creation process for this instance */
+ struct mutex devreq_mtx;
};
#define handle_to_scmi_info(h) container_of(h, struct scmi_info, handle)
@@ -255,6 +260,40 @@ void scmi_protocol_unregister(const struct scmi_protocol *proto)
}
EXPORT_SYMBOL_GPL(scmi_protocol_unregister);
+/**
+ * scmi_create_protocol_devices - Create devices for all pending requests for
+ * this SCMI instance.
+ *
+ * @np: The device node describing the protocol
+ * @info: The SCMI instance descriptor
+ * @prot_id: The protocol ID
+ * @name: The optional name of the device to be created: if not provided this
+ * call will lead to the creation of all the devices currently requested
+ * for the specified protocol.
+ */
+static void scmi_create_protocol_devices(struct device_node *np,
+ struct scmi_info *info,
+ int prot_id, const char *name)
+{
+ struct scmi_device *sdev;
+
+ mutex_lock(&info->devreq_mtx);
+ sdev = scmi_device_create(np, info->dev, prot_id, name);
+ if (name && !sdev)
+ dev_err(info->dev,
+ "failed to create device for protocol 0x%X (%s)\n",
+ prot_id, name);
+ mutex_unlock(&info->devreq_mtx);
+}
+
+static void scmi_destroy_protocol_devices(struct scmi_info *info,
+ int prot_id, const char *name)
+{
+ mutex_lock(&info->devreq_mtx);
+ scmi_device_destroy(info->dev, prot_id, name);
+ mutex_unlock(&info->devreq_mtx);
+}
+
void scmi_notification_instance_data_set(const struct scmi_handle *handle,
void *priv)
{
@@ -2283,8 +2322,12 @@ static int scmi_device_request_notifier(struct notifier_block *nb,
switch (action) {
case SCMI_BUS_NOTIFY_DEVICE_REQUEST:
+ scmi_create_protocol_devices(np, info, id_table->protocol_id,
+ id_table->name);
break;
case SCMI_BUS_NOTIFY_DEVICE_UNREQUEST:
+ scmi_destroy_protocol_devices(info, id_table->protocol_id,
+ id_table->name);
break;
default:
return NOTIFY_DONE;
@@ -2318,6 +2361,7 @@ static int scmi_probe(struct platform_device *pdev)
idr_init(&info->protocols);
mutex_init(&info->protocols_mtx);
idr_init(&info->active_protocols);
+ mutex_init(&info->devreq_mtx);
platform_set_drvdata(pdev, info);
idr_init(&info->tx_idr);
@@ -2412,6 +2456,7 @@ static int scmi_probe(struct platform_device *pdev)
}
of_node_get(child);
+ scmi_create_protocol_devices(child, info, prot_id, NULL);
}
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 8/9] firmware: arm_scmi: Introduce a new lifecycle for protocol devices
@ 2022-12-22 18:50 ` Cristian Marussi
0 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Protocol devices are created or destroyed depending on the related device
request/unrequest events emitted on the scmi_requested_devices_nh
notification chain by the SCMI bus and served in the driver by the
scmi_device_request_notifier.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/driver.c | 45 ++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 83a43a5f7bb4..115baaa4aca9 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -14,6 +14,8 @@
* Copyright (C) 2018-2021 ARM Ltd.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/bitmap.h>
#include <linux/device.h>
#include <linux/export.h>
@@ -140,6 +142,7 @@ struct scmi_protocol_instance {
* @bus_nb: A notifier to listen for device bind/unbind on the scmi bus
* @dev_req_nb: A notifier to listen for device request/unrequest on the scmi
* bus
+ * @devreq_mtx: A mutex to serialize device creation for this SCMI instance
*/
struct scmi_info {
struct device *dev;
@@ -161,6 +164,8 @@ struct scmi_info {
int users;
struct notifier_block bus_nb;
struct notifier_block dev_req_nb;
+ /* Serialize device creation process for this instance */
+ struct mutex devreq_mtx;
};
#define handle_to_scmi_info(h) container_of(h, struct scmi_info, handle)
@@ -255,6 +260,40 @@ void scmi_protocol_unregister(const struct scmi_protocol *proto)
}
EXPORT_SYMBOL_GPL(scmi_protocol_unregister);
+/**
+ * scmi_create_protocol_devices - Create devices for all pending requests for
+ * this SCMI instance.
+ *
+ * @np: The device node describing the protocol
+ * @info: The SCMI instance descriptor
+ * @prot_id: The protocol ID
+ * @name: The optional name of the device to be created: if not provided this
+ * call will lead to the creation of all the devices currently requested
+ * for the specified protocol.
+ */
+static void scmi_create_protocol_devices(struct device_node *np,
+ struct scmi_info *info,
+ int prot_id, const char *name)
+{
+ struct scmi_device *sdev;
+
+ mutex_lock(&info->devreq_mtx);
+ sdev = scmi_device_create(np, info->dev, prot_id, name);
+ if (name && !sdev)
+ dev_err(info->dev,
+ "failed to create device for protocol 0x%X (%s)\n",
+ prot_id, name);
+ mutex_unlock(&info->devreq_mtx);
+}
+
+static void scmi_destroy_protocol_devices(struct scmi_info *info,
+ int prot_id, const char *name)
+{
+ mutex_lock(&info->devreq_mtx);
+ scmi_device_destroy(info->dev, prot_id, name);
+ mutex_unlock(&info->devreq_mtx);
+}
+
void scmi_notification_instance_data_set(const struct scmi_handle *handle,
void *priv)
{
@@ -2283,8 +2322,12 @@ static int scmi_device_request_notifier(struct notifier_block *nb,
switch (action) {
case SCMI_BUS_NOTIFY_DEVICE_REQUEST:
+ scmi_create_protocol_devices(np, info, id_table->protocol_id,
+ id_table->name);
break;
case SCMI_BUS_NOTIFY_DEVICE_UNREQUEST:
+ scmi_destroy_protocol_devices(info, id_table->protocol_id,
+ id_table->name);
break;
default:
return NOTIFY_DONE;
@@ -2318,6 +2361,7 @@ static int scmi_probe(struct platform_device *pdev)
idr_init(&info->protocols);
mutex_init(&info->protocols_mtx);
idr_init(&info->active_protocols);
+ mutex_init(&info->devreq_mtx);
platform_set_drvdata(pdev, info);
idr_init(&info->tx_idr);
@@ -2412,6 +2456,7 @@ static int scmi_probe(struct platform_device *pdev)
}
of_node_get(child);
+ scmi_create_protocol_devices(child, info, prot_id, NULL);
}
return 0;
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 9/9] firmware: arm_scmi: Split bus and driver into distinct modules
2022-12-22 18:50 ` Cristian Marussi
@ 2022-12-22 18:50 ` Cristian Marussi
-1 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Make the SCMI bus on its own as a distinct module initialized at
subsys_initcall level when builtin.
Keep the SCMI driver core stack, together with any configured transport,
in a different module initialized as module_init level.
SCMI Drivers initialization remain unchanged at module_init level.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/Makefile | 8 ++++++--
drivers/firmware/arm_scmi/bus.c | 15 ++++++++++++---
drivers/firmware/arm_scmi/common.h | 2 --
drivers/firmware/arm_scmi/driver.c | 6 +-----
4 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index 9ea86f8cc8f7..f0596c386c62 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
scmi-bus-y = bus.o
+scmi-core-objs := $(scmi-bus-y)
+
scmi-driver-y = driver.o notify.o
scmi-transport-$(CONFIG_ARM_SCMI_HAVE_SHMEM) = shmem.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += mailbox.o
@@ -8,9 +10,11 @@ scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
-scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \
- $(scmi-transport-y)
+scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
+
+obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
+
obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o
obj-$(CONFIG_ARM_SCMI_POWER_CONTROL) += scmi_power_control.o
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 190999a658b2..6228f1581fe3 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -476,18 +476,21 @@ static void scmi_devices_unregister(void)
bus_for_each_dev(&scmi_bus_type, NULL, NULL, __scmi_devices_unregister);
}
-int __init scmi_bus_init(void)
+static int __init scmi_bus_init(void)
{
int retval;
retval = bus_register(&scmi_bus_type);
if (retval)
- pr_err("scmi protocol bus register failed (%d)\n", retval);
+ pr_err("SCMI protocol bus register failed (%d)\n", retval);
+
+ pr_info("SCMI protocol bus registered\n");
return retval;
}
+subsys_initcall(scmi_bus_init);
-void __exit scmi_bus_exit(void)
+static void __exit scmi_bus_exit(void)
{
/*
* Destroy all remaining devices: just in case the drivers were
@@ -497,3 +500,9 @@ void __exit scmi_bus_exit(void)
bus_unregister(&scmi_bus_type);
ida_destroy(&scmi_bus_id);
}
+module_exit(scmi_bus_exit);
+
+MODULE_ALIAS("scmi-core");
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("ARM SCMI protocol bus");
+MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 19726a037991..80f63fc8ca14 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -99,8 +99,6 @@ scmi_revision_area_get(const struct scmi_protocol_handle *ph);
void scmi_setup_protocol_implemented(const struct scmi_protocol_handle *ph,
u8 *prot_imp);
-int __init scmi_bus_init(void);
-void __exit scmi_bus_exit(void);
extern struct bus_type scmi_bus_type;
#define SCMI_BUS_NOTIFY_DEVICE_REQUEST 0
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 115baaa4aca9..50267eef10fa 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2638,8 +2638,6 @@ static int __init scmi_driver_init(void)
if (WARN_ON(!IS_ENABLED(CONFIG_ARM_SCMI_HAVE_TRANSPORT)))
return -EINVAL;
- scmi_bus_init();
-
/* Initialize any compiled-in transport which provided an init/exit */
ret = scmi_transports_init();
if (ret)
@@ -2658,7 +2656,7 @@ static int __init scmi_driver_init(void)
return platform_driver_register(&scmi_driver);
}
-subsys_initcall(scmi_driver_init);
+module_init(scmi_driver_init);
static void __exit scmi_driver_exit(void)
{
@@ -2673,8 +2671,6 @@ static void __exit scmi_driver_exit(void)
scmi_system_unregister();
scmi_powercap_unregister();
- scmi_bus_exit();
-
scmi_transports_exit();
platform_driver_unregister(&scmi_driver);
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 9/9] firmware: arm_scmi: Split bus and driver into distinct modules
@ 2022-12-22 18:50 ` Cristian Marussi
0 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-22 18:50 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, f.fainelli, etienne.carriere,
vincent.guittot, Ludvig.Parsson, cristian.marussi
Make the SCMI bus on its own as a distinct module initialized at
subsys_initcall level when builtin.
Keep the SCMI driver core stack, together with any configured transport,
in a different module initialized as module_init level.
SCMI Drivers initialization remain unchanged at module_init level.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/Makefile | 8 ++++++--
drivers/firmware/arm_scmi/bus.c | 15 ++++++++++++---
drivers/firmware/arm_scmi/common.h | 2 --
drivers/firmware/arm_scmi/driver.c | 6 +-----
4 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index 9ea86f8cc8f7..f0596c386c62 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
scmi-bus-y = bus.o
+scmi-core-objs := $(scmi-bus-y)
+
scmi-driver-y = driver.o notify.o
scmi-transport-$(CONFIG_ARM_SCMI_HAVE_SHMEM) = shmem.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += mailbox.o
@@ -8,9 +10,11 @@ scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
-scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \
- $(scmi-transport-y)
+scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
+
+obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
+
obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o
obj-$(CONFIG_ARM_SCMI_POWER_CONTROL) += scmi_power_control.o
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 190999a658b2..6228f1581fe3 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -476,18 +476,21 @@ static void scmi_devices_unregister(void)
bus_for_each_dev(&scmi_bus_type, NULL, NULL, __scmi_devices_unregister);
}
-int __init scmi_bus_init(void)
+static int __init scmi_bus_init(void)
{
int retval;
retval = bus_register(&scmi_bus_type);
if (retval)
- pr_err("scmi protocol bus register failed (%d)\n", retval);
+ pr_err("SCMI protocol bus register failed (%d)\n", retval);
+
+ pr_info("SCMI protocol bus registered\n");
return retval;
}
+subsys_initcall(scmi_bus_init);
-void __exit scmi_bus_exit(void)
+static void __exit scmi_bus_exit(void)
{
/*
* Destroy all remaining devices: just in case the drivers were
@@ -497,3 +500,9 @@ void __exit scmi_bus_exit(void)
bus_unregister(&scmi_bus_type);
ida_destroy(&scmi_bus_id);
}
+module_exit(scmi_bus_exit);
+
+MODULE_ALIAS("scmi-core");
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("ARM SCMI protocol bus");
+MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 19726a037991..80f63fc8ca14 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -99,8 +99,6 @@ scmi_revision_area_get(const struct scmi_protocol_handle *ph);
void scmi_setup_protocol_implemented(const struct scmi_protocol_handle *ph,
u8 *prot_imp);
-int __init scmi_bus_init(void);
-void __exit scmi_bus_exit(void);
extern struct bus_type scmi_bus_type;
#define SCMI_BUS_NOTIFY_DEVICE_REQUEST 0
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 115baaa4aca9..50267eef10fa 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2638,8 +2638,6 @@ static int __init scmi_driver_init(void)
if (WARN_ON(!IS_ENABLED(CONFIG_ARM_SCMI_HAVE_TRANSPORT)))
return -EINVAL;
- scmi_bus_init();
-
/* Initialize any compiled-in transport which provided an init/exit */
ret = scmi_transports_init();
if (ret)
@@ -2658,7 +2656,7 @@ static int __init scmi_driver_init(void)
return platform_driver_register(&scmi_driver);
}
-subsys_initcall(scmi_driver_init);
+module_init(scmi_driver_init);
static void __exit scmi_driver_exit(void)
{
@@ -2673,8 +2671,6 @@ static void __exit scmi_driver_exit(void)
scmi_system_unregister();
scmi_powercap_unregister();
- scmi_bus_exit();
-
scmi_transports_exit();
platform_driver_unregister(&scmi_driver);
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] Rework SCMI initialization and probing sequence
2022-12-22 18:50 ` Cristian Marussi
@ 2022-12-23 5:36 ` Sumit Garg
-1 siblings, 0 replies; 28+ messages in thread
From: Sumit Garg @ 2022-12-23 5:36 UTC (permalink / raw)
To: Cristian Marussi
Cc: linux-kernel, linux-arm-kernel, sudeep.holla, james.quinlan,
f.fainelli, etienne.carriere, vincent.guittot, Ludvig.Parsson
On Fri, 23 Dec 2022 at 00:22, Cristian Marussi <cristian.marussi@arm.com> wrote:
>
> Hi,
>
> under some configurations the SCMI core stack, which is now initialized
> as a whole at the subsys_initcall level, can be dependent on some other
> Kernel subsystems (like TEE) when some SCMI transport backend like optee
> is used.
Thanks Cristian for the rework, but this doesn't seem to address
reluctance to carry forward the DT legacy (see [1]).
TLDR, it has led to misrepresentation of OP-TEE transport as follows:
First represented as a platform device via DT (compatible =
"linaro,scmi-optee";) and then
Migrated to being a TEE bus device (UUID: 0xa8cfe406, 0xd4f5,
0x4a2e, 0x9f, 0x8d, 0xa2, 0x5d, 0xc7, 0x54, 0xc0, 0x99)
Do we really need to have a platform device for every SCMI transport?
[1] https://lore.kernel.org/lkml/CAFA6WYPwku8d7EiJ8rF5pVh568oy+jXMXLdxSr6r476e0SD2nw@mail.gmail.com/
-Sumit
>
> This has been reported to lead to some awkward probe loop which, even
> though successful at the end, leaves a track of errors in the logs coming
> directly from the core Linux driver model facilities.
>
> In order to solve this issue and cleaning up a bit the SCMI stack startup
> sequence, this small series reviews and reworks the SCMI core stack
> initialization and probe logic.
>
> Basically the SCMI Bus is split into its own module (scmi-core.ko) which is
> initialized at subsys_initcall, while the SCMI core stack, including its
> various transport backends (like optee, mailbox, virtio, smc), is kept into
> a distinct module (scmi-module.ko) which get initialized at module_init.
>
> The SCMI driver users initlevel, instead, remains unchanged at module_init.
>
> No change is made to the Kconfig: the main ARM_SCMI_PROTOCOL option will
> now cause both modules to be built.
>
> This allows the other possibly needed subsystems to be up and running
> well before the core SCMI stack and its dependent transport backends, so
> solving the reported issue.
>
> Tested with SCMI transports mailbox/virtio and, in a previous draft, optee,
> in a number of different load/unload/bind/unbind combinations both as
> builtin and as LKMs.
>
> Applies on v6.1.
>
> Any feedback, testing welcome.
>
> Thanks,
> Cristian
>
> Cristian Marussi (9):
> firmware: arm_scmi: Simplify chan_available transport operation
> firmware: arm_scmi: Use dedicated devices to initialize channels
> firmware: arm_scmi: Move protocol registration helpers
> firmware: arm_scmi: Add common notifier helpers
> firmware: arm_scmi: Refactor protocol device creation
> firmware: arm_scmi: Move handle get/set helpers
> firmware: arm_scmi: Refactor device create/destroy helpers
> firmware: arm_scmi: Introduce a new lifecycle for protocol devices
> firmware: arm_scmi: Split bus and driver into distinct modules
>
> drivers/firmware/arm_scmi/Makefile | 8 +-
> drivers/firmware/arm_scmi/bus.c | 388 ++++++++++++-----
> drivers/firmware/arm_scmi/common.h | 25 +-
> drivers/firmware/arm_scmi/driver.c | 623 ++++++++++++++--------------
> drivers/firmware/arm_scmi/mailbox.c | 6 +-
> drivers/firmware/arm_scmi/optee.c | 6 +-
> drivers/firmware/arm_scmi/smc.c | 6 +-
> drivers/firmware/arm_scmi/virtio.c | 4 +-
> include/linux/scmi_protocol.h | 5 -
> 9 files changed, 622 insertions(+), 449 deletions(-)
>
> --
> 2.34.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] Rework SCMI initialization and probing sequence
@ 2022-12-23 5:36 ` Sumit Garg
0 siblings, 0 replies; 28+ messages in thread
From: Sumit Garg @ 2022-12-23 5:36 UTC (permalink / raw)
To: Cristian Marussi
Cc: linux-kernel, linux-arm-kernel, sudeep.holla, james.quinlan,
f.fainelli, etienne.carriere, vincent.guittot, Ludvig.Parsson
On Fri, 23 Dec 2022 at 00:22, Cristian Marussi <cristian.marussi@arm.com> wrote:
>
> Hi,
>
> under some configurations the SCMI core stack, which is now initialized
> as a whole at the subsys_initcall level, can be dependent on some other
> Kernel subsystems (like TEE) when some SCMI transport backend like optee
> is used.
Thanks Cristian for the rework, but this doesn't seem to address
reluctance to carry forward the DT legacy (see [1]).
TLDR, it has led to misrepresentation of OP-TEE transport as follows:
First represented as a platform device via DT (compatible =
"linaro,scmi-optee";) and then
Migrated to being a TEE bus device (UUID: 0xa8cfe406, 0xd4f5,
0x4a2e, 0x9f, 0x8d, 0xa2, 0x5d, 0xc7, 0x54, 0xc0, 0x99)
Do we really need to have a platform device for every SCMI transport?
[1] https://lore.kernel.org/lkml/CAFA6WYPwku8d7EiJ8rF5pVh568oy+jXMXLdxSr6r476e0SD2nw@mail.gmail.com/
-Sumit
>
> This has been reported to lead to some awkward probe loop which, even
> though successful at the end, leaves a track of errors in the logs coming
> directly from the core Linux driver model facilities.
>
> In order to solve this issue and cleaning up a bit the SCMI stack startup
> sequence, this small series reviews and reworks the SCMI core stack
> initialization and probe logic.
>
> Basically the SCMI Bus is split into its own module (scmi-core.ko) which is
> initialized at subsys_initcall, while the SCMI core stack, including its
> various transport backends (like optee, mailbox, virtio, smc), is kept into
> a distinct module (scmi-module.ko) which get initialized at module_init.
>
> The SCMI driver users initlevel, instead, remains unchanged at module_init.
>
> No change is made to the Kconfig: the main ARM_SCMI_PROTOCOL option will
> now cause both modules to be built.
>
> This allows the other possibly needed subsystems to be up and running
> well before the core SCMI stack and its dependent transport backends, so
> solving the reported issue.
>
> Tested with SCMI transports mailbox/virtio and, in a previous draft, optee,
> in a number of different load/unload/bind/unbind combinations both as
> builtin and as LKMs.
>
> Applies on v6.1.
>
> Any feedback, testing welcome.
>
> Thanks,
> Cristian
>
> Cristian Marussi (9):
> firmware: arm_scmi: Simplify chan_available transport operation
> firmware: arm_scmi: Use dedicated devices to initialize channels
> firmware: arm_scmi: Move protocol registration helpers
> firmware: arm_scmi: Add common notifier helpers
> firmware: arm_scmi: Refactor protocol device creation
> firmware: arm_scmi: Move handle get/set helpers
> firmware: arm_scmi: Refactor device create/destroy helpers
> firmware: arm_scmi: Introduce a new lifecycle for protocol devices
> firmware: arm_scmi: Split bus and driver into distinct modules
>
> drivers/firmware/arm_scmi/Makefile | 8 +-
> drivers/firmware/arm_scmi/bus.c | 388 ++++++++++++-----
> drivers/firmware/arm_scmi/common.h | 25 +-
> drivers/firmware/arm_scmi/driver.c | 623 ++++++++++++++--------------
> drivers/firmware/arm_scmi/mailbox.c | 6 +-
> drivers/firmware/arm_scmi/optee.c | 6 +-
> drivers/firmware/arm_scmi/smc.c | 6 +-
> drivers/firmware/arm_scmi/virtio.c | 4 +-
> include/linux/scmi_protocol.h | 5 -
> 9 files changed, 622 insertions(+), 449 deletions(-)
>
> --
> 2.34.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] Rework SCMI initialization and probing sequence
2022-12-23 5:36 ` Sumit Garg
@ 2022-12-23 11:37 ` Cristian Marussi
-1 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-23 11:37 UTC (permalink / raw)
To: Sumit Garg
Cc: linux-kernel, linux-arm-kernel, sudeep.holla, james.quinlan,
f.fainelli, etienne.carriere, vincent.guittot, Ludvig.Parsson
On Fri, Dec 23, 2022 at 11:06:29AM +0530, Sumit Garg wrote:
> On Fri, 23 Dec 2022 at 00:22, Cristian Marussi <cristian.marussi@arm.com> wrote:
> >
> > Hi,
> >
> > under some configurations the SCMI core stack, which is now initialized
> > as a whole at the subsys_initcall level, can be dependent on some other
> > Kernel subsystems (like TEE) when some SCMI transport backend like optee
> > is used.
>
> Thanks Cristian for the rework, but this doesn't seem to address
> reluctance to carry forward the DT legacy (see [1]).
>
> TLDR, it has led to misrepresentation of OP-TEE transport as follows:
>
> First represented as a platform device via DT (compatible =
> "linaro,scmi-optee";) and then
> Migrated to being a TEE bus device (UUID: 0xa8cfe406, 0xd4f5,
> 0x4a2e, 0x9f, 0x8d, 0xa2, 0x5d, 0xc7, 0x54, 0xc0, 0x99)
>
> Do we really need to have a platform device for every SCMI transport?
>
> [1] https://lore.kernel.org/lkml/CAFA6WYPwku8d7EiJ8rF5pVh568oy+jXMXLdxSr6r476e0SD2nw@mail.gmail.com/
>
Hi Sumit,
thanks for the feedback first of all.
This series represents really a long standing point on my todo-list and it
is meant to start addressing/reviewing the whole SCMI stack init/probe
sequencing and transports setup while taking the chance/opportunity to
fix the issue reported by Ludvig.
The natural next step in my (and Sudeep) view would be to split out the SCMI
transports too into proper full fledged drivers, that can be probed by their
own susbsys eventually (when possible) and that will then register with the
SCMI core as available transports; so that we can avoid some of the cruft
when multiple backend subsystems are involved...
...it is just that I have NOT dug deep into this further evolution and I did
NOT want to do it in this series, but just starting laying out some basic rework
toward this direction while fixing Ludvig issue. (... also because there are a
lot of bit and pieces to get right in SCMI around protocols/modules and DT
parsing and I was trying not to break too many things at a time :P...)
Anyway, even in the perspective of the above possible evolution into full
fledged drivers, I doubt that we can get rid completely of the DT based
per-transport platform devices since their DT nodes can carry a bit of
transport related information (even for auto-discoverable transport I think)
...it will just be that such devices, bound to the compatibles, will be used
probably in a different way (also for backward compatibility with DT
bindings...)...indeed...such platform devices now DO carry some information
about the underlying transport to use BUT most of all they represent also
an SCMI platform instance, so that will not definitely go away completely,
it will just loose most of the transport related functionalities
..but... as said...I have not dived too much into this further evolution so
I maybe wrong here on the details... anyway the plan going further, as spoken
also with Sudeep offline, could/should be that depicted above.
Not sure if this answers all of your questions but I'll keep you posted
on this series and next evolutions...
Thanks,
Cristian
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] Rework SCMI initialization and probing sequence
@ 2022-12-23 11:37 ` Cristian Marussi
0 siblings, 0 replies; 28+ messages in thread
From: Cristian Marussi @ 2022-12-23 11:37 UTC (permalink / raw)
To: Sumit Garg
Cc: linux-kernel, linux-arm-kernel, sudeep.holla, james.quinlan,
f.fainelli, etienne.carriere, vincent.guittot, Ludvig.Parsson
On Fri, Dec 23, 2022 at 11:06:29AM +0530, Sumit Garg wrote:
> On Fri, 23 Dec 2022 at 00:22, Cristian Marussi <cristian.marussi@arm.com> wrote:
> >
> > Hi,
> >
> > under some configurations the SCMI core stack, which is now initialized
> > as a whole at the subsys_initcall level, can be dependent on some other
> > Kernel subsystems (like TEE) when some SCMI transport backend like optee
> > is used.
>
> Thanks Cristian for the rework, but this doesn't seem to address
> reluctance to carry forward the DT legacy (see [1]).
>
> TLDR, it has led to misrepresentation of OP-TEE transport as follows:
>
> First represented as a platform device via DT (compatible =
> "linaro,scmi-optee";) and then
> Migrated to being a TEE bus device (UUID: 0xa8cfe406, 0xd4f5,
> 0x4a2e, 0x9f, 0x8d, 0xa2, 0x5d, 0xc7, 0x54, 0xc0, 0x99)
>
> Do we really need to have a platform device for every SCMI transport?
>
> [1] https://lore.kernel.org/lkml/CAFA6WYPwku8d7EiJ8rF5pVh568oy+jXMXLdxSr6r476e0SD2nw@mail.gmail.com/
>
Hi Sumit,
thanks for the feedback first of all.
This series represents really a long standing point on my todo-list and it
is meant to start addressing/reviewing the whole SCMI stack init/probe
sequencing and transports setup while taking the chance/opportunity to
fix the issue reported by Ludvig.
The natural next step in my (and Sudeep) view would be to split out the SCMI
transports too into proper full fledged drivers, that can be probed by their
own susbsys eventually (when possible) and that will then register with the
SCMI core as available transports; so that we can avoid some of the cruft
when multiple backend subsystems are involved...
...it is just that I have NOT dug deep into this further evolution and I did
NOT want to do it in this series, but just starting laying out some basic rework
toward this direction while fixing Ludvig issue. (... also because there are a
lot of bit and pieces to get right in SCMI around protocols/modules and DT
parsing and I was trying not to break too many things at a time :P...)
Anyway, even in the perspective of the above possible evolution into full
fledged drivers, I doubt that we can get rid completely of the DT based
per-transport platform devices since their DT nodes can carry a bit of
transport related information (even for auto-discoverable transport I think)
...it will just be that such devices, bound to the compatibles, will be used
probably in a different way (also for backward compatibility with DT
bindings...)...indeed...such platform devices now DO carry some information
about the underlying transport to use BUT most of all they represent also
an SCMI platform instance, so that will not definitely go away completely,
it will just loose most of the transport related functionalities
..but... as said...I have not dived too much into this further evolution so
I maybe wrong here on the details... anyway the plan going further, as spoken
also with Sudeep offline, could/should be that depicted above.
Not sure if this answers all of your questions but I'll keep you posted
on this series and next evolutions...
Thanks,
Cristian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] Rework SCMI initialization and probing sequence
2022-12-23 11:37 ` Cristian Marussi
@ 2022-12-26 13:54 ` Sumit Garg
-1 siblings, 0 replies; 28+ messages in thread
From: Sumit Garg @ 2022-12-26 13:54 UTC (permalink / raw)
To: Cristian Marussi
Cc: linux-kernel, linux-arm-kernel, sudeep.holla, james.quinlan,
f.fainelli, etienne.carriere, vincent.guittot, Ludvig.Parsson
On Fri, 23 Dec 2022 at 17:07, Cristian Marussi <cristian.marussi@arm.com> wrote:
>
> On Fri, Dec 23, 2022 at 11:06:29AM +0530, Sumit Garg wrote:
> > On Fri, 23 Dec 2022 at 00:22, Cristian Marussi <cristian.marussi@arm.com> wrote:
> > >
> > > Hi,
> > >
> > > under some configurations the SCMI core stack, which is now initialized
> > > as a whole at the subsys_initcall level, can be dependent on some other
> > > Kernel subsystems (like TEE) when some SCMI transport backend like optee
> > > is used.
> >
> > Thanks Cristian for the rework, but this doesn't seem to address
> > reluctance to carry forward the DT legacy (see [1]).
> >
> > TLDR, it has led to misrepresentation of OP-TEE transport as follows:
> >
> > First represented as a platform device via DT (compatible =
> > "linaro,scmi-optee";) and then
> > Migrated to being a TEE bus device (UUID: 0xa8cfe406, 0xd4f5,
> > 0x4a2e, 0x9f, 0x8d, 0xa2, 0x5d, 0xc7, 0x54, 0xc0, 0x99)
> >
> > Do we really need to have a platform device for every SCMI transport?
> >
> > [1] https://lore.kernel.org/lkml/CAFA6WYPwku8d7EiJ8rF5pVh568oy+jXMXLdxSr6r476e0SD2nw@mail.gmail.com/
> >
>
> Hi Sumit,
>
> thanks for the feedback first of all.
>
> This series represents really a long standing point on my todo-list and it
> is meant to start addressing/reviewing the whole SCMI stack init/probe
> sequencing and transports setup while taking the chance/opportunity to
> fix the issue reported by Ludvig.
>
> The natural next step in my (and Sudeep) view would be to split out the SCMI
> transports too into proper full fledged drivers, that can be probed by their
> own susbsys eventually (when possible) and that will then register with the
> SCMI core as available transports; so that we can avoid some of the cruft
> when multiple backend subsystems are involved...
>
> ...it is just that I have NOT dug deep into this further evolution and I did
> NOT want to do it in this series, but just starting laying out some basic rework
> toward this direction while fixing Ludvig issue. (... also because there are a
> lot of bit and pieces to get right in SCMI around protocols/modules and DT
> parsing and I was trying not to break too many things at a time :P...)
>
> Anyway, even in the perspective of the above possible evolution into full
> fledged drivers, I doubt that we can get rid completely of the DT based
> per-transport platform devices since their DT nodes can carry a bit of
> transport related information (even for auto-discoverable transport I think)
>
> ...it will just be that such devices, bound to the compatibles, will be used
> probably in a different way (also for backward compatibility with DT
> bindings...)...indeed...such platform devices now DO carry some information
> about the underlying transport to use BUT most of all they represent also
> an SCMI platform instance, so that will not definitely go away completely,
> it will just loose most of the transport related functionalities
>
> ..but... as said...I have not dived too much into this further evolution so
> I maybe wrong here on the details... anyway the plan going further, as spoken
> also with Sudeep offline, could/should be that depicted above.
>
> Not sure if this answers all of your questions but I'll keep you posted
> on this series and next evolutions...
Thanks for the detailed clarification. I don't have the deep insights
regarding how SCMI subsystem works but generally dealing with a device
being probed on multiple buses is prone to system integration problems
such as:
- Is the device present on the platform bus (in DT)? Is the device
present on a discoverable bus (eg. TEE bus)?
- Do both buses represent synchronised device views? IOW, version skew problems.
I hope we should be able to address those with the evolution you are planning.
-Sumit
>
> Thanks,
> Cristian
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] Rework SCMI initialization and probing sequence
@ 2022-12-26 13:54 ` Sumit Garg
0 siblings, 0 replies; 28+ messages in thread
From: Sumit Garg @ 2022-12-26 13:54 UTC (permalink / raw)
To: Cristian Marussi
Cc: linux-kernel, linux-arm-kernel, sudeep.holla, james.quinlan,
f.fainelli, etienne.carriere, vincent.guittot, Ludvig.Parsson
On Fri, 23 Dec 2022 at 17:07, Cristian Marussi <cristian.marussi@arm.com> wrote:
>
> On Fri, Dec 23, 2022 at 11:06:29AM +0530, Sumit Garg wrote:
> > On Fri, 23 Dec 2022 at 00:22, Cristian Marussi <cristian.marussi@arm.com> wrote:
> > >
> > > Hi,
> > >
> > > under some configurations the SCMI core stack, which is now initialized
> > > as a whole at the subsys_initcall level, can be dependent on some other
> > > Kernel subsystems (like TEE) when some SCMI transport backend like optee
> > > is used.
> >
> > Thanks Cristian for the rework, but this doesn't seem to address
> > reluctance to carry forward the DT legacy (see [1]).
> >
> > TLDR, it has led to misrepresentation of OP-TEE transport as follows:
> >
> > First represented as a platform device via DT (compatible =
> > "linaro,scmi-optee";) and then
> > Migrated to being a TEE bus device (UUID: 0xa8cfe406, 0xd4f5,
> > 0x4a2e, 0x9f, 0x8d, 0xa2, 0x5d, 0xc7, 0x54, 0xc0, 0x99)
> >
> > Do we really need to have a platform device for every SCMI transport?
> >
> > [1] https://lore.kernel.org/lkml/CAFA6WYPwku8d7EiJ8rF5pVh568oy+jXMXLdxSr6r476e0SD2nw@mail.gmail.com/
> >
>
> Hi Sumit,
>
> thanks for the feedback first of all.
>
> This series represents really a long standing point on my todo-list and it
> is meant to start addressing/reviewing the whole SCMI stack init/probe
> sequencing and transports setup while taking the chance/opportunity to
> fix the issue reported by Ludvig.
>
> The natural next step in my (and Sudeep) view would be to split out the SCMI
> transports too into proper full fledged drivers, that can be probed by their
> own susbsys eventually (when possible) and that will then register with the
> SCMI core as available transports; so that we can avoid some of the cruft
> when multiple backend subsystems are involved...
>
> ...it is just that I have NOT dug deep into this further evolution and I did
> NOT want to do it in this series, but just starting laying out some basic rework
> toward this direction while fixing Ludvig issue. (... also because there are a
> lot of bit and pieces to get right in SCMI around protocols/modules and DT
> parsing and I was trying not to break too many things at a time :P...)
>
> Anyway, even in the perspective of the above possible evolution into full
> fledged drivers, I doubt that we can get rid completely of the DT based
> per-transport platform devices since their DT nodes can carry a bit of
> transport related information (even for auto-discoverable transport I think)
>
> ...it will just be that such devices, bound to the compatibles, will be used
> probably in a different way (also for backward compatibility with DT
> bindings...)...indeed...such platform devices now DO carry some information
> about the underlying transport to use BUT most of all they represent also
> an SCMI platform instance, so that will not definitely go away completely,
> it will just loose most of the transport related functionalities
>
> ..but... as said...I have not dived too much into this further evolution so
> I maybe wrong here on the details... anyway the plan going further, as spoken
> also with Sudeep offline, could/should be that depicted above.
>
> Not sure if this answers all of your questions but I'll keep you posted
> on this series and next evolutions...
Thanks for the detailed clarification. I don't have the deep insights
regarding how SCMI subsystem works but generally dealing with a device
being probed on multiple buses is prone to system integration problems
such as:
- Is the device present on the platform bus (in DT)? Is the device
present on a discoverable bus (eg. TEE bus)?
- Do both buses represent synchronised device views? IOW, version skew problems.
I hope we should be able to address those with the evolution you are planning.
-Sumit
>
> Thanks,
> Cristian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] Rework SCMI initialization and probing sequence
2022-12-22 18:50 ` Cristian Marussi
@ 2023-01-19 19:31 ` Sudeep Holla
-1 siblings, 0 replies; 28+ messages in thread
From: Sudeep Holla @ 2023-01-19 19:31 UTC (permalink / raw)
To: Cristian Marussi, linux-arm-kernel, linux-kernel
Cc: Sudeep Holla, Ludvig.Parsson, etienne.carriere, vincent.guittot,
f.fainelli, james.quinlan
On Thu, 22 Dec 2022 18:50:40 +0000, Cristian Marussi wrote:
> under some configurations the SCMI core stack, which is now initialized
> as a whole at the subsys_initcall level, can be dependent on some other
> Kernel subsystems (like TEE) when some SCMI transport backend like optee
> is used.
>
> This has been reported to lead to some awkward probe loop which, even
> though successful at the end, leaves a track of errors in the logs coming
> directly from the core Linux driver model facilities.
>
> [...]
Applied to sudeep.holla/linux (for-next/scmi), thanks!
[1/9] firmware: arm_scmi: Simplify chan_available transport operation
https://git.kernel.org/sudeep.holla/c/7a75b7afd8ff
[2/9] firmware: arm_scmi: Use dedicated devices to initialize channels
https://git.kernel.org/sudeep.holla/c/05a2801d8b90
[3/9] firmware: arm_scmi: Move protocol registration helpers
https://git.kernel.org/sudeep.holla/c/9115c20ac1ee
[4/9] firmware: arm_scmi: Add common notifier helpers
https://git.kernel.org/sudeep.holla/c/53b8c25df708
[5/9] firmware: arm_scmi: Refactor protocol device creation
https://git.kernel.org/sudeep.holla/c/d3cd7c525fd2
[6/9] firmware: arm_scmi: Move handle get/set helpers
https://git.kernel.org/sudeep.holla/c/971fc0665f13
[7/9] firmware: arm_scmi: Refactor device create/destroy helpers
https://git.kernel.org/sudeep.holla/c/2c3e674465e7
[8/9] firmware: arm_scmi: Introduce a new lifecycle for protocol devices
https://git.kernel.org/sudeep.holla/c/ee5dcedaf72d
[9/9] firmware: arm_scmi: Split bus and driver into distinct modules
https://git.kernel.org/sudeep.holla/c/37b5be828032
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] Rework SCMI initialization and probing sequence
@ 2023-01-19 19:31 ` Sudeep Holla
0 siblings, 0 replies; 28+ messages in thread
From: Sudeep Holla @ 2023-01-19 19:31 UTC (permalink / raw)
To: Cristian Marussi, linux-arm-kernel, linux-kernel
Cc: Sudeep Holla, Ludvig.Parsson, etienne.carriere, vincent.guittot,
f.fainelli, james.quinlan
On Thu, 22 Dec 2022 18:50:40 +0000, Cristian Marussi wrote:
> under some configurations the SCMI core stack, which is now initialized
> as a whole at the subsys_initcall level, can be dependent on some other
> Kernel subsystems (like TEE) when some SCMI transport backend like optee
> is used.
>
> This has been reported to lead to some awkward probe loop which, even
> though successful at the end, leaves a track of errors in the logs coming
> directly from the core Linux driver model facilities.
>
> [...]
Applied to sudeep.holla/linux (for-next/scmi), thanks!
[1/9] firmware: arm_scmi: Simplify chan_available transport operation
https://git.kernel.org/sudeep.holla/c/7a75b7afd8ff
[2/9] firmware: arm_scmi: Use dedicated devices to initialize channels
https://git.kernel.org/sudeep.holla/c/05a2801d8b90
[3/9] firmware: arm_scmi: Move protocol registration helpers
https://git.kernel.org/sudeep.holla/c/9115c20ac1ee
[4/9] firmware: arm_scmi: Add common notifier helpers
https://git.kernel.org/sudeep.holla/c/53b8c25df708
[5/9] firmware: arm_scmi: Refactor protocol device creation
https://git.kernel.org/sudeep.holla/c/d3cd7c525fd2
[6/9] firmware: arm_scmi: Move handle get/set helpers
https://git.kernel.org/sudeep.holla/c/971fc0665f13
[7/9] firmware: arm_scmi: Refactor device create/destroy helpers
https://git.kernel.org/sudeep.holla/c/2c3e674465e7
[8/9] firmware: arm_scmi: Introduce a new lifecycle for protocol devices
https://git.kernel.org/sudeep.holla/c/ee5dcedaf72d
[9/9] firmware: arm_scmi: Split bus and driver into distinct modules
https://git.kernel.org/sudeep.holla/c/37b5be828032
--
Regards,
Sudeep
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2023-01-19 19:33 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-22 18:50 [PATCH 0/9] Rework SCMI initialization and probing sequence Cristian Marussi
2022-12-22 18:50 ` Cristian Marussi
2022-12-22 18:50 ` [PATCH 1/9] firmware: arm_scmi: Simplify chan_available transport operation Cristian Marussi
2022-12-22 18:50 ` Cristian Marussi
2022-12-22 18:50 ` [PATCH 2/9] firmware: arm_scmi: Use dedicated devices to initialize channels Cristian Marussi
2022-12-22 18:50 ` Cristian Marussi
2022-12-22 18:50 ` [PATCH 3/9] firmware: arm_scmi: Move protocol registration helpers Cristian Marussi
2022-12-22 18:50 ` Cristian Marussi
2022-12-22 18:50 ` [PATCH 4/9] firmware: arm_scmi: Add common notifier helpers Cristian Marussi
2022-12-22 18:50 ` Cristian Marussi
2022-12-22 18:50 ` [PATCH 5/9] firmware: arm_scmi: Refactor protocol device creation Cristian Marussi
2022-12-22 18:50 ` Cristian Marussi
2022-12-22 18:50 ` [PATCH 6/9] firmware: arm_scmi: Move handle get/set helpers Cristian Marussi
2022-12-22 18:50 ` Cristian Marussi
2022-12-22 18:50 ` [PATCH 7/9] firmware: arm_scmi: Refactor device create/destroy helpers Cristian Marussi
2022-12-22 18:50 ` Cristian Marussi
2022-12-22 18:50 ` [PATCH 8/9] firmware: arm_scmi: Introduce a new lifecycle for protocol devices Cristian Marussi
2022-12-22 18:50 ` Cristian Marussi
2022-12-22 18:50 ` [PATCH 9/9] firmware: arm_scmi: Split bus and driver into distinct modules Cristian Marussi
2022-12-22 18:50 ` Cristian Marussi
2022-12-23 5:36 ` [PATCH 0/9] Rework SCMI initialization and probing sequence Sumit Garg
2022-12-23 5:36 ` Sumit Garg
2022-12-23 11:37 ` Cristian Marussi
2022-12-23 11:37 ` Cristian Marussi
2022-12-26 13:54 ` Sumit Garg
2022-12-26 13:54 ` Sumit Garg
2023-01-19 19:31 ` Sudeep Holla
2023-01-19 19:31 ` Sudeep Holla
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.