Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1407480 > unrolled thread

[RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants Neil Armstrong <narmstrong@baylibre.com> - 2016-05-26 11:40 +0200
    [RFC PATCH 1/2] firmware: Add a SCPI framework to handle multiple vendors implementation Neil Armstrong <narmstrong@baylibre.com> - 2016-05-26 11:40 +0200
    Re: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors  variants Sudeep Holla <sudeep.holla@arm.com> - 2016-05-26 18:30 +0200
      Re: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors  variants Neil Armstrong <narmstrong@baylibre.com> - 2016-05-27 10:20 +0200
        Re: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors  variants Neil Armstrong <narmstrong@baylibre.com> - 2016-05-27 17:20 +0200
        Re: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors  variants Neil Armstrong <narmstrong@baylibre.com> - 2016-05-30 10:40 +0200

#1407480 — [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants

Since the current SCPI implementation, based on [0]:
- is (at leat) JUNO specific
- does not specify a strong "standard"
- does not specify a strong MHU interface specification

SoC vendors could implement a variant with slight changes in message
indexes, new messages types, different messages data format or different MHU
communication scheme.

To keep the spirit of the SCPI interface, add a thin "register" layer to get
the ops from the parent node and switch the drivers using the ops to use
the new of_scpi_ops_get() call.

[0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html

Neil Armstrong (2):
  firmware: Add a SCPI framework to handle multiple vendors
    implementation
  firmware: scpi: Switch scpi drivers to use new Framework calls

 drivers/clk/clk-scpi.c         |  18 ++++---
 drivers/cpufreq/scpi-cpufreq.c |   7 +--
 drivers/firmware/Makefile      |   1 +
 drivers/firmware/arm_scpi.c    |  18 +++----
 drivers/firmware/scpi.c        | 110 +++++++++++++++++++++++++++++++++++++++++
 drivers/hwmon/scpi-hwmon.c     |   6 +--
 include/linux/scpi_protocol.h  |  33 ++++++++++++-
 7 files changed, 169 insertions(+), 24 deletions(-)
 create mode 100644 drivers/firmware/scpi.c

-- 
2.7.0

[toc] | [next] | [standalone]

#1407484 — [RFC PATCH 1/2] firmware: Add a SCPI framework to handle multiple vendors implementation

Add a thin "register" interface for SCPI driver in order to register their
ops along their node.

Since nodes using the SCPI ops are currently sub-nodes, does not implement
phandle xlate stuff.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/firmware/Makefile |   1 +
 drivers/firmware/scpi.c   | 110 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+)
 create mode 100644 drivers/firmware/scpi.c

diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 474bada..701a791 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -2,6 +2,7 @@
 # Makefile for the linux kernel.
 #
 obj-$(CONFIG_ARM_PSCI_FW)	+= psci.o
+obj-$(CONFIG_ARM_SCPI_PROTOCOL)	+= scpi.o
 obj-$(CONFIG_ARM_SCPI_PROTOCOL)	+= arm_scpi.o
 obj-$(CONFIG_DMI)		+= dmi_scan.o
 obj-$(CONFIG_DMI_SYSFS)		+= dmi-sysfs.o
diff --git a/drivers/firmware/scpi.c b/drivers/firmware/scpi.c
new file mode 100644
index 0000000..ba94ac4
--- /dev/null
+++ b/drivers/firmware/scpi.c
@@ -0,0 +1,110 @@
+/*
+ * System Control and Power Interface (SCPI) Message Protocol framework
+ *
+ * SCPI Message Protocol is used between the System Control Processor(SCP)
+ * and the Application Processors(AP). The Message Handling Unit(MHU)
+ * provides a mechanism for inter-processor communication between SCP's
+ * Cortex M3 and AP.
+ *
+ * SCP offers control and management of the core/cluster power states,
+ * various power domain DVFS including the core/cluster, certain system
+ * clocks configuration, thermal sensors and many others.
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ * Copyright (C) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/of.h>
+#include <linux/scpi_protocol.h>
+#include <linux/spinlock.h>
+
+static DEFINE_MUTEX(scpi_list_mutex);
+static LIST_HEAD(scpi_drivers_list);
+
+struct scpi_ops *of_scpi_ops_get(struct device_node *node)
+{
+	struct scpi_ops *ops = NULL;
+	struct scpi_driver *r;
+
+	if (!node)
+		return ERR_PTR(-EINVAL);
+
+	mutex_lock(&scpi_list_mutex);
+	list_for_each_entry(r, &scpi_drivers_list, list) {
+		if (node == r->node) {
+			ops = r->ops;
+			break;
+		}
+	}
+	mutex_unlock(&scpi_list_mutex);
+
+	if (!ops)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return ops;
+}
+EXPORT_SYMBOL_GPL(of_scpi_ops_get);
+
+int scpi_driver_register(struct scpi_driver *drv)
+{
+	mutex_lock(&scpi_list_mutex);
+	list_add(&drv->list, &scpi_drivers_list);
+	mutex_unlock(&scpi_list_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(scpi_driver_register);
+
+void scpi_driver_unregister(struct scpi_driver *drv)
+{
+	mutex_lock(&scpi_list_mutex);
+	list_del(&drv->list);
+	mutex_unlock(&scpi_list_mutex);
+}
+EXPORT_SYMBOL_GPL(scpi_driver_unregister);
+
+static void devm_scpi_driver_unregister(struct device *dev, void *res)
+{
+	scpi_driver_unregister(*(struct scpi_driver **)res);
+}
+
+int devm_scpi_driver_register(struct device *dev,
+				struct scpi_driver *drv)
+{
+	struct scpi_driver **rcdrv;
+	int ret;
+
+	rcdrv = devres_alloc(devm_scpi_driver_unregister, sizeof(*drv),
+			     GFP_KERNEL);
+	if (!rcdrv)
+		return -ENOMEM;
+
+	ret = scpi_driver_register(drv);
+	if (!ret) {
+		*rcdrv = drv;
+		devres_add(dev, rcdrv);
+	} else
+		devres_free(rcdrv);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_scpi_driver_register);
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1407621 — Re: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants

Hi Neil,

On 26/05/16 10:38, Neil Armstrong wrote:
> Since the current SCPI implementation, based on [0]:
> - is (at leat) JUNO specific

Agreed.

> - does not specify a strong "standard"

Not exactly, it's extensible. Refer section 3.2.2. Get SCP capability
Extended Set Enabled bit.

> - does not specify a strong MHU interface specification
>

Not really required, any mailbox must do.

> SoC vendors could implement a variant with slight changes in message
> indexes,

I assume you mean command index here

> new messages types,

Also fine with extended command set.

> different messages data format or

you mean the header or payload ? If they don't follow the header, then
how can be group them as same protocol ?

> different MHU communication scheme.

Not a problem as I mentioned above.

>
> To keep the spirit of the SCPI interface, add a thin "register" layer to get
> the ops from the parent node and switch the drivers using the ops to use
> the new of_scpi_ops_get() call.
>

All I can see is that you share the code to register such drivers which
is hardly anything. It's hard to say all drivers use same protocol or
interface after this change. I may be missing to see something here so
it would be easy to appreciate/review this change with one user of this.

My idea of extending this driver if vendor implements extensions based
on SCPI specification is something like below:

-- 
Regards,
Sudeep

-->8

diff --git i/drivers/firmware/arm_scpi.c w/drivers/firmware/arm_scpi.c
index 7e3e595c9f30..7e46aa103690 100644
--- i/drivers/firmware/arm_scpi.c
+++ w/drivers/firmware/arm_scpi.c
@@ -46,6 +46,8 @@

  #define CMD_ID_SHIFT		0
  #define CMD_ID_MASK		0x7f
+#define CMD_SET_SHIFT		7
+#define CMD_SET_MASK		0x1
  #define CMD_TOKEN_ID_SHIFT	8
  #define CMD_TOKEN_ID_MASK	0xff
  #define CMD_DATA_SIZE_SHIFT	16
@@ -53,6 +55,10 @@
  #define PACK_SCPI_CMD(cmd_id, tx_sz)			\
  	((((cmd_id) & CMD_ID_MASK) << CMD_ID_SHIFT) |	\
  	(((tx_sz) & CMD_DATA_SIZE_MASK) << CMD_DATA_SIZE_SHIFT))
+#define PACK_EXT_SCPI_CMD(cmd_id, tx_sz)		\
+	((((cmd_id) & CMD_ID_MASK) << CMD_ID_SHIFT) |	\
+	(CMD_SET_MASK << CMD_SET_SHIFT) |	\
+	(((tx_sz) & CMD_DATA_SIZE_MASK) << CMD_DATA_SIZE_SHIFT))
  #define ADD_SCPI_TOKEN(cmd, token)			\
  	((cmd) |= (((token) & CMD_TOKEN_ID_MASK) << CMD_TOKEN_ID_SHIFT))

@@ -132,6 +138,9 @@ enum scpi_std_cmd {
  	SCPI_CMD_COUNT
  };

+enum scpi_vendor_ext_cmd {
+};
+
  struct scpi_xfer {
  	u32 slot; /* has to be first element */
  	u32 cmd;
@@ -165,6 +174,7 @@ struct scpi_drvinfo {
  	struct scpi_ops *scpi_ops;
  	struct scpi_chan *channels;
  	struct scpi_dvfs_info *dvfs[MAX_DVFS_DOMAINS];
+	void *scpi_ext_ops;
  };

  /*
@@ -343,8 +353,8 @@ static void put_scpi_xfer(struct scpi_xfer *t, 
struct scpi_chan *ch)
  	mutex_unlock(&ch->xfers_lock);
  }

-static int scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len,
-			     void *rx_buf, unsigned int rx_len)
+static int __scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len,
+			       void *rx_buf, unsigned int rx_len, bool extn)
  {
  	int ret;
  	u8 chan;
@@ -359,7 +369,8 @@ static int scpi_send_message(u8 cmd, void *tx_buf, 
unsigned int tx_len,
  		return -ENOMEM;

  	msg->slot = BIT(SCPI_SLOT);
-	msg->cmd = PACK_SCPI_CMD(cmd, tx_len);
+	msg->cmd = extn ? PACK_EXT_SCPI_CMD(cmd, tx_len) :
+			  PACK_SCPI_CMD(cmd, tx_len);
  	msg->tx_buf = tx_buf;
  	msg->tx_len = tx_len;
  	msg->rx_buf = rx_buf;
@@ -384,6 +395,18 @@ static int scpi_send_message(u8 cmd, void *tx_buf, 
unsigned int tx_len,
  	return ret > 0 ? scpi_to_linux_errno(ret) : ret;
  }

+static int scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len,
+			     void *rx_buf, unsigned int rx_len)
+{
+	return __scpi_send_message(cmd, tx_buf, tx_len, rx_buf, rx_len, false);
+}
+
+static int scpi_send_ext_message(u8 cmd, void *tx_buf, unsigned int tx_len,
+				 void *rx_buf, unsigned int rx_len)
+{
+	return __scpi_send_message(cmd, tx_buf, tx_len, rx_buf, rx_len, true);
+}
+
  static u32 scpi_get_version(void)
  {
  	return scpi_info->protocol_version;
@@ -574,6 +597,29 @@ static int scpi_init_versions(struct scpi_drvinfo 
*info)
  	return ret;
  }

+static struct scpi_vendor_ext_ops scpi_vendor_ext_ops = {
+};
+
+void *get_scpi_ext_ops(void)
+{
+	return scpi_info ? scpi_info->scpi_ext_ops : NULL;
+}
+EXPORT_SYMBOL_GPL(get_scpi_ext_ops);
+
+static const struct of_device_id scpi_extentions_of_match[] = {
+	{.compatible = "vendor,scpi", .data = &scpi_vendor_ext_ops},
+	{},
+};
+
+static void
+scpi_init_extensions(struct scpi_drvinfo *info, struct device_node *np)
+{
+	const struct of_device_id *of_id;
+
+	if (np && (of_id = of_match_node(scpi_extentions_of_match, np)))
+		info->scpi_ext_ops = (void *)of_id->data;
+}
+
  static ssize_t protocol_version_show(struct device *dev,
  				     struct device_attribute *attr, char *buf)
  {
@@ -745,6 +791,8 @@ static int scpi_probe(struct platform_device *pdev)
  		  FW_REV_PATCH(scpi_info->firmware_version));
  	scpi_info->scpi_ops = &scpi_ops;

+	scpi_init_extensions(scpi_info, np);
+
  	ret = sysfs_create_groups(&dev->kobj, versions_groups);
  	if (ret)
  		dev_err(dev, "unable to create sysfs version group\n");
diff --git i/include/linux/scpi_protocol.h w/include/linux/scpi_protocol.h
index 35de50a65665..052f6aa1ae4b 100644
--- i/include/linux/scpi_protocol.h
+++ w/include/linux/scpi_protocol.h
@@ -72,8 +72,13 @@ struct scpi_ops {
  	int (*sensor_get_value)(u16, u64 *);
  };

+struct scpi_vendor_ext_ops {
+};
+
  #if IS_REACHABLE(CONFIG_ARM_SCPI_PROTOCOL)
  struct scpi_ops *get_scpi_ops(void);
+void *get_scpi_ext_ops(void);
  #else
  static inline struct scpi_ops *get_scpi_ops(void) { return NULL; }
+static inline void *get_scpi_ext_ops(void) { return NULL; }
  #endif

[toc] | [prev] | [next] | [standalone]

#1407908 — Re: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants

Hi,
On 05/26/2016 06:29 PM, Sudeep Holla wrote:
> Hi Neil,
> 
> On 26/05/16 10:38, Neil Armstrong wrote:
>> Since the current SCPI implementation, based on [0]:
>> - is (at leat) JUNO specific
> 
> Agreed.
> 
>> - does not specify a strong "standard"
> 
> Not exactly, it's extensible. Refer section 3.2.2. Get SCP capability
> Extended Set Enabled bit.

Well, I was not thinking about an extension, but this should be upstreamed for sure.

> 
>> - does not specify a strong MHU interface specification
>>
> 
> Not really required, any mailbox must do.
> 
>> SoC vendors could implement a variant with slight changes in message
>> indexes,
> 
> I assume you mean command index here
> 
>> new messages types,
> 
> Also fine with extended command set.
> 
>> different messages data format or
> 
> you mean the header or payload ? If they don't follow the header, then
> how can be group them as same protocol ?
> 
>> different MHU communication scheme.
> 
> Not a problem as I mentioned above.
> 
>>
>> To keep the spirit of the SCPI interface, add a thin "register" layer to get
>> the ops from the parent node and switch the drivers using the ops to use
>> the new of_scpi_ops_get() call.
>>
> 
> All I can see is that you share the code to register such drivers which
> is hardly anything. It's hard to say all drivers use same protocol or
> interface after this change. I may be missing to see something here so
> it would be easy to appreciate/review this change with one user of this.

I'm actually working on an "SCPI" implementation from Amlogic for their arm64 plaform,
and their implementation is slightly different but enough to need a separate driver.

For instance, the differences are :
- they do not implement virtual channels
- they pass the command word by the mailbox interface
- they use the "sender id" to group the command in a sort of "class"
- the payload size is shifted by 20 instead of 16
- the command word is not in the SRAM, so we cannot use a rx command queue, only a single command by channel at a single time
- the command indexes are slightly shifted
- in clk_set_value, "rate" is first instead of last entry
- in sensor_value, they only use a 32bit entry
- some commands are only accepted on the high priority channel, the others only on the low priority
- MAX_DVFS_DOMAINS is 3 instead of 8
- MAX_DVFS_OPPS is 16 instead of 8

But, It's still a "SCPI" interface by design and usage because :
- they implement 90% of the same commands, in the same way
- the usage is exactly the same and architecture is similar
- they use the same error code scheme

Finally, it would be stupid to add an exact copy of the scpi-sensors and scpi-clocks !
The scpi-cpufreq is another story since they only have a single A53 cluster, it's not adapted.

This is why I wrote a very thin layer, and it can also clean up the arm_scpi and the scpi driver to use a cleaner registry interface.
It would also be a good point to add a private_data to the ops and pass it to the scpi callbacks, to completely remove the global variables.

I found an issue with scpi-cpufreq, since the device is created by the scpi-clocks probe, it does not have a proper node and my current of_scpi_ops_get won't work...
The scpi-cpufreq should have a proper DT node and use the deffered probe to register after the scpi-clocks.

> 
> My idea of extending this driver if vendor implements extensions based
> on SCPI specification is something like below:
> 

Neil

[toc] | [prev] | [next] | [standalone]

#1408128 — Re: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants

On 05/27/2016 10:17 AM, Neil Armstrong wrote:
> Hi,
> On 05/26/2016 06:29 PM, Sudeep Holla wrote:
>> Hi Neil,
>>> To keep the spirit of the SCPI interface, add a thin "register" layer to get
>>> the ops from the parent node and switch the drivers using the ops to use
>>> the new of_scpi_ops_get() call.
>>>
>>
>> All I can see is that you share the code to register such drivers which
>> is hardly anything. It's hard to say all drivers use same protocol or
>> interface after this change. I may be missing to see something here so
>> it would be easy to appreciate/review this change with one user of this.
> 
> I'm actually working on a "SCPI" implementation from Amlogic for their arm64 platform,
> and their implementation is slightly different but enough to need a separate driver.
> 
> For instance, the differences are :
> - they do not implement virtual channels
> - they pass the command word by the mailbox interface
> - they use the "sender id" to group the command in a sort of "class"
> - the payload size is shifted by 20 instead of 16
> - the command word is not in the SRAM, so we cannot use a rx command queue, only a single command by channel at a single time
> - the command indexes are slightly shifted
> - in clk_set_value, "rate" is first instead of last entry
> - in sensor_value, they only use a 32bit entry
> - some commands are only accepted on the high priority channel, the others only on the low priority
> - MAX_DVFS_DOMAINS is 3 instead of 8
> - MAX_DVFS_OPPS is 16 instead of 8
> 
> But, It's still a "SCPI" interface by design and usage because :
> - they implement 90% of the same commands, in the same way
> - the usage is exactly the same and architecture is similar
> - they use the same error code scheme
> 
> Finally, it would be stupid to add an exact copy of the scpi-sensors and scpi-clocks !
> The scpi-cpufreq is another story since they only have a single A53 cluster, it's not adapted.
> 
> This is why I wrote a very thin layer, and it can also clean up the arm_scpi and the scpi driver to use a cleaner registry interface.
> It would also be a good point to add a private_data to the ops and pass it to the scpi callbacks, to completely remove the global variables.
> 
> I found an issue with scpi-cpufreq, since the device is created by the scpi-clocks probe, it does not have a proper node and my current of_scpi_ops_get won't work...
> The scpi-cpufreq should have a proper DT node and use the deffered probe to register after the scpi-clocks.

Actually I managed to make the Amlogic SCPI SCPI implementation work using a separate driver and the registry layer just fine.
The cpufreq works, like the scpi-dvfs-clocks and the scpi-hwmon drivers.

You can have a look at my github tree :
https://github.com/superna9999/linux/compare/28165ec7a99be98123aa89540bf2cfc24df19498...superna9999:amlogic/v4.7/scpi+fw-v0

The Amlogic SCPI implementation lies on top of this RFC patchset.
The current patchset need a fix in scpi-hwmon and for scpi-cpufreq but it works like a charm.

Neil

[toc] | [prev] | [next] | [standalone]

#1408863 — Re: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants

On 05/27/2016 10:17 AM, Neil Armstrong wrote:
> Hi,
> On 05/26/2016 06:29 PM, Sudeep Holla wrote:
>> Hi Neil,
>>
>> On 26/05/16 10:38, Neil Armstrong wrote:
>>> Since the current SCPI implementation, based on [0]:
>>> - is (at leat) JUNO specific
>>
>> Agreed.
>>
>>> - does not specify a strong "standard"
>>
>> Not exactly, it's extensible. Refer section 3.2.2. Get SCP capability
>> Extended Set Enabled bit.
> 
> Well, I was not thinking about an extension, but this should be upstreamed for sure.
> 
>>
>>> - does not specify a strong MHU interface specification
>>>
>>
>> Not really required, any mailbox must do.
>>
>>> SoC vendors could implement a variant with slight changes in message
>>> indexes,
>>
>> I assume you mean command index here
>>
>>> new messages types,
>>
>> Also fine with extended command set.
>>
>>> different messages data format or
>>
>> you mean the header or payload ? If they don't follow the header, then
>> how can be group them as same protocol ?
>>
>>> different MHU communication scheme.
>>
>> Not a problem as I mentioned above.
>>
>>>
>>> To keep the spirit of the SCPI interface, add a thin "register" layer to get
>>> the ops from the parent node and switch the drivers using the ops to use
>>> the new of_scpi_ops_get() call.
>>>
>>
>> All I can see is that you share the code to register such drivers which
>> is hardly anything. It's hard to say all drivers use same protocol or
>> interface after this change. I may be missing to see something here so
>> it would be easy to appreciate/review this change with one user of this.
> 
> I'm actually working on an "SCPI" implementation from Amlogic for their arm64 plaform,
> and their implementation is slightly different but enough to need a separate driver.
> 
> For instance, the differences are :
> - they do not implement virtual channels
> - they pass the command word by the mailbox interface
> - they use the "sender id" to group the command in a sort of "class"
> - the payload size is shifted by 20 instead of 16
> - the command word is not in the SRAM, so we cannot use a rx command queue, only a single command by channel at a single time
> - the command indexes are slightly shifted
> - in clk_set_value, "rate" is first instead of last entry
> - in sensor_value, they only use a 32bit entry
> - some commands are only accepted on the high priority channel, the others only on the low priority
> - MAX_DVFS_DOMAINS is 3 instead of 8
> - MAX_DVFS_OPPS is 16 instead of 8
> 
> But, It's still a "SCPI" interface by design and usage because :
> - they implement 90% of the same commands, in the same way
> - the usage is exactly the same and architecture is similar
> - they use the same error code scheme
> 
> Finally, it would be stupid to add an exact copy of the scpi-sensors and scpi-clocks !
> The scpi-cpufreq is another story since they only have a single A53 cluster, it's not adapted.
> 
> This is why I wrote a very thin layer, and it can also clean up the arm_scpi and the scpi driver to use a cleaner registry interface.
> It would also be a good point to add a private_data to the ops and pass it to the scpi callbacks, to completely remove the global variables.
> 
> I found an issue with scpi-cpufreq, since the device is created by the scpi-clocks probe, it does not have a proper node and my current of_scpi_ops_get won't work...
> The scpi-cpufreq should have a proper DT node and use the deffered probe to register after the scpi-clocks.
> 
>>
>> My idea of extending this driver if vendor implements extensions based
>> on SCPI specification is something like below:
>>

While looking for other ARMv8 based platform, I found that the RK3368 platform has the same SCPI implementation as Amlogic.

They extended it with DDR, system and thermal commands.

Look at :
https://github.com/geekboxzone/mmallow_kernel/blob/geekbox/drivers/mailbox/scpi_cmd.h
https://github.com/geekboxzone/mmallow_kernel/blob/geekbox/drivers/mailbox/scpi_protocol.c

So the SCPI must have a framework to allow different protocol versions, and must allow command extension.
Grouping Rockchip and Amlogic should be done, thus needing a generic name like vendor_scpi or with a version.

Sudeep, could you somehow find out which version of the protocol AmLogic and Rockchip based their SCPI development ?


Thanks !
Neil

> Neil
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web