diff mbox series

[RFC,net-next] net: phy: aquantia: add firmware load support

Message ID 20230930104008.234831-1-robimarko@gmail.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series [RFC,net-next] net: phy: aquantia: add firmware load support | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 1340 this patch: 1348
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 1363 this patch: 1363
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 1363 this patch: 1371
netdev/checkpatch warning WARNING: line length of 123 exceeds 80 columns WARNING: line length of 131 exceeds 80 columns WARNING: line length of 81 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns WARNING: line length of 99 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Robert Marko Sept. 30, 2023, 10:39 a.m. UTC
Aquantia PHY-s require firmware to be loaded before they start operating.
It can be automatically loaded in case when there is a SPI-NOR connected
to Aquantia PHY-s or can be loaded from the host via MDIO.

This patch adds support for loading the firmware via MDIO as in most cases
there is no SPI-NOR being used to save on cost.
Firmware loading code itself is ported from mainline U-boot with cleanups.

The firmware has mixed values both in big and little endian.
PHY core itself is big-endian but it expects values to be in little-endian.
The firmware is little-endian but CRC-16 value for it is stored at the end
of firmware in big-endian.

It seems the PHY does the conversion internally from firmware that is
little-endian to the PHY that is big-endian on using the mailbox
but mailbox returns a big-endian CRC-16 to verify the written data
integrity.

Co-developed-by: Christian Marangi <ansuelsmth@gmail.com>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 drivers/net/phy/Kconfig         |   1 +
 drivers/net/phy/aquantia_main.c | 276 ++++++++++++++++++++++++++++++++
 2 files changed, 277 insertions(+)

Comments

Andrew Lunn Oct. 2, 2023, 8:18 p.m. UTC | #1
> +/* load data into the phy's memory */
> +static int aquantia_load_memory(struct phy_device *phydev, u32 addr,
> +				const u8 *data, size_t len)
> +{
> +	u16 crc = 0, up_crc;
> +	size_t pos;
> +
> +	/* PHY expect addr in LE */
> +	addr = cpu_to_le32(addr);
> +
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1,
> +		      VEND1_GLOBAL_MAILBOX_INTERFACE1,
> +		      VEND1_GLOBAL_MAILBOX_INTERFACE1_CRC_RESET);
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1,
> +		      VEND1_GLOBAL_MAILBOX_INTERFACE3,
> +		      VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR(addr));
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1,
> +		      VEND1_GLOBAL_MAILBOX_INTERFACE4,
> +		      VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR(addr));
> +
> +	for (pos = 0; pos < len; pos += min(sizeof(u32), len - pos)) {
> +		u32 word = 0;
> +
> +		memcpy(&word, data + pos, min(sizeof(u32), len - pos));
> +
> +		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE5,
> +			      VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA(word));
> +		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE6,
> +			      VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA(word));
> +
> +		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE1,
> +			      VEND1_GLOBAL_MAILBOX_INTERFACE1_EXECUTE |
> +			      VEND1_GLOBAL_MAILBOX_INTERFACE1_WRITE);
> +
> +		/* calculate CRC as we load data to the mailbox.
> +		 * We convert word to big-endiang as PHY is BE and ailbox will
> +		 * return a BE crc.

_m_ailbox.

And i would consistently uses CRC in comments.

> +static int aqr_fw_boot(struct phy_device *phydev, const u8 *data, size_t size)
> +{
> +	const struct aqr_fw_header *header;
> +	u32 iram_offset = 0, iram_size = 0;
> +	u32 dram_offset = 0, dram_size = 0;
> +	char version[VERSION_STRING_SIZE];
> +	u16 calculated_crc, read_crc;
> +	u32 primary_offset = 0;
> +	int ret;
> +
> +	/* extract saved crc at the end of the fw */
> +	memcpy(&read_crc, data + size - 2, sizeof(read_crc));
> +	/* crc is saved in big-endian as PHY is BE */
> +	read_crc = be16_to_cpu(read_crc);
> +	calculated_crc = crc_ccitt_false(0, data, size - 2);
> +	if (read_crc != calculated_crc) {
> +		phydev_err(phydev, "bad firmware CRC: file 0x%04x calculated 0x%04x\n",
> +			   read_crc, calculated_crc);
> +		return -EINVAL;
> +	}
> +
> +	/* Get the primary offset to extract DRAM and IRAM sections. */
> +	memcpy(&primary_offset, data + PRIMARY_OFFSET_OFFSET, sizeof(u16));

Please add some sanity checks. We should not fully trust the
firmware. Is PRIMARY_OFFSET_OFFSET + sizeof(u16) actually inside the
firmware blob?

> +	primary_offset = PRIMARY_OFFSET(le32_to_cpu(primary_offset));
> +
> +	/* Find the DRAM and IRAM sections within the firmware file. */
> +	header = (struct aqr_fw_header *)(data + primary_offset + HEADER_OFFSET);

