All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niek Linnenbank <nieklinnenbank@gmail.com>
To: qianfanguijin@163.com
Cc: qemu-arm@nongnu.org, qemu-devel@nongnu.org,
	"Strahinja Jankovic" <strahinja.p.jankovic@gmail.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Beniamino Galvani" <b.galvani@gmail.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>
Subject: Re: [PATCH v2 07/12] hw: sd: allwinner-sdhost: Add sun50i-a64 SoC support
Date: Thu, 6 Apr 2023 22:18:57 +0200	[thread overview]
Message-ID: <CAPan3Wpm6KqrXrdtrSqQPGAQGwJpiraUj0n3FKJOAoSrR1c71w@mail.gmail.com> (raw)
In-Reply-To: <20230328054654.18620-8-qianfanguijin@163.com>

[-- Attachment #1: Type: text/plain, Size: 8529 bytes --]

On Tue, Mar 28, 2023 at 7:47 AM <qianfanguijin@163.com> wrote:

> From: qianfan Zhao <qianfanguijin@163.com>
>
> A64's sd register was similar to H3, and it introduced a new register
> named SAMP_DL_REG location at 0x144. The dma descriptor buffer size of
> mmc2 is only 8K and the other mmc controllers has 64K.
>
> Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
> ---
>  hw/sd/allwinner-sdhost.c         | 70 ++++++++++++++++++++++++++++++--
>  include/hw/sd/allwinner-sdhost.h |  9 ++++
>  2 files changed, 76 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c
> index 51e5e90830..38e7844399 100644
> --- a/hw/sd/allwinner-sdhost.c
> +++ b/hw/sd/allwinner-sdhost.c
> @@ -77,6 +77,7 @@ enum {
>      REG_SD_DATA1_CRC  = 0x12C, /* CRC Data 1 from card/eMMC */
>      REG_SD_DATA0_CRC  = 0x130, /* CRC Data 0 from card/eMMC */
>      REG_SD_CRC_STA    = 0x134, /* CRC status from card/eMMC during write
> */
> +    REG_SD_SAMP_DL    = 0x144, /* Sample Delay Control (sun50i-a64) */
>      REG_SD_FIFO       = 0x200, /* Read/Write FIFO */
>  };
>
> @@ -158,6 +159,7 @@ enum {
>      REG_SD_RES_CRC_RST      = 0x0,
>      REG_SD_DATA_CRC_RST     = 0x0,
>      REG_SD_CRC_STA_RST      = 0x0,
> +    REG_SD_SAMPLE_DL_RST    = 0x00002000,
>      REG_SD_FIFO_RST         = 0x0,
>  };
>
> @@ -438,6 +440,7 @@ static uint64_t allwinner_sdhost_read(void *opaque,
> hwaddr offset,
>  {
>      AwSdHostState *s = AW_SDHOST(opaque);
>      AwSdHostClass *sc = AW_SDHOST_GET_CLASS(s);
> +    bool out_of_bounds = false;
>      uint32_t res = 0;
>
>      switch (offset) {
> @@ -556,13 +559,24 @@ static uint64_t allwinner_sdhost_read(void *opaque,
> hwaddr offset,
>      case REG_SD_FIFO:      /* Read/Write FIFO */
>          res = allwinner_sdhost_fifo_read(s);
>          break;
> +    case REG_SD_SAMP_DL: /* Sample Delay */
>
Sample Delay Control


> +        if (sc->can_calibrate) {
> +            res = s->sample_delay;
> +        } else {
> +            out_of_bounds = true;
> +        }
> +        break;
>      default:
> -        qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds offset %"
> -                      HWADDR_PRIx"\n", __func__, offset);
> +        out_of_bounds = true;
>          res = 0;
>          break;
>      }
>
> +    if (out_of_bounds) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds offset %"
> +                      HWADDR_PRIx"\n", __func__, offset);
> +    }
> +
>      trace_allwinner_sdhost_read(offset, res, size);
>      return res;
>  }
> @@ -581,6 +595,7 @@ static void allwinner_sdhost_write(void *opaque,
> hwaddr offset,
>  {
>      AwSdHostState *s = AW_SDHOST(opaque);
>      AwSdHostClass *sc = AW_SDHOST_GET_CLASS(s);
> +    bool out_of_bounds = false;
>
>      trace_allwinner_sdhost_write(offset, value, size);
>
> @@ -704,10 +719,21 @@ static void allwinner_sdhost_write(void *opaque,
> hwaddr offset,
>      case REG_SD_DATA0_CRC: /* CRC Data 0 from card/eMMC */
>      case REG_SD_CRC_STA:   /* CRC status from card/eMMC in write
> operation */
>          break;
> +    case REG_SD_SAMP_DL: /* Sample delay control */
> +        if (sc->can_calibrate) {
> +            s->sample_delay = value;
> +        } else {
> +            out_of_bounds = true;
> +        }
> +        break;
>      default:
> +        out_of_bounds = true;
> +        break;
> +    }
> +
> +    if (out_of_bounds) {
>          qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds offset %"
>                        HWADDR_PRIx"\n", __func__, offset);
> -        break;
>      }
>  }
>
> @@ -756,6 +782,7 @@ static const VMStateDescription
> vmstate_allwinner_sdhost = {
>          VMSTATE_UINT32(response_crc, AwSdHostState),
>          VMSTATE_UINT32_ARRAY(data_crc, AwSdHostState, 8),
>          VMSTATE_UINT32(status_crc, AwSdHostState),
> +        VMSTATE_UINT32(sample_delay, AwSdHostState),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> @@ -794,6 +821,7 @@ static void allwinner_sdhost_realize(DeviceState *dev,
> Error **errp)
>  static void allwinner_sdhost_reset(DeviceState *dev)
>  {
>      AwSdHostState *s = AW_SDHOST(dev);
> +    AwSdHostClass *sc = AW_SDHOST_GET_CLASS(s);
>
>      s->global_ctl = REG_SD_GCTL_RST;
>      s->clock_ctl = REG_SD_CKCR_RST;
> @@ -834,6 +862,10 @@ static void allwinner_sdhost_reset(DeviceState *dev)
>      }
>
>      s->status_crc = REG_SD_CRC_STA_RST;
> +
> +    if (sc->can_calibrate) {
> +        s->sample_delay = REG_SD_SAMPLE_DL_RST;
> +    }
>  }
>
>  static void allwinner_sdhost_bus_class_init(ObjectClass *klass, void
> *data)
> @@ -867,6 +899,24 @@ static void
> allwinner_sdhost_sun5i_class_init(ObjectClass *klass, void *data)
>      sc->is_sun4i = false;
>  }
>
> +static void allwinner_sdhost_sun50i_a64_class_init(ObjectClass *klass,
> +                                                   void *data)
> +{
> +    AwSdHostClass *sc = AW_SDHOST_CLASS(klass);
> +    sc->max_desc_size = 64 * KiB;
> +    sc->is_sun4i = false;
> +    sc->can_calibrate = true;
>

perhaps in the other two existing _init() functions for sun4i/sun5i, we
should also explicitly set the new can_calibrate value to false,
to avoid the risk of using uninitialized data in the other machines/socs.


> +}
> +
> +static void allwinner_sdhost_sun50i_a64_emmc_class_init(ObjectClass
> *klass,
> +                                                        void *data)
> +{
> +    AwSdHostClass *sc = AW_SDHOST_CLASS(klass);
> +    sc->max_desc_size = 8 * KiB;
> +    sc->is_sun4i = false;
> +    sc->can_calibrate = true;
> +}
> +
>  static const TypeInfo allwinner_sdhost_info = {
>      .name          = TYPE_AW_SDHOST,
>      .parent        = TYPE_SYS_BUS_DEVICE,
> @@ -889,6 +939,18 @@ static const TypeInfo allwinner_sdhost_sun5i_info = {
>      .class_init    = allwinner_sdhost_sun5i_class_init,
>  };
>
> +static const TypeInfo allwinner_sdhost_sun50i_a64_info = {
> +    .name          = TYPE_AW_SDHOST_SUN50I_A64,
> +    .parent        = TYPE_AW_SDHOST,
> +    .class_init    = allwinner_sdhost_sun50i_a64_class_init,
> +};
> +
> +static const TypeInfo allwinner_sdhost_sun50i_a64_emmc_info = {
> +    .name          = TYPE_AW_SDHOST_SUN50I_A64_EMMC,
> +    .parent        = TYPE_AW_SDHOST,
> +    .class_init    = allwinner_sdhost_sun50i_a64_emmc_class_init,
> +};
> +
>  static const TypeInfo allwinner_sdhost_bus_info = {
>      .name = TYPE_AW_SDHOST_BUS,
>      .parent = TYPE_SD_BUS,
> @@ -901,6 +963,8 @@ static void allwinner_sdhost_register_types(void)
>      type_register_static(&allwinner_sdhost_info);
>      type_register_static(&allwinner_sdhost_sun4i_info);
>      type_register_static(&allwinner_sdhost_sun5i_info);
> +    type_register_static(&allwinner_sdhost_sun50i_a64_info);
> +    type_register_static(&allwinner_sdhost_sun50i_a64_emmc_info);
>      type_register_static(&allwinner_sdhost_bus_info);
>  }
>
> diff --git a/include/hw/sd/allwinner-sdhost.h
> b/include/hw/sd/allwinner-sdhost.h
> index 30c1e60404..1b951177dd 100644
> --- a/include/hw/sd/allwinner-sdhost.h
> +++ b/include/hw/sd/allwinner-sdhost.h
> @@ -38,6 +38,12 @@
>  /** Allwinner sun5i family and newer (A13, H2+, H3, etc) */
>  #define TYPE_AW_SDHOST_SUN5I TYPE_AW_SDHOST "-sun5i"
>
> +/** Allwinner sun50i-a64 */
> +#define TYPE_AW_SDHOST_SUN50I_A64 TYPE_AW_SDHOST "-sun50i-a64"
> +
> +/** Allwinner sun50i-a64 emmc */
> +#define TYPE_AW_SDHOST_SUN50I_A64_EMMC  TYPE_AW_SDHOST "-sun50i-a64-emmc"
> +
>  /** @} */
>
>  /**
> @@ -110,6 +116,7 @@ struct AwSdHostState {
>      uint32_t startbit_detect;   /**< eMMC DDR Start Bit Detection Control
> */
>      uint32_t response_crc;      /**< Response CRC */
>      uint32_t data_crc[8];       /**< Data CRC */
> +    uint32_t sample_delay;      /**< Sample delay control */
>      uint32_t status_crc;        /**< Status CRC */
>
>      /** @} */
> @@ -132,6 +139,8 @@ struct AwSdHostClass {
>      size_t max_desc_size;
>      bool   is_sun4i;
>
> +    /** does the IP block support autocalibration? */
> +    bool can_calibrate;
>  };
>
>  #endif /* HW_SD_ALLWINNER_SDHOST_H */
> --
> 2.25.1
>
>
In this patch, I don't see any update to the new allwinner-r40.c file.
If you make the required changes to allwinner-r40.c in this patch, you can
also avoid having patch 08.

Regards,
Niek

-- 
Niek Linnenbank

[-- Attachment #2: Type: text/html, Size: 10631 bytes --]

  reply	other threads:[~2023-04-06 20:19 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-28  5:46 [PATCH v2 00/12] *** add allwinner-r40 support *** qianfanguijin
2023-03-28  5:46 ` [PATCH v2 01/12] hw: arm: Add bananapi M2-Ultra and allwinner-r40 support qianfanguijin
2023-04-06 19:12   ` Niek Linnenbank
2023-03-28  5:46 ` [PATCH v2 02/12] hw/arm/allwinner-r40: add Clock Control Unit qianfanguijin
2023-04-06 19:17   ` Niek Linnenbank
2023-03-28  5:46 ` [PATCH v2 03/12] hw: allwinner-r40: Complete uart devices qianfanguijin
2023-04-11 18:06   ` Strahinja Jankovic
2023-03-28  5:46 ` [PATCH v2 04/12] hw: arm: allwinner-r40: Add i2c0 device qianfanguijin
2023-04-11 18:02   ` Strahinja Jankovic
2023-03-28  5:46 ` [PATCH v2 05/12] hw/misc: Rename axp209 to axp22x and add support AXP221 PMU qianfanguijin
2023-03-28  5:46 ` [PATCH v2 06/12] hw/arm/allwinner-r40: add SDRAM controller device qianfanguijin
2023-03-28  5:46 ` [PATCH v2 07/12] hw: sd: allwinner-sdhost: Add sun50i-a64 SoC support qianfanguijin
2023-04-06 20:18   ` Niek Linnenbank [this message]
2023-04-18 10:58     ` qianfan
2023-03-28  5:46 ` [PATCH v2 08/12] hw: arm: allwinner-r40: Fix the mmc controller's type qianfanguijin
2023-04-06 19:22   ` Niek Linnenbank
2023-04-18 10:56     ` qianfan
2023-03-28  5:46 ` [PATCH v2 09/12] hw: arm: allwinner-r40: Add emac and gmac support qianfanguijin
2023-03-28  5:46 ` [PATCH v2 10/12] hw: arm: allwinner-sramc: Add SRAM Controller support for R40 qianfanguijin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPan3Wpm6KqrXrdtrSqQPGAQGwJpiraUj0n3FKJOAoSrR1c71w@mail.gmail.com \
    --to=nieklinnenbank@gmail.com \
    --cc=b.galvani@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qianfanguijin@163.com \
    --cc=strahinja.p.jankovic@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.