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

Groups > linux.kernel > #1468518 > unrolled thread

[PATCH v2 0/7] scpi: Add support for legacy SCPI protocol

Started byNeil Armstrong <narmstrong@baylibre.com>
First post2016-08-23 14:00 +0200
Last post2016-08-25 18:50 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/7] scpi: Add support for legacy SCPI protocol Neil Armstrong <narmstrong@baylibre.com> - 2016-08-23 14:00 +0200
    Re: [PATCH v2 0/7] scpi: Add support for legacy SCPI protocol Neil Armstrong <narmstrong@baylibre.com> - 2016-08-25 15:30 +0200
      Re: [PATCH v2 0/7] scpi: Add support for legacy SCPI protocol Sudeep Holla <sudeep.holla@arm.com> - 2016-08-25 15:50 +0200
        Re: [PATCH v2 0/7] scpi: Add support for legacy SCPI protocol Sudeep Holla <sudeep.holla@arm.com> - 2016-08-25 18:50 +0200

#1468518 — [PATCH v2 0/7] scpi: Add support for legacy SCPI protocol

FromNeil Armstrong <narmstrong@baylibre.com>
Date2016-08-23 14:00 +0200
Subject[PATCH v2 0/7] scpi: Add support for legacy SCPI protocol
Message-ID<s9inn-3ui-3@gated-at.bofh.it>
This patchset aims to support the legacy SCPI firmware implementation that was
delivered as early technology preview for the JUNO platform.

Finally a stable, maintained and public implementation for the SCPI protocol
has been upstreamed part of the JUNO support and it is the recommended way
of implementing SCP communication on ARMv8 platforms.

The Amlogic GXBB platform is using this legacy protocol, as the RK3368 & RK3399
platforms. This patchset will only add support for Amlogic GXBB SoC.

This patchset add support for the legacy protocol in the arm_scpi.c file,
avoiding code duplication.

Last RFC discution tread can be found at : https://lkml.org/lkml/2016/8/9/210

The last patch depends on the "Platform MHU" dtsi patch.

Changes since v1 at : http://lkml.kernel.org/r/1471515066-3626-1-git-send-email-narmstrong@baylibre.com
 - Dropped vendor_send_message and rockchip vendor mechanism patches
 - Merged alternate functions into main functions using is_legacy boolean
 - Added DT match table to set is_legacy to true
 - Kept alternate scpi_ops structure for legacy

Neil Armstrong (7):
  scpi: Add alternative legacy structures, functions and macros
  scpi: Use legacy variants command index calling scpi_send_message
  scpi: Add support for Legacy match table for Amlogic GXBB SoC
  scpi: grow MAX_DVFS_OPPS to 16 entries
  dt-bindings: Add support for Amlogic GXBB SCPI Interface
  ARM64: dts: meson-gxbb: Add SRAM node
  ARM64: dts: meson-gxbb: Add SCPI with cpufreq & sensors Nodes

 Documentation/devicetree/bindings/arm/arm,scpi.txt |   8 +-
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |  45 ++++
 drivers/firmware/arm_scpi.c                        | 279 +++++++++++++++++++--
 3 files changed, 302 insertions(+), 30 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1470139