Is header actually inside the firmware blob?

> +	memcpy(&iram_offset, &header->iram_offset, sizeof(u8) * 3);
> +	memcpy(&iram_size, &header->iram_size, sizeof(u8) * 3);
> +	memcpy(&dram_offset, &header->dram_offset, sizeof(u8) * 3);
> +	memcpy(&dram_size, &header->dram_size, sizeof(u8) * 3);
> +
> +	/* offset are in LE and values needs to be converted to cpu endian */
> +	iram_offset = le32_to_cpu(iram_offset);
> +	iram_size = le32_to_cpu(iram_size);
> +	dram_offset = le32_to_cpu(dram_offset);
> +	dram_size = le32_to_cpu(dram_size);
> +
> +	/* Increment the offset with the primary offset. */
> +	iram_offset += primary_offset;
> +	dram_offset += primary_offset;
> +
> +	phydev_dbg(phydev, "primary %d IRAM offset=%d size=%d DRAM offset=%d size=%d\n",
> +		   primary_offset, iram_offset, iram_size, dram_offset, dram_size);
> +
> +	strscpy(version, (char *)data + dram_offset + VERSION_STRING_OFFSET,
> +		VERSION_STRING_SIZE);

Is version inside the blob....

> +static int aqr_firmware_load_nvmem(struct phy_device *phydev)
> +{
> +	struct nvmem_cell *cell;
> +	size_t size;
> +	u8 *buf;
> +	int ret;
> +
> +	cell = nvmem_cell_get(&phydev->mdio.dev, "firmware");

Does this need properties in device tree? Please update the binding.

> +
> +static int aqr_firmware_load_sysfs(struct phy_device *phydev)

_sysfs seems a bit odd here. Does request_firmware still use the user
mode helper? I _thought_ it just went direct to the filesystem?

> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	const struct firmware *fw;
> +	const char *fw_name;
> +	int ret;
> +
> +	ret = of_property_read_string(dev->of_node, "firmware-name",
> +				      &fw_name);

Please update the device tree binding.

> +static int aqr_firmware_load(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_FW_ID);
> +	if (ret > 0)
> +		goto exit;

I assume this means a value of 0 indicates there is no firmware
running? Maybe a comment or a #define for 0?

	 Andrew
Christian Marangi Oct. 2, 2023, 8:22 p.m. UTC | #2
On Mon, Oct 02, 2023 at 10:18:05PM +0200, Andrew Lunn wrote:
> > +/* load data into the phy's memory */
> > +static int aquantia_load_memory(struct phy_device *phydev, u32 addr,
> > +				const u8 *data, size_t len)
> > +{
> > +	u16 crc = 0, up_crc;
> > +	size_t pos;
> > +
> > +	/* PHY expect addr in LE */
> > +	addr = cpu_to_le32(addr);
> > +
> > +	phy_write_mmd(phydev, MDIO_MMD_VEND1,
> > +		      VEND1_GLOBAL_MAILBOX_INTERFACE1,
> > +		      VEND1_GLOBAL_MAILBOX_INTERFACE1_CRC_RESET);
> > +	phy_write_mmd(phydev, MDIO_MMD_VEND1,
> > +		      VEND1_GLOBAL_MAILBOX_INTERFACE3,
> > +		      VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR(addr));
> > +	phy_write_mmd(phydev, MDIO_MMD_VEND1,
> > +		      VEND1_GLOBAL_MAILBOX_INTERFACE4,
> > +		      VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR(addr));
> > +
> > +	for (pos = 0; pos < len; pos += min(sizeof(u32), len - pos)) {
> > +		u32 word = 0;
> > +
> > +		memcpy(&word, data + pos, min(sizeof(u32), len - pos));
> > +
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE5,
> > +			      VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA(word));
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE6,
> > +			      VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA(word));
> > +
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE1,
> > +			      VEND1_GLOBAL_MAILBOX_INTERFACE1_EXECUTE |
> > +			      VEND1_GLOBAL_MAILBOX_INTERFACE1_WRITE);
> > +
> > +		/* calculate CRC as we load data to the mailbox.
> > +		 * We convert word to big-endiang as PHY is BE and ailbox will
> > +		 * return a BE crc.
> 
> _m_ailbox.
> 
> And i would consistently uses CRC in comments.
> 
> > +static int aqr_fw_boot(struct phy_device *phydev, const u8 *data, size_t size)
> > +{
> > +	const struct aqr_fw_header *header;
> > +	u32 iram_offset = 0, iram_size = 0;
> > +	u32 dram_offset = 0, dram_size = 0;
> > +	char version[VERSION_STRING_SIZE];
> > +	u16 calculated_crc, read_crc;
> > +	u32 primary_offset = 0;
> > +	int ret;
> > +
> > +	/* extract saved crc at the end of the fw */
> > +	memcpy(&read_crc, data + size - 2, sizeof(read_crc));
> > +	/* crc is saved in big-endian as PHY is BE */
> > +	read_crc = be16_to_cpu(read_crc);
> > +	calculated_crc = crc_ccitt_false(0, data, size - 2);
> > +	if (read_crc != calculated_crc) {
> > +		phydev_err(phydev, "bad firmware CRC: file 0x%04x calculated 0x%04x\n",
> > +			   read_crc, calculated_crc);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Get the primary offset to extract DRAM and IRAM sections. */
> > +	memcpy(&primary_offset, data + PRIMARY_OFFSET_OFFSET, sizeof(u16));
> 
> Please add some sanity checks. We should not fully trust the
> firmware. Is PRIMARY_OFFSET_OFFSET + sizeof(u16) actually inside the
> firmware blob?
> 
> > +	primary_offset = PRIMARY_OFFSET(le32_to_cpu(primary_offset));
> > +
> > +	/* Find the DRAM and IRAM sections within the firmware file. */
> > +	header = (struct aqr_fw_header *)(data + primary_offset + HEADER_OFFSET);
> 
> Is header actually inside the firmware blob?
> 
> > +	memcpy(&iram_offset, &header->iram_offset, sizeof(u8) * 3);
> > +	memcpy(&iram_size, &header->iram_size, sizeof(u8) * 3);
> > +	memcpy(&dram_offset, &header->dram_offset, sizeof(u8) * 3);
> > +	memcpy(&dram_size, &header->dram_size, sizeof(u8) * 3);
> > +
> > +	/* offset are in LE and values needs to be converted to cpu endian */
> > +	iram_offset = le32_to_cpu(iram_offset);
> > +	iram_size = le32_to_cpu(iram_size);
> > +	dram_offset = le32_to_cpu(dram_offset);
> > +	dram_size = le32_to_cpu(dram_size);
> > +
> > +	/* Increment the offset with the primary offset. */
> > +	iram_offset += primary_offset;
> > +	dram_offset += primary_offset;
> > +
> > +	phydev_dbg(phydev, "primary %d IRAM offset=%d size=%d DRAM offset=%d size=%d\n",
> > +		   primary_offset, iram_offset, iram_size, dram_offset, dram_size);
> > +
> > +	strscpy(version, (char *)data + dram_offset + VERSION_STRING_OFFSET,
> > +		VERSION_STRING_SIZE);
> 
> Is version inside the blob....
> 
> > +static int aqr_firmware_load_nvmem(struct phy_device *phydev)
> > +{
> > +	struct nvmem_cell *cell;
> > +	size_t size;
> > +	u8 *buf;
> > +	int ret;
> > +
> > +	cell = nvmem_cell_get(&phydev->mdio.dev, "firmware");
> 
> Does this need properties in device tree? Please update the binding.
>

This is problematic... Since this is a plain standard PHY and we don't
have a compatible (as it's matched with the PHY id) we don't have DT to
add this... Sooo how to add this? Should we update the generic-phy dt?

Should we create a dummy dt and add a compatible adding
ethernet-phy.ID... just for this properties?

This is why we were a bit confused about adding a DT commit to this.

> > +
> > +static int aqr_firmware_load_sysfs(struct phy_device *phydev)
> 
> _sysfs seems a bit odd here. Does request_firmware still use the user
> mode helper? I _thought_ it just went direct to the filesystem?
> 
> > +{
> > +	struct device *dev = &phydev->mdio.dev;
> > +	const struct firmware *fw;
> > +	const char *fw_name;
> > +	int ret;
> > +
> > +	ret = of_property_read_string(dev->of_node, "firmware-name",
> > +				      &fw_name);
> 
> Please update the device tree binding.
> 
> > +static int aqr_firmware_load(struct phy_device *phydev)
> > +{
> > +	int ret;
> > +
> > +	ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_FW_ID);
> > +	if (ret > 0)
> > +		goto exit;
> 
> I assume this means a value of 0 indicates there is no firmware
> running? Maybe a comment or a #define for 0?
> 
> 	 Andrew
Andrew Lunn Oct. 2, 2023, 9:07 p.m. UTC | #3
> This is problematic... Since this is a plain standard PHY and we don't
> have a compatible (as it's matched with the PHY id) we don't have DT to
> add this... Sooo how to add this? Should we update the generic-phy dt?
> 
> Should we create a dummy dt and add a compatible adding
> ethernet-phy.ID... just for this properties?
> 
> This is why we were a bit confused about adding a DT commit to this.

Just do what other PHYs do. ti,dp83869.yaml, motorcomm,yt8xxx.yaml,
nxp,tja11xx.yaml, etc.

	Andrew
Christian Marangi Oct. 3, 2023, 10:21 a.m. UTC | #4
On Mon, Oct 02, 2023 at 11:07:25PM +0200, Andrew Lunn wrote:
> > This is problematic... Since this is a plain standard PHY and we don't
> > have a compatible (as it's matched with the PHY id) we don't have DT to
> > add this... Sooo how to add this? Should we update the generic-phy dt?
> > 
> > Should we create a dummy dt and add a compatible adding
> > ethernet-phy.ID... just for this properties?
> > 
> > This is why we were a bit confused about adding a DT commit to this.
> 
> Just do what other PHYs do. ti,dp83869.yaml, motorcomm,yt8xxx.yaml,
> nxp,tja11xx.yaml, etc.
> 

Thanks I prepared a DT hoping it will be good in v2.
Simon Horman Oct. 3, 2023, 3:20 p.m. UTC | #5
On Sat, Sep 30, 2023 at 12:39:44PM +0200, Robert Marko wrote:
> Aquantia PHY-s require firmware to be loaded before they start operating.
> It can be automatically loaded in case when there is a SPI-NOR connected
> to Aquantia PHY-s or can be loaded from the host via MDIO.
> 
> This patch adds support for loading the firmware via MDIO as in most cases
> there is no SPI-NOR being used to save on cost.
> Firmware loading code itself is ported from mainline U-boot with cleanups.
> 
> The firmware has mixed values both in big and little endian.
> PHY core itself is big-endian but it expects values to be in little-endian.
> The firmware is little-endian but CRC-16 value for it is stored at the end
> of firmware in big-endian.
> 
> It seems the PHY does the conversion internally from firmware that is
> little-endian to the PHY that is big-endian on using the mailbox
> but mailbox returns a big-endian CRC-16 to verify the written data
> integrity.
> 
> Co-developed-by: Christian Marangi <ansuelsmth@gmail.com>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> Signed-off-by: Robert Marko <robimarko@gmail.com>

...

> diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c

...

> @@ -677,6 +735,142 @@ static int aqr107_wait_processor_intensive_op(struct phy_device *phydev)
>  	return 0;
>  }
>  
> +/* load data into the phy's memory */
> +static int aquantia_load_memory(struct phy_device *phydev, u32 addr,
> +				const u8 *data, size_t len)
> +{
> +	u16 crc = 0, up_crc;
> +	size_t pos;
> +
> +	/* PHY expect addr in LE */
> +	addr = cpu_to_le32(addr);

Hi Christian and Robert,


The type of addr us u32, but here it is assigned a __le32 value.

As flagged by Sparse.

> +
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1,
> +		      VEND1_GLOBAL_MAILBOX_INTERFACE1,
> +		      VEND1_GLOBAL_MAILBOX_INTERFACE1_CRC_RESET);
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1,
> +		      VEND1_GLOBAL_MAILBOX_INTERFACE3,
> +		      VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR(addr));
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1,
> +		      VEND1_GLOBAL_MAILBOX_INTERFACE4,
> +		      VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR(addr));
> +
> +	for (pos = 0; pos < len; pos += min(sizeof(u32), len - pos)) {
> +		u32 word = 0;
> +
> +		memcpy(&word, data + pos, min(sizeof(u32), len - pos));
> +
> +		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE5,
> +			      VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA(word));
> +		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE6,
> +			      VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA(word));
> +
> +		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE1,
> +			      VEND1_GLOBAL_MAILBOX_INTERFACE1_EXECUTE |
> +			      VEND1_GLOBAL_MAILBOX_INTERFACE1_WRITE);
> +
> +		/* calculate CRC as we load data to the mailbox.
> +		 * We convert word to big-endiang as PHY is BE and ailbox will
> +		 * return a BE crc.
> +		 */
> +		word = cpu_to_be32(word);
> +		crc = crc_ccitt_false(crc, (u8 *)&word, sizeof(word));
> +	}
> +
> +	up_crc = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE2);
> +	if (crc != up_crc) {
> +		phydev_err(phydev, "CRC mismatch: calculated 0x%04x PHY 0x%04x\n",
> +			   crc, up_crc);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int aqr_fw_boot(struct phy_device *phydev, const u8 *data, size_t size)
> +{
> +	const struct aqr_fw_header *header;
> +	u32 iram_offset = 0, iram_size = 0;
> +	u32 dram_offset = 0, dram_size = 0;
> +	char version[VERSION_STRING_SIZE];
> +	u16 calculated_crc, read_crc;
> +	u32 primary_offset = 0;
> +	int ret;
> +
> +	/* extract saved crc at the end of the fw */
> +	memcpy(&read_crc, data + size - 2, sizeof(read_crc));
> +	/* crc is saved in big-endian as PHY is BE */
> +	read_crc = be16_to_cpu(read_crc);

The type of read_crc is u16.
But be16_to_cpu expects a __be16 argument.

As flagged by Sparse.


> +	calculated_crc = crc_ccitt_false(0, data, size - 2);
> +	if (read_crc != calculated_crc) {
> +		phydev_err(phydev, "bad firmware CRC: file 0x%04x calculated 0x%04x\n",
> +			   read_crc, calculated_crc);
> +		return -EINVAL;
> +	}
> +
> +	/* Get the primary offset to extract DRAM and IRAM sections. */
> +	memcpy(&primary_offset, data + PRIMARY_OFFSET_OFFSET, sizeof(u16));
> +	primary_offset = PRIMARY_OFFSET(le32_to_cpu(primary_offset));

Similarly here.

> +
> +	/* Find the DRAM and IRAM sections within the firmware file. */
> +	header = (struct aqr_fw_header *)(data + primary_offset + HEADER_OFFSET);
> +	memcpy(&iram_offset, &header->iram_offset, sizeof(u8) * 3);
> +	memcpy(&iram_size, &header->iram_size, sizeof(u8) * 3);
> +	memcpy(&dram_offset, &header->dram_offset, sizeof(u8) * 3);
> +	memcpy(&dram_size, &header->dram_size, sizeof(u8) * 3);
> +
> +	/* offset are in LE and values needs to be converted to cpu endian */
> +	iram_offset = le32_to_cpu(iram_offset);
> +	iram_size = le32_to_cpu(iram_size);
> +	dram_offset = le32_to_cpu(dram_offset);
> +	dram_size = le32_to_cpu(dram_size);

And here (x4).

> +
> +	/* Increment the offset with the primary offset. */
> +	iram_offset += primary_offset;
> +	dram_offset += primary_offset;
> +
> +	phydev_dbg(phydev, "primary %d IRAM offset=%d size=%d DRAM offset=%d size=%d\n",
> +		   primary_offset, iram_offset, iram_size, dram_offset, dram_size);
> +
> +	strscpy(version, (char *)data + dram_offset + VERSION_STRING_OFFSET,
> +		VERSION_STRING_SIZE);
> +	phydev_info(phydev, "loading firmware version '%s'\n", version);
> +
> +	/* stall the microcprocessor */
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_CONTROL2,
> +		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL | VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD);
> +
> +	phydev_dbg(phydev, "loading DRAM 0x%08x from offset=%d size=%d\n",
> +		   DRAM_BASE_ADDR, dram_offset, dram_size);
> +	ret = aquantia_load_memory(phydev, DRAM_BASE_ADDR, data + dram_offset,
> +				   dram_size);
> +	if (ret)
> +		return ret;
> +
> +	phydev_dbg(phydev, "loading IRAM 0x%08x from offset=%d size=%d\n",
> +		   IRAM_BASE_ADDR, iram_offset, iram_size);
> +	ret = aquantia_load_memory(phydev, IRAM_BASE_ADDR, data + iram_offset,
> +				   iram_size);
> +	if (ret)
> +		return ret;
> +
> +	/* make sure soft reset and low power mode are clear */
> +	phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_SC,
> +			   VEND1_GLOBAL_SC_SOFT_RESET | VEND1_GLOBAL_SC_LOW_POWER);
> +
> +	/* Release the microprocessor. UP_RESET must be held for 100 usec. */
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_CONTROL2,
> +		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL |
> +		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD |
> +		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_RST);
> +	usleep_range(UP_RESET_SLEEP, UP_RESET_SLEEP * 2);
> +
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_CONTROL2,
> +		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD);
> +
> +	return 0;
> +}

