From: Cristian Marussi <cristian.marussi@arm.com>
To: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Cc: linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, james.quinlan@broadcom.com,
lukasz.luba@arm.com, sudeep.holla@arm.com
Subject: Re: [RFC PATCH 01/11] firmware: arm_scmi: Add receive buffer support for notifications
Date: Fri, 14 Feb 2020 15:25:57 +0000 [thread overview]
Message-ID: <97365c4e-3672-64ec-50c7-28dc839a2d35@arm.com> (raw)
In-Reply-To: <20200127170713.000013ee@Huawei.com>
Hi
On 27/01/2020 17:07, Jonathan Cameron wrote:
> On Mon, 20 Jan 2020 12:23:23 +0000
> Cristian Marussi <cristian.marussi@arm.com> wrote:
>
>> From: Sudeep Holla <sudeep.holla@arm.com>
>>
>> With all the plumbing in place, let's just add the separate dedicated
>> receive buffers to handle notifications that can arrive asynchronously
>> from the platform firmware to OS.
>>
>> Also add check to see if the platform supports any receive channels
>> before allocating the receive buffers.
>
> Perhaps hand hold the reader a tiny bit more by saying that we need
> to move the initialization later so that we can know *if* the receive
> channels are supported. Took me a moment to figure out why you did that ;)
>
Addressed in v2.
> One minor suggestion inline.
>
>>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>> drivers/firmware/arm_scmi/driver.c | 24 ++++++++++++++++++------
>> 1 file changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
>> index 2c96f6b5a7d8..9611e8037d77 100644
>> --- a/drivers/firmware/arm_scmi/driver.c
>> +++ b/drivers/firmware/arm_scmi/driver.c
>> @@ -123,6 +123,7 @@ struct scmi_chan_info {
>> * @version: SCMI revision information containing protocol version,
>> * implementation version and (sub-)vendor identification.
>> * @tx_minfo: Universal Transmit Message management info
>> + * @rx_minfo: Universal Receive Message management info
>> * @tx_idr: IDR object to map protocol id to Tx channel info pointer
>> * @rx_idr: IDR object to map protocol id to Rx channel info pointer
>> * @protocols_imp: List of protocols implemented, currently maximum of
>> @@ -136,6 +137,7 @@ struct scmi_info {
>> struct scmi_revision_info version;
>> struct scmi_handle handle;
>> struct scmi_xfers_info tx_minfo;
>> + struct scmi_xfers_info rx_minfo;
>> struct idr tx_idr;
>> struct idr rx_idr;
>> u8 *protocols_imp;
>> @@ -690,13 +692,13 @@ int scmi_handle_put(const struct scmi_handle *handle)
>> return 0;
>> }
>>
>> -static int scmi_xfer_info_init(struct scmi_info *sinfo)
>> +static int __scmi_xfer_info_init(struct scmi_info *sinfo, bool tx)
>> {
>> int i;
>> struct scmi_xfer *xfer;
>> struct device *dev = sinfo->dev;
>> const struct scmi_desc *desc = sinfo->desc;
>> - struct scmi_xfers_info *info = &sinfo->tx_minfo;
>> + struct scmi_xfers_info *info = tx ? &sinfo->tx_minfo : &sinfo->rx_minfo;
>
> Perhaps cleaner to just pass in the relevant info structure rather than a boolean
> to pick it. Saves people having to check if the boolean is saying it's
> tx or rx when reading the call sites.
>
Done in the upcoming v2.
Regards
Cristian
>>
>> /* Pre-allocated messages, no more than what hdr.seq can support */
>> if (WARN_ON(desc->max_msg >= MSG_TOKEN_MAX)) {
>> @@ -731,6 +733,16 @@ static int scmi_xfer_info_init(struct scmi_info *sinfo)
>> return 0;
>> }
>>
>> +static int scmi_xfer_info_init(struct scmi_info *sinfo)
>> +{
>> + int ret = __scmi_xfer_info_init(sinfo, true);
>> +
>> + if (!ret && idr_find(&sinfo->rx_idr, SCMI_PROTOCOL_BASE))
>> + ret = __scmi_xfer_info_init(sinfo, false);
>> +
>> + return ret;
>> +}
>> +
>> static int scmi_mailbox_check(struct device_node *np, int idx)
>> {
>> return of_parse_phandle_with_args(np, "mboxes", "#mbox-cells",
>> @@ -908,10 +920,6 @@ static int scmi_probe(struct platform_device *pdev)
>> info->desc = desc;
>> INIT_LIST_HEAD(&info->node);
>>
>> - ret = scmi_xfer_info_init(info);
>> - if (ret)
>> - return ret;
>> -
>> platform_set_drvdata(pdev, info);
>> idr_init(&info->tx_idr);
>> idr_init(&info->rx_idr);
>> @@ -924,6 +932,10 @@ static int scmi_probe(struct platform_device *pdev)
>> if (ret)
>> return ret;
>>
>> + ret = scmi_xfer_info_init(info);
>> + if (ret)
>> + return ret;
>> +
>> ret = scmi_base_protocol_init(handle);
>> if (ret) {
>> dev_err(dev, "unable to communicate with SCMI(%d)\n", ret);
>
>
next prev parent reply other threads:[~2020-02-14 15:26 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-20 12:23 [RFC PATCH 00/11] SCMI Notifications Support Cristian Marussi
2020-01-20 12:23 ` [RFC PATCH 01/11] firmware: arm_scmi: Add receive buffer support for notifications Cristian Marussi
2020-01-27 17:07 ` Jonathan Cameron
2020-02-14 15:25 ` Cristian Marussi [this message]
2020-01-20 12:23 ` [RFC PATCH 02/11] firmware: arm_scmi: Update protocol commands and notification list Cristian Marussi
2020-01-20 12:23 ` [RFC PATCH 03/11] firmware: arm_scmi: Add support for notifications message processing Cristian Marussi
[not found] ` <4c59008e-6010-fb98-d7bf-8677454d1e4f@broadcom.com>
2020-01-23 10:58 ` Cristian Marussi
2020-01-27 17:32 ` Jonathan Cameron
2020-02-14 15:28 ` Cristian Marussi
2020-01-20 12:23 ` [RFC PATCH 04/11] firmware: arm_scmi: Add core notifications support Cristian Marussi
2020-01-21 17:43 ` Cristian Marussi
2020-01-27 18:11 ` Jonathan Cameron
2020-01-27 18:52 ` Cristian Marussi
2020-02-14 15:32 ` Cristian Marussi
2020-01-20 12:23 ` [RFC PATCH 05/11] firmware: arm_scmi: Add notifications anti-tampering Cristian Marussi
2020-01-20 12:23 ` [RFC PATCH 06/11] firmware: arm_scmi: Enable core notifications Cristian Marussi
2020-01-20 12:23 ` [RFC PATCH 07/11] firmware: arm_scmi: Add Power notifications support Cristian Marussi
2020-01-20 12:23 ` [RFC PATCH 08/11] firmware: arm_scmi: Add Perf " Cristian Marussi
2020-01-20 12:23 ` [RFC PATCH 09/11] firmware: arm_scmi: Add Sensor " Cristian Marussi
2020-01-20 12:23 ` [RFC PATCH 10/11] firmware: arm_scmi: Add Reset " Cristian Marussi
2020-01-20 12:23 ` [RFC PATCH 11/11] firmware: arm_scmi: Add Base " Cristian Marussi
2020-01-23 11:02 ` [RFC PATCH 00/11] SCMI Notifications Support Cristian Marussi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=97365c4e-3672-64ec-50c7-28dc839a2d35@arm.com \
--to=cristian.marussi@arm.com \
--cc=Jonathan.Cameron@Huawei.com \
--cc=james.quinlan@broadcom.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lukasz.luba@arm.com \
--cc=sudeep.holla@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).