Re: [PATCH] mmc: sdhci-pxav3: add support for the Armada 38x SDHCI controller

[Arnd Bergmann <arnd@arndb.de>]

On Tuesday 18 February 2014 20:26:16 Thomas Petazzoni wrote:
> On Tue, 18 Feb 2014 19:02:47 +0100, Arnd Bergmann wrote:
> 
> > > In order to achieve this, the sdhci-pxav3 driver is extended with an
> > > additional compatible string "marvell,armada-380-sdhci". When this
> > > compatible string is used, the MBus windows are initialized in a way
> > > that is identical to what all other DMA-capable drivers for Marvell
> > > EBU platforms do.
> > 
> > It seems odd to do this in the sdhci driver, when the configuration
> > is done in registers that belong to mbus.
> 
> No, those registers don't belong to MBus. They belong to the device
> itself, as they configure the device -> memory windows.
> 
> Because SDHCI has a standard register interface, they separated the
> SDHCI registers (standard) from the registers to configure the device
> -> Mbus windows (Marvell-specific).
> 
> But in all other IPs, the device -> memory windows are configured in
> registers that are just in the middle of the other registers for this
> device.

Ok, got it.


> > 
> > > +/*
> > > + * These registers are relative to the second register region, for the
> > > + * MBus bridge.
> > > + */
> > > +#define SDHCI_WINDOW_CTRL(i)   (0x80 + ((i) << 3))
> > > +#define SDHCI_WINDOW_BASE(i)   (0x84 + ((i) << 3))
> > > +#define SDHCI_MAX_WIN_NUM      8
> > 
> > These look similar to the outbound mbus windows that are used for MMIO,
> > but it's not really clear from the code what they really do.
> 
> There are two completely different mechanisms:
> 
>  * The CPU -> { memory, device } windows. These windows are managed by
>    the mvebu-mbus driver, as they are configured using global
>    registers, owned by the mvebu-driver.
> 
>  * The device -> memory windows. These windows are needed for a given
>    device to access memory in order to do DMA. These windows are
>    configured through registers that are part of each peripheral
>    register area.

Yes, I understand the difference. The former corresponds to
the DT 'ranges' property, while the latter is the 'dma-ranges'
property.

> > I assume there are more the same register ranges for each bus master
> > behind mbus (PCI being special again). How about adding an exported
> > function to the mbus driver that sets up all the windows for one
> > bus master correctly, passing only the number of the bus master?
> 
> This is certainly a possible refactoring, but it involves changing a
> fairly large number of drivers, since many drivers are using
> mv_mbus_dram_info() (this function and all the code spread in drivers
> to configure windows predates the mach-mvebu thing and all the DT
> conversion).

Is the layout of the mbus configuration windows in each device
the same?

> Therefore, I'd like to have the possibility of handling sdhci-pxav3.c
> like all other drivers for now, and then do a cleanup of this area.
> Would this be possible?

Yes, sounds reasonable.

Thanks for the clarification.

	Arnd