...
Jakub Kicinski Oct. 4, 2023, 11:28 p.m. UTC | #6
On Sat, 30 Sep 2023 12:39:44 +0200 Robert Marko wrote:
> +	ret = of_property_read_string(dev->of_node, "firmware-name",
> +				      &fw_name);

Perhaps a well established weirdness of the embedded world but why read
the fw name from OF?! You can identify what PHY it is and decide the
file name based on that. And also put that fw name in MODULE_FIRMWARE()
so that initramfs can be built with appropriate file in place :S

> +	ret = request_firmware(&fw, fw_name, dev);
Andrew Lunn Oct. 5, 2023, 2:43 a.m. UTC | #7
On Wed, Oct 04, 2023 at 04:28:31PM -0700, Jakub Kicinski wrote:
> On Sat, 30 Sep 2023 12:39:44 +0200 Robert Marko wrote:
> > +	ret = of_property_read_string(dev->of_node, "firmware-name",
> > +				      &fw_name);
> 
> Perhaps a well established weirdness of the embedded world but why read
> the fw name from OF?! You can identify what PHY it is and decide the
> file name based on that. And also put that fw name in MODULE_FIRMWARE()
> so that initramfs can be built with appropriate file in place :S

The Aquantia PHY and its `firmware` is just weird. It is more than
just firmware, it also contains what i think they call provisioning.
That is basically the reset defaults for registers. And not everything
is documented, and i think parts of that provision contains SERDES eye
configuration. So i think you end up with a custom firmware per board?
And you can never trust the firmware in one device will do the same
thing as a different firmware in another device, because the reset
defaults are a bit fuzzy. The PHY driver is somewhat built on sand,
since you cannot really trust any register to have any specific reset
value.

