All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Zhang, Ning A" <ning.a.zhang@intel.com>
To: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
Cc: "pombredanne@nexb.com" <pombredanne@nexb.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Li, Ting" <ting.li@intel.com>,
	"yamada.masahiro@socionext.com" <yamada.masahiro@socionext.com>,
	"kstewart@linuxfoundation.org" <kstewart@linuxfoundation.org>,
	"markus@trippelsdorf.de" <markus@trippelsdorf.de>
Subject: Re: [PATCH] firmware: make sure builtin firmware is page alignment
Date: Tue, 7 Aug 2018 02:27:31 +0000	[thread overview]
Message-ID: <1533608851.20683.78.camel@intel.com> (raw)
In-Reply-To: <20180806140512.GA19844@kroah.com>

在 2018-08-06一的 16:05 +0200,gregkh@linuxfoundation.org写道:
> On Mon, Aug 06, 2018 at 01:48:44AM +0000, Zhang, Ning A wrote:
> > 在 2018-08-03五的 12:31 +0200,gregkh@linuxfoundation.org写道:
> > > On Fri, Aug 03, 2018 at 08:42:25AM +0000, Zhang, Ning A wrote:
> > > > 在 2018-08-03五的 07:39 +0200,Greg KH写道:
> > > > > On Fri, Aug 03, 2018 at 09:45:21AM +0800, Zhang Ning wrote:
> > > > > > when firmware is in filesystem, request_firmware will load
> > > > > > it,
> > > > > > and copy it to vmalloc memory, that is page align memory.
> > > > > > 
> > > > > > but when firmware is builtin, it is 8 bytes or 4 bytes
> > > > > > alignment.
> > > > > > 
> > > > > > make sure builtin firmware is page algnment, that can
> > > > > > simplify
> > > > > > algorithm
> > > > > > to handle firmware.
> > > > > 
> > > > > How is it simplified?  I don't see any such change like that
> > > > > here
> > > > > :(
> > > > > 
> > > > 
> > > > Thank you for review this patch.
> > > > 
> > > > When driver handles its firmware based on  page, like below:
> > > > 
> > > > 	struct page *p;
> > > > 	p = vmalloc_to_page(fw->data);  // for filesystem
> > > > firmware
> > > > 	p = virt_to_page(fw->data); // for builtin firmware
> > > > 
> > > > but if builtin firmware is not page alignment, page pointer for
> > > > builtin
> > > > firmware is wrong, it contains memory not belong to firmware.
> > > > drivers
> > > > has to use additional code to handle this. 
> > > > 
> > > > if builtin firmware is also page alignment, no need additional
> > > > code
> > > > to
> > > > handle builtin firmware. simplified.
> > > 
> > > But you did not change anything like this in your code, so why
> > > would
> > > I
> > > know this?
> > 
> > I understand it is very difficult to review this patch without
> > context.
> > The driver is not opensource, I can't show the patch for driver.
> 
> Then I can not accept your patch.  Go talk to your corporate lawyers
> about changing core kernel code for a closed source driver and what
> that
> implies about that closed driver (hint, your driver can not be
> closed...) :)

It's very sad, but anyway, thank you for your review.
your review will definitely help us improve our work.

BR.
Ning.

> 
> Then come back with a proper open sourced driver, that's the only way
> Linux drivers can be written sorry.
> 
> best of luck,
> 
> greg k-h
> 

  reply	other threads:[~2018-08-07  2:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-03  1:45 [PATCH] firmware: make sure builtin firmware is page alignment Zhang Ning
2018-08-03  5:39 ` Greg KH
2018-08-03  8:42   ` Zhang, Ning A
2018-08-03 10:31     ` gregkh
2018-08-06  1:48       ` Zhang, Ning A
2018-08-06  5:13         ` Zhang, Ning A
2018-08-06 14:05         ` gregkh
2018-08-07  2:27           ` Zhang, Ning A [this message]
2018-08-07  8:12             ` gregkh

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=1533608851.20683.78.camel@intel.com \
    --to=ning.a.zhang@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kstewart@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markus@trippelsdorf.de \
    --cc=pombredanne@nexb.com \
    --cc=ting.li@intel.com \
    --cc=yamada.masahiro@socionext.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 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.