FromNeil Armstrong <narmstrong@baylibre.com>
Date2016-08-25 15:30 +0200
Message-ID<sa2Tf-1tU-3@gated-at.bofh.it>
In reply to#1468518
On 08/23/2016 01:46 PM, Neil Armstrong wrote:
> This patchset aims to support the legacy SCPI firmware implementation that was
> delivered as early technology preview for the JUNO platform.
> 
> Finally a stable, maintained and public implementation for the SCPI protocol
> has been upstreamed part of the JUNO support and it is the recommended way
> of implementing SCP communication on ARMv8 platforms.
> 
> The Amlogic GXBB platform is using this legacy protocol, as the RK3368 & RK3399
> platforms. This patchset will only add support for Amlogic GXBB SoC.
> 
> This patchset add support for the legacy protocol in the arm_scpi.c file,
> avoiding code duplication.
> 
> Last RFC discution tread can be found at : https://lkml.org/lkml/2016/8/9/210
> 
> The last patch depends on the "Platform MHU" dtsi patch.
> 
> Changes since v1 at : http://lkml.kernel.org/r/1471515066-3626-1-git-send-email-narmstrong@baylibre.com
>  - Dropped vendor_send_message and rockchip vendor mechanism patches
>  - Merged alternate functions into main functions using is_legacy boolean
>  - Added DT match table to set is_legacy to true
>  - Kept alternate scpi_ops structure for legacy
> 
> Neil Armstrong (7):
>   scpi: Add alternative legacy structures, functions and macros
>   scpi: Use legacy variants command index calling scpi_send_message
>   scpi: Add support for Legacy match table for Amlogic GXBB SoC
>   scpi: grow MAX_DVFS_OPPS to 16 entries
>   dt-bindings: Add support for Amlogic GXBB SCPI Interface
>   ARM64: dts: meson-gxbb: Add SRAM node
>   ARM64: dts: meson-gxbb: Add SCPI with cpufreq & sensors Nodes
> 
>  Documentation/devicetree/bindings/arm/arm,scpi.txt |   8 +-
>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |  45 ++++
>  drivers/firmware/arm_scpi.c                        | 279 +++++++++++++++++++--
>  3 files changed, 302 insertions(+), 30 deletions(-)
> 

Hi Sudeep,

Sorry but I posted this V2 before you had time to look at my previous v1 replies...

In this serie, I merged the scpi_send_message, but I must still evaluate how it's possible
to use the list to queue commands.

Here I used if(is_legacy) to stop duplicating functions, is this ok for you ?

Thanks,
Neil

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

#1470162

FromSudeep Holla <sudeep.holla@arm.com>
Date2016-08-25 15:50 +0200
Message-ID<sa3cC-1AH-27@gated-at.bofh.it>
In reply to#1470139

On 25/08/16 14:18, Neil Armstrong wrote:
> On 08/23/2016 01:46 PM, Neil Armstrong wrote:
>> This patchset aims to support the legacy SCPI firmware implementation that was
>> delivered as early technology preview for the JUNO platform.
>>
>> Finally a stable, maintained and public implementation for the SCPI protocol
>> has been upstreamed part of the JUNO support and it is the recommended way
>> of implementing SCP communication on ARMv8 platforms.
>>
>> The Amlogic GXBB platform is using this legacy protocol, as the RK3368 & RK3399
>> platforms. This patchset will only add support for Amlogic GXBB SoC.
>>
>> This patchset add support for the legacy protocol in the arm_scpi.c file,
>> avoiding code duplication.
>>
>> Last RFC discution tread can be found at : https://lkml.org/lkml/2016/8/9/210
>>
>> The last patch depends on the "Platform MHU" dtsi patch.
>>
>> Changes since v1 at : http://lkml.kernel.org/r/1471515066-3626-1-git-send-email-narmstrong@baylibre.com
>>  - Dropped vendor_send_message and rockchip vendor mechanism patches
>>  - Merged alternate functions into main functions using is_legacy boolean
>>  - Added DT match table to set is_legacy to true
>>  - Kept alternate scpi_ops structure for legacy
>>
>> Neil Armstrong (7):
>>   scpi: Add alternative legacy structures, functions and macros
>>   scpi: Use legacy variants command index calling scpi_send_message
>>   scpi: Add support for Legacy match table for Amlogic GXBB SoC
>>   scpi: grow MAX_DVFS_OPPS to 16 entries
>>   dt-bindings: Add support for Amlogic GXBB SCPI Interface
>>   ARM64: dts: meson-gxbb: Add SRAM node
>>   ARM64: dts: meson-gxbb: Add SCPI with cpufreq & sensors Nodes
>>
>>  Documentation/devicetree/bindings/arm/arm,scpi.txt |   8 +-
>>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |  45 ++++
>>  drivers/firmware/arm_scpi.c                        | 279 +++++++++++++++++++--
>>  3 files changed, 302 insertions(+), 30 deletions(-)
>>
>
> Hi Sudeep,
>
> Sorry but I posted this V2 before you had time to look at my
> previous  v1 replies...
>