So i can understand putting the board specific firmware name in DT,
and that the firmware will never be in linux-firmware because it would
not scale, and there never being one firmware usable for all
boards. And this odd way of doing things means the usual mechanisms
for getting the firmware in initramfs does not work.

I suppose the question is, do we want to say this is all too ugly for
Linux, solve it in the bootloader, or spend the extra $0.50 for a
flash chip and put the firmware in at the factory. As a kernel
developer i would want the boot loader to solve this, so i can TFTP
boot the kernel. I would also like having a rescue mechanism for when
i brick Linux on the box and need to boot a Debian install image to
recover it.

    Andrew
Jakub Kicinski Oct. 5, 2023, 2:24 p.m. UTC | #8
On Thu, 5 Oct 2023 04:43:51 +0200 Andrew Lunn wrote:
> The Aquantia PHY and its `firmware` is just weird. It is more than
> just firmware, it also contains what i think they call provisioning.
> That is basically the reset defaults for registers. And not everything
> is documented, and i think parts of that provision contains SERDES eye
> configuration. So i think you end up with a custom firmware per board?

Ah, that makes sense, thanks for explaining.
diff mbox series

Patch

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 107880d13d21..558fef3d9491 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -98,6 +98,7 @@  config ADIN1100_PHY
 
 config AQUANTIA_PHY
 	tristate "Aquantia PHYs"
