Re: [PATCH v1 1/3] MIPS: ath79: provide driver for Atheros ART partition

[Sam Ravnborg <sam@ravnborg.org>]

Hi Oleksij

Some nitpicks.

On Tue, Jun 05, 2018 at 08:10:19PM +0200, Oleksij Rempel wrote:
> From: Oleksij Rempel <linux@rempel-privat.de>
> 
> this partition contains calibration data for WiFi and
> some board specific data, like MAC address.
> 
> For now we care only about MAC.
> 
> Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
> ---
> +static int art_read_mac(struct device_d *dev, const char *file)
> +{
> +	int fd, rbytes;
> +	struct ar9300_eeprom eeprom;
> +
> +	fd = open_and_lseek(file, O_RDONLY, 0x1000);
> +	if (fd < 0) {
> +		dev_err(dev, "Failed to open eeprom path %s %d\n",
> +		       file, -errno);
> +		return -errno;
> +	}
open_and_lseek says:
otherwise a negative error code is returned
So it is wrong to convert this to a positive errocode

> +
> +	rbytes = read_full(fd, &eeprom, sizeof(eeprom));
> +	close(fd);
> +	if (rbytes <= 0 || rbytes < sizeof(eeprom)) {
> +		dev_err(dev, "Failed to read %s\n", file);
> +		return -EIO;
> +	}
Because here you convert a positive number to a negative number.

> +
> +	dev_dbg(dev, "ART version: %x.%x\n",
> +		 eeprom.eeprom_version, eeprom.template_version);
> +	dev_dbg(dev, "mac: %02x:%02x:%02x:%02x:%02x:%02x\n",
> +	       eeprom.mac_addr[0],
> +	       eeprom.mac_addr[1],
> +	       eeprom.mac_addr[2],
> +	       eeprom.mac_addr[3],
> +	       eeprom.mac_addr[4],
> +	       eeprom.mac_addr[5]);
We should add support for %pM...
But this is not done, so the above is fine

> +
> +	if (!is_valid_ether_addr(&eeprom.mac_addr[0])) {
> +		dev_err(dev, "bad MAC addr\n");
> +		return -EILSEQ;
> +	}
> +
> +	return art_set_mac(dev, &eeprom);
> +}

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox