From: "Jon Medhurst (Tixy)" <tixy@linaro.org> To: Sudeep Holla <sudeep.holla@arm.com> Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Liviu Dudau <Liviu.Dudau@arm.com>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Jassi Brar <jassisinghbrar@gmail.com> Subject: Re: [PATCH v2 2/5] firmware: add support for ARM System Control and Power Interface(SCPI) protocol Date: Wed, 20 May 2015 12:17:16 +0100 [thread overview] Message-ID: <1432120636.3231.24.camel@linaro.org> (raw) In-Reply-To: <1431618155-3132-3-git-send-email-sudeep.holla@arm.com> On Thu, 2015-05-14 at 16:42 +0100, Sudeep Holla wrote: > This patch adds support for System Control and Power Interface (SCPI) > Message Protocol used between the Application Cores(AP) and the System > Control Processor(SCP). The MHU peripheral provides a mechanism for > inter-processor communication between SCP's M3 processor 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. > > This protocol driver provides interface for all the client drivers using > SCPI to make use of the features offered by the SCP. > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > CC: Jassi Brar <jassisinghbrar@gmail.com> > Cc: Liviu Dudau <Liviu.Dudau@arm.com> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Cc: Jon Medhurst (Tixy) <tixy@linaro.org> > --- Sorry for the delay in looking at this. I have one nitpick below but anyway, here's a Reviewed-by: Jon Medhurst (Tixy) <tixy@linaro.org> [...] > +++ b/drivers/firmware/arm_scpi.c [...] > +static void scpi_process_cmd(struct scpi_chan *ch, u32 cmd) > +{ > + unsigned long flags; > + struct scpi_xfer *t, *match = NULL; > + > + spin_lock_irqsave(&ch->rx_lock, flags); > + if (list_empty(&ch->rx_pending)) { > + spin_unlock_irqrestore(&ch->rx_lock, flags); > + return; > + } > + > + list_for_each_entry(t, &ch->rx_pending, node) > + if (CMD_XTRACT_UNIQ(t->cmd) == CMD_XTRACT_UNIQ(cmd)) { > + list_del(&t->node); > + match = t; > + break; > + } > + /* check if wait_for_completion is in progress or timed-out */ > + if (match && !completion_done(&match->done)) { > + struct scpi_shared_mem *mem = ch->rx_payload; > + int len = min(match->rx_len, CMD_SIZE(cmd)); > + > + match->status = le32_to_cpu(mem->status); > + memcpy_fromio(match->rx_buf, mem->payload, len); > + if (match->rx_len > len) rx_len is unsigned and len is signed and so I had to go refresh my memory from the C standard before I could convince myself that this if statement was OK. Might be clearer if len was unsigned, especially as it's the 'min' of two unsigned values. -- Tixy
WARNING: multiple messages have this Message-ID (diff)
From: tixy@linaro.org (Jon Medhurst (Tixy)) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 2/5] firmware: add support for ARM System Control and Power Interface(SCPI) protocol Date: Wed, 20 May 2015 12:17:16 +0100 [thread overview] Message-ID: <1432120636.3231.24.camel@linaro.org> (raw) In-Reply-To: <1431618155-3132-3-git-send-email-sudeep.holla@arm.com> On Thu, 2015-05-14 at 16:42 +0100, Sudeep Holla wrote: > This patch adds support for System Control and Power Interface (SCPI) > Message Protocol used between the Application Cores(AP) and the System > Control Processor(SCP). The MHU peripheral provides a mechanism for > inter-processor communication between SCP's M3 processor 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. > > This protocol driver provides interface for all the client drivers using > SCPI to make use of the features offered by the SCP. > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > CC: Jassi Brar <jassisinghbrar@gmail.com> > Cc: Liviu Dudau <Liviu.Dudau@arm.com> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Cc: Jon Medhurst (Tixy) <tixy@linaro.org> > --- Sorry for the delay in looking at this. I have one nitpick below but anyway, here's a Reviewed-by: Jon Medhurst (Tixy) <tixy@linaro.org> [...] > +++ b/drivers/firmware/arm_scpi.c [...] > +static void scpi_process_cmd(struct scpi_chan *ch, u32 cmd) > +{ > + unsigned long flags; > + struct scpi_xfer *t, *match = NULL; > + > + spin_lock_irqsave(&ch->rx_lock, flags); > + if (list_empty(&ch->rx_pending)) { > + spin_unlock_irqrestore(&ch->rx_lock, flags); > + return; > + } > + > + list_for_each_entry(t, &ch->rx_pending, node) > + if (CMD_XTRACT_UNIQ(t->cmd) == CMD_XTRACT_UNIQ(cmd)) { > + list_del(&t->node); > + match = t; > + break; > + } > + /* check if wait_for_completion is in progress or timed-out */ > + if (match && !completion_done(&match->done)) { > + struct scpi_shared_mem *mem = ch->rx_payload; > + int len = min(match->rx_len, CMD_SIZE(cmd)); > + > + match->status = le32_to_cpu(mem->status); > + memcpy_fromio(match->rx_buf, mem->payload, len); > + if (match->rx_len > len) rx_len is unsigned and len is signed and so I had to go refresh my memory from the C standard before I could convince myself that this if statement was OK. Might be clearer if len was unsigned, especially as it's the 'min' of two unsigned values. -- Tixy
next prev parent reply other threads:[~2015-05-20 11:17 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-05-14 15:42 [PATCH v2 0/5] ARM64: juno: add SCPI mailbox protocol, clock and CPUFreq support Sudeep Holla 2015-05-14 15:42 ` Sudeep Holla 2015-05-14 15:42 ` [PATCH v2 1/5] Documentation: add DT binding for ARM System Control and Power Interface(SCPI) protocol Sudeep Holla 2015-05-14 15:42 ` Sudeep Holla 2015-05-14 15:42 ` [PATCH v2 2/5] firmware: add support " Sudeep Holla 2015-05-14 15:42 ` Sudeep Holla 2015-05-20 11:17 ` Jon Medhurst (Tixy) [this message] 2015-05-20 11:17 ` Jon Medhurst (Tixy) 2015-05-26 11:18 ` Sudeep Holla 2015-05-26 11:18 ` Sudeep Holla 2015-05-26 11:18 ` Sudeep Holla 2015-05-26 11:18 ` Sudeep Holla 2015-05-14 15:42 ` [PATCH v2 3/5] clk: add support for clocks provided by SCP(System Control Processor) Sudeep Holla 2015-05-14 15:42 ` Sudeep Holla 2015-05-14 15:42 ` [PATCH v2 4/5] clk: scpi: add support for cpufreq virtual device Sudeep Holla 2015-05-14 15:42 ` Sudeep Holla 2015-05-14 15:42 ` [PATCH v2 5/5] cpufreq: arm_big_little: add SCPI interface driver Sudeep Holla 2015-05-14 15:42 ` Sudeep Holla 2015-05-14 22:03 ` Rafael J. Wysocki 2015-05-14 22:03 ` Rafael J. Wysocki 2015-05-15 5:39 ` Viresh Kumar 2015-05-15 5:39 ` Viresh Kumar
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=1432120636.3231.24.camel@linaro.org \ --to=tixy@linaro.org \ --cc=Liviu.Dudau@arm.com \ --cc=jassisinghbrar@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=lorenzo.pieralisi@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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.