+	select CRC_CCITT
 	help
 	  Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405
 
diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c
index 334a6904ca5a..d343df3d0e61 100644
--- a/drivers/net/phy/aquantia_main.c
+++ b/drivers/net/phy/aquantia_main.c
@@ -12,6 +12,10 @@ 
 #include <linux/delay.h>
 #include <linux/bitfield.h>
 #include <linux/phy.h>
+#include <linux/of.h>
+#include <linux/firmware.h>
+#include <linux/crc-ccitt.h>
+#include <linux/nvmem-consumer.h>
 
 #include "aquantia.h"
 
@@ -92,10 +96,40 @@ 
 #define MDIO_C22EXT_STAT_SGMII_TX_RUNT_FRAMES		0xd31b
 
 /* Vendor specific 1, MDIO_MMD_VEND1 */
+#define VEND1_GLOBAL_SC				0x0
+#define VEND1_GLOBAL_SC_SOFT_RESET		BIT(15)
+#define VEND1_GLOBAL_SC_LOW_POWER		BIT(11)
+
 #define VEND1_GLOBAL_FW_ID			0x0020
 #define VEND1_GLOBAL_FW_ID_MAJOR		GENMASK(15, 8)
 #define VEND1_GLOBAL_FW_ID_MINOR		GENMASK(7, 0)
 
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1			0x0200
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1_EXECUTE		BIT(15)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1_WRITE		BIT(14)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1_CRC_RESET	BIT(12)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1_BUSY		BIT(8)
+
+#define VEND1_GLOBAL_MAILBOX_INTERFACE2			0x0201
+#define VEND1_GLOBAL_MAILBOX_INTERFACE3			0x0202
+#define VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR_MASK	GENMASK(15, 0)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR(x)	FIELD_PREP(VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR_MASK, (u16)((x) >> 16))
+#define VEND1_GLOBAL_MAILBOX_INTERFACE4			0x0203
+#define VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR_MASK	GENMASK(15, 2)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR(x)	FIELD_PREP(VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR_MASK, (u16)(x))
+
+#define VEND1_GLOBAL_MAILBOX_INTERFACE5			0x0204
+#define VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA_MASK	GENMASK(15, 0)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA(x)	FIELD_PREP(VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA_MASK, (u16)((x) >> 16))
+#define VEND1_GLOBAL_MAILBOX_INTERFACE6			0x0205
+#define VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA_MASK	GENMASK(15, 0)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA(x)	FIELD_PREP(VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA_MASK, (u16)(x))
+
+#define VEND1_GLOBAL_CONTROL2			0xc001
+#define VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_RST	BIT(15)
+#define VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD	BIT(6)
+#define VEND1_GLOBAL_CONTROL2_UP_RUN_STALL	BIT(0)
+
 #define VEND1_GLOBAL_GEN_STAT2			0xc831
 #define VEND1_GLOBAL_GEN_STAT2_OP_IN_PROG	BIT(15)
 
