2012.12.18. 23:22 keltezéssel, Stanislaw Gruszka írta:

On Tue, Dec 18, 2012 at 05:22:23PM +0100, Gabor Juhos wrote:

quoted

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 <redacted>

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.

quoted

+	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

quoted

+		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