Re: wl1251 & mac address & calibration data

From: Pali RohÃr
Date: Thu Nov 24 2016 - 11:49:47 EST


On Thursday 24 November 2016 17:08:30 Sebastian Reichel wrote:
> Hi,
>
> On Thu, Nov 24, 2016 at 04:20:45PM +0100, Pali RohÃr wrote:
> > On Thursday 24 November 2016 16:13:17 Sebastian Reichel wrote:
> > > On Thu, Nov 24, 2016 at 09:33:29AM +0100, Pali RohÃr wrote:
> > > > On Thursday 24 November 2016 08:51:04 Pavel Machek wrote:
> > > > > > > "ifconfig hw ether XX" normally sets the address. I guess
> > > > > > > that's ioctl?
> > > > > >
> > > > > > This sets temporary address and it is ioctl. IIRC same as
> > > > > > what ethtool uses. (ifconfig is already deprecated).
> > > > > >
> > > > > > > And I guess we should use similar mechanism for permanent
> > > > > > > address.
> > > > > >
> > > > > > I'm not sure here... Above ioctl âââ is for changing
> > > > > > temporary mac address. But here we do not want to change
> > > > > > permanent mac address. We want to tell kernel driver
> > > > > > current permanent mac address which is stored
> > > > >
> > > > > Well... I'd still use similar mechanism :-).
> > > >
> > > > Thats problematic, because in time when wlan0 interface is
> > > > registered into system and visible in ifconfig output it
> > > > already needs to have permanent mac address assigned.
> > > >
> > > > We should assign permanent mac address before wlan0 of wl1251
> > > > is registered into system.
> > >
> > > You can just add the MAC address to the NVS data, which is also
> > > required for the device initialization.
> >
> > NVS data file has fixed size, there is IIRC no place for it.
> >
> > But one of my suggestion was to use another request_firmware for
> > MAC address. So this is similar to what you are proposing, as NVS
> > data are loaded by request_firmware too...
>
> Just append it to NVS data and modify the size check accordingly?

First that would mean to have request_firmware() function which will not
use direct VFS access, but instead use userspace helper.

NVS data file with some default values (not suitable for usage) is
already present in linux-firmware and available in /lib/firmware/...

Also I'm not sure if such thing is allowed by license:
https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/LICENCE.ti-connectivity

> > > I wonder if those information could be put into DT. Iirc some
> > > network devices get their MAC address from DT. Maybe we can add
> > > all NVS info to DT? How much data is it?
> >
> > Proprietary, signed and closed bootloader NOLO does not support DT.
> > So for booting you need to append DTS file to kernel image.
>
> Yeah, so NOLO without U-Boot will depend on userspace to fixup DT.
>
> > U-Boot is optional and can be used as intermediate bootloader
> > between NOLO and kernel. But still it has problems with reading
> > from nand, so cannot read NVS data nor MAC address.
>
> It may in the future?

I already tried that, but I failed. I was not able to access N900's nand
from u-boot. No idea where was problem...

And if somebody fix onenand in u-boot, then needs to reimplement Nokia's
proprietary parser of that partition where is stored NVS and mac address
&& make this support in upstream u-boot.

So... I doubt it will be in any future.

+ no men power

> > > Userspace application can add all those information to the DT
> > > using a DT overlay. Also the u-boot could parse and add it at
> > > some point in the future.
> >
> > In case when wl1251 is statically linked into kernel image, it is
> > loaded and initialized before even userspace applications starts.
> >
> > So no... adding NVS data or MAC address into DT or DT overlay is
> > not a solution.
>
> Actually with data loaded from DT you *can* load data quite early in
> the boot process, while your suggestions always require userspace.
> So you argument against yourself?

You cannot add DTS in uboot (no support). And if you modify DTS on
running kernel from userspace, then it is too late when wl1251 is
statically linked into kernel image.

wl1251 will not wait until some userspace modify DTS and add data...

But request_firmware() in fallback mode *can* wait for userspace so
wl1251 can postpone its operation until mdev/udev/whatever starts
listening for events and push needed firmware data to kernel.

So no, there is no argument against... request_firmware() in fallback
mode with userspace helper is by design blocking and waiting for
userspace. But waiting for some change in DST in kernel is just
nonsense.

--
Pali RohÃr
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part.