@@ -152,6 +186,30 @@ 
 #define AQR107_OP_IN_PROG_SLEEP		1000
 #define AQR107_OP_IN_PROG_TIMEOUT	100000
 
+#define UP_RESET_SLEEP		100
+
+/* addresses of memory segments in the phy */
+#define DRAM_BASE_ADDR		0x3FFE0000
+#define IRAM_BASE_ADDR		0x40000000
+
+/* firmware image format constants */
+#define VERSION_STRING_SIZE		0x40
+#define VERSION_STRING_OFFSET		0x0200
+/* primary offset is written at an offset from the start of the fw blob */
+#define PRIMARY_OFFSET_OFFSET		0x8
+/* primary offset needs to be then added to a base offset */
+#define PRIMARY_OFFSET_SHIFT		12
+#define PRIMARY_OFFSET(x)		((x) << PRIMARY_OFFSET_SHIFT)
+#define HEADER_OFFSET			0x300
+
+struct aqr_fw_header {
+	u32 padding;
+	u8 iram_offset[3];
+	u8 iram_size[3];
+	u8 dram_offset[3];
+	u8 dram_size[3];
+} __packed;
+
 struct aqr107_hw_stat {
 	const char *name;
 	int reg;
@@ -677,6 +735,142 @@  static int aqr107_wait_processor_intensive_op(struct phy_device *phydev)
 	return 0;
 }
 
