On Thursday 24 November 2016 19:11:39 Sebastian Reichel wrote:

Hi,

On Thu, Nov 24, 2016 at 05:49:33PM +0100, Pali Rohár wrote:

quoted

On Thursday 24 November 2016 17:08:30 Sebastian Reichel wrote:

quoted

On Thu, Nov 24, 2016 at 04:20:45PM +0100, Pali Rohár wrote:

quoted

On Thursday 24 November 2016 16:13:17 Sebastian Reichel wrote:

quoted

On Thu, Nov 24, 2016 at 09:33:29AM +0100, Pali Rohár wrote:

quoted

On Thursday 24 November 2016 08:51:04 Pavel Machek wrote:

quoted

quoted

quoted

"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).

quoted

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.

Permanent MAC is device specific init data, NVS is device specific
init data => load together.

The userspace helper stuff is only needed to get the data from
the NAND calibration area on the fly.

But it is needed... and currently request_firmware() prefer direct VFS 
access...

quoted

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

You mentioned NVS data is fixed size, so this should be enough?

switch(loaded_size)
    case IMAGE_SIZE + MAC_SIZE:
        /* extract mac */
        /* fallthrough */
    case IMAGE_SIZE:
        /* load NVS */
        break;
    default:
        /* fail */
}

quoted

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

concating data is not a modification, otherwise you can't
put the file in a filesystem.

Ok, if net maintainers agree.

quoted

quoted

quoted

quoted

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.

quoted

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.

Are we talking about this?

https://github.com/community-ssu/libcal/blob/master/cal.c

That's ~1k loc for read-only.

Yes. There is also read-only alternative library:
https://github.com/pali/0xFFFF/blob/master/src/cal.c

But still, this is proprietary format useful only for one device (or 
two) and I do not know if such thing could be accepted by u-boot 
developers...

Getting NAND working looks harder than porting this to me.

Right.

quoted

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

+ no men power

yeah :(

But with that reasoning you should just extract the NVS data
from NAND and put it in your rootfs.

Not a clean solution. Some rootfs parts can used as readonly. And 
normally rootfs is flashed into nand, so I still will say that roofs 
(image) should not contain any device specific data.

quoted

quoted

quoted

quoted

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).

AFAIK that's supported:

http://www.denx.de/wiki/DULG/LinuxFDTBlob

"Note that U-Boot makes some automatic modifications to the blob
before passing it to the kernel - mainly adding and modifying
information that is learnt at run-time."

I mean we do not have support for due to n900 nand.

quoted

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

if (nvs info missing)
    return -EPROBE_DEFER;

Forever? Only N times? How long? Only N second?

Here we do not know if userspace is going to send data or not.

My guess is that such code will not be accepted into net/wireless 
subsystem or drivers by maintainers.

quoted

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.

As you said one workaround is waiting. That's not a solution, that
only works with request_firmware().

Yes, but request_firmware() uses interaction with userspace. Your 
proposed solution does not do any interaction with userspace that some 
process needs to fill missing data into DT.

And request_firmware() is already used for loading NVS data.

quoted

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 DTS in
kernel is just nonsense.

I would just mark the wlan device with status = "disabled" and
enable it in the overlay together with adding the NVS & MAC info.

So if you think that this solution make sense, we can wait what net 
wireless maintainers say about it...

For me it looks like that solution can be:

extending request_firmware() to use only userspace helper

and load mac address also via request_firmware() either by appending it 
into NVS data or via separate call

-- 
Pali Rohár
pali.rohar@gmail.com