From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753919AbdASQlW (ORCPT ); Thu, 19 Jan 2017 11:41:22 -0500 Received: from mail-oi0-f53.google.com ([209.85.218.53]:33131 "EHLO mail-oi0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753888AbdASQlG (ORCPT ); Thu, 19 Jan 2017 11:41:06 -0500 Date: Thu, 19 Jan 2017 10:16:58 -0600 From: Andy Gross To: Russell King - ARM Linux Cc: lorenzo.pieralisi@arm.com, linux-arm-msm@vger.kernel.org, will.deacon@arm.com, linux-kernel@vger.kernel.org, Bjorn Andersson , Srinivas Kandagatla , Kevin Hilman , linux-arm-kernel@lists.infradead.org Subject: Re: [Patch v3 1/2] arm: kernel: Add SMC structure parameter Message-ID: <20170119161658.GC9631@hector.attlocal.net> References: <1484173918-25090-1-git-send-email-andy.gross@linaro.org> <1484173918-25090-2-git-send-email-andy.gross@linaro.org> <20170119144008.GA27312@n2100.armlinux.org.uk> <20170119154506.GB9631@hector.attlocal.net> <20170119154818.GE27312@n2100.armlinux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170119154818.GE27312@n2100.armlinux.org.uk> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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