Re: [PATCH V3] ssb: Implement virtual SPROM

[Michael Buesch <mb@bu3sch.de>]

On Tuesday 23 March 2010 20:13:54 Larry Finger wrote:
> Some recent BCM43XX devices lack an on-board SPROM. The pertinent data
> from the SPROM could be included in the kernel; however, this presents
> a problem in the generation of a unique, reproducible MAC address. The
> solution is to use a file in /lib/firmware/ssb, which has some "random"
> information generated by /bin/dbus-uuidgen. This file is loaded using the
> asynchronous firmware facility and a MAC address is generated from those
> data. To prevent multiple devices in a single box from having the same
> MAC address, the last two digits are replaced by the bus numbers of the
> device.
> 
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
> 
> John,
> 
> Unless someone comes up with a unique way to generate a MAC address using
> only udev rules, I think this is ready.

Please take a look at Calvin Walton's RFC.
It would avoid the need for abusing the firmware loading mechanism.

> +	/* Input MAC address data in ASCII - get low nibble in binary */
> +	for (i = 0 ; i < 8; i++) {
> +		u8 tmp = ascii_to_bin(fw->data[2 * i]);
> +		u8 tmp1 = ascii_to_bin(fw->data[2 * i + 1]);
> +		buf2[i] = tmp << 4 | tmp1;
> +	}

You would basically initialize the mac address to FFFFFFFFFFFF here and
udev would take care of the rest.

>  static int ssb_pci_sprom_get(struct ssb_bus *bus,
>  			     struct ssb_sprom *sprom)
>  {
> @@ -620,8 +735,18 @@ static int ssb_pci_sprom_get(struct ssb_
>  	int err = -ENOMEM;
>  	u16 *buf;
>  
> -	if (!ssb_is_sprom_available(bus))
> -		return -ENODEV;
> +	if (!ssb_is_sprom_available(bus)) {
> +		/* This device has no SPROM. Try for a random MAC address */
> +		err = request_firmware_nowait(THIS_MODULE,
> +			FW_ACTION_HOTPLUG, "ssb/mac_addr",
> +			&bus->host_pci->dev, GFP_KERNEL, bus,
> +			ssb_get_vsprom);

Well, I'm not sure how this is supposed to work. What happens if a part of ssb
or b43 uses the SPROM data structure, but ssb_get_vsprom() didn't run, yet?

So: Get rid of the firmware fetching stuff completely and do it via udev.
It reduces the headache by a few magnitudes of headachenesses.

> +		if (err) {
> +			ssb_vsprom_load_failed();
> +			return err;
> +		}
> +		return 0;
> +	}
>  
>  	buf = kcalloc(SSB_SPROMSIZE_WORDS_R123, sizeof(u16), GFP_KERNEL);
>  	if (!buf)
-- 
Greetings, Michael.