That's fine.

> In this series, I merged the scpi_send_message, but I must still
> evaluate how it's possible  to use the list to queue commands.
>

Ah OK.

> Here I used if(is_legacy) to stop duplicating functions, is this ok
> for you ?
>

I am still thinking if it can be abstracted well, some kind of mapping
but haven't thought too much about that yet. Also I was thinking about
bitmap for high priority commands. I remember doing something before but
seem to have lost that copy. I will try to dig it out..

-- 
Regards,
Sudeep

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

#1470275

FromSudeep Holla <sudeep.holla@arm.com>
Date2016-08-25 18:50 +0200
Message-ID<sa60O-3vb-11@gated-at.bofh.it>
In reply to#1470162

On 25/08/16 14:45, Sudeep Holla wrote:
>
>
> On 25/08/16 14:18, Neil Armstrong wrote:

[...]

>> Here I used if(is_legacy) to stop duplicating functions, is this ok
>> for you ?
>>
>
> I am still thinking if it can be abstracted well, some kind of mapping
> but haven't thought too much about that yet. Also I was thinking about
> bitmap for high priority commands. I remember doing something before but
> seem to have lost that copy. I will try to dig it out..
>

OK how about something like:

1. in struct scpi_drvinfo
	DECLARE_BITMAP(cmd_priority, SCPI_CMD_COUNT);

2. scpi_send_message
	scpi_chan = test_bit(cmd, scpi_info->cmd_priority) ?
		    scpi_info->channels + 1 : scpi_info->channels;

3. probe
	for (idx = 0; idx < ARRAY_SIZE(hpriority_cmds); idx++)
		set_bit(hpriority_cmds[idx], scpi_info->cmd_priority);

For commands, I am thinking some kind of indirection like the below
patch. See if that helps, I will add the description later, but you can
build your patches on top of it if you think that works and keeps code
simple.

Regards,
Sudeep


-->8

 From afde0d2f1d3381d443445301ab5ec111276934e5 Mon Sep 17 00:00:00 2001
From: Sudeep Holla <sudeep.holla@arm.com>
Date: Thu, 25 Aug 2016 17:21:49 +0100
Subject: [PATCH] firmware: arm_scpi: create command indirection to support
  legacy commands

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
  drivers/firmware/arm_scpi.c | 68 
