linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: 'Greg KH' <gregkh@linuxfoundation.org>
To: Jolly Shah <jolly.shah@xilinx.com>
Cc: "keescook@chromium.org" <keescook@chromium.org>,
	"ard.biesheuvel@linaro.org" <ard.biesheuvel@linaro.org>,
	"matt@codeblueprint.co.uk" <matt@codeblueprint.co.uk>,
	"dmitry.torokhov@gmail.com" <dmitry.torokhov@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Rajan Vaja <RAJANV@xilinx.com>, Michal Simek <michals@xilinx.com>,
	"sudeep.holla@arm.com" <sudeep.holla@arm.com>,
	"mingo@kernel.org" <mingo@kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"hkallweit1@gmail.com" <hkallweit1@gmail.com>
Subject: Re: [PATCH v2 1/4] firmware: xilinx: Add sysfs interface
Date: Fri, 14 Feb 2020 19:52:35 -0500	[thread overview]
Message-ID: <20200215005235.GA32359@kroah.com> (raw)
In-Reply-To: <c2914eae-bf95-ad51-79a4-07f199f37e27@xilinx.com>

On Fri, Feb 14, 2020 at 04:37:16PM -0800, Jolly Shah wrote:
> > Just make the direct call to the firmware driver, no need to muck around
> > with tables of function pointers.  In fact, with the spectre changes,
> > you just made things slower than needed, and you can get back a bunch of
> > throughput by removing that whole middle layer.
> > 
> 
> arm,scpi is doing the same way and we thought this approach will be more
> acceptable than direct function calls but happy to change as suggested.

Just because one random tiny thing does it the wrong way does not mean
to focus on that design pattern and ignore the thousands of other
apis/interfaces in the kernel that do not do it that way :)

> > So go do that first please, before adding any new stuff.
> > 
> > Now for the ioctl, yeah, that's not a "normal" pattern either.  But
> > right now you only have 2 "different" ioctls that you call.  So why not
> > just turn those 2 into real function calls as well that then makes the
> > "ioctl" call to the hardware?  That makes things a lot more obvious on
> > the kernel driver side exactly what is going on.
> > 
> 
> Sure as i understand firmware driver will provide real function calls to be
> used by user drivers and underneath it will call ioctl for desired
> operation. Please correct if I misunderstood.

You do not misunderstand.

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-02-15  0:58 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-08 23:54 [PATCH v2 0/4] firmware: xilinx: Add xilinx specific sysfs interface Jolly Shah
2020-01-08 23:54 ` [PATCH v2 1/4] firmware: xilinx: Add " Jolly Shah
2020-01-14 14:52   ` Greg KH
2020-01-23 23:47     ` Jolly Shah
2020-01-24  6:03       ` Greg KH
2020-01-24 11:32         ` Sudeep Holla
2020-01-27 22:46           ` Jolly Shah
2020-01-27 23:01         ` Jolly Shah
2020-01-28  6:28           ` Greg KH
2020-01-30 23:59             ` Jolly Shah
2020-01-31  6:10               ` Greg KH
2020-01-31  9:05                 ` Rajan Vaja
2020-01-31  9:36                   ` 'Greg KH'
2020-02-11  0:57                     ` Jolly Shah
2020-02-14 17:10                       ` 'Greg KH'
2020-02-15  0:37                         ` Jolly Shah
2020-02-15  0:52                           ` 'Greg KH' [this message]
2020-02-19 22:50                             ` Jolly Shah
2020-03-06 23:55                             ` Jolly Shah
2020-03-07  8:47                               ` 'Greg KH'
2020-01-08 23:54 ` [PATCH v2 2/4] firmware: xilinx: Add system shutdown API interface Jolly Shah
2020-01-08 23:54 ` [PATCH v2 3/4] firmware: xilinx: Add sysfs to set shutdown scope Jolly Shah
2020-01-08 23:54 ` [PATCH v2 4/4] firmware: xilinx: Add sysfs and IOCTL to set boot health status Jolly Shah

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=20200215005235.GA32359@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=RAJANV@xilinx.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=jolly.shah@xilinx.com \
    --cc=keescook@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt@codeblueprint.co.uk \
    --cc=michals@xilinx.com \
    --cc=mingo@kernel.org \
    --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).