2012.12.18. 23:22 keltezéssel, Stanislaw Gruszka írta:
> On Tue, Dec 18, 2012 at 05:22:23PM +0100, Gabor Juhos wrote:
>> Currently the driver fetches the EEPROM data
>> from a fixed memory location for SoC devices
>> for SoC devices with a built-in wireless MAC.
>>
>> The usability of this approach is quite
>> limited, because it is only suitable if the
>> location of the EEPROM data is mapped into
>> the memory. This condition is true on embedded
>> boards equipped which are using a parallel NOR
>> flash, but it is not true for boards equipped
>> with SPI or NAND flashes. The fixed location
>> also does not work in all cases, because the
>> offset of the EEPROM data varies between
>> different boards.
>>
>> Additionally, various embedded boards are using
>> a PCI/PCIe chip soldered directly onto the PCB.
>> Such boards usually does not have a separate
>> EEPROM chip for the PCI/PCIe devices, the data
>> of the EEPROM is stored in the main flash
>> instead.
>>
>> The patch makes it possible to load the EEPROM
>> data via firmware API. This new method works
>> regardless of the type of the flash, and it is
>> usable with built-in and with PCI/PCIe devices.
>>
>> Signed-off-by: Gabor Juhos <juhosg@xxxxxxxxxxx>
>
> I understand this patch will not broke NOR boards, which use
> ioremap approach currently?
The change will break those obviously, so those boards must be converted to use
the new method. I have added sanity check into the 'rt2800soc_probe' function
which ensures that the users of such boards will be informed about that. FWIW,
that approach is used by out-of-tree boards only.
>> + init_completion(&ec.complete);
>> + retval = request_firmware_nowait(THIS_MODULE, 1, name,
>> + rt2x00dev->dev, GFP_KERNEL, &ec,
>> + rt2800pci_eeprom_request_cb);
>> + if (retval < 0) {
>> + ERROR(rt2x00dev, "EEPROM request failed\n");
>> + return retval;
>> + }
>> +
>> + wait_for_completion(&ec.complete);
> Since we use completion here, why we can not just use normal synchronous
> version of request_firmware? I heard of request_firmware drawbacks, so
> this approach can be correct. Just want to know if we do not complicate
> things not necessarily here.
If the driver is built into the kernel, then the synchronous version would fail
because user-space is not up during probe time.
The initial version of the patch used the synchronous version, but Gertjan had
concerns about that:
http://rt2x00.serialmonkey.com/pipermail/users_rt2x00.serialmonkey.com/2012-December/005526.html
>> + goto release_eeprom;
>> + }
>> +
>> + memcpy(rt2x00dev->eeprom, ec.blob->data, EEPROM_SIZE);
>> + retval = 0;
>> +
>> +release_eeprom:
> We do not free memory - I guess we should do relase_firmware(ec.blob)?
Yes. I'm sure that I have added that call once, but it seems lost in the rebase
process. Will send and updated version.
Thank you for the comments!
-Gabor
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
