From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754459AbcHSBCC (ORCPT ); Thu, 18 Aug 2016 21:02:02 -0400 Received: from foss.arm.com ([217.140.101.70]:33027 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754316AbcHSBCA (ORCPT ); Thu, 18 Aug 2016 21:02:00 -0400 Subject: Re: [PATCH 02/13] scpi: Add alternative legacy structures and macros To: Neil Armstrong , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <1471515066-3626-1-git-send-email-narmstrong@baylibre.com> <1471515066-3626-3-git-send-email-narmstrong@baylibre.com> Cc: Sudeep Holla , linux-amlogic@lists.infradead.org, khilman@baylibre.com, heiko@sntech.de, wxt@rock-chips.com, frank.wang@rock-chips.com From: Sudeep Holla Organization: ARM Message-ID: <2b345fec-4dbd-2144-cd34-d8451bb314c0@arm.com> Date: Thu, 18 Aug 2016 18:16:34 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1471515066-3626-3-git-send-email-narmstrong@baylibre.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 18/08/16 11:10, Neil Armstrong wrote: > In order to support the legacy SCPI protocol variant, add back the structures > and macros that varies against the final specification. > > Signed-off-by: Neil Armstrong > --- > I checked against Amlogic implementation documentation and on-device, the channel selection > via legacy_scpi_get_chan() is needed and fails without it. > The sender_id is not needed so it was dropped. > > drivers/firmware/arm_scpi.c | 84 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 84 insertions(+) > > diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c > index 403783a..0bb6134 100644 > --- a/drivers/firmware/arm_scpi.c > +++ b/drivers/firmware/arm_scpi.c [...] > @@ -183,6 +224,11 @@ struct scpi_shared_mem { > u8 payload[0]; > } __packed; > > +struct legacy_scpi_shared_mem { > + __le32 status; > + u8 payload[0]; > +} __packed; > + > struct scp_capabilities { > __le32 protocol_version; > __le32 event_version; > @@ -208,6 +254,12 @@ struct clk_set_value { > __le32 rate; > } __packed; > > +struct legacy_clk_set_value { > + __le32 rate; > + __le16 id; > + __le16 reserved; > +} __packed; > + > struct dvfs_info { > __le32 header; > struct { > @@ -237,6 +289,10 @@ struct sensor_value { > __le32 hi_val; > } __packed; > > +struct legacy_sensor_value { > + __le32 val; > +} __packed; > + Not required, new one is backward compatible. Otherwise this patch looks fine. -- Regards, Sudeep From mboxrd@z Thu Jan 1 00:00:00 1970 From: sudeep.holla@arm.com (Sudeep Holla) Date: Thu, 18 Aug 2016 18:16:34 +0100 Subject: [PATCH 02/13] scpi: Add alternative legacy structures and macros In-Reply-To: <1471515066-3626-3-git-send-email-narmstrong@baylibre.com> References: <1471515066-3626-1-git-send-email-narmstrong@baylibre.com> <1471515066-3626-3-git-send-email-narmstrong@baylibre.com> Message-ID: <2b345fec-4dbd-2144-cd34-d8451bb314c0@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 18/08/16 11:10, Neil Armstrong wrote: > In order to support the legacy SCPI protocol variant, add back the structures > and macros that varies against the final specification. > > Signed-off-by: Neil Armstrong > --- > I checked against Amlogic implementation documentation and on-device, the channel selection > via legacy_scpi_get_chan() is needed and fails without it. > The sender_id is not needed so it was dropped. > > drivers/firmware/arm_scpi.c | 84 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 84 insertions(+) > > diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c > index 403783a..0bb6134 100644 > --- a/drivers/firmware/arm_scpi.c > +++ b/drivers/firmware/arm_scpi.c [...] > @@ -183,6 +224,11 @@ struct scpi_shared_mem { > u8 payload[0]; > } __packed; > > +struct legacy_scpi_shared_mem { > + __le32 status; > + u8 payload[0]; > +} __packed; > + > struct scp_capabilities { > __le32 protocol_version; > __le32 event_version; > @@ -208,6 +254,12 @@ struct clk_set_value { > __le32 rate; > } __packed; > > +struct legacy_clk_set_value { > + __le32 rate; > + __le16 id; > + __le16 reserved; > +} __packed; > + > struct dvfs_info { > __le32 header; > struct { > @@ -237,6 +289,10 @@ struct sensor_value { > __le32 hi_val; > } __packed; > > +struct legacy_sensor_value { > + __le32 val; > +} __packed; > + Not required, new one is backward compatible. Otherwise this patch looks fine. -- Regards, Sudeep From mboxrd@z Thu Jan 1 00:00:00 1970 From: sudeep.holla@arm.com (Sudeep Holla) Date: Thu, 18 Aug 2016 18:16:34 +0100 Subject: [PATCH 02/13] scpi: Add alternative legacy structures and macros In-Reply-To: <1471515066-3626-3-git-send-email-narmstrong@baylibre.com> References: <1471515066-3626-1-git-send-email-narmstrong@baylibre.com> <1471515066-3626-3-git-send-email-narmstrong@baylibre.com> Message-ID: <2b345fec-4dbd-2144-cd34-d8451bb314c0@arm.com> To: linus-amlogic@lists.infradead.org List-Id: linus-amlogic.lists.infradead.org On 18/08/16 11:10, Neil Armstrong wrote: > In order to support the legacy SCPI protocol variant, add back the structures > and macros that varies against the final specification. > > Signed-off-by: Neil Armstrong > --- > I checked against Amlogic implementation documentation and on-device, the channel selection > via legacy_scpi_get_chan() is needed and fails without it. > The sender_id is not needed so it was dropped. > > drivers/firmware/arm_scpi.c | 84 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 84 insertions(+) > > diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c > index 403783a..0bb6134 100644 > --- a/drivers/firmware/arm_scpi.c > +++ b/drivers/firmware/arm_scpi.c [...] > @@ -183,6 +224,11 @@ struct scpi_shared_mem { > u8 payload[0]; > } __packed; > > +struct legacy_scpi_shared_mem { > + __le32 status; > + u8 payload[0]; > +} __packed; > + > struct scp_capabilities { > __le32 protocol_version; > __le32 event_version; > @@ -208,6 +254,12 @@ struct clk_set_value { > __le32 rate; > } __packed; > > +struct legacy_clk_set_value { > + __le32 rate; > + __le16 id; > + __le16 reserved; > +} __packed; > + > struct dvfs_info { > __le32 header; > struct { > @@ -237,6 +289,10 @@ struct sensor_value { > __le32 hi_val; > } __packed; > > +struct legacy_sensor_value { > + __le32 val; > +} __packed; > + Not required, new one is backward compatible. Otherwise this patch looks fine. -- Regards, Sudeep