[alsa-devel] [PATCH 1/1] hdsp: allow firmware loading from inside the kernel

[Takashi Iwai]

At Tue, 12 May 2009 10:01:20 +0200,
Raphaël Doursenaud wrote:
> 
> Takashi Iwai a écrit :
> > At Tue, 12 May 2009 09:41:36 +0200,
> > Raphaël Doursenaud wrote:
> >> -----BEGIN PGP SIGNED MESSAGE-----
> >> Hash: SHA1
> >>
> >> Takashi Iwai a écrit :
> >>> At Tue, 12 May 2009 09:25:27 +0200,
> >>> Raphaël Doursenaud wrote:
> >>>> -----BEGIN PGP SIGNED MESSAGE-----
> >>>> Hash: SHA1
> >>>>
> >>>> Takashi Iwai a écrit :
> >>>>> At Tue, 12 May 2009 09:05:19 +0200,
> >>>>> Raphaël Doursenaud wrote:
> >>>>>> -----BEGIN PGP SIGNED MESSAGE-----
> >>>>>> Hash: SHA1
> >>>>>>
> >>>>>> Takashi Iwai a écrit :
> >>>>>>> At Tue, 12 May 2009 08:47:29 +0200,
> >>>>>>> Raphaël Doursenaud wrote:
> >>>>>>>> -----BEGIN PGP SIGNED MESSAGE-----
> >>>>>>>> Hash: SHA1
> >>>>>>>>
> >>>>>>>> Takashi Iwai a écrit :
> >>>>>>>>> At Tue, 12 May 2009 08:16:08 +0200,
> >>>>>>>>> Raphaël Doursenaud wrote:
> >>>>>>>>>> From: Raphaël Doursenaud <rdoursenaud at free.fr>
> >>>>>>>>>>
> >>>>>>>>>> Allow the use of  the FIRMWARE_IN_KERNEL option with hdsp cards and
> >>>>>>>>>> in-kernel driver.
> >>>>>>>>> Did it really work without problems?
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Takashi
> >>>>>>>> Tested over the weekend with two multifaces in my DAW.
> >>>>>>>> Got no problem.
> >>>>>>> Interesting.
> >>>>>>> Did you build the firmware file into the kernel, or not?
> >>>>>>>
> >>>>>>>
> >>>>>>> Takashi
> >>>>>> Yes I built all hdsp fimware files (multiface_firmware.bin
> >>>>>> multiface_firmware_rev11.bin digiface_firmware.bin
> >>>>>> digiface_firmware_rev11.bin) into the kernel.
> >>>>>> It's the aim of this patch.
> >>>>> Well, the problem I'm concerned is that the driver can be compiled
> >>>>> in even if you have no built-in firmware.  And there is no restriction
> >>>>> or dependency check in Kconfig, so far.
> >>>>>
> >>>>> Could you test how the kernel behaves without the built-in firmware?
> >>>>> Does it hang or give any critical error?
> >>>>>
> >>>>>
> >>>>> thanks,
> >>>>>
> >>>>> Takashi
> >>>> Could you be more specific?
> >>>> I'm not sure to understand why it could be a problem.
> >>>> Do you think that if I set FIRMWARE_IN_KERNEL without compiling the
> >>>> firmware(s) in-kernel the request_firmware() will not resolve and cause
> >>>> an error?
> >>> Yes, exactly.
> >>> request_firmware() shall fail in that case likely after a long
> >>> time-out (unless you have the firmware files in initrd) because there
> >>> is really no file / data available at the time it's called.
> >>> And I'm not sure whether this could lead to a fatal operation error.
> >>>
> >>>
> >>> Takashi
> >> AFAIK this is handled in the code (from line 5080) and should lead to
> >> "Hammerfall-DSP: couldn't get firmware from userspace. try using hdsploader"
> > 
> > Yep, a fallback mechanism is found in hdsp driver itself.
> > But the driver probe hangs for too long time, and I'm just worried
> > about its influence on the whole kernel boot-up.
> > 
> >> I'm building a kernel to test that case.
> > 
> > Thanks.
> > 
> > 
> > Takashi
> Wow, you're right! Didn't think about that...
> 
> It hangs for about 45s on :
> Hammerfall-DSP: wait for FIFO status <= 0 failed after 30 iterations
> RME Hammerfall DSP 0000:05:02.0: firmware: requesting
> multiface_firmware_rev11.bin
> 
> before going on with :
> Hammerfall-DSP: cannot load firmware multiface_firmware_rev11.bin
> Hammerfall-DSP: couldn't get firmware from userspace. try using hdsploader
> Hammerfall-DSP: card initialization pending : waiting for firmware

OK, so it doesn't lead to any critical error?
Then it's better than the worst case I thought of :)

> I wonder what would be the best way to deal with that while still
> enabling in-kernel firmware.

Well, it's basically a problem of the request_firmware() itself, so
we can't fix the behavior properly in the driver side.  There is an
option to use *_nowait() call, but this will end up with a mess of
code flow, I'm afraid.

So, I'm going to apply your patch.  But, maybe we should add a warning
in Kconfig comment, such as

comment "Don't forget to add built-in firmwares for HDSP driver"
	depends on SND_HDSP=y

or so...  Suggestions of a better text are appreciated.


thanks,

Takashi