Re: [PATCH] Revert "mmc: mvsdio: Use sg_miter for PIO"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 27 Sept 2024 at 17:54, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> This reverts commit 2761822c00e8c271f10a10affdbd4917d900d7ea.
>
> When testing on real hardware the patch does not work.
> Revert, try to acquire real hardware, and retry.
> These systems typically don't have highmem anyway so the
> impact is likely zero.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Charlie <g4sra@xxxxxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>

Applied for fixes, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/mvsdio.c | 71 ++++++++++++-----------------------------------
>  1 file changed, 18 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
> index af7f21888e27..ca01b7d204ba 100644
> --- a/drivers/mmc/host/mvsdio.c
> +++ b/drivers/mmc/host/mvsdio.c
> @@ -38,9 +38,8 @@ struct mvsd_host {
>         unsigned int xfer_mode;
>         unsigned int intr_en;
>         unsigned int ctrl;
> -       bool use_pio;
> -       struct sg_mapping_iter sg_miter;
>         unsigned int pio_size;
> +       void *pio_ptr;
>         unsigned int sg_frags;
>         unsigned int ns_per_clk;
>         unsigned int clock;
> @@ -115,18 +114,11 @@ static int mvsd_setup_data(struct mvsd_host *host, struct mmc_data *data)
>                  * data when the buffer is not aligned on a 64 byte
>                  * boundary.
>                  */
> -               unsigned int miter_flags = SG_MITER_ATOMIC; /* Used from IRQ */
> -
> -               if (data->flags & MMC_DATA_READ)
> -                       miter_flags |= SG_MITER_TO_SG;
> -               else
> -                       miter_flags |= SG_MITER_FROM_SG;
> -
>                 host->pio_size = data->blocks * data->blksz;
> -               sg_miter_start(&host->sg_miter, data->sg, data->sg_len, miter_flags);
> +               host->pio_ptr = sg_virt(data->sg);
>                 if (!nodma)
> -                       dev_dbg(host->dev, "fallback to PIO for data\n");
> -               host->use_pio = true;
> +                       dev_dbg(host->dev, "fallback to PIO for data at 0x%p size %d\n",
> +                               host->pio_ptr, host->pio_size);
>                 return 1;
>         } else {
>                 dma_addr_t phys_addr;
> @@ -137,7 +129,6 @@ static int mvsd_setup_data(struct mvsd_host *host, struct mmc_data *data)
>                 phys_addr = sg_dma_address(data->sg);
>                 mvsd_write(MVSD_SYS_ADDR_LOW, (u32)phys_addr & 0xffff);
>                 mvsd_write(MVSD_SYS_ADDR_HI,  (u32)phys_addr >> 16);
> -               host->use_pio = false;
>                 return 0;
>         }
>  }
> @@ -297,8 +288,8 @@ static u32 mvsd_finish_data(struct mvsd_host *host, struct mmc_data *data,
>  {
>         void __iomem *iobase = host->base;
>
> -       if (host->use_pio) {
> -               sg_miter_stop(&host->sg_miter);
> +       if (host->pio_ptr) {
> +               host->pio_ptr = NULL;
>                 host->pio_size = 0;
>         } else {
>                 dma_unmap_sg(mmc_dev(host->mmc), data->sg, host->sg_frags,
> @@ -353,12 +344,9 @@ static u32 mvsd_finish_data(struct mvsd_host *host, struct mmc_data *data,
>  static irqreturn_t mvsd_irq(int irq, void *dev)
>  {
>         struct mvsd_host *host = dev;
> -       struct sg_mapping_iter *sgm = &host->sg_miter;
>         void __iomem *iobase = host->base;
>         u32 intr_status, intr_done_mask;
>         int irq_handled = 0;
> -       u16 *p;
> -       int s;
>
>         intr_status = mvsd_read(MVSD_NOR_INTR_STATUS);
>         dev_dbg(host->dev, "intr 0x%04x intr_en 0x%04x hw_state 0x%04x\n",
> @@ -382,36 +370,15 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
>         spin_lock(&host->lock);
>
>         /* PIO handling, if needed. Messy business... */
> -       if (host->use_pio) {
> -               /*
> -                * As we set sgm->consumed this always gives a valid buffer
> -                * position.
> -                */
> -               if (!sg_miter_next(sgm)) {
> -                       /* This should not happen */
> -                       dev_err(host->dev, "ran out of scatter segments\n");
> -                       spin_unlock(&host->lock);
> -                       host->intr_en &=
> -                               ~(MVSD_NOR_RX_READY | MVSD_NOR_RX_FIFO_8W |
> -                                 MVSD_NOR_TX_AVAIL | MVSD_NOR_TX_FIFO_8W);
> -                       mvsd_write(MVSD_NOR_INTR_EN, host->intr_en);
> -                       return IRQ_HANDLED;
> -               }
> -               p = sgm->addr;
> -               s = sgm->length;
> -               if (s > host->pio_size)
> -                       s = host->pio_size;
> -       }
> -
> -       if (host->use_pio &&
> +       if (host->pio_size &&
>             (intr_status & host->intr_en &
>              (MVSD_NOR_RX_READY | MVSD_NOR_RX_FIFO_8W))) {
> -
> +               u16 *p = host->pio_ptr;
> +               int s = host->pio_size;
>                 while (s >= 32 && (intr_status & MVSD_NOR_RX_FIFO_8W)) {
>                         readsw(iobase + MVSD_FIFO, p, 16);
>                         p += 16;
>                         s -= 32;
> -                       sgm->consumed += 32;
>                         intr_status = mvsd_read(MVSD_NOR_INTR_STATUS);
>                 }
>                 /*
> @@ -424,7 +391,6 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
>                                 put_unaligned(mvsd_read(MVSD_FIFO), p++);
>                                 put_unaligned(mvsd_read(MVSD_FIFO), p++);
>                                 s -= 4;
> -                               sgm->consumed += 4;
>                                 intr_status = mvsd_read(MVSD_NOR_INTR_STATUS);
>                         }
>                         if (s && s < 4 && (intr_status & MVSD_NOR_RX_READY)) {
> @@ -432,13 +398,10 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
>                                 val[0] = mvsd_read(MVSD_FIFO);
>                                 val[1] = mvsd_read(MVSD_FIFO);
>                                 memcpy(p, ((void *)&val) + 4 - s, s);
> -                               sgm->consumed += s;
>                                 s = 0;
>                                 intr_status = mvsd_read(MVSD_NOR_INTR_STATUS);
>                         }
> -                       /* PIO transfer done */
> -                       host->pio_size -= sgm->consumed;
> -                       if (host->pio_size == 0) {
> +                       if (s == 0) {
>                                 host->intr_en &=
>                                      ~(MVSD_NOR_RX_READY | MVSD_NOR_RX_FIFO_8W);
>                                 mvsd_write(MVSD_NOR_INTR_EN, host->intr_en);
> @@ -450,10 +413,14 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
>                 }
>                 dev_dbg(host->dev, "pio %d intr 0x%04x hw_state 0x%04x\n",
>                         s, intr_status, mvsd_read(MVSD_HW_STATE));
> +               host->pio_ptr = p;
> +               host->pio_size = s;
>                 irq_handled = 1;
> -       } else if (host->use_pio &&
> +       } else if (host->pio_size &&
>                    (intr_status & host->intr_en &
>                     (MVSD_NOR_TX_AVAIL | MVSD_NOR_TX_FIFO_8W))) {
> +               u16 *p = host->pio_ptr;
> +               int s = host->pio_size;
>                 /*
>                  * The TX_FIFO_8W bit is unreliable. When set, bursting
>                  * 16 halfwords all at once in the FIFO drops data. Actually
> @@ -464,7 +431,6 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
>                         mvsd_write(MVSD_FIFO, get_unaligned(p++));
>                         mvsd_write(MVSD_FIFO, get_unaligned(p++));
>                         s -= 4;
> -                       sgm->consumed += 4;
>                         intr_status = mvsd_read(MVSD_NOR_INTR_STATUS);
>                 }
>                 if (s < 4) {
> @@ -473,13 +439,10 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
>                                 memcpy(((void *)&val) + 4 - s, p, s);
>                                 mvsd_write(MVSD_FIFO, val[0]);
>                                 mvsd_write(MVSD_FIFO, val[1]);
> -                               sgm->consumed += s;
>                                 s = 0;
>                                 intr_status = mvsd_read(MVSD_NOR_INTR_STATUS);
>                         }
> -                       /* PIO transfer done */
> -                       host->pio_size -= sgm->consumed;
> -                       if (host->pio_size == 0) {
> +                       if (s == 0) {
>                                 host->intr_en &=
>                                      ~(MVSD_NOR_TX_AVAIL | MVSD_NOR_TX_FIFO_8W);
>                                 mvsd_write(MVSD_NOR_INTR_EN, host->intr_en);
> @@ -487,6 +450,8 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
>                 }
>                 dev_dbg(host->dev, "pio %d intr 0x%04x hw_state 0x%04x\n",
>                         s, intr_status, mvsd_read(MVSD_HW_STATE));
> +               host->pio_ptr = p;
> +               host->pio_size = s;
>                 irq_handled = 1;
>         }
>
>
> ---
> base-commit: 075dbe9f6e3c21596c5245826a4ee1f1c1676eb8
> change-id: 20240927-kirkwood-mmc-regression-986c598d4c9e
>
> Best regards,
> --
> Linus Walleij <linus.walleij@xxxxxxxxxx>
>




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux