[PATCH v3 11/12] mmc: Introduce mmc_phys_to_bus()/mmc_bus_to_phys()

[Simon Glass]

Hi Nicolas,

On Mon, 21 Dec 2020 at 12:52, Nicolas Saenz Julienne
<nsaenzjulienne at suse.de> wrote:
>
> On Mon, 2020-12-21 at 09:47 -0700, Simon Glass wrote:
> > Hi Nicolas,
> >
> > On Mon, 21 Dec 2020 at 07:23, Nicolas Saenz Julienne
> > <nsaenzjulienne at suse.de> wrote:
> > >
> > > On Fri, 2020-12-18 at 19:28 -0700, Simon Glass wrote:
> > > > On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
> > > > <nsaenzjulienne at suse.de> wrote:
> > > > >
> > > > > This will allow us to use DM variants of phys_to_bus()/bus_to_phys()
> > > > > when relevant.
> > > > >
> > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne at suse.de>
> > > > > ---
> > > > >  drivers/mmc/sdhci.c |  7 ++++---
> > > > >  include/mmc.h       | 10 ++++++++++
> > > > >  2 files changed, 14 insertions(+), 3 deletions(-)
> > > >
> > > > Reviewed-by: Simon Glass <sjg at chromium.org>
> > > >
> > > >
> > > > >
> > > > > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> > > > > index 0628934312..2086d7cdb1 100644
> > > > > --- a/drivers/mmc/sdhci.c
> > > > > +++ b/drivers/mmc/sdhci.c
> > > > > @@ -19,7 +19,6 @@
> > > > >  #include <linux/bitops.h>
> > > > >  #include <linux/delay.h>
> > > > >  #include <linux/dma-mapping.h>
> > > > > -#include <phys2bus.h>
> > > > >
> > > > >  static void sdhci_reset(struct sdhci_host *host, u8 mask)
> > > > >  {
> > > > > @@ -103,7 +102,8 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
> > > > >                                           mmc_get_dma_dir(data));
> > > > >
> > > > >         if (host->flags & USE_SDMA) {
> > > > > -               sdhci_writel(host, phys_to_bus((ulong)host->start_addr),
> > > > > +               sdhci_writel(host,
> > > > > +                            mmc_phys_to_bus(host->mmc, (ulong)host->start_addr),
> > > > >                                 SDHCI_DMA_ADDRESS);
> > > > >         }
> > > > >  #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
> > > > > @@ -162,7 +162,8 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
> > > > >                                 start_addr &=
> > > > >                                 ~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1);
> > > > >                                 start_addr += SDHCI_DEFAULT_BOUNDARY_SIZE;
> > > > > -                               sdhci_writel(host, phys_to_bus((ulong)start_addr),
> > > > > +                               sdhci_writel(host,
> > > > > +                                            mmc_phys_to_bus(host->mmc, (ulong)start_addr),
> > > > >                                              SDHCI_DMA_ADDRESS);
> > > > >                         }
> > > > >                 }
> > > > > diff --git a/include/mmc.h b/include/mmc.h
> > > > > index 1d377e0281..5fe1ef1dfc 100644
> > > > > --- a/include/mmc.h
> > > > > +++ b/include/mmc.h
> > > > > @@ -15,6 +15,7 @@
> > > > >  #include <linux/compiler.h>
> > > > >  #include <linux/dma-direction.h>
> > > > >  #include <part.h>
> > > > > +#include <phys2bus.h>
> > > > >
> > > > >  struct bd_info;
> > > > >
> > > > > @@ -977,4 +978,13 @@ static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
> > > > >         return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
> > > > >  }
> > > > >
> > > > > +static inline dma_addr_t mmc_phys_to_bus(struct mmc *mmc, phys_addr_t addr)
> > > > > +{
> > > > > +#if CONFIG_IS_ENABLED(DM_DMA)
> > > >
> > > > if() ?
> > >
> > > Noted.
> > >
> > > > Also makes me wonder if dev_phys_to_bus() should call phys_to_bus() if
> > > > DM_DMA is not enabled?
> > >
> > > I'd still need some sort per-subsystem macro to cater for !DM configurations
> > > where the device pointer isn't defined in the controller struct. Which IMO
> > > isn't much of an enhancement to this approach.
> >
> > We don't need to worry about non-DM for new features. In fact we don't
> > generally accept code that adds new features to non-DM code. It is
> > deprecated and slated for removal, so it just prolongs the migration.
>
> Understood, so let's not call the plain phys_to_bus() and assume a 1:1 address
> translation on non DM_DMA users (for both mmc and xhci). I'd say it's pretty
> safe to do anyway, as it seems phys2bus.h was introduced specially for the RPi
> family.

That sounds right to me.

>
> There still are some rogue users of the non dev dependent phys_to_bus() API in
> RPi platform code. I'll try to clean that up in a subsequent series.

OK ta.

Regards,
Simon