在 2018-08-06一的 01:48 +0000，Zhang, Ning A写道：
> 在 2018-08-03五的 12:31 +0200，gre...@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.
>
> this patch changes kernel common code, and it has value to upstream,
> so
> I submit this patch for review and also look forward some advises
> from
> community. Once we decide to opensource, the driver changes will be
> there.
>
>
as said our driver handles firmware based on page, and if firmware is
builtin, page point from virt_to_page(fw->data) contains memory doesn't
belong to firmware. there are two way to fix it.
1, copy builtin firmware to vmalloc memory, this is additional work to
handle firmware. I create a wrap function for firmware request.
int request_XYZ_fw(const struct firmware **firmware_p, const char
*name,
struct device *device)
{
const struct firmware *fw;
struct firmware *tmp;
int ret;
ret = request_firmware(&fw, name, device);
if (ret < 0)
return ret;
if (is_vmalloc_addr(fw->data))
*firmware_p = fw;
else {
tmp = (struct firmware *)kzalloc(sizeof(struct
firmware), GFP_KERNEL);
if (!tmp)
return -ENOMEM;
tmp->size = fw->size;
tmp->data = vmalloc(fw->size);
memcpy(tmp->data, fw->data, fw->size);
*firmware_p = tmp;
}
return ret;
}
2, make builtin firmware is also page alignment. the change in driver
code is to check memory type of fw->data, and use different XXX_to_page
function.
struct page *p;
if (is_vmalloc_addr(fw->data)) // new
p = vmalloc_to_page(fw->data);
else // new
p = virt_to_page(fw->data); // new
compare to solution, solution 2 is simple, this is what I said
simplified. and solution 2 also save memory (I don't know the affect
change memory alignment on memory usage.)
Hi, Greg
is this easier for you to review?
BR.
Ning.
>
>
Please fix this up submit this properly.
greg k-h
