Re: [PATCH 2/7] firmware: add offset to request_firmware_into_buf

[Scott Branden]

Hi Takashi,

On 2019-08-26 10:12 a.m., Takashi Iwai wrote:
> On Mon, 26 Aug 2019 17:41:40 +0200,
> Scott Branden wrote:
> > HI Takashi,
> >
> > On 2019-08-26 8:20 a.m., Takashi Iwai wrote:
> > > On Fri, 23 Aug 2019 21:44:42 +0200,
> > > Scott Branden wrote:
> > > > Hi Takashi,
> > > >
> > > > Thanks for review. comments below.
> > > >
> > > > On 2019-08-23 3:05 a.m., Takashi Iwai wrote:
> > > > > On Thu, 22 Aug 2019 21:24:46 +0200,
> > > > > Scott Branden wrote:
> > > > > > Add offset to request_firmware_into_buf to allow for portions
> > > > > > of firmware file to be read into a buffer.  Necessary where firmware
> > > > > > needs to be loaded in portions from file in memory constrained systems.
> > > > > AFAIU, this won't work with the fallback user helper, right?
> > > > Seems to work fine in the fw_run_tests.sh with fallbacks.
> > > But how?  You patch doesn't change anything about the fallback loading
> > > mechanism.
> > Correct - I didn't change any of the underlying mechanisms,
> > so however request_firmware_into_buf worked before it still does.
> > >    Or, if the expected behavior is to load the whole content
> > > and then copy a part, what's the merit of this API?
> > The merit of the API is that the entire file is not copied into a buffer.
> > In my use case, the buffer is a memory region in PCIe space that isn't
> > even large enough for the whole file. So the only way to get the file
> > is to read it
> > in portions.
> BTW: does the use case above mean that the firmware API directly
> writes onto the given PCI iomem region?  If so, I'm not sure whether
> it would work as expected on all architectures.  There must be a
> reason of the presence of iomem-related API like memcpy_toio()...
Yes, we access the PCI region directly in the driver and thus also 
through request_firmware_into_buf.
I will admit I am not familiar with every subtlety of PCI accesses. Any 
comments to the Valkyrie driver in this patch series are appreciated.
But not all drivers need to work on all architectures. I can add a 
depends on x86 64bit architectures to the driver to limit it to such.
> thanks,
>
> Takashi