From: Andy Gross <andy.gross@linaro.org>
To: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: lorenzo.pieralisi@arm.com, linux-arm-msm@vger.kernel.org,
will.deacon@arm.com, linux-kernel@vger.kernel.org,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
Kevin Hilman <khilman@baylibre.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [Patch v3 1/2] arm: kernel: Add SMC structure parameter
Date: Thu, 19 Jan 2017 10:16:58 -0600 [thread overview]
Message-ID: <20170119161658.GC9631@hector.attlocal.net> (raw)
In-Reply-To: <20170119154818.GE27312@n2100.armlinux.org.uk>
On Thu, Jan 19, 2017 at 03:48:18PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 19, 2017 at 09:45:06AM -0600, Andy Gross wrote:
> > On Thu, Jan 19, 2017 at 02:40:08PM +0000, Russell King - ARM Linux wrote:
> > > On Wed, Jan 11, 2017 at 04:31:57PM -0600, Andy Gross wrote:
> > > > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> > > > index b5abfda..3e28d08 100644
> > > > --- a/include/linux/arm-smccc.h
> > > > +++ b/include/linux/arm-smccc.h
> > > > @@ -72,19 +72,33 @@ struct arm_smccc_res {
> > > > };
> > > >
> > > > /**
> > > > - * arm_smccc_smc() - make SMC calls
> > > > + * struct arm_smccc_quirk - Contains quirk information
> > > > + * id contains quirk identification
> > > > + * state contains the quirk specific information
> > >
> > > Given that this is a kerneldoc comment, it should really conform to the
> > > kerneldoc requirements - see Documentation/kernel-doc-nano-HOWTO.txt:
> > >
> > > /**
> > > * struct arm_smccc_quirk - Contains quirk information
> > > * @id: quirk identification
> > > * @state: the quirk specific information
> >
> > Ah, I flubbed that. I'll fix this.
> >
> > >
> > > > + */
> > > > +struct arm_smccc_quirk {
> > > > + int id;
> > > > + union {
> > > > + unsigned long a6;
> > > > + } state;
> > > > +};
> > > > +
> > > > +/**
> > > > + * __arm_smccc_smc() - make SMC calls
> > > > * @a0-a7: arguments passed in registers 0 to 7
> > > > * @res: result values from registers 0 to 3
> > > > + * @quirk: optional quirk structure
> > > > *
> > > > * This function is used to make SMC calls following SMC Calling Convention.
> > > > * The content of the supplied param are copied to registers 0 to 7 prior
> > > > * to the SMC instruction. The return values are updated with the content
> > > > - * from register 0 to 3 on return from the SMC instruction.
> > > > + * from register 0 to 3 on return from the SMC instruction. An optional
> > > > + * quirk structure provides vendor specific behavior.
> > >
> > > It's quite odd to have the result buried in the middle of arguments
> > > passed to a function, but I guess for the sake of simplicity in the
> > > assembly code that's what we need.
> > >
> > > Also:
> > >
> > > "@quirk points to an arm_smccc_quirk, or NULL when no quirks are required."
> >
> > Fixed.
> >
> > >
> > > And... should this not be const? Are we expecting anyone to modify
> > > the quirk structure?
> >
> > No, unfortunately. For qcom, we are stuffing the contents of a6 into the
> > structure so the caller can get that information.
>
> Please add that detail to the kerneldoc comments for the structure
> concerning state.a6. It's important that such information is not
> lost in the depths of history.
For documentation, should I split out the union separately? I don't see any
good examples for documenting union members in unions defined inside structures.
Regards,
Andy
next prev parent reply other threads:[~2017-01-19 16:41 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-11 22:31 [Patch v3 0/2] Support ARM SMCC SoC vendor quirks Andy Gross
2017-01-11 22:31 ` [Patch v3 1/2] arm: kernel: Add SMC structure parameter Andy Gross
2017-01-19 14:40 ` Russell King - ARM Linux
2017-01-19 15:45 ` Andy Gross
2017-01-19 15:48 ` Russell King - ARM Linux
2017-01-19 16:16 ` Andy Gross [this message]
2017-01-19 16:24 ` Russell King - ARM Linux
2017-01-11 22:31 ` [Patch v3 2/2] firmware: qcom: scm: Fix interrupted SCM calls Andy Gross
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=20170119161658.GC9631@hector.attlocal.net \
--to=andy.gross@linaro.org \
--cc=bjorn.andersson@linaro.org \
--cc=khilman@baylibre.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=lorenzo.pieralisi@arm.com \
--cc=srinivas.kandagatla@linaro.org \
--cc=will.deacon@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).