On Thu, May 23, 2013 at 8:05 PM, Takashi Iwai <ti...@suse.de> wrote: > At Thu, 23 May 2013 18:45:29 +0800, > Ming Lei wrote: >> >> On Thu, May 23, 2013 at 6:36 PM, Takashi Iwai <ti...@suse.de> wrote: >> > >> > No, f/w loader always fall back to user mode helper, as long as its >> > support is built in. And doing that for microcode driver in that code >> > path isn't only superfluous but also broken due to request_firmware >> > call in module init. >> >> Firstly, it is not good to do this since some distributions doesn't support >> direct loading and doesn't have udevd(such as, android). >> >> Secondly, returning failure from request_firmware_direct() doesn't mean >> the firmware doesn't exist since distribution may put the firmware other >> where. > > Right, the non-standard path is the problem, and basically the only > problem. The distribution that doesn't support the direct loading > means nothing but that.
Suppose it is, it is the fact, and it isn't OK to break this distribution. Also udev supports user-defined rules to load firmware, which means some drivers may not put their firmware in the default path of distribution's firmware. > >> Anyway, this example is very specific(no firmware can be accepted), and >> request_firmware_nowait() should be OK for the situation. > > Oh no, rewriting with request_firmware_nowait() should be really the > last choice. It would change the code flow awfully bad in most > cases. > > The new kernel driver has a better firmware mechanism. If it's only > the question of paths, we should move on toward that direction and > drop the too complex old way. I'd vote for a warning shown when a Simply dropping the old way may cause user space regression. > firmware file is loaded via user mode helper (except for explicit > cases like FW_ACTION_NOHOTPLUG), for example. As it is a very driver specific problem, it is better to solve it inside driver. > > >> >> wrt. this problem, I think we >> >> need to know why the direct loading is failed. >> > >> > The reason is obvious: the requested f/w file doesn't exist. >> > And it's fine, because the microcode update is an optional operation. >> > If no f/w file is found, it's not handled as an error. It just means >> > that no need to update, continuing to work. >> >> OK, as said above, the example is very specific, and might be >> workarounded by request_firmware_nowait(). > > It's not that easy in this case. The microcode loader driver core > module doesn't invoke request_firmware() directly but it's via cpu > driver. And the same callback is called in different code paths, not > only at init but also via sysfs write. Thus the request_firmware() > call must be synchronous there. I don't think the way is too difficult to implement. In the path which requires synchronization, it can be waited on one completion after calling request_firmware_nowait(). Thanks, -- Ming Lei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/