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 <[email protected]>
Signed-off-by: Christian Marangi <[email protected]>
Signed-off-by: Robert Marko <[email protected]>
---
drivers/net/phy/Kconfig | 1 +
drivers/net/phy/aquantia_main.c | 276 ++++++++++++++++++++++++++++++++
2 files changed, 277 insertions(+)
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);
}
--
2.41.0
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
--
Ansuel
> 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
> +/* 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
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.
--
Ansuel
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 <[email protected]>
> Signed-off-by: Christian Marangi <[email protected]>
> Signed-off-by: Robert Marko <[email protected]>
...
> 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;
> +}
...
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);
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
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.