+/* load data into the phy's memory */
+static int aquantia_load_memory(struct phy_device *phydev, u32 addr,
+				const u8 *data, size_t len)
+{
+	u16 crc = 0, up_crc;
+	size_t pos;
+
+	/* PHY expect addr in LE */
+	addr = cpu_to_le32(addr);
+
+	phy_write_mmd(phydev, MDIO_MMD_VEND1,
+		      VEND1_GLOBAL_MAILBOX_INTERFACE1,
+		      VEND1_GLOBAL_MAILBOX_INTERFACE1_CRC_RESET);
+	phy_write_mmd(phydev, MDIO_MMD_VEND1,
+		      VEND1_GLOBAL_MAILBOX_INTERFACE3,
+		      VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR(addr));
+	phy_write_mmd(phydev, MDIO_MMD_VEND1,
+		      VEND1_GLOBAL_MAILBOX_INTERFACE4,
+		      VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR(addr));
+
+	for (pos = 0; pos < len; pos += min(sizeof(u32), len - pos)) {
+		u32 word = 0;
+
+		memcpy(&word, data + pos, min(sizeof(u32), len - pos));
+
+		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE5,
+			      VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA(word));
+		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE6,
+			      VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA(word));
+
+		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE1,
+			      VEND1_GLOBAL_MAILBOX_INTERFACE1_EXECUTE |
+			      VEND1_GLOBAL_MAILBOX_INTERFACE1_WRITE);
+
+		/* calculate CRC as we load data to the mailbox.
+		 * We convert word to big-endiang as PHY is BE and ailbox will
+		 * return a BE crc.
+		 */
+		word = cpu_to_be32(word);
+		crc = crc_ccitt_false(crc, (u8 *)&word, sizeof(word));
+	}
+
+	up_crc = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE2);
+	if (crc != up_crc) {
+		phydev_err(phydev, "CRC mismatch: calculated 0x%04x PHY 0x%04x\n",
+			   crc, up_crc);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int aqr_fw_boot(struct phy_device *phydev, const u8 *data, size_t size)
+{
+	const struct aqr_fw_header *header;
+	u32 iram_offset = 0, iram_size = 0;
+	u32 dram_offset = 0, dram_size = 0;
+	char version[VERSION_STRING_SIZE];
+	u16 calculated_crc, read_crc;
+	u32 primary_offset = 0;
+	int ret;
+
+	/* extract saved crc at the end of the fw */
+	memcpy(&read_crc, data + size - 2, sizeof(read_crc));
+	/* crc is saved in big-endian as PHY is BE */
+	read_crc = be16_to_cpu(read_crc);
+	calculated_crc = crc_ccitt_false(0, data, size - 2);
+	if (read_crc != calculated_crc) {
+		phydev_err(phydev, "bad firmware CRC: file 0x%04x calculated 0x%04x\n",
+			   read_crc, calculated_crc);
+		return -EINVAL;
+	}
+
+	/* Get the primary offset to extract DRAM and IRAM sections. */
+	memcpy(&primary_offset, data + PRIMARY_OFFSET_OFFSET, sizeof(u16));
+	primary_offset = PRIMARY_OFFSET(le32_to_cpu(primary_offset));
+
+	/* Find the DRAM and IRAM sections within the firmware file. */
+	header = (struct aqr_fw_header *)(data + primary_offset + HEADER_OFFSET);
+	memcpy(&iram_offset, &header->iram_offset, sizeof(u8) * 3);
+	memcpy(&iram_size, &header->iram_size, sizeof(u8) * 3);
+	memcpy(&dram_offset, &header->dram_offset, sizeof(u8) * 3);
+	memcpy(&dram_size, &header->dram_size, sizeof(u8) * 3);
+
+	/* offset are in LE and values needs to be converted to cpu endian */
+	iram_offset = le32_to_cpu(iram_offset);
+	iram_size = le32_to_cpu(iram_size);
+	dram_offset = le32_to_cpu(dram_offset);
+	dram_size = le32_to_cpu(dram_size);
+
+	/* Increment the offset with the primary offset. */
+	iram_offset += primary_offset;
+	dram_offset += primary_offset;
+
+	phydev_dbg(phydev, "primary %d IRAM offset=%d size=%d DRAM offset=%d size=%d\n",
+		   primary_offset, iram_offset, iram_size, dram_offset, dram_size);
+
+	strscpy(version, (char *)data + dram_offset + VERSION_STRING_OFFSET,
+		VERSION_STRING_SIZE);
+	phydev_info(phydev, "loading firmware version '%s'\n", version);
+
+	/* stall the microcprocessor */
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_CONTROL2,
+		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL | VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD);
+
+	phydev_dbg(phydev, "loading DRAM 0x%08x from offset=%d size=%d\n",
+		   DRAM_BASE_ADDR, dram_offset, dram_size);
+	ret = aquantia_load_memory(phydev, DRAM_BASE_ADDR, data + dram_offset,
+				   dram_size);
+	if (ret)
+		return ret;
+
+	phydev_dbg(phydev, "loading IRAM 0x%08x from offset=%d size=%d\n",
+		   IRAM_BASE_ADDR, iram_offset, iram_size);
+	ret = aquantia_load_memory(phydev, IRAM_BASE_ADDR, data + iram_offset,
+				   iram_size);
+	if (ret)
+		return ret;
+
+	/* make sure soft reset and low power mode are clear */
+	phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_SC,
+			   VEND1_GLOBAL_SC_SOFT_RESET | VEND1_GLOBAL_SC_LOW_POWER);
+
+	/* Release the microprocessor. UP_RESET must be held for 100 usec. */
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_CONTROL2,
+		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL |
+		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD |
+		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_RST);
+	usleep_range(UP_RESET_SLEEP, UP_RESET_SLEEP * 2);
+
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_CONTROL2,
+		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD);
+
+	return 0;
+}
+
 static int aqr107_get_rate_matching(struct phy_device *phydev,
 				    phy_interface_t iface)
 {
@@ -711,13 +905,95 @@  static int aqr107_resume(struct phy_device *phydev)
 	return aqr107_wait_processor_intensive_op(phydev);
 }
 
