| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <94266bc2-ae44-d7a2-61e9-4e09c29bd18d@linux.intel.com>
Date: Wed, 29 Apr 2020 17:39:26 +0300
From: Mathias Nyman <mathias.nyman@...ux.intel.com>
To: Vinod Koul <vkoul@...nel.org>,
Mathias Nyman <mathias.nyman@...el.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: linux-arm-msm@...r.kernel.org,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
Christian Lamparter <chunkeey@...glemail.com>,
John Stultz <john.stultz@...aro.org>,
Alan Stern <stern@...land.harvard.edu>,
Andreas Böhler <dev@...ehler.at>,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v10 4/5] usb: renesas-xhci: Add ROM loader for uPD720201
On 24.4.2020 13.14, Vinod Koul wrote:
> uPD720201 supports ROM and allows software to program the ROM and boot
> from it. Add support for detecting if ROM is present, if so load the ROM
> if not programmed earlier.
>
> Signed-off-by: Vinod Koul <vkoul@...nel.org>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
> Cc: Christian Lamparter <chunkeey@...glemail.com>
> ---
> drivers/usb/host/xhci-pci-renesas.c | 342 +++++++++++++++++++++++++++-
> 1 file changed, 341 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
> index 402e86912c9f..6bb537999754 100644
> --- a/drivers/usb/host/xhci-pci-renesas.c
> +++ b/drivers/usb/host/xhci-pci-renesas.c
> @@ -50,6 +50,22 @@
> #define RENESAS_RETRY 10000
> #define RENESAS_DELAY 10
>
> +#define ROM_VALID_01 0x2013
> +#define ROM_VALID_02 0x2026
> +
> +static int renesas_verify_fw_version(struct pci_dev *pdev, u32 version)
> +{
> + switch (version) {
> + case ROM_VALID_01:
> + case ROM_VALID_02:
> + return 0;
> + default:
> + dev_err(&pdev->dev, "FW has invalid version :%d\n", version);
> + return 1;
> + }
> + return -EINVAL;
This never returns -EINVAL
Maybe just get rid of the default case and print
the error message before returning
> +}
> +
> static int renesas_fw_download_image(struct pci_dev *dev,
> const u32 *fw,
> size_t step)
> @@ -144,10 +160,84 @@ static int renesas_fw_verify(const void *fw_data,
> return 0;
> }
>
> -static int renesas_fw_check_running(struct pci_dev *pdev)
> +static bool renesas_check_rom(struct pci_dev *pdev)
> {
> + u16 rom_status;
> + int retval;
> +
> + /* Check if external ROM exists */
> + retval = pci_read_config_word(pdev, RENESAS_ROM_STATUS, &rom_status);
> + if (retval)
> + return false;
> +
> + rom_status &= RENESAS_ROM_STATUS_ROM_EXISTS;
> + if (rom_status) {
> + dev_dbg(&pdev->dev, "External ROM exists\n");
> + return true; /* External ROM exists */
> + }
> +
> + return false;
> +}
> +
> +static int renesas_check_rom_state(struct pci_dev *pdev)
> +{
> + u16 rom_state;
> + u32 version;
> int err;
> +
> + /* check FW version */
> + err = pci_read_config_dword(pdev, RENESAS_FW_VERSION, &version);
> + if (err)
> + return pcibios_err_to_errno(err);
> +
> + version &= RENESAS_FW_VERSION_FIELD;
> + version = version >> RENESAS_FW_VERSION_OFFSET;
> +
> + err = renesas_verify_fw_version(pdev, version);
> + if (err)
> + return err;
> +
> + /*
> + * Test if ROM is present and loaded, if so we can skip everything
> + */
> + err = pci_read_config_word(pdev, RENESAS_ROM_STATUS, &rom_state);
> + if (err)
> + return pcibios_err_to_errno(err);
> +
> + if (rom_state & BIT(15)) {
> + /* ROM exists */
> + dev_dbg(&pdev->dev, "ROM exists\n");
> +
> + /* Check the "Result Code" Bits (6:4) and act accordingly */
> + switch (rom_state & RENESAS_ROM_STATUS_RESULT) {
> + case RENESAS_ROM_STATUS_SUCCESS:
> + return 0;
> +
> + case RENESAS_ROM_STATUS_NO_RESULT: /* No result yet */
> + return 0;
> +
> + case RENESAS_ROM_STATUS_ERROR: /* Error State */
> + default: /* All other states are marked as "Reserved states" */
> + dev_err(&pdev->dev, "Invalid ROM..");
> + break;
> + }
> + }
> +
> + return -EIO;
> +}
> +
> +static int renesas_fw_check_running(struct pci_dev *pdev)
> +{
> u8 fw_state;
> + int err;
> +
> + /* Check if device has ROM and loaded, if so skip everything */
> + err = renesas_check_rom(pdev);
> + if (err) { /* we have rom */
> + err = renesas_check_rom_state(pdev);
> + if (!err)
> + return err;
> + }
>
> /*
> * Test if the device is actually needing the firmware. As most
> @@ -303,11 +393,261 @@ static int renesas_fw_download(struct pci_dev *pdev,
> return 0;
> }
>
> +static void renesas_rom_erase(struct pci_dev *pdev)
> +{
> + int retval, i;
> + u8 status;
> +
> + dev_dbg(&pdev->dev, "Performing ROM Erase...\n");
> + retval = pci_write_config_dword(pdev, RENESAS_DATA0,
> + RENESAS_ROM_ERASE_MAGIC);
> + if (retval) {
> + dev_err(&pdev->dev, "ROM erase, magic word write failed: %d\n",
> + pcibios_err_to_errno(retval));
> + return;
> + }
> +
> + retval = pci_read_config_byte(pdev, RENESAS_ROM_STATUS, &status);
> + if (retval) {
> + dev_err(&pdev->dev, "ROM status read failed: %d\n",
> + pcibios_err_to_errno(retval));
> + return;
> + }
> + status |= RENESAS_ROM_STATUS_ERASE;
> + retval = pci_write_config_byte(pdev, RENESAS_ROM_STATUS, status);
> + if (retval) {
> + dev_err(&pdev->dev, "ROM erase set word write failed\n");
> + return;
> + }
> +
> + /* sleep a bit while ROM is erased */
> + msleep(20);
> +
> + for (i = 0; i < RENESAS_RETRY; i++) {
> + retval = pci_read_config_byte(pdev, RENESAS_ROM_STATUS,
> + &status);
> + status &= RENESAS_ROM_STATUS_ERASE;
> + if (!status)
> + break;
> +
> + mdelay(RENESAS_DELAY);
> + }
> +
> + if (i == RENESAS_RETRY)
> + dev_dbg(&pdev->dev, "Chip erase timedout: %x\n", status);
> +
> + dev_dbg(&pdev->dev, "ROM Erase... Done success\n");
> +}
> +
> +static bool renesas_download_rom(struct pci_dev *pdev,
> + const u32 *fw, size_t step)
> +{
> + bool data0_or_data1;
> + u8 fw_status;
> + size_t i;
> + int err;
> +
> + /*
> + * The hardware does alternate between two 32-bit pages.
> + * (This is because each row of the firmware is 8 bytes).
> + *
> + * for even steps we use DATA0, for odd steps DATA1.
> + */
> + data0_or_data1 = (step & 1) == 1;
> +
> + /* Read "Set DATAX" and confirm it is cleared. */
> + for (i = 0; i < RENESAS_RETRY; i++) {
> + err = pci_read_config_byte(pdev, RENESAS_ROM_STATUS_MSB,
> + &fw_status);
> + if (err) {
> + dev_err(&pdev->dev, "Read ROM Status failed: %d\n",
> + pcibios_err_to_errno(err));
> + return false;
> + }
> + if (!(fw_status & BIT(data0_or_data1)))
> + break;
> +
> + udelay(RENESAS_DELAY);
> + }
> + if (i == RENESAS_RETRY) {
> + dev_err(&pdev->dev, "Timeout for Set DATAX step: %zd\n", step);
> + return false;
> + }
> +
> + /*
> + * Write FW data to "DATAX".
> + * "LSB is left" => force little endian
> + */
> + err = pci_write_config_dword(pdev, data0_or_data1 ?
> + RENESAS_DATA1 : RENESAS_DATA0,
> + (__force u32)cpu_to_le32(fw[step]));
> + if (err) {
> + dev_err(&pdev->dev, "Write to DATAX failed: %d\n",
> + pcibios_err_to_errno(err));
> + return false;
> + }
> +
> + udelay(100);
> +
> + /* Set "Set DATAX". */
> + err = pci_write_config_byte(pdev, RENESAS_ROM_STATUS_MSB,
> + BIT(data0_or_data1));
> + if (err) {
> + dev_err(&pdev->dev, "Write config for DATAX failed: %d\n",
> + pcibios_err_to_errno(err));
> + return false;
> + }
> +
> + return true;
> +}
The above function is almost identical to renesas_fw_download_image() added in a
previous patch.
To avoid code duplication I'm sure one function that handles both cases would be possible.
-Mathias
Powered by blists - more mailing lists