From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 37718C38A2D for ; Wed, 26 Oct 2022 17:55:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234262AbiJZRzK (ORCPT ); Wed, 26 Oct 2022 13:55:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234263AbiJZRzG (ORCPT ); Wed, 26 Oct 2022 13:55:06 -0400 Received: from mail-pj1-x1029.google.com (mail-pj1-x1029.google.com [IPv6:2607:f8b0:4864:20::1029]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 35D7C18B1F for ; Wed, 26 Oct 2022 10:55:00 -0700 (PDT) Received: by mail-pj1-x1029.google.com with SMTP id u8-20020a17090a5e4800b002106dcdd4a0so3402087pji.1 for ; Wed, 26 Oct 2022 10:55:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=plC5l3eW8NLTPswr03UrFtb1fJfJcQ4i9k2SpOtUr38=; b=zZDxS9pn1W2QmACkqj07UdwIqFEDd9O44rzzvX8f8K4K5DPgO0tEsbVMoPmj4IK22N k5h1pMqNmbGO1upupBC1JeUTVRjg62u7x/sXdLSyetEvKXTHZ5lct5OCTSfgs+UWu0A+ xCk/RKHcgP7JbU9coSSyd3WRthiuJWp7X4upphMQ89plnzVDyK+iw55YeJhJZON+o3+k 96P1E4yPL/U9qYCwXEXGu6sO0hz96J53SDHcQoMd+Z0jGM74OjKVgeA0DvSUt9Hk8dS5 usLKUTCE7N3NxCtkA1QUTsVrmDL9qq37flVeTIQysugGy9pN9crHwmhs65B7x1mZVpOi bW0g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=plC5l3eW8NLTPswr03UrFtb1fJfJcQ4i9k2SpOtUr38=; b=KKnF0876D1vekmBbslEyfMvopLUUKhvydQ1hk2qaMpB/QpMVMPFxv+cVmbKJkmbIi7 +zqGBKWEuI4WXTInPMyIQ8yO/lMFJ0Su68BXIfe02N+bkXpCy+fieYJ1s2WE2Z78U5sq TD+to1QMOx2rhYEQIS4NgbNDGS/LG420Lpf+lH1DMrGYJK9F98YkXkUMHBp/JBZYNX6g Oe6vDLrigZCg3pPfpyCyDL0r9Ht3IX61hEeEIwoiWmxgJbkmdl1My3VCj/20R+XOS0Iz v06KEtMvJ3WWASw6kzeF+Q8H3mLi7UAxYBMs8Fw2EIVNhrET1sT8dAbTgQmTPFOv1ijG UfhA== X-Gm-Message-State: ACrzQf0d2AIP0uQZNgqlZWtJiYxC2sUk8v+RQnhf1ndEOJjbFJny1uH4 EGf5S5UvTOeNv2uSETCKGJJrDtkq1/rX0Q== X-Google-Smtp-Source: AMsMyM6wjepfziEwpGXc/1nNckLqcF/KGnNqfopH/DxsG/4h0K520rOA/6LwfnH2dfCx/Z/YuA8fGg== X-Received: by 2002:a17:902:860a:b0:186:7eab:afa2 with SMTP id f10-20020a170902860a00b001867eabafa2mr27931705plo.46.1666806900081; Wed, 26 Oct 2022 10:55:00 -0700 (PDT) Received: from p14s (S0106889e681aac74.cg.shawcable.net. [68.147.0.187]) by smtp.gmail.com with ESMTPSA id c81-20020a621c54000000b0056abff42a8bsm3278491pfc.69.2022.10.26.10.54.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Oct 2022 10:54:59 -0700 (PDT) Date: Wed, 26 Oct 2022 11:54:57 -0600 From: Mathieu Poirier To: Daniel Kestrel Cc: Bjorn Andersson , Rob Herring , linux-remoteproc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 3/3] remoteproc: Add AVM WASP driver Message-ID: <20221026175457.GA1171927@p14s> References: <20220804210806.4053-1-kestrelseventyfour@gmail.com> <20220804210806.4053-4-kestrelseventyfour@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20220804210806.4053-4-kestrelseventyfour@gmail.com> Precedence: bulk List-ID: X-Mailing-List: linux-remoteproc@vger.kernel.org On Thu, Aug 04, 2022 at 11:08:06PM +0200, Daniel Kestrel wrote: > Some AVM Fritzbox router boards (3390, 3490, 5490, 5491, 7490), > that are Lantiq XRX200 based, have a memory only ATH79 based > WASP (Wireless Assistant Support Processor) SoC that has wifi > cards connected to it. It does not share anything with the > Lantiq host and has no persistent storage. It has an mdio based > connection for bringing up a small network boot firmware and is > connected to the Lantiq GSWIP switch via gigabit ethernet. This > is used to load an initramfs linux image to it, after the > network boot firmware was started. > > In order to initialize this remote processor we need to: > - power on the SoC using power gpio > - reset the SoC using the reset gpio > - send the network boot firmware using mdio > - send the linux image using raw ethernet frames > > This driver allows to start and stop the WASP SoC. > > Signed-off-by: Daniel Kestrel > Tested-by: Timo Dorfner # tested on Fritzbox 7490 > --- > drivers/remoteproc/Kconfig | 10 + > drivers/remoteproc/Makefile | 1 + > drivers/remoteproc/avm_wasp.c | 1051 +++++++++++++++++++++++++++++++++ > 3 files changed, 1062 insertions(+) > create mode 100644 drivers/remoteproc/avm_wasp.c > > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig > index 166019786653..a761186c5171 100644 > --- a/drivers/remoteproc/Kconfig > +++ b/drivers/remoteproc/Kconfig > @@ -23,6 +23,16 @@ config REMOTEPROC_CDEV > > It's safe to say N if you don't want to use this interface. > > +config AVM_WASP_REMOTEPROC > + tristate "AVM WASP remoteproc support" > + depends on NET_DSA_LANTIQ_GSWIP > + help > + Say y here to support booting the secondary SoC ATH79 target > + called Wireless Assistant Support Processor (WASP) that some > + AVM Fritzbox devices (3390, 3490, 5490, 5491, 7490) have built in. > + > + It's safe to say N here. > + > config IMX_REMOTEPROC > tristate "i.MX remoteproc support" > depends on ARCH_MXC > diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile > index 5478c7cb9e07..0ae175c6722f 100644 > --- a/drivers/remoteproc/Makefile > +++ b/drivers/remoteproc/Makefile > @@ -11,6 +11,7 @@ remoteproc-y += remoteproc_sysfs.o > remoteproc-y += remoteproc_virtio.o > remoteproc-y += remoteproc_elf_loader.o > obj-$(CONFIG_REMOTEPROC_CDEV) += remoteproc_cdev.o > +obj-$(CONFIG_AVM_WASP_REMOTEPROC) += avm_wasp.o > obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o > obj-$(CONFIG_IMX_DSP_REMOTEPROC) += imx_dsp_rproc.o > obj-$(CONFIG_INGENIC_VPU_RPROC) += ingenic_rproc.o > diff --git a/drivers/remoteproc/avm_wasp.c b/drivers/remoteproc/avm_wasp.c > new file mode 100644 > index 000000000000..6eda4db5cf4d > --- /dev/null > +++ b/drivers/remoteproc/avm_wasp.c > @@ -0,0 +1,1051 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * AVM WASP Remote Processor driver > + * > + * Copyright (c) 2019-2020 Andreas Böhler > + * Copyright (c) 2021-2022 Daniel Kestrel > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "remoteproc_internal.h" > + > +#define WASP_CHUNK_SIZE 14 > +#define WASP_ADDR 0x07 > +#define WASP_TIMEOUT_COUNT 1000 > +#define WASP_WAIT_TIMEOUT_COUNT 20 > +#define WASP_CHECK_LEN_DIVBY4_MASK 0x3 > + > +#define WASP_WAIT_SLEEP_US_LOW 50000 > +#define WASP_WAIT_SLEEP_US 100000 > +#define WASP_POLL_SLEEP_US 200 > + > +#define WASP_RESP_OK 0x0002 > +#define WASP_RESP_READY_TO_START 0x0202 > + > +#define WASP_CMD_SET_PARAMS 0x0c01 > +#define WASP_CMD_SET_CHECKSUM_3390 0x0801 > +#define WASP_CMD_SET_CHECKSUM_X490 0x0401 > +#define WASP_CMD_SET_DATA 0x0e01 > +#define WASP_CMD_START_FIRMWARE_3390 0x0201 > +#define WASP_CMD_START_FIRMWARE_X490 0x0001 > +#define WASP_CMD_START_FIRMWARE2_X490 0x0101 > + > +#define ETH_TYPE_ATH_ECPS_FRAME 0x88bd > +#define ETH_BUF_SIZE 1056 > +#define ETH_SEND_LOOP_TIMEOUT_SECS 60 > +#define ETH_MAX_DATA_SIZE 1028 > +#define ETH_DATA_SIZE 1024 > +#define ETH_WASP_PACKET_ID 0x1200 > + > +#define CMD_FIRMWARE_DATA 0x0104 > +#define CMD_START_FIRMWARE 0xd400 > + > +#define RESP_DISCOVER 0x0000 > +#define RESP_OK 0x0100 > +#define RESP_STARTING 0x0200 > +#define RESP_ERROR 0x0300 > + > +static const u32 m_load_addr = 0x81a00000; > +static const u32 m_start_and_exec_addr = 0xbd003000; > + > +static const char mac_data[WASP_CHUNK_SIZE] = {0xaa, 0xaa, 0xaa, 0xaa, 0xaa, > + 0xaa, 0x04, 0x20, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00}; > + > +static char wasp_mac[] = {0x00, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa}; > + > +enum { > + MODEL_3390, > + MODEL_X490, > + MODEL_UNKNOWN > +} m_model = MODEL_UNKNOWN; > + > +struct wasp_packet { > + struct ethhdr eh; > + struct __packed { > + u16 packet_start; > + u8 pad_one[5]; > + u16 command; > + u16 response; > + u16 counter; > + u8 pad_two; > + } hdr; > + u8 payload[ETH_MAX_DATA_SIZE]; > +} __packed; > + > +static char *firmware = "ath9k-eeprom-ahb-18100000.wmac.bin"; > +module_param(firmware, charp, 0444); > +MODULE_PARM_DESC(firmware, > + "Filename of the ath9k eeprom to be loaded"); > + > +static char *caldata = "ath10k/cal-pci-0000:00:00.0.bin"; > +module_param(caldata, charp, 0444); > +MODULE_PARM_DESC(caldata, > + "Filename of the ath10k caldata to be loaded"); > + > +static char *netboot = "netboot.fw"; > +module_param(netboot, charp, 0444); > +MODULE_PARM_DESC(netboot, > + "Filename of the network boot firmware for WASP"); > + > +static char *image = "wasp-image.bin"; > +module_param(image, charp, 0444); > +MODULE_PARM_DESC(image, > + "Filename of the linux image to be loaded to WASP"); > + > +/** > + * struct avm_wasp_rproc - avmwasp remote processor priv > + * @rproc: rproc handle > + * @pdev: pointer to platform device > + * @fw_blob: pointer to load and save any firmware > + * @linux_blob: pointer to access initramfs image > + * @mdio_bus: pointer to mii_bus of gswip device for gpio > + * @power_gpio: store WASP power gpio descriptor > + * @reset_gpio: store WASP reset gpio descriptor > + * @loader_port: store name of the port wasp is connected to > + * @buffer: recv buffer for feedback from WASP > + * @ifindex: interface index used for WASP communication > + */ > +struct avm_wasp_rproc { > + struct rproc *rproc; > + struct platform_device *pdev; > + const struct firmware *fw_blob, *linux_blob; > + struct mii_bus *mdio_bus; > + struct gpio_desc *power_gpio, *reset_gpio; > + char *loader_port; > + char buffer[ETH_BUF_SIZE]; > + int ifindex; > +}; > + > +/** > + * avm_wasp_netboot_mdio_write_u32_split() - write 32bit value > + * @avmwasp: pointer to drivers private avm_wasp_rproc structure > + * @mdio_reg: register start value for two mdio registers to write to > + * @value: value to be written to the register > + * > + * As the mdio registers are 16bit, this function writes a 32bit value > + * to two subsequent mdio registers starting with the specified register > + * for the mdio address that is used for the connection to the WASP SoC > + * > + * Return: 0 on Success, -ETIMEDOUT if avm_wasp_netboot_mdio_write fails > + */ > +static int avm_wasp_netboot_mdio_write_u32_split(struct avm_wasp_rproc *avmwasp, > + u32 mdio_reg, const u32 value) > +{ > + struct device *dev = &avmwasp->pdev->dev; > + int ret; > + > + ret = mdiobus_write(avmwasp->mdio_bus, WASP_ADDR, mdio_reg, > + ((value & 0xffff0000) >> 16)); > + if (ret < 0) > + goto err; > + > + ret = mdiobus_write(avmwasp->mdio_bus, WASP_ADDR, mdio_reg + 2, > + (value & 0x0000ffff)); > + if (ret < 0) > + goto err; > + > + return 0; > +err: > + dev_err(dev, "mdio split write failed\n"); > + return ret; > +} > + > +/** > + * avm_wasp_read_poll_timeout() - wrap read_poll_timeout_macro > + * @avmwasp: pointer to drivers private avm_wasp_rproc structure > + * @udelay: microseconds to wait after read > + * @utimeout: timeout value in microseconds > + * @checkval: value to be checked against > + * > + * This function checks checkval against the WASP mdio status register, > + * waits udelay before repeating the read and times out after utimeout. > + * Separate function because every other use of read_poll_timeout makes > + * the kernel module around half a kB larger. > + * > + * Return: 0 when checkval was read or -ETIMEDOUT if not > + */ > +static int avm_wasp_read_poll_timeout(struct avm_wasp_rproc *avmwasp, > + u32 udelay, u32 utimeout, u32 checkval) > +{ > + u32 val; > + > + return read_poll_timeout(mdiobus_read, val, > + (val == checkval), udelay, utimeout, false, > + avmwasp->mdio_bus, WASP_ADDR, 0x0); > +} > + > +/** > + * avm_wasp_netboot_write_header() - write header to WASP > + * @avmwasp: pointer to drivers private avm_wasp_rproc structure > + * @start_exec_addr: address where to load the firmware to on WASP and where > + * to start it from > + * @len: length of the network boot firmware > + * > + * Writes the header to WASP using mdio to initiate the start of > + * transferring the network boot firmware to WASP > + * > + * Return: 0 on Success, -ETIMEDOUT if writing header failed based on return > + * code from WASP or the write methods > + */ > +static int avm_wasp_netboot_write_header(struct avm_wasp_rproc *avmwasp, > + const u32 start_exec_addr, Do you see @start_exec_addr changing? If so it should probably be taken from the device tree. Otherwise I see little value in it being a function parameter. > + const u32 len) > +{ > + struct device *dev = &avmwasp->pdev->dev; > + int ret; > + > + ret = avm_wasp_netboot_mdio_write_u32_split(avmwasp, 0x2, Please use #defines rather than hard code values. > + start_exec_addr); > + if (ret < 0) > + goto err; > + > + ret = avm_wasp_netboot_mdio_write_u32_split(avmwasp, 0x6, len); > + if (ret < 0) > + goto err; > + > + ret = avm_wasp_netboot_mdio_write_u32_split(avmwasp, 0xA, > + start_exec_addr); > + if (ret < 0) > + goto err; > + > + ret = mdiobus_write(avmwasp->mdio_bus, WASP_ADDR, 0x0, > + WASP_CMD_SET_PARAMS); > + if (ret < 0) > + goto err; > + > + ret = avm_wasp_read_poll_timeout(avmwasp, WASP_POLL_SLEEP_US, > + WASP_TIMEOUT_COUNT * WASP_POLL_SLEEP_US, > + WASP_RESP_OK); > + if (ret < 0) > + goto err; > + > + return 0; > +err: > + dev_err(dev, "mdio write for header failed\n"); > + return ret; > +} > + > +/** > + * avm_wasp_netboot_write_checksum() - write checksum to WASP > + * @avmwasp: pointer to drivers private avm_wasp_rproc structure > + * @checksum: calculated checksum value to be sent to WASP > + * > + * Writes the calculated checksum for the given network boot firmware > + * to WASP using mdio as the second step > + * > + * Return: 0 on Success, -ETIMEDOUT if writing checksum failed based on > + * return code from WASP or the write methods > + */ > +static int avm_wasp_netboot_write_checksum(struct avm_wasp_rproc *avmwasp, > + const u32 checksum) > +{ > + struct device *dev = &avmwasp->pdev->dev; > + int ret; > + > + ret = avm_wasp_netboot_mdio_write_u32_split(avmwasp, 0x2, checksum); > + if (ret < 0) > + goto err; > + > + if (m_model == MODEL_3390) { > + ret = avm_wasp_netboot_mdio_write_u32_split(avmwasp, 0x6, 0x0000); > + if (ret < 0) > + goto err; > + > + ret = mdiobus_write(avmwasp->mdio_bus, WASP_ADDR, 0x0, > + WASP_CMD_SET_CHECKSUM_3390); > + } else if (m_model == MODEL_X490) { > + ret = mdiobus_write(avmwasp->mdio_bus, WASP_ADDR, 0x0, > + WASP_CMD_SET_CHECKSUM_X490); > + } Seems like writing the checksum to the remote processor follows a different protocol based on the model. Some comments, along with using #defines rather than hard coded values, would be useful. > + if (ret < 0) > + goto err; > + > + ret = avm_wasp_read_poll_timeout(avmwasp, WASP_POLL_SLEEP_US, > + WASP_TIMEOUT_COUNT * WASP_POLL_SLEEP_US, > + WASP_RESP_OK); > + if (ret < 0) > + goto err; > + > + return 0; > +err: > + dev_err(dev, "mdio write for checksum failed\n"); > + return ret; > +} > + > +/** > + * avm_wasp_netboot_write_chunk() - write chunk of data to WASP > + * @avmwasp: pointer to drivers private avm_wasp_rproc structure > + * @data: pointer to data > + * @len: length of data (should not exceed 14 bytes) > + * > + * Writes up to 14 bytes of data into the 7 16bit mdio registers > + * to WASP using mdio > + * > + * Return: 0 on Success, -EFAULT if data length is more than 14 bytes or > + * -ETIMEDOUT if writing the data failed based on return code from WASP > + * or the write methods > + */ > +static int avm_wasp_netboot_write_chunk(struct avm_wasp_rproc *avmwasp, > + const char *data, size_t len) > +{ > + struct device *dev = &avmwasp->pdev->dev; > + int i, ret = 0; > + > + if (len > WASP_CHUNK_SIZE || !data) > + return -EFAULT; > + > + for (i = 0; i < len && ret >= 0; i += 2) > + ret = mdiobus_write(avmwasp->mdio_bus, WASP_ADDR, i + 2, > + *((u16 *)(data + i))); > + if (ret < 0) > + goto err; > + > + ret = mdiobus_write(avmwasp->mdio_bus, WASP_ADDR, 0x0, WASP_CMD_SET_DATA); > + if (ret < 0) > + goto err; > + > + ret = avm_wasp_read_poll_timeout(avmwasp, WASP_POLL_SLEEP_US, > + WASP_TIMEOUT_COUNT * WASP_POLL_SLEEP_US, > + WASP_RESP_OK); > + if (ret < 0) > + goto err; > + > + return 0; > +err: > + dev_err(dev, "mdio write for data chunk failed\n"); > + return ret; > +} > + > +/** > + * avm_wasp_netboot_calc_checksum() - calculate netboot firmware checksum > + * @avmwasp: pointer to drivers private avm_wasp_rproc structure > + * > + * Calculates the checksum by using the firmware in fw_blob from the private > + * avm_wasp_rproc structure > + * > + * Return: Calculated checksum > + */ > +static u32 avm_wasp_netboot_calc_checksum(struct avm_wasp_rproc *avmwasp) > +{ > + u32 checksum = U32_MAX, count = U32_MAX, cs; > + const u8 *p_firmware, *p_firmware_end; > + > + p_firmware = avmwasp->fw_blob->data; > + p_firmware_end = p_firmware + avmwasp->fw_blob->size; > + > + while (p_firmware < p_firmware_end) { > + cs = be32_to_cpu(*(u32 *)p_firmware); > + checksum = checksum - cs; > + count++; > + p_firmware += 4; > + } > + > + checksum = checksum - count; > + return checksum; > +} > + > +/** > + * avm_wasp_netboot_load_firmware() - load netboot firmware to WASP > + * @avmwasp: pointer to drivers private avm_wasp_rproc structure > + * > + * First the status is checked if poweron and reset were successful. > + * Implements the process to send header, checksum and the firmware > + * blob in 14 byte chunks to the WASP processor using mdio > + * Includes checks between the steps and sending commands to start > + * the network boot firmware > + * > + * Return: 0 on Success, -ENODEV if WASP not ready after reset, > + * -ETIMEDOUT if there was a timeout on polling or > + * -EFAULT if other errors have occurred > + */ > +static int avm_wasp_netboot_load_firmware(struct avm_wasp_rproc *avmwasp) > +{ > + struct device *dev = &avmwasp->pdev->dev; > + struct mii_bus *mdio_bus = avmwasp->mdio_bus; > + const u8 *p_firmware, *p_firmware_end; > + int regval, regval2, ret, cont = 1; > + u32 checksum, left; > + > + ret = avm_wasp_read_poll_timeout(avmwasp, WASP_WAIT_SLEEP_US, > + WASP_WAIT_TIMEOUT_COUNT * > + WASP_WAIT_SLEEP_US, WASP_RESP_OK); > + if (ret) { > + dev_err(dev, "error WASP processor not in ready status\n"); > + return -ENODEV; > + } > + > + p_firmware = avmwasp->fw_blob->data; > + p_firmware_end = p_firmware + avmwasp->fw_blob->size; > + > + if ((avmwasp->fw_blob->size & WASP_CHECK_LEN_DIVBY4_MASK) || Some comments as to what is happening here would be appreciated. > + avmwasp->fw_blob->size > U16_MAX) { > + dev_err(dev, "error network boot firmware size\n"); > + return -EFAULT; > + } > + > + ret = avm_wasp_netboot_write_header(avmwasp, m_start_and_exec_addr, > + avmwasp->fw_blob->size); > + if (ret < 0) > + return ret; > + > + checksum = avm_wasp_netboot_calc_checksum(avmwasp); > + ret = avm_wasp_netboot_write_checksum(avmwasp, checksum); > + > + if (ret < 0) > + return ret; > + > + while (p_firmware < p_firmware_end) { > + left = p_firmware_end - p_firmware; > + if (left > WASP_CHUNK_SIZE) > + left = WASP_CHUNK_SIZE; > + ret = avm_wasp_netboot_write_chunk(avmwasp, p_firmware, left); > + if (ret < 0) > + return ret; > + > + p_firmware += left; > + } > + > + usleep_range(WASP_WAIT_SLEEP_US_LOW, WASP_WAIT_SLEEP_US); > + > + if (m_model == MODEL_3390) > + ret = mdiobus_write(mdio_bus, WASP_ADDR, 0x0, > + WASP_CMD_START_FIRMWARE_3390); > + else if (m_model == MODEL_X490) > + ret = mdiobus_write(mdio_bus, WASP_ADDR, 0x0, > + WASP_CMD_START_FIRMWARE_X490); > + if (ret < 0) { > + dev_err(dev, "writing command failed\n"); > + return ret; > + } > + > + usleep_range(WASP_WAIT_SLEEP_US_LOW, WASP_WAIT_SLEEP_US); > + > + ret = avm_wasp_read_poll_timeout(avmwasp, WASP_WAIT_SLEEP_US, > + WASP_WAIT_TIMEOUT_COUNT * > + WASP_WAIT_SLEEP_US, > + WASP_RESP_READY_TO_START); > + if (ret) { > + dev_err(dev, "timed out waiting for WASP ready to start\n"); > + return ret; > + } > + > + if (m_model == MODEL_3390) > + ret = mdiobus_write(mdio_bus, WASP_ADDR, 0x0, > + WASP_CMD_START_FIRMWARE_3390); > + else if (m_model == MODEL_X490) > + ret = mdiobus_write(mdio_bus, WASP_ADDR, 0x0, > + WASP_CMD_SET_CHECKSUM_X490); Are you sure this is WASP_CMD_SET_CHECKSUM_X490 and not WASP_CMD_START_FIRMWARE_X490? It also raises the question as to why it is done twice in a row. > + if (ret < 0) { > + dev_err(dev, "writing command failed\n"); > + return ret; > + } > + > + usleep_range(WASP_WAIT_SLEEP_US_LOW, WASP_WAIT_SLEEP_US); > + > + if (m_model == MODEL_3390) { > + ret = avm_wasp_read_poll_timeout(avmwasp, WASP_WAIT_SLEEP_US, > + WASP_WAIT_TIMEOUT_COUNT * > + WASP_WAIT_SLEEP_US * 10, > + WASP_RESP_OK); > + if (ret) { > + dev_err(dev, "timed out waiting for WASP OK\n"); > + return ret; > + } > + if (avm_wasp_netboot_write_chunk(avmwasp, mac_data, > + ARRAY_SIZE(mac_data)) < 0) { > + dev_err(dev, "error sending MAC address\n"); > + return -EFAULT; > + } > + } else if (m_model == MODEL_X490) { > + while (cont) { > + ret = avm_wasp_read_poll_timeout(avmwasp, > + WASP_WAIT_SLEEP_US, > + WASP_WAIT_TIMEOUT_COUNT * > + WASP_WAIT_SLEEP_US, > + WASP_RESP_OK); > + if (ret) { > + dev_err(dev, > + "timed out waiting for WASP OK\n"); > + return ret; > + } > + regval = mdiobus_read(mdio_bus, WASP_ADDR, 0x2); > + if (regval < 0) { > + dev_err(dev, "mdio read failed\n"); > + return ret; > + } > + regval2 = mdiobus_read(mdio_bus, WASP_ADDR, 0x4); > + if (regval2 < 0) { > + dev_err(dev, "mdio read failed\n"); > + return ret; > + } > + ret = mdiobus_write(mdio_bus, WASP_ADDR, 0x0, > + WASP_CMD_SET_CHECKSUM_X490); > + if (ret < 0) { > + dev_err(dev, "writing command failed\n"); > + return ret; > + } > + > + if (regval == 0 && regval2 != 0) > + cont = regval2; > + else > + cont--; What is this dance with @regval and @regval2 for X490? The overall absence of documentation in this driver makes reviewing it quite difficult. > + } > + > + ret = avm_wasp_read_poll_timeout(avmwasp, > + WASP_POLL_SLEEP_US, > + WASP_TIMEOUT_COUNT * > + WASP_POLL_SLEEP_US, > + WASP_RESP_OK); > + if (ret) { > + dev_err(dev, > + "error waiting for checksum OK response\n"); > + return ret; > + } > + > + ret = mdiobus_write(mdio_bus, WASP_ADDR, 0x2, 0x00); > + if (ret < 0) { > + dev_err(dev, "mdio write failed\n"); > + return ret; > + } > + ret = mdiobus_write(mdio_bus, WASP_ADDR, 0x0, > + WASP_CMD_START_FIRMWARE2_X490); > + if (ret < 0) { > + dev_err(dev, "writing command failed\n"); > + return ret; > + } > + > + regval = mdiobus_read(mdio_bus, WASP_ADDR, 0x0); > + if (regval != WASP_RESP_OK) { > + dev_err(dev, > + "error starting WASP network boot: 0x%x\n", > + regval); > + return -EFAULT; > + } > + } > + > + return 0; > +} > + > +/** > + * avm_wasp_load_initramfs_image() - load initramfs image to WASP > + * @avmwasp: pointer to drivers private avm_wasp_rproc structure > + * > + * Uses the lan port specified from DT to load the initramfs to > + * WASP after the network boot firmware was successfully started. > + * Communication is done by using raw sockets. > + * There are several commands and status values which are checked. > + * First a discovery packet is received and then each data packet > + * is acknowledged by the WASP network boot firmware. > + * First packet needs to prepend the load address and last packet > + * needs to append the execution address. > + * > + * Return: 0 on Success, -EFAULT if errors with the WASP send protocol > + * have occurred or if no firmware is found, -EAGAIN if the wasp network > + * interface is down or the error returned from the failed operating > + * system function or service > + */ > +static int avm_wasp_load_initramfs_image(struct avm_wasp_rproc *avmwasp) > +{ > + bool done = false; > + int ret; > + u32 num_chunks = 0, chunk_counter = 0; > + short interface_flags; > + const u8 *p_firmware, *p_firmware_end; > + struct device *dev = &avmwasp->pdev->dev; > + struct kvec socket_kvec; > + struct msghdr socket_msghdr; > + struct net_device *send_netdev; > + struct sockaddr send_sock_addr; > + struct sockaddr_ll send_socket_address; > + struct socket *wasp_socket; > + struct wasp_packet *packet = (struct wasp_packet *) > + (avmwasp->buffer); > + struct __kernel_old_timeval timeout; > + time64_t start_time, current_time; > + > + if (!avmwasp->linux_blob) { > + dev_err(dev, "error accessing initramfs image\n"); > + ret = -EFAULT; > + goto err; > + } > + > + p_firmware = avmwasp->linux_blob->data; > + p_firmware_end = p_firmware + avmwasp->linux_blob->size; > + > + ret = sock_create_kern(&init_net, PF_PACKET, SOCK_RAW, > + htons(ETH_TYPE_ATH_ECPS_FRAME), > + &wasp_socket); > + if (ret < 0) { > + dev_err(dev, "error opening recv socket: %d\n", ret); > + goto err; > + } > + > + timeout.tv_sec = 10; > + timeout.tv_usec = 0; 10 seconds! Why so much? > + ret = sock_setsockopt(wasp_socket, SOL_SOCKET, SO_RCVTIMEO_OLD, > + KERNEL_SOCKPTR(&timeout), sizeof(timeout)); > + if (ret < 0) { > + dev_err(dev, "error SO_RCVTIMEO recv socket: %d\n", ret); > + goto err_socket; > + } > + > + ret = sock_setsockopt(wasp_socket, SOL_SOCKET, SO_SNDTIMEO_OLD, > + KERNEL_SOCKPTR(&timeout), sizeof(timeout)); > + if (ret < 0) { > + dev_err(dev, "error SO_SNDTIMEO send socket: %d\n", ret); > + goto err_socket; > + } > + > + rcu_read_lock(); > + send_netdev = dev_get_by_name_rcu(sock_net(wasp_socket->sk), > + avmwasp->loader_port); > + if (send_netdev) > + interface_flags = (short)dev_get_flags(send_netdev); > + rcu_read_unlock(); > + > + if (IS_ERR_OR_NULL(send_netdev)) { Same comment as in my previous post. > + dev_err(dev, "error accessing net device\n"); > + ret = -ENODEV; > + goto err_socket; > + } > + > + if (!(interface_flags & IFF_UP && interface_flags & IFF_RUNNING)) { > + dev_err(dev, "error wasp interface %s is down\n", > + avmwasp->loader_port); > + ret = -EAGAIN; > + goto err_socket; > + } > + > + avmwasp->ifindex = send_netdev->ifindex; > + ret = dev_get_mac_address(&send_sock_addr, &init_net, > + avmwasp->loader_port); > + if (ret < 0) { > + dev_err(dev, "error getting mac address: %d\n", ret); > + goto err_socket; > + } > + > + send_socket_address.sll_halen = ETH_ALEN; > + send_socket_address.sll_ifindex = avmwasp->ifindex; > + memset(&socket_msghdr, 0, sizeof(socket_msghdr)); > + socket_msghdr.msg_name = (struct sockaddr *)&send_socket_address; > + socket_msghdr.msg_namelen = sizeof(struct sockaddr_ll); > + > + start_time = ktime_get_seconds(); > + > + while (!done) { > + current_time = ktime_get_seconds(); > + if ((current_time - start_time) > ETH_SEND_LOOP_TIMEOUT_SECS) { > + dev_err(dev, > + "waiting for packet from WASP timed out\n"); > + ret = -ETIMEDOUT; > + goto err_socket; > + } > + > + socket_kvec.iov_base = avmwasp->buffer; > + socket_kvec.iov_len = ETH_BUF_SIZE; > + ret = kernel_recvmsg(wasp_socket, > + &socket_msghdr, &socket_kvec, 1, > + ETH_BUF_SIZE, 0); > + > + if (ret < 0) { > + dev_err(dev, > + "error receiving any packet or timeout: %d\n", > + ret); > + goto err_socket; > + } > + > + if (ret < (sizeof(struct ethhdr) + sizeof(packet->hdr))) { > + dev_err(dev, > + "packet too small, discard and continue\n"); > + continue; > + } > + > + if (packet->eh.h_proto != ETH_TYPE_ATH_ECPS_FRAME) > + continue; > + > + memcpy(wasp_mac, packet->eh.h_source, sizeof(wasp_mac)); > + > + if (packet->hdr.packet_start == ETH_WASP_PACKET_ID) { if (packet->hdr.packet_start != ETH_WASP_PACKET_ID) { continue; That way you can push back everything that follows. > + switch (packet->hdr.response) { > + case RESP_DISCOVER: > + chunk_counter = 1; > + num_chunks = DIV_ROUND_UP(avmwasp->linux_blob->size, > + ETH_DATA_SIZE); > + fallthrough; > + case RESP_OK: > + memcpy(packet->eh.h_dest, wasp_mac, sizeof(packet->eh.h_dest)); > + packet->eh.h_proto = ETH_TYPE_ATH_ECPS_FRAME; > + memcpy(packet->eh.h_source, send_sock_addr.sa_data, > + sizeof(packet->eh.h_source)); > + > + if (p_firmware < p_firmware_end) { > + size_t bytestosend, send_len; > + u32 data_offset = 0; > + > + if (chunk_counter == 1) { > + memcpy(packet->payload, > + &m_load_addr, > + sizeof(m_load_addr)); > + data_offset = sizeof(m_load_addr); > + } > + > + if ((p_firmware_end - p_firmware) >= > + ETH_DATA_SIZE) Just put the above all on the same line. > + bytestosend = ETH_DATA_SIZE; > + else > + bytestosend = p_firmware_end - > + p_firmware; Same > + memcpy(&packet->payload[data_offset], > + p_firmware, bytestosend); > + p_firmware = p_firmware + ETH_DATA_SIZE; > + > + packet->hdr.packet_start = > + ETH_WASP_PACKET_ID; Same > + if (chunk_counter == num_chunks) { > + packet->hdr.response = > + CMD_START_FIRMWARE; same > + memcpy(&packet->payload > + [data_offset + bytestosend], > + &m_load_addr, > + sizeof(m_load_addr)); > + bytestosend += sizeof(m_load_addr); > + } else { > + packet->hdr.command = > + CMD_FIRMWARE_DATA; Same > + } > + packet->hdr.counter = > + (chunk_counter - 1) * 4; Same > + > + send_len = sizeof(struct ethhdr) > + + sizeof(packet->hdr) + bytestosend + > + data_offset; > + > + socket_kvec.iov_len = send_len; > + socket_kvec.iov_base = avmwasp->buffer; > + > + ret = kernel_sendmsg(wasp_socket, > + &socket_msghdr, > + &socket_kvec, > + 1, send_len); > + if (ret < 0) { > + dev_err(dev, > + "error sending to WASP %d\n", > + ret); Same > + goto err_socket; > + } > + > + chunk_counter++; > + } > + break; > + case RESP_ERROR: > + dev_err(dev, > + "received an WASP error packet\n"); Same > + ret = -EFAULT; > + goto err_socket; > + case RESP_STARTING: > + done = true; > + ret = 0; > + continue; > + break; > + default: > + dev_err(dev, "unknown packet, continue\n"); > + continue; > + break; > + } Overall the above is extremely hard to read. > + } > + } > + > +err_socket: > + wasp_socket->ops->release(wasp_socket); > +err: > + return ret; > +} > + > +/** > + * avm_wasp_rproc_start() - start the remote processor > + * @rproc: pointer to the rproc structure > + * > + * Starts the remote processor by initiating the reset process using > + * the reset_gpio. > + * As the first step, the network boot firmware is tried to be loaded > + * and started. > + * As a second step, the initramfs image is tried to be loaded > + * and started. > + * > + * Return: 0 on Success, -ENODEV or return code from the called function > + * if any other error occurred in the process of starting and loading > + * the firmware files to the WASP processor > + */ > +static int avm_wasp_rproc_start(struct rproc *rproc) > +{ > + struct avm_wasp_rproc *avmwasp = rproc->priv; > + struct device *dev = &avmwasp->pdev->dev; > + u32 pval; > + int ret; > + > + gpiod_set_value(avmwasp->reset_gpio, 0); > + usleep_range(WASP_WAIT_SLEEP_US_LOW, WASP_WAIT_SLEEP_US); > + gpiod_set_value(avmwasp->reset_gpio, 1); > + usleep_range(WASP_WAIT_SLEEP_US_LOW, WASP_WAIT_SLEEP_US); > + > + ret = request_firmware_direct((const struct firmware **) > + &avmwasp->fw_blob, netboot, dev); > + if (ret) { > + dev_err(dev, "could not load network boot firmware\n"); > + goto err; > + } > + > + ret = of_property_read_u32(dev->of_node, "mdio-device", &pval); > + if (ret) { > + dev_err(dev, "no mdio-device given\n"); > + goto err_release_fw; > + } else { > + struct device_node *mdio_node = > + of_find_node_by_phandle(pval); > + > + if (!mdio_node) { > + dev_err(dev, "get mdio-device failed\n"); > + ret = -ENODEV; > + goto err_release_fw; > + } else { > + avmwasp->mdio_bus = of_mdio_find_bus(mdio_node); > + of_node_put(mdio_node); > + if (!avmwasp->mdio_bus) { > + dev_err(dev, "mdio bus not found\n"); > + ret = -ENODEV; > + goto err_release_fw; > + } > + } > + } > + > + ret = avm_wasp_netboot_load_firmware(avmwasp); > + if (ret) > + goto err_put_device; > + > + ret = avm_wasp_load_initramfs_image(avmwasp); > + > +err_put_device: > + put_device(&avmwasp->mdio_bus->dev); > +err_release_fw: > + release_firmware(avmwasp->fw_blob); > +err: > + return ret; > +} > + > +/** > + * avm_wasp_rproc_stop() - stop the remote processor > + * @rproc: pointer to the rproc structure > + * > + * To stop the remote processor the reset gpio is used > + * > + * Return: 0 on Success > + */ > +static int avm_wasp_rproc_stop(struct rproc *rproc) > +{ > + struct avm_wasp_rproc *avmwasp = rproc->priv; > + > + gpiod_set_value(avmwasp->reset_gpio, 0); > + usleep_range(WASP_WAIT_SLEEP_US_LOW, WASP_WAIT_SLEEP_US); > + gpiod_set_value(avmwasp->reset_gpio, 1); > + usleep_range(WASP_WAIT_SLEEP_US_LOW, WASP_WAIT_SLEEP_US); > + > + return 0; > +} > + > +/** > + * avm_wasp_rproc_load() - noop to avoid the ELF binary defaults > + * @rproc: pointer to the rproc structure > + * @fw: pointer to firmware struct > + * > + * If a load function is not defined in the rproc_ops, then all the settings > + * like checking the firmware binary will default to ELF checks, which fail > + * in case of the bootable and compressed initramfs image for WASP. > + * This function stores the initramfs image that is loaded by the remote > + * processor framework during boot process into the priv for access by > + * the initramfs load function avm_wasp_load_initramfs_image(). > + * > + * Return: Always 0 > + */ > +static int avm_wasp_rproc_load(struct rproc *rproc, const struct firmware *fw) > +{ > + struct avm_wasp_rproc *avmwasp = rproc->priv; > + > + avmwasp->linux_blob = fw; Please add a comment to the effect that ->linux_blob is not valid outside of avm_wasp_rproc_start(). I am done reviewing this set. Thanks for the patience, Mathieu > + > + return 0; > +} > + > +static const struct rproc_ops avm_wasp_rproc_ops = { > + .start = avm_wasp_rproc_start, > + .stop = avm_wasp_rproc_stop, > + .load = avm_wasp_rproc_load, > +}; > + > +static int avm_wasp_rproc_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct avm_wasp_rproc *avmwasp; > + struct rproc *rproc; > + struct net_device *netdev; > + int ret; > + short interface_flags; > + const u32 *match_data; > + u32 pval; > + > + match_data = of_device_get_match_data(dev); > + if (IS_ERR_OR_NULL(match_data)) { > + dev_err_probe(dev, PTR_ERR(match_data), > + "model specific data is not defined\n"); > + ret = -ENODEV; > + goto err; > + } > + m_model = *match_data; > + > + rproc = devm_rproc_alloc(dev, "avm,wasp", &avm_wasp_rproc_ops, > + image, sizeof(*avmwasp)); > + if (!rproc) { > + ret = -ENOMEM; > + goto err; > + } > + > + rproc->auto_boot = true; > + > + avmwasp = rproc->priv; > + avmwasp->rproc = rproc; > + avmwasp->pdev = pdev; > + > + ret = request_firmware((const struct firmware **)&avmwasp->fw_blob, > + firmware, dev); > + if (ret) > + dev_err(dev, "could not load ath9k firmware\n"); > + release_firmware(avmwasp->fw_blob); > + > + if (m_model == MODEL_X490) { > + ret = request_firmware((const struct firmware **) > + &avmwasp->fw_blob, caldata, dev); > + if (ret) > + dev_err(dev, "could not load ath10k caldata\n"); > + release_firmware(avmwasp->fw_blob); > + } > + > + ret = of_property_read_u32(dev->of_node, "link-interface", > + &pval); > + if (ret) { > + dev_err(dev, "no wasp-port given\n"); > + goto err; > + } else { > + struct device_node *child = of_find_node_by_phandle(pval); > + > + if (!child) { > + dev_err(dev, "get link-interface node failed\n"); > + ret = -ENODEV; > + goto err; > + } else { > + ret = of_property_read_string(child, "label", > + (const char **) > + &avmwasp->loader_port); > + of_node_put(child); > + if (ret) { > + dev_err(dev, "get link-interface label failed\n"); > + goto err; > + } > + } > + } > + > + rcu_read_lock(); > + netdev = dev_get_by_name_rcu(&init_net, avmwasp->loader_port); > + if (netdev) > + interface_flags = (short)dev_get_flags(netdev); > + rcu_read_unlock(); > + > + if (IS_ERR_OR_NULL(netdev)) { > + dev_err_probe(dev, PTR_ERR(netdev), > + "error accessing net device\n"); > + ret = -ENODEV; > + goto err; > + } > + > + if (!(interface_flags & IFF_UP && interface_flags & IFF_RUNNING)) { > + dev_err(dev, "error link-interface %s down\n", > + avmwasp->loader_port); > + ret = -EPROBE_DEFER; > + goto err; > + } > + > + avmwasp->power_gpio = devm_gpiod_get(dev, "power", GPIOD_OUT_LOW); > + if (IS_ERR(avmwasp->power_gpio)) { > + ret = dev_err_probe(dev, PTR_ERR(avmwasp->power_gpio), > + "failed to get power gpio\n"); > + goto err; > + } > + > + avmwasp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(avmwasp->reset_gpio)) { > + ret = dev_err_probe(dev, PTR_ERR(avmwasp->reset_gpio), > + "failed to get reset gpio\n"); > + goto err; > + } > + > + platform_set_drvdata(pdev, rproc); > + > + ret = devm_rproc_add(dev, rproc); > + if (ret) { > + dev_err(dev, "rproc_add failed\n"); > + goto err; > + } > + > + gpiod_set_value(avmwasp->power_gpio, 1); > + usleep_range(WASP_WAIT_SLEEP_US_LOW, WASP_WAIT_SLEEP_US); > + > +err: > + return ret; > +} > + > +static int avm_wasp_rproc_remove(struct platform_device *pdev) > +{ > + struct rproc *rproc = platform_get_drvdata(pdev); > + struct avm_wasp_rproc *avmwasp = rproc->priv; > + > + gpiod_set_value(avmwasp->power_gpio, 0); > + > + return 0; > +} > + > +static const u32 model_3390 = MODEL_3390; > +static const u32 model_x490 = MODEL_X490; > + > +static const struct of_device_id avm_wasp_rproc_of_match[] = { > + { .compatible = "avm,fritzbox3390-wasp", .data = &model_3390 }, > + { .compatible = "avm,fritzbox3490-wasp", .data = &model_x490 }, > + { .compatible = "avm,fritzbox5490-wasp", .data = &model_x490 }, > + { .compatible = "avm,fritzbox5491-wasp", .data = &model_x490 }, > + { .compatible = "avm,fritzbox7490-wasp", .data = &model_x490 }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, avm_wasp_rproc_of_match); > + > +static struct platform_driver avm_wasp_rproc_driver = { > + .probe = avm_wasp_rproc_probe, > + .remove = avm_wasp_rproc_remove, > + .driver = { > + .name = "avm_wasp_rproc", > + .of_match_table = avm_wasp_rproc_of_match, > + }, > +}; > + > +module_platform_driver(avm_wasp_rproc_driver); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("AVM WASP remote processor boot driver"); > +MODULE_AUTHOR("Daniel Kestrel "); > -- > 2.17.1 >