+++++++++++++++++++++++++++++++++------------
  1 file changed, 51 insertions(+), 17 deletions(-)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index 438893762076..c2063cb76b08 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -129,7 +129,39 @@ enum scpi_std_cmd {
  	SCPI_CMD_SENSOR_ASYNC_VALUE	= 0x1a,
  	SCPI_CMD_SET_DEVICE_PWR_STATE	= 0x1b,
  	SCPI_CMD_GET_DEVICE_PWR_STATE	= 0x1c,
-	SCPI_CMD_COUNT
+};
+
+enum scpi_drv_cmd {
+	SCPI_CAPABILITIES	= 0,
+	GET_DVFS_INFO		= 1,
+	SET_DVFS		= 2,
+	GET_DVFS		= 3,
+	GET_CLOCK_INFO		= 4,
+	SET_CLOCK_VALUE		= 5,
+	GET_CLOCK_VALUE		= 6,
+	PSU_CAPABILITIES	= 7,
+	SENSOR_CAPABILITIES	= 8,
+	SENSOR_INFO		= 9,
+	SENSOR_VALUE		= 10,
+	SET_DEV_PWR_STATE	= 11,
+	GET_DEV_PWR_STATE	= 12,
+	CMD_MAX_COUNT
+};
+
+static int scpi_std_commands[CMD_MAX_COUNT] = {
+	SCPI_CMD_SCPI_CAPABILITIES,
+	SCPI_CMD_GET_DVFS_INFO,
+	SCPI_CMD_SET_DVFS,
+	SCPI_CMD_GET_DVFS,
+	SCPI_CMD_GET_CLOCK_INFO,
+	SCPI_CMD_SET_CLOCK_VALUE,
+	SCPI_CMD_GET_CLOCK_VALUE,
+	SCPI_CMD_PSU_CAPABILITIES,
+	SCPI_CMD_SENSOR_CAPABILITIES,
+	SCPI_CMD_SENSOR_INFO,
+	SCPI_CMD_SENSOR_VALUE,
+	SCPI_CMD_SET_DEVICE_PWR_STATE,
+	SCPI_CMD_GET_DEVICE_PWR_STATE,
  };
   struct scpi_xfer {
@@ -161,6 +193,7 @@ struct scpi_drvinfo {
  	u32 protocol_version;
  	u32 firmware_version;
  	int num_chans;
+	int *cmds;
  	atomic_t next_chan;
  	struct scpi_ops *scpi_ops;
  	struct scpi_chan *channels;
@@ -397,7 +430,7 @@ scpi_clk_get_range(u16 clk_id, unsigned long *min, 
unsigned long *max)
  	struct clk_get_info clk;
  	__le16 le_clk_id = cpu_to_le16(clk_id);
  -	ret = scpi_send_message(SCPI_CMD_GET_CLOCK_INFO, &le_clk_id,
+	ret = scpi_send_message(scpi_info->cmds[GET_CLOCK_INFO], &le_clk_id,
  				sizeof(le_clk_id), &clk, sizeof(clk));
  	if (!ret) {
  		*min = le32_to_cpu(clk.min_rate);
@@ -412,7 +445,7 @@ static unsigned long scpi_clk_get_val(u16 clk_id)
  	struct clk_get_value clk;
  	__le16 le_clk_id = cpu_to_le16(clk_id);
  -	ret = scpi_send_message(SCPI_CMD_GET_CLOCK_VALUE, &le_clk_id,
+	ret = scpi_send_message(scpi_info->cmds[GET_CLOCK_VALUE], &le_clk_id,
  				sizeof(le_clk_id), &clk, sizeof(clk));
  	return ret ? ret : le32_to_cpu(clk.rate);
  }
@@ -425,8 +458,8 @@ static int scpi_clk_set_val(u16 clk_id, unsigned 
long rate)
  		.rate = cpu_to_le32(rate)
  	};
  -	return scpi_send_message(SCPI_CMD_SET_CLOCK_VALUE, &clk, sizeof(clk),
-				 &stat, sizeof(stat));
+	return scpi_send_message(scpi_info->cmds[SET_CLOCK_VALUE], &clk,
+				 sizeof(clk), &stat, sizeof(stat));
  }
   static int scpi_dvfs_get_idx(u8 domain)
@@ -434,8 +467,8 @@ static int scpi_dvfs_get_idx(u8 domain)
  	int ret;
  	u8 dvfs_idx;
  -	ret = scpi_send_message(SCPI_CMD_GET_DVFS, &domain, sizeof(domain),
-				&dvfs_idx, sizeof(dvfs_idx));
+	ret = scpi_send_message(scpi_info->cmds[GET_DVFS], &domain,
+				sizeof(domain),	&dvfs_idx, sizeof(dvfs_idx));
  	return ret ? ret : dvfs_idx;
  }
  @@ -444,7 +477,7 @@ static int scpi_dvfs_set_idx(u8 domain, u8 index)
  	int stat;
  	struct dvfs_set dvfs = {domain, index};
  -	return scpi_send_message(SCPI_CMD_SET_DVFS, &dvfs, sizeof(dvfs),
+	return scpi_send_message(scpi_info->cmds[SET_DVFS], &dvfs, sizeof(dvfs),
  				 &stat, sizeof(stat));
  }
  @@ -468,8 +501,8 @@ static struct scpi_dvfs_info 
*scpi_dvfs_get_info(u8 domain)
  	if (scpi_info->dvfs[domain])	/* data already populated */
  		return scpi_info->dvfs[domain];
  -	ret = scpi_send_message(SCPI_CMD_GET_DVFS_INFO, &domain, sizeof(domain),
-				&buf, sizeof(buf));
+	ret = scpi_send_message(scpi_info->cmds[GET_DVFS_INFO], &domain,
+				sizeof(domain),	&buf, sizeof(buf));
   	if (ret)
  		return ERR_PTR(ret);
@@ -503,8 +536,8 @@ static int scpi_sensor_get_capability(u16 *sensors)
  	struct sensor_capabilities cap_buf;
  	int ret;
  -	ret = scpi_send_message(SCPI_CMD_SENSOR_CAPABILITIES, NULL, 0, &cap_buf,
-				sizeof(cap_buf));
+	ret = scpi_send_message(scpi_info->cmds[SENSOR_CAPABILITIES], NULL, 0,
+				&cap_buf, sizeof(cap_buf));
  	if (!ret)
  		*sensors = le16_to_cpu(cap_buf.sensors);
  @@ -517,7 +550,7 @@ static int scpi_sensor_get_info(u16 sensor_id, 
struct scpi_sensor_info *info)
  	struct _scpi_sensor_info _info;
  	int ret;
  -	ret = scpi_send_message(SCPI_CMD_SENSOR_INFO, &id, sizeof(id),
+	ret = scpi_send_message(scpi_info->cmds[SENSOR_INFO], &id, sizeof(id),
  				&_info, sizeof(_info));
  	if (!ret) {
  		memcpy(info, &_info, sizeof(*info));
@@ -533,7 +566,7 @@ static int scpi_sensor_get_value(u16 sensor, u64 *val)
  	struct sensor_value buf;
  	int ret;
  -	ret = scpi_send_message(SCPI_CMD_SENSOR_VALUE, &id, sizeof(id),
+	ret = scpi_send_message(scpi_info->cmds[SENSOR_VALUE], &id, sizeof(id),
  				&buf, sizeof(buf));
  	if (!ret)
  		*val = (u64)le32_to_cpu(buf.hi_val) << 32 |
@@ -548,7 +581,7 @@ static int scpi_device_get_power_state(u16 dev_id)
  	u8 pstate;
  	__le16 id = cpu_to_le16(dev_id);
  -	ret = scpi_send_message(SCPI_CMD_GET_DEVICE_PWR_STATE, &id,
+	ret = scpi_send_message(scpi_info->cmds[GET_DEV_PWR_STATE], &id,
  				sizeof(id), &pstate, sizeof(pstate));
  	return ret ? ret : pstate;
  }
@@ -561,7 +594,7 @@ static int scpi_device_set_power_state(u16 dev_id, 
u8 pstate)
  		.pstate = pstate,
  	};
  -	return scpi_send_message(SCPI_CMD_SET_DEVICE_PWR_STATE, &dev_set,
+	return scpi_send_message(scpi_info->cmds[SET_DEV_PWR_STATE], &dev_set,
  				 sizeof(dev_set), &stat, sizeof(stat));
  }
  @@ -591,7 +624,7 @@ static int scpi_init_versions(struct scpi_drvinfo 
*info)
  	int ret;
  	struct scp_capabilities caps;
  -	ret = scpi_send_message(SCPI_CMD_SCPI_CAPABILITIES, NULL, 0,
+	ret = scpi_send_message(info->cmds[SCPI_CAPABILITIES], NULL, 0,
  				&caps, sizeof(caps));
  	if (!ret) {
  		info->protocol_version = le32_to_cpu(caps.protocol_version);
@@ -754,6 +787,7 @@ static int scpi_probe(struct platform_device *pdev)
   	scpi_info->channels = scpi_chan;
  	scpi_info->num_chans = count;
+	scpi_info->cmds = scpi_std_commands;
  	platform_set_drvdata(pdev, scpi_info);
   	ret = scpi_init_versions(scpi_info);
-- 
2.7.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web