[PATCH 0/2] firmware: arm_scmi: create scmi devices for protocols that not have of_node

Peng Fan (OSS) posted 2 patches 1 year, 5 months ago
There is a newer version of this series
drivers/firmware/arm_scmi/driver.c  | 33 ++++++++++++++++++++++++++++++++-
drivers/firmware/arm_scmi/mailbox.c |  2 ++
drivers/firmware/arm_scmi/optee.c   |  3 +++
drivers/firmware/arm_scmi/smc.c     |  7 ++++++-
drivers/firmware/arm_scmi/virtio.c  |  3 +++
5 files changed, 46 insertions(+), 2 deletions(-)
[PATCH 0/2] firmware: arm_scmi: create scmi devices for protocols that not have of_node
Posted by Peng Fan (OSS) 1 year, 5 months ago
Per
https://lore.kernel.org/all/20230125141113.kkbowopusikuogx6@bogus/
"
In short we shouldn't have to add a node if there are no consumers. It
was one of the topic of discussion initially when SCMI binding was added
and they exist only for the consumers otherwise we don't need it as
everything is discoverable from the interface.
"
https://lore.kernel.org/all/Y9JLUIioxFPn4BS0@e120937-lin/
If a node has its own channel, the of_node is still needed.

i.MX95 SCMI firmware not have dedicated channel for 0x12, and no need
of_node. This patchset is to support protocol 0x12 without the procotol
node in device tree.

Without of_node, still need to create the scmi devices. As of now,
it is based on an array 'protocols[]' in 'scmi_probe'. 

And no of_node, means no per protocol channel, so reuse the base
protocol channel. Need patch chan_available to support.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (2):
      firmware: arm_scmi: channel unavailable if no of_node
      firmware: arm_scmi: create scmi_devices that not have of_node

 drivers/firmware/arm_scmi/driver.c  | 33 ++++++++++++++++++++++++++++++++-
 drivers/firmware/arm_scmi/mailbox.c |  2 ++
 drivers/firmware/arm_scmi/optee.c   |  3 +++
 drivers/firmware/arm_scmi/smc.c     |  7 ++++++-
 drivers/firmware/arm_scmi/virtio.c  |  3 +++
 5 files changed, 46 insertions(+), 2 deletions(-)
---
base-commit: d8003eb2eb0200352b5d63af77ec0912a52e79ad
change-id: 20240626-scmi-driver-96dc61b036a2

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>
Re: [PATCH 0/2] firmware: arm_scmi: create scmi devices for protocols that not have of_node
Posted by Cristian Marussi 1 year, 5 months ago
On Wed, Jun 26, 2024 at 02:58:38PM +0800, Peng Fan (OSS) wrote:
> Per

Hi,

> https://lore.kernel.org/all/20230125141113.kkbowopusikuogx6@bogus/
> "
> In short we shouldn't have to add a node if there are no consumers. It
> was one of the topic of discussion initially when SCMI binding was added
> and they exist only for the consumers otherwise we don't need it as
> everything is discoverable from the interface.
> "
> https://lore.kernel.org/all/Y9JLUIioxFPn4BS0@e120937-lin/
> If a node has its own channel, the of_node is still needed.
> 
> i.MX95 SCMI firmware not have dedicated channel for 0x12, and no need
> of_node. This patchset is to support protocol 0x12 without the procotol
> node in device tree.
> 

With this patch you change a bit of the core logic to allow for
protocols not explicitly described in the DT to be instantiated, and you
use a static builtin array to list such protocols...so any future change
or any downstream vendor protocols that want to use this, we will have to
patch and extend such protocols[] array.

Moreover, if anyone DO want to use a per-protocol channel in the future
on some of these protocols, it will work fine with your solution on the code
side, BUT you will still have anyway a DT binding check error when you
try to add that 0x12 node to contain a channel description, right ?
... because in that case you will have re-added a (supposedly) empty
protocol node in order to containn the channels definitions and that wont
be yaml-compliant, am I right ?

IOW this solves your issue in the immediate, while adding complexity to
the core code and changing the core behaviour around protocols, but it
wont stand any future addition or different usage.

For these reasons, I still think that the cleanest solution is to just let
protocol nodes to exist even if not referenced anywhere from the DT (your
original patch to add protocol0x12 I think) simply because we allow
per-protocol channel definitions and so any empty unreferenced protocol
node could be needed in the future for this reason.

In this way we'll also keep treating protocols in an uniform way.

Just my opinion, though, I'll settle with what is finally decided
anyway.

Thank
Cristian