Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Luis Chamberlain <mcgrof@kernel.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Scott Branden <scott.branden@broadcom.com>,
	David Brown <david.brown@linaro.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Shuah Khan <shuah@kernel.org>,
	bjorn.andersson@linaro.org,
	Shuah Khan <skhan@linuxfoundation.org>,
	Arnd Bergmann <arnd@arndb.de>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	BCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>,
	Olof Johansson <olof@lixom.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Colin Ian King <colin.king@canonical.com>,
	Kees Cook <keescook@chromium.org>, Takashi Iwai <tiwai@suse.de>,
	linux-kselftest@vger.kernel.org, Andy Gross <agross@kernel.org>,
	Desmond Yan <desmond.yan@broadcom.com>,
	James Hu <james.hu@broadcom.com>
Subject: Re: [PATCH v5 6/7] misc: bcm-vk: add Broadcom VK driver
Date: Wed, 13 May 2020 12:30:33 +0000	[thread overview]
Message-ID: <20200513123033.GL11244@42.do-not-panic.com> (raw)
In-Reply-To: <20200513065046.GA764247@kroah.com>

On Wed, May 13, 2020 at 08:50:46AM +0200, Greg Kroah-Hartman wrote:
> On Tue, May 12, 2020 at 11:31:28PM -0700, Scott Branden wrote:
> > Hi Luis,
> > 
> > On 2020-05-12 5:38 p.m., Luis Chamberlain wrote:
> > > On Thu, May 07, 2020 at 05:27:38PM -0700, Scott Branden wrote:
> > > > +#if defined(CONFIG_REQ_FW_INTO_BUF_PRIV)
> > > > +
> > > > +#define KERNEL_PREAD_FLAG_PART	0x0001 /* Allow reading part of file */
> > > > +#define REQUEST_FIRMWARE_INTO_BUF request_firmware_into_buf_priv
> > > > +int request_firmware_into_buf_priv(const struct firmware **firmware_p,
> > > > +				   const char *name, struct device *device,
> > > > +				   void *buf, size_t size,
> > > > +				   size_t offset, unsigned int pread_flags);
> > > > +
> > > > +#else
> > > > +
> > > > +#define REQUEST_FIRMWARE_INTO_BUF request_firmware_into_buf
> > > > +
> > > > +#endif
> > > > +
> > > > +#endif
> > > Please clean this up, the code must reflect only the code upstream. No
> > > config stuff like this should be used on the driver. I had to stop my
> > > review here.
> > The CONFIG_ prefix shouldn't have been there as there is no Kconfig option
> > to select this.
> > Would like to just change it to a normal define without CONFIG_ prefix
> > instead?
> > This code is here to allow a limited version of the driver to run on older
> > kernels which do not have the necessary partial file read support.
> > By having it in the upstream codebase we don't need to maintain an internal
> > version of the driver.  User can take the upstream kernel module and compile
> > it against an old version of the kernel by via the define.
> 
> That's not how kernel drivers in the tree work, sorry.  They do not
> contain "older kernel support" in them, they work as a whole with the
> rest of the kernel they ship with only.
> 
> Otherwise all drivers would be a total mess over time, can you imagine
> doing this for the next 20+ years?  Not maintainable.

Scott, now imagine the amount of cleanup you'd need to do to your driver
to get it to a state where it doesn't depend on any old kernel. That's
the exact shape of the driver we want.

To backport, you can look into the backports project which strives to
backport drivers automatically [0] to older kernels.

[0] https://backports.wiki.kernel.org/index.php/Main_Page

  Luis

  reply	other threads:[~2020-05-13 12:30 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-08  0:27 [PATCH v5 0/7] firmware: add partial read support in request_firmware_into_buf Scott Branden
2020-05-08  0:27 ` [PATCH v5 1/7] fs: introduce kernel_pread_file* support Scott Branden
2020-05-13  0:27   ` Luis Chamberlain
2020-05-13  6:23     ` Scott Branden
2020-05-13  6:51       ` Greg Kroah-Hartman
2020-05-13  8:16         ` Scott Branden
2020-05-13 18:39   ` Mimi Zohar
2020-05-13 18:53     ` Scott Branden
2020-05-13 18:57       ` Scott Branden
2020-05-13 19:03       ` Mimi Zohar
2020-05-13 19:18         ` Scott Branden
2020-05-13 19:39           ` Mimi Zohar
2020-05-13 19:41             ` Scott Branden
2020-05-13 21:20               ` Mimi Zohar
2020-05-13 21:28                 ` Luis Chamberlain
2020-05-13 22:12                   ` Mimi Zohar
2020-05-13 22:48                     ` Scott Branden
2020-05-13 23:00                       ` Mimi Zohar
2020-05-13 23:34                         ` Kees Cook
2020-05-13 23:58                           ` Mimi Zohar
2020-05-08  0:27 ` [PATCH v5 2/7] firmware: add offset to request_firmware_into_buf Scott Branden
2020-05-13  0:33   ` Luis Chamberlain
2020-05-13 18:35     ` Scott Branden
2020-05-15 20:44       ` Luis Chamberlain
2020-05-08  0:27 ` [PATCH v5 3/7] test_firmware: add partial read support for request_firmware_into_buf Scott Branden
2020-05-13  0:35   ` Luis Chamberlain
2020-05-08  0:27 ` [PATCH v5 4/7] firmware: test partial file reads of request_firmware_into_buf Scott Branden
2020-05-08  0:27 ` [PATCH v5 5/7] bcm-vk: add bcm_vk UAPI Scott Branden
2020-05-08  0:27 ` [PATCH v5 7/7] MAINTAINERS: bcm-vk: add maintainer for Broadcom VK Driver Scott Branden
     [not found] ` <20200508002739.19360-7-scott.branden@broadcom.com>
2020-05-13  0:38   ` [PATCH v5 6/7] misc: bcm-vk: add Broadcom VK driver Luis Chamberlain
2020-05-13  6:31     ` Scott Branden
2020-05-13  6:50       ` Greg Kroah-Hartman
2020-05-13 12:30         ` Luis Chamberlain [this message]
2020-05-13 18:39           ` Scott Branden
2020-05-13 16:23 ` [PATCH v5 0/7] firmware: add partial read support in request_firmware_into_buf Mimi Zohar
2020-05-15 20:47   ` Luis Chamberlain
2020-05-15 23:28     ` Scott Branden
2020-05-16  1:05       ` Luis Chamberlain

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=20200513123033.GL11244@42.do-not-panic.com \
    --to=mcgrof@kernel.org \
    --cc=agross@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=colin.king@canonical.com \
    --cc=dan.carpenter@oracle.com \
    --cc=david.brown@linaro.org \
    --cc=desmond.yan@broadcom.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=james.hu@broadcom.com \
    --cc=keescook@chromium.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=olof@lixom.net \
    --cc=rafael@kernel.org \
    --cc=scott.branden@broadcom.com \
    --cc=shuah@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=tiwai@suse.de \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH v5 6/7] misc: bcm-vk: add Broadcom VK driver' \
    /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

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).