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 <vk...@kernel.org>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda...@renesas.com>
> Cc: Christian Lamparter <chunk...@googlemail.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

Reply via email to