+static int aqr_firmware_load_nvmem(struct phy_device *phydev)
+{
+	struct nvmem_cell *cell;
+	size_t size;
+	u8 *buf;
+	int ret;
+
+	cell = nvmem_cell_get(&phydev->mdio.dev, "firmware");
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	buf = nvmem_cell_read(cell, &size);
+	if (IS_ERR(buf)) {
+		ret = PTR_ERR(buf);
+		goto exit;
+	}
+
+	ret = aqr_fw_boot(phydev, buf, size);
+	if (ret)
+		phydev_err(phydev, "firmware loading failed: %d\n", ret);
+
+exit:
+	nvmem_cell_put(cell);
+
+	return ret;
+}
+
+static int aqr_firmware_load_sysfs(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	const struct firmware *fw;
+	const char *fw_name;
+	int ret;
+
+	ret = of_property_read_string(dev->of_node, "firmware-name",
+				      &fw_name);
+	if (ret)
+		return ret;
+
+	ret = request_firmware(&fw, fw_name, dev);
+	if (ret) {
+		phydev_err(phydev, "failed to find FW file %s (%d)\n",
+			   fw_name, ret);
+		goto exit;
+	}
+
+	ret = aqr_fw_boot(phydev, fw->data, fw->size);
+	if (ret)
+		phydev_err(phydev, "firmware loading failed: %d\n", ret);
+
+exit:
+	release_firmware(fw);
+
+	return ret;
+}
+
+static int aqr_firmware_load(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_FW_ID);
+	if (ret > 0)
+		goto exit;
+
+	ret = aqr_firmware_load_nvmem(phydev);
+	if (!ret)
+		goto exit;
+
+	ret = aqr_firmware_load_sysfs(phydev);
+	if (ret)
+		return ret;
+
+exit:
+	return 0;
+}
+
 static int aqr107_probe(struct phy_device *phydev)
 {
+	int ret;
+
 	phydev->priv = devm_kzalloc(&phydev->mdio.dev,
 				    sizeof(struct aqr107_priv), GFP_KERNEL);
 	if (!phydev->priv)
 		return -ENOMEM;
 
+	ret = aqr_firmware_load(phydev);
+	if (ret)
+		return ret;
+
 	return aqr_hwmon_probe(phydev);
 }