[U-Boot] [PATCH v5 00/19] arm: rpi: Enable USB, Ethernet, MMC, Video driver model on Raspberry Pi

[U-Boot] [PATCH v5 00/19] arm: rpi: Enable USB, Ethernet, MMC, Video driver model on Raspberry Pi

[Simon Glass]

Raspberry Pi uses a DWC2 USB controller and a SMSC USB Ethernet adaptor.
Driver model support for these is available.

This series does the following:
- Enable CONFIG_DM_ETH and CONFIG_DM_USB on Raspberry Pi
- Convert the MMC driver to driver model
- Convert the video driver to driver model
- Fixes a driver model video bug which accessed beyond the frame buffer
- Fixes start-up of MMC with driver model (e.g. at present it does not
    support env_fat)
- Clean up a few loose ends

With Ethernet active the device list looks something like this:

U-Boot> dm tree
 Class       Probed   Name
----------------------------------------
 root        [ + ]    root_driver
 simple_bus  [ + ]    |-- soc
 gpio        [ + ]    |   |-- gpio at 7e200000
 serial      [ + ]    |   |-- serial at 7e215040
 mmc         [ + ]    |   |-- sdhci at 7e300000
 blk         [ + ]    |   |   `-- sdhci at 7e300000.blk
 video       [ + ]    |   |-- hdmi at 7e902000
 vidconsole0 [ + ]    |   |   `-- hdmi at 7e902000.vidconsole0
 usb         [ + ]    |   `-- usb at 7e980000
 usb_hub     [ + ]    |       `-- usb_hub
 usb_hub     [ + ]    |           `-- usb_hub
 eth         [ + ]    |               `-- smsc95xx_eth
 simple_bus  [   ]    `-- clocks

With version 4 a problem was discovered where tftpboot does not correctly
read a file on rpi_2 and rpi_3. The cause is unknown but for now this
series includes a work-around in the dwc2 driver. See here for details:

https://lists.denx.de/pipermail/u-boot/2017-April/285451.html

Changes in v5:
- Add new patch for dwc2 to se separate input and output buffers
- Add new patch for smsc95xx to correct free_pkt() implementation
- Rebase to master

Changes in v4:
- Add patches to convert video and MMC to driver model also
- Rebase to master

Changes in v3:
- Drop applied patches from series
- Drop patch to introduce usbethaddr for driver model

Simon Glass (19):
  net: smsc95xx: Correct free_pkt() implementation
  usb: dwc2: Use separate input and output buffers
  dm: mmc: Set up the MMC device when controller is probed
  dm: video: Correct line clearing code
  string: Use memcpy() within memmove() when we can
  arm: rpi: Drop the GPIO device addresses
  arm: rpi: Drop CONFIG_CONS_INDEX
  dm: arm: rpi: Move to driver model for USB
  dm: arm: rpi: Use driver model for Ethernet
  arm: rpi: Add a file to handle messages
  arm: rpi: Add a function to obtain the MMC clock
  dm: mmc: rpi: Convert Raspberry Pi to driver model for MMC
  dm: arm: rpi: Drop CONFIG_OF_EMBED
  video: arm: rpi: Move the video query out of the driver
  video: arm: rpi: Move the video settings out of the driver
  dm: video: Refactor lcd_simplefb to prepare for driver model
  dm: video: Add driver-model support to lcd_simplefb
  dm: video: arm: rpi: Convert to use driver model for video
  arm: rpi: Add a TODO to move all messages into the msg handler

 arch/arm/mach-bcm283x/Makefile            |   2 +-
 arch/arm/mach-bcm283x/include/mach/gpio.h |   5 -
 arch/arm/mach-bcm283x/include/mach/msg.h  |  51 ++++++++++
 arch/arm/mach-bcm283x/msg.c               | 154 ++++++++++++++++++++++++++++++
 board/raspberrypi/rpi/rpi.c               |  56 +----------
 common/lcd_simplefb.c                     |  47 +++++++--
 configs/rpi_2_defconfig                   |   6 +-
 configs/rpi_3_32b_defconfig               |   6 +-
 configs/rpi_3_defconfig                   |   6 +-
 configs/rpi_defconfig                     |   6 +-
 drivers/mmc/bcm2835_sdhci.c               |  81 ++++++++++++----
 drivers/mmc/mmc-uclass.c                  |  12 +++
 drivers/usb/eth/smsc95xx.c                |   4 +-
 drivers/usb/host/dwc2.c                   |  27 ++++--
 drivers/video/bcm2835.c                   | 140 +++++++--------------------
 drivers/video/console_normal.c            |   3 +-
 include/configs/rpi.h                     |  17 +---
 lib/string.c                              |  11 +--
 18 files changed, 404 insertions(+), 230 deletions(-)
 create mode 100644 arch/arm/mach-bcm283x/include/mach/msg.h
 create mode 100644 arch/arm/mach-bcm283x/msg.c

-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 01/19] net: smsc95xx: Correct free_pkt() implementation

[Simon Glass]

On further review this returns the wrong packet length from the driver.
It may not be noticed since protocols will take care of it. Fix it by
subtracting the header length from the packet length returned.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5:
- Add new patch for smsc95xx to correct free_pkt() implementation

Changes in v4: None
Changes in v3: None

 drivers/usb/eth/smsc95xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/eth/smsc95xx.c b/drivers/usb/eth/smsc95xx.c
index d4c8ea4a98..26000a5a45 100644
--- a/drivers/usb/eth/smsc95xx.c
+++ b/drivers/usb/eth/smsc95xx.c
@@ -998,7 +998,7 @@ int smsc95xx_eth_recv(struct udevice *dev, int flags, uchar **packetp)
 	}
 
 	*packetp = ptr + sizeof(packet_len);
-	return packet_len;
+	return packet_len - 4;
 
 err:
 	usb_ether_advance_rxbuf(ueth, -1);
@@ -1009,7 +1009,7 @@ static int smsc95xx_free_pkt(struct udevice *dev, uchar *packet, int packet_len)
 {
 	struct smsc95xx_private *priv = dev_get_priv(dev);
 
-	packet_len = ALIGN(packet_len, 4);
+	packet_len = ALIGN(packet_len + sizeof(u32), 4);
 	usb_ether_advance_rxbuf(&priv->ueth, sizeof(u32) + packet_len);
 
 	return 0;
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 01/19] net: smsc95xx: Correct free_pkt() implementation

[Joe Hershberger]

On Sat, Apr 1, 2017 at 1:05 PM, Simon Glass <sjg@chromium.org> wrote:
> On further review this returns the wrong packet length from the driver.
> It may not be noticed since protocols will take care of it. Fix it by
> subtracting the header length from the packet length returned.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Simon Glass]

On Raspberry Pi 2 and 3 a problem was noticed when enabling driver model
for USB: the cache invalidate after an incoming transfer does not seem to
work correctly.

This may be a problem with the underlying caching implementation on armv7
and armv8 but this seems very unlikely. As a work-around, use separate
buffers for input and output. This ensures that the input buffer will not
hold dirty cache data.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5:
- Add new patch for dwc2 to se separate input and output buffers

Changes in v4: None
Changes in v3: None

 drivers/usb/host/dwc2.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
index d253b946f3..a65ed4f66c 100644
--- a/drivers/usb/host/dwc2.c
+++ b/drivers/usb/host/dwc2.c
@@ -31,10 +31,14 @@ DECLARE_GLOBAL_DATA_PTR;
 
 struct dwc2_priv {
 #ifdef CONFIG_DM_USB
-	uint8_t aligned_buffer[DWC2_DATA_BUF_SIZE] __aligned(ARCH_DMA_MINALIGN);
+	uint8_t aligned_buffer_in[DWC2_DATA_BUF_SIZE]
+			__aligned(ARCH_DMA_MINALIGN);
+	uint8_t aligned_buffer_out[DWC2_DATA_BUF_SIZE]
+			__aligned(ARCH_DMA_MINALIGN);
 	uint8_t status_buffer[DWC2_STATUS_BUF_SIZE] __aligned(ARCH_DMA_MINALIGN);
 #else
-	uint8_t *aligned_buffer;
+	uint8_t *aligned_buffer_in;
+	uint8_t *aligned_buffer_out;
 	uint8_t *status_buffer;
 #endif
 	u8 in_data_toggle[MAX_DEVICE][MAX_ENDPOINT];
@@ -769,10 +773,12 @@ static int dwc2_eptype[] = {
 	DWC2_HCCHAR_EPTYPE_BULK,
 };
 
-static int transfer_chunk(struct dwc2_hc_regs *hc_regs, void *aligned_buffer,
-			  u8 *pid, int in, void *buffer, int num_packets,
-			  int xfer_len, int *actual_len, int odd_frame)
+static int transfer_chunk(struct dwc2_hc_regs *hc_regs, void *aligned_buffer_in,
+			  void *aligned_buffer_out, u8 *pid, int in,
+			  void *buffer, int num_packets, int xfer_len,
+			  int *actual_len, int odd_frame)
 {
+	void *aligned_buffer;
 	int ret = 0;
 	uint32_t sub;
 
@@ -785,11 +791,14 @@ static int transfer_chunk(struct dwc2_hc_regs *hc_regs, void *aligned_buffer,
 	       &hc_regs->hctsiz);
 
 	if (!in && xfer_len) {
+		aligned_buffer = aligned_buffer_in;
 		memcpy(aligned_buffer, buffer, xfer_len);
 
 		flush_dcache_range((unsigned long)aligned_buffer,
 				   (unsigned long)aligned_buffer +
 				   roundup(xfer_len, ARCH_DMA_MINALIGN));
+	} else {
+		aligned_buffer = aligned_buffer_out;
 	}
 
 	writel(phys_to_bus((unsigned long)aligned_buffer), &hc_regs->hcdma);
@@ -901,7 +910,8 @@ int chunk_msg(struct dwc2_priv *priv, struct usb_device *dev,
 				odd_frame = 1;
 		}
 
-		ret = transfer_chunk(hc_regs, priv->aligned_buffer, pid,
+		ret = transfer_chunk(hc_regs, priv->aligned_buffer_in,
+				     priv->aligned_buffer_out, pid,
 				     in, (char *)buffer + done, num_packets,
 				     xfer_len, &actual_len, odd_frame);
 
@@ -1136,7 +1146,10 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
 	memset(priv, '\0', sizeof(*priv));
 	priv->root_hub_devnum = 0;
 	priv->regs = (struct dwc2_core_regs *)CONFIG_USB_DWC2_REG_ADDR;
-	priv->aligned_buffer = aligned_buffer_addr;
+
+	/* We can use the same buffer for input and output */
+	priv->aligned_buffer_in = aligned_buffer_addr;
+	priv->aligned_buffer_out = aligned_buffer_addr;
 	priv->status_buffer = status_buffer_addr;
 
 	/* board-dependant init */
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Marek Vasut]

On 04/01/2017 08:05 PM, Simon Glass wrote:
> On Raspberry Pi 2 and 3 a problem was noticed when enabling driver model
> for USB: the cache invalidate after an incoming transfer does not seem to
> work correctly.
> 
> This may be a problem with the underlying caching implementation on armv7
> and armv8 but this seems very unlikely. As a work-around, use separate
> buffers for input and output. This ensures that the input buffer will not
> hold dirty cache data.

What do you think of this patch:
[U-Boot] usb: dwc2: invalidate the dcache before starting the DMA

> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v5:
> - Add new patch for dwc2 to se separate input and output buffers
> 
> Changes in v4: None
> Changes in v3: None
> 
>  drivers/usb/host/dwc2.c | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
> index d253b946f3..a65ed4f66c 100644
> --- a/drivers/usb/host/dwc2.c
> +++ b/drivers/usb/host/dwc2.c
> @@ -31,10 +31,14 @@ DECLARE_GLOBAL_DATA_PTR;
>  
>  struct dwc2_priv {
>  #ifdef CONFIG_DM_USB
> -	uint8_t aligned_buffer[DWC2_DATA_BUF_SIZE] __aligned(ARCH_DMA_MINALIGN);
> +	uint8_t aligned_buffer_in[DWC2_DATA_BUF_SIZE]
> +			__aligned(ARCH_DMA_MINALIGN);
> +	uint8_t aligned_buffer_out[DWC2_DATA_BUF_SIZE]
> +			__aligned(ARCH_DMA_MINALIGN);
>  	uint8_t status_buffer[DWC2_STATUS_BUF_SIZE] __aligned(ARCH_DMA_MINALIGN);
>  #else
> -	uint8_t *aligned_buffer;
> +	uint8_t *aligned_buffer_in;
> +	uint8_t *aligned_buffer_out;
>  	uint8_t *status_buffer;
>  #endif
>  	u8 in_data_toggle[MAX_DEVICE][MAX_ENDPOINT];
> @@ -769,10 +773,12 @@ static int dwc2_eptype[] = {
>  	DWC2_HCCHAR_EPTYPE_BULK,
>  };
>  
> -static int transfer_chunk(struct dwc2_hc_regs *hc_regs, void *aligned_buffer,
> -			  u8 *pid, int in, void *buffer, int num_packets,
> -			  int xfer_len, int *actual_len, int odd_frame)
> +static int transfer_chunk(struct dwc2_hc_regs *hc_regs, void *aligned_buffer_in,
> +			  void *aligned_buffer_out, u8 *pid, int in,
> +			  void *buffer, int num_packets, int xfer_len,
> +			  int *actual_len, int odd_frame)
>  {
> +	void *aligned_buffer;
>  	int ret = 0;
>  	uint32_t sub;
>  
> @@ -785,11 +791,14 @@ static int transfer_chunk(struct dwc2_hc_regs *hc_regs, void *aligned_buffer,
>  	       &hc_regs->hctsiz);
>  
>  	if (!in && xfer_len) {
> +		aligned_buffer = aligned_buffer_in;
>  		memcpy(aligned_buffer, buffer, xfer_len);
>  
>  		flush_dcache_range((unsigned long)aligned_buffer,
>  				   (unsigned long)aligned_buffer +
>  				   roundup(xfer_len, ARCH_DMA_MINALIGN));
> +	} else {
> +		aligned_buffer = aligned_buffer_out;
>  	}
>  
>  	writel(phys_to_bus((unsigned long)aligned_buffer), &hc_regs->hcdma);
> @@ -901,7 +910,8 @@ int chunk_msg(struct dwc2_priv *priv, struct usb_device *dev,
>  				odd_frame = 1;
>  		}
>  
> -		ret = transfer_chunk(hc_regs, priv->aligned_buffer, pid,
> +		ret = transfer_chunk(hc_regs, priv->aligned_buffer_in,
> +				     priv->aligned_buffer_out, pid,
>  				     in, (char *)buffer + done, num_packets,
>  				     xfer_len, &actual_len, odd_frame);
>  
> @@ -1136,7 +1146,10 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
>  	memset(priv, '\0', sizeof(*priv));
>  	priv->root_hub_devnum = 0;
>  	priv->regs = (struct dwc2_core_regs *)CONFIG_USB_DWC2_REG_ADDR;
> -	priv->aligned_buffer = aligned_buffer_addr;
> +
> +	/* We can use the same buffer for input and output */
> +	priv->aligned_buffer_in = aligned_buffer_addr;
> +	priv->aligned_buffer_out = aligned_buffer_addr;
>  	priv->status_buffer = status_buffer_addr;
>  
>  	/* board-dependant init */
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Simon Glass]

Hi Marek,

On 1 April 2017 at 14:15, Marek Vasut <marex@denx.de> wrote:
> On 04/01/2017 08:05 PM, Simon Glass wrote:
>> On Raspberry Pi 2 and 3 a problem was noticed when enabling driver model
>> for USB: the cache invalidate after an incoming transfer does not seem to
>> work correctly.
>>
>> This may be a problem with the underlying caching implementation on armv7
>> and armv8 but this seems very unlikely. As a work-around, use separate
>> buffers for input and output. This ensures that the input buffer will not
>> hold dirty cache data.
>
> What do you think of this patch:
> [U-Boot] usb: dwc2: invalidate the dcache before starting the DMA

Yes that matches what I did as a hack. I didn't realise that the DMA
would go through the cache. Thanks for the pointer.

Regards,
Simon

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Marek Vasut]

On 04/02/2017 01:40 AM, Simon Glass wrote:
> Hi Marek,
> 
> On 1 April 2017 at 14:15, Marek Vasut <marex@denx.de> wrote:
>> On 04/01/2017 08:05 PM, Simon Glass wrote:
>>> On Raspberry Pi 2 and 3 a problem was noticed when enabling driver model
>>> for USB: the cache invalidate after an incoming transfer does not seem to
>>> work correctly.
>>>
>>> This may be a problem with the underlying caching implementation on armv7
>>> and armv8 but this seems very unlikely. As a work-around, use separate
>>> buffers for input and output. This ensures that the input buffer will not
>>> hold dirty cache data.
>>
>> What do you think of this patch:
>> [U-Boot] usb: dwc2: invalidate the dcache before starting the DMA
> 
> Yes that matches what I did as a hack. I didn't realise that the DMA
> would go through the cache. Thanks for the pointer.

DMA should not go through the cache. I have yet to review that patch,
but IMO it's relevant to this problem you observe.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Stefan Bruens]

On Sonntag, 2. April 2017 05:01:41 CEST Marek Vasut wrote:
> On 04/02/2017 01:40 AM, Simon Glass wrote:
> > Hi Marek,
> > 
> > On 1 April 2017 at 14:15, Marek Vasut <marex@denx.de> wrote:
> >> On 04/01/2017 08:05 PM, Simon Glass wrote:
> >>> On Raspberry Pi 2 and 3 a problem was noticed when enabling driver model
> >>> for USB: the cache invalidate after an incoming transfer does not seem
> >>> to
> >>> work correctly.
> >>> 
> >>> This may be a problem with the underlying caching implementation on
> >>> armv7
> >>> and armv8 but this seems very unlikely. As a work-around, use separate
> >>> buffers for input and output. This ensures that the input buffer will
> >>> not
> >>> hold dirty cache data.
> >> 
> >> What do you think of this patch:
> >> [U-Boot] usb: dwc2: invalidate the dcache before starting the DMA
> > 
> > Yes that matches what I did as a hack. I didn't realise that the DMA
> > would go through the cache. Thanks for the pointer.
> 
> DMA should not go through the cache. I have yet to review that patch,
> but IMO it's relevant to this problem you observe.

DMA transfers not going through the cache is probably the problem here:

Assume we have the aligned_buffer at address 0xdead0000

1. The cpu writes to address 0xdead0002. This is fine, as it is the current 
owner of the address. The cacheline is marked dirty.
2. The cpu no longer needs the corresponding address range, and it is 
reallocated (i.e. freed and then allocated from dwc2) or reused (i.e. formerly 
out buffer, now in buffer).
3. The CPU starts the DMA transfer
4. The DMA transfer writes to e.g. 0xdead0000-0xdead0200 in memory.
5. The CPU fetches an address aliasing with 0xdead0000. The dirty cache line 
is evicted, and the 0xdead0000-0xdead0040 memory contents are overwritten.

Obviously, the dirty cache line from (1.) has to be cleared at the beginning 
of (3.), as Eddys patch does.

Kind regards,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019
work: +49 2405 49936-424

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Simon Glass]

Hi Stefan,

On 2 April 2017 at 07:10, Stefan Bruens <stefan.bruens@rwth-aachen.de> wrote:
> On Sonntag, 2. April 2017 05:01:41 CEST Marek Vasut wrote:
>> On 04/02/2017 01:40 AM, Simon Glass wrote:
>> > Hi Marek,
>> >
>> > On 1 April 2017 at 14:15, Marek Vasut <marex@denx.de> wrote:
>> >> On 04/01/2017 08:05 PM, Simon Glass wrote:
>> >>> On Raspberry Pi 2 and 3 a problem was noticed when enabling driver model
>> >>> for USB: the cache invalidate after an incoming transfer does not seem
>> >>> to
>> >>> work correctly.
>> >>>
>> >>> This may be a problem with the underlying caching implementation on
>> >>> armv7
>> >>> and armv8 but this seems very unlikely. As a work-around, use separate
>> >>> buffers for input and output. This ensures that the input buffer will
>> >>> not
>> >>> hold dirty cache data.
>> >>
>> >> What do you think of this patch:
>> >> [U-Boot] usb: dwc2: invalidate the dcache before starting the DMA
>> >
>> > Yes that matches what I did as a hack. I didn't realise that the DMA
>> > would go through the cache. Thanks for the pointer.
>>
>> DMA should not go through the cache. I have yet to review that patch,
>> but IMO it's relevant to this problem you observe.
>
> DMA transfers not going through the cache is probably the problem here:
>
> Assume we have the aligned_buffer at address 0xdead0000
>
> 1. The cpu writes to address 0xdead0002. This is fine, as it is the current
> owner of the address. The cacheline is marked dirty.
> 2. The cpu no longer needs the corresponding address range, and it is
> reallocated (i.e. freed and then allocated from dwc2) or reused (i.e. formerly
> out buffer, now in buffer).
> 3. The CPU starts the DMA transfer
> 4. The DMA transfer writes to e.g. 0xdead0000-0xdead0200 in memory.
> 5. The CPU fetches an address aliasing with 0xdead0000. The dirty cache line
> is evicted, and the 0xdead0000-0xdead0040 memory contents are overwritten.

This is the part I don't understand. This should be an invalidate, not
a clean and invalidate, so there should be not memory write.

Also if the CPU fetches from cached 0xdead0000 without an invalidate,
it will not cause a cash clean. It will simple read the data from the
cache and ignore what the DMA wrote.

On armv8 we appear not to suppose invalidate in the code, so it makes
sense for rpi_3.

But for rpi_2 which seems to do a proper invalidate, I still don't see
the problem.

>
> Obviously, the dirty cache line from (1.) has to be cleared at the beginning
> of (3.), as Eddys patch does.

But I still don't understand why we have to clean instead of just invalidate?

Regards,
Simon

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Stefan Bruens]

On Sonntag, 2. April 2017 17:43:38 CEST Simon Glass wrote:
> Hi Stefan,
> 
> On 2 April 2017 at 07:10, Stefan Bruens <stefan.bruens@rwth-aachen.de> 
wrote:
> > On Sonntag, 2. April 2017 05:01:41 CEST Marek Vasut wrote:
> >> On 04/02/2017 01:40 AM, Simon Glass wrote:
> >> > Hi Marek,
> >> > 
> >> > On 1 April 2017 at 14:15, Marek Vasut <marex@denx.de> wrote:
> >> >> On 04/01/2017 08:05 PM, Simon Glass wrote:
> >> >>> On Raspberry Pi 2 and 3 a problem was noticed when enabling driver
> >> >>> model
> >> >>> for USB: the cache invalidate after an incoming transfer does not
> >> >>> seem
> >> >>> to
> >> >>> work correctly.
> >> >>> 
> >> >>> This may be a problem with the underlying caching implementation on
> >> >>> armv7
> >> >>> and armv8 but this seems very unlikely. As a work-around, use
> >> >>> separate
> >> >>> buffers for input and output. This ensures that the input buffer will
> >> >>> not
> >> >>> hold dirty cache data.
> >> >> 
> >> >> What do you think of this patch:
> >> >> [U-Boot] usb: dwc2: invalidate the dcache before starting the DMA
> >> > 
> >> > Yes that matches what I did as a hack. I didn't realise that the DMA
> >> > would go through the cache. Thanks for the pointer.
> >> 
> >> DMA should not go through the cache. I have yet to review that patch,
> >> but IMO it's relevant to this problem you observe.
> > 
> > DMA transfers not going through the cache is probably the problem here:
> > 
> > Assume we have the aligned_buffer at address 0xdead0000
> > 
> > 1. The cpu writes to address 0xdead0002. This is fine, as it is the
> > current
> > owner of the address. The cacheline is marked dirty.
> > 2. The cpu no longer needs the corresponding address range, and it is
> > reallocated (i.e. freed and then allocated from dwc2) or reused (i.e.
> > formerly out buffer, now in buffer).
> > 3. The CPU starts the DMA transfer
> > 4. The DMA transfer writes to e.g. 0xdead0000-0xdead0200 in memory.
> > 5. The CPU fetches an address aliasing with 0xdead0000. The dirty cache
> > line is evicted, and the 0xdead0000-0xdead0040 memory contents are
> > overwritten.
> This is the part I don't understand. This should be an invalidate, not
> a clean and invalidate, so there should be not memory write.
> 
> Also if the CPU fetches from cached 0xdead0000 without an invalidate,
> it will not cause a cash clean. It will simple read the data from the
> cache and ignore what the DMA wrote.

The CPU does not fetch 0xdead0000, but from an address *aliasing* with 
0xdead000. As 0xdead0000 is *dirty* (we have neither flushed (clears dirty 
bit) or invalidated (implicitly clears dirty for the address)), the cache 
controller has to write out the 0xdead0000 cache line to memory.

> On armv8 we appear not to suppose invalidate in the code, so it makes
> sense for rpi_3.

> But for rpi_2 which seems to do a proper invalidate, I still don't see
> the problem.

Which part of the code is different between rpi2 and rpi3? The dwc2 code is 
identical, is the memory invalidated in some other place?
 
> > Obviously, the dirty cache line from (1.) has to be cleared at the
> > beginning of (3.), as Eddys patch does.
> 
> But I still don't understand why we have to clean instead of just
> invalidate?

The patch by Eddie Cai just does an invalidate_dcache_range on the transfer 
buffer, nothing else. Where do you see a "clean" (whatever that refers to)?

Kind regards,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019
work: +49 2405 49936-424

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Simon Glass]

Hi Stefan,

On 2 April 2017 at 15:34, Stefan Bruens <stefan.bruens@rwth-aachen.de> wrote:
> On Sonntag, 2. April 2017 17:43:38 CEST Simon Glass wrote:
>> Hi Stefan,
>>
>> On 2 April 2017 at 07:10, Stefan Bruens <stefan.bruens@rwth-aachen.de>
> wrote:
>> > On Sonntag, 2. April 2017 05:01:41 CEST Marek Vasut wrote:
>> >> On 04/02/2017 01:40 AM, Simon Glass wrote:
>> >> > Hi Marek,
>> >> >
>> >> > On 1 April 2017 at 14:15, Marek Vasut <marex@denx.de> wrote:
>> >> >> On 04/01/2017 08:05 PM, Simon Glass wrote:
>> >> >>> On Raspberry Pi 2 and 3 a problem was noticed when enabling driver
>> >> >>> model
>> >> >>> for USB: the cache invalidate after an incoming transfer does not
>> >> >>> seem
>> >> >>> to
>> >> >>> work correctly.
>> >> >>>
>> >> >>> This may be a problem with the underlying caching implementation on
>> >> >>> armv7
>> >> >>> and armv8 but this seems very unlikely. As a work-around, use
>> >> >>> separate
>> >> >>> buffers for input and output. This ensures that the input buffer will
>> >> >>> not
>> >> >>> hold dirty cache data.
>> >> >>
>> >> >> What do you think of this patch:
>> >> >> [U-Boot] usb: dwc2: invalidate the dcache before starting the DMA
>> >> >
>> >> > Yes that matches what I did as a hack. I didn't realise that the DMA
>> >> > would go through the cache. Thanks for the pointer.
>> >>
>> >> DMA should not go through the cache. I have yet to review that patch,
>> >> but IMO it's relevant to this problem you observe.
>> >
>> > DMA transfers not going through the cache is probably the problem here:
>> >
>> > Assume we have the aligned_buffer at address 0xdead0000
>> >
>> > 1. The cpu writes to address 0xdead0002. This is fine, as it is the
>> > current
>> > owner of the address. The cacheline is marked dirty.
>> > 2. The cpu no longer needs the corresponding address range, and it is
>> > reallocated (i.e. freed and then allocated from dwc2) or reused (i.e.
>> > formerly out buffer, now in buffer).
>> > 3. The CPU starts the DMA transfer
>> > 4. The DMA transfer writes to e.g. 0xdead0000-0xdead0200 in memory.
>> > 5. The CPU fetches an address aliasing with 0xdead0000. The dirty cache
>> > line is evicted, and the 0xdead0000-0xdead0040 memory contents are
>> > overwritten.
>> This is the part I don't understand. This should be an invalidate, not
>> a clean and invalidate, so there should be not memory write.
>>
>> Also if the CPU fetches from cached 0xdead0000 without an invalidate,
>> it will not cause a cash clean. It will simple read the data from the
>> cache and ignore what the DMA wrote.
>
> The CPU does not fetch 0xdead0000, but from an address *aliasing* with
> 0xdead000. As 0xdead0000 is *dirty* (we have neither flushed (clears dirty
> bit) or invalidated (implicitly clears dirty for the address)), the cache
> controller has to write out the 0xdead0000 cache line to memory.

That doesn't make any sense to me. Can you explain it a bit more?

If the CPU fetches from a cache-alias of 0xdead0000, say 0xa11a5000
then I expect the cache line would contain the data for that alias,
not for 0xdead0000. So a later invalidate of 0xdead0000 should at most
clean the cache line and write to memory at 0xa11a5000. If it were to
write cached data intended for 0xa11a5000 to memory at 0xdead0000,
then surely this would be a bug?

Therefore I cannot see the situation where the CPU should write to
0xdead0000 when that address is invalidated.

I suspect what is happening is something like this:

1. Driver writes to memory to USB, using 0xdead0000 as an output buffer
2. In the process of this, the data ends up in the cache, since the
CPU allocates cache lines on write
3. Later the driver wants to do a read, using 0xdead0000 as an input buffer
4. The cache still contains the data at 0xdead0000 from the previous
write operation
5. DMA reads the data from USB and puts it at 0xdead0000
6. The CPU invalidates the cache at 0xdead0000 (i.e. throws away its
previously cached data)
7. The CPU reads the data (which should now come from the cache)

I think something is happening at 6 which I don't understand.

>
>> On armv8 we appear not to suppose invalidate in the code, so it makes
>> sense for rpi_3.
>
>> But for rpi_2 which seems to do a proper invalidate, I still don't see
>> the problem.
>
> Which part of the code is different between rpi2 and rpi3? The dwc2 code is
> identical, is the memory invalidated in some other place?

It is the invalidate_dcache_range() function.

>
>> > Obviously, the dirty cache line from (1.) has to be cleared at the
>> > beginning of (3.), as Eddys patch does.
>>
>> But I still don't understand why we have to clean instead of just
>> invalidate?
>
> The patch by Eddie Cai just does an invalidate_dcache_range on the transfer
> buffer, nothing else. Where do you see a "clean" (whatever that refers to)?

In the armv8  invalidate_dcache_range() function.
>

Regards,
Simon

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Brüns, Stefan]

On Montag, 3. April 2017 01:23:17 CEST you wrote:
> Hi Stefan,
> 
> On 2 April 2017 at 15:34, Stefan Bruens <stefan.bruens@rwth-aachen.de> 
wrote:
> > On Sonntag, 2. April 2017 17:43:38 CEST Simon Glass wrote:
> >> Hi Stefan,
> >> 
> >> On 2 April 2017 at 07:10, Stefan Bruens <stefan.bruens@rwth-aachen.de>
> > 
> > wrote:
> >> > On Sonntag, 2. April 2017 05:01:41 CEST Marek Vasut wrote:
> >> >> On 04/02/2017 01:40 AM, Simon Glass wrote:
> >> >> > Hi Marek,
> >> >> > 
> >> >> > On 1 April 2017 at 14:15, Marek Vasut <marex@denx.de> wrote:
> >> >> >> On 04/01/2017 08:05 PM, Simon Glass wrote:
> >> >> >>> On Raspberry Pi 2 and 3 a problem was noticed when enabling driver
> >> >> >>> model
> >> >> >>> for USB: the cache invalidate after an incoming transfer does not
> >> >> >>> seem
> >> >> >>> to
> >> >> >>> work correctly.
> >> >> >>> 
> >> >> >>> This may be a problem with the underlying caching implementation
> >> >> >>> on
> >> >> >>> armv7
> >> >> >>> and armv8 but this seems very unlikely. As a work-around, use
> >> >> >>> separate
> >> >> >>> buffers for input and output. This ensures that the input buffer
> >> >> >>> will
> >> >> >>> not
> >> >> >>> hold dirty cache data.
> >> >> >> 
> >> >> >> What do you think of this patch:
> >> >> >> [U-Boot] usb: dwc2: invalidate the dcache before starting the DMA
> >> >> > 
> >> >> > Yes that matches what I did as a hack. I didn't realise that the DMA
> >> >> > would go through the cache. Thanks for the pointer.
> >> >> 
> >> >> DMA should not go through the cache. I have yet to review that patch,
> >> >> but IMO it's relevant to this problem you observe.
> >> > 
> >> > DMA transfers not going through the cache is probably the problem here:
> >> > 
> >> > Assume we have the aligned_buffer at address 0xdead0000
> >> > 
> >> > 1. The cpu writes to address 0xdead0002. This is fine, as it is the
> >> > current
> >> > owner of the address. The cacheline is marked dirty.
> >> > 2. The cpu no longer needs the corresponding address range, and it is
> >> > reallocated (i.e. freed and then allocated from dwc2) or reused (i.e.
> >> > formerly out buffer, now in buffer).
> >> > 3. The CPU starts the DMA transfer
> >> > 4. The DMA transfer writes to e.g. 0xdead0000-0xdead0200 in memory.
> >> > 5. The CPU fetches an address aliasing with 0xdead0000. The dirty cache
> >> > line is evicted, and the 0xdead0000-0xdead0040 memory contents are
> >> > overwritten.
> >> 
> >> This is the part I don't understand. This should be an invalidate, not
> >> a clean and invalidate, so there should be not memory write.
> >> 
> >> Also if the CPU fetches from cached 0xdead0000 without an invalidate,
> >> it will not cause a cash clean. It will simple read the data from the
> >> cache and ignore what the DMA wrote.
> > 
> > The CPU does not fetch 0xdead0000, but from an address *aliasing* with
> > 0xdead000. As 0xdead0000 is *dirty* (we have neither flushed (clears dirty
> > bit) or invalidated (implicitly clears dirty for the address)), the cache
> > controller has to write out the 0xdead0000 cache line to memory.
> 
> That doesn't make any sense to me. Can you explain it a bit more?
> 
> If the CPU fetches from a cache-alias of 0xdead0000, say 0xa11a5000
> then I expect the cache line would contain the data for that alias,
> not for 0xdead0000.

The important part is the dirty flag in the 0xdead0000 cacheline. By reading 
an aliasing address, you are causing eviction of the current cache line 
contents, and writing back its contents to memory. Reading of an address may 
cause write of a different address. You can see it as an dcache_flush_range 
done by the cache controller.

I think you are assuming a write-through cache here, which leads to your 
confusion.

> So a later invalidate of 0xdead0000 should at most
> clean the cache line and write to memory at 0xa11a5000. If it were to
> write cached data intended for 0xa11a5000 to memory at 0xdead0000,
> then surely this would be a bug?

After the cache line for 0xdead0000 has been evicted, any flush/invalidate 
operations are noops for that address.

> Therefore I cannot see the situation where the CPU should write to
> 0xdead0000 when that address is invalidated.

It is not the invalidation which causes the write, but eviction from the 
cache.

 > >> On armv8 we appear not to suppose invalidate in the code, so it makes
> >> sense for rpi_3.
> >> 
> >> But for rpi_2 which seems to do a proper invalidate, I still don't see
> >> the problem.
> > 
> > Which part of the code is different between rpi2 and rpi3? The dwc2 code
> > is
> > identical, is the memory invalidated in some other place?
> 
> It is the invalidate_dcache_range() function.

Thats for pointing that out, for anyone not having read the code:

ARMv7 has different operations for flush_dcache_range and 
invalidate_dcache_range, the former does a writeback of dirty cache lines and 
sets the invalid tags for the corresponding cache lines, the latter only does 
the invalidate part.

ARMv8 does a writeback for both operations. I assume thats what you call 
"improper".

The important part is, calling flush multiple times in a row is *exactly* the 
same thing as calling flush once. Calling flush instead of invalidate is the 
same thing *if* the dirty flag is not set, as the writeback part is skipped in 
that case.
 
> >> > Obviously, the dirty cache line from (1.) has to be cleared at the
> >> > beginning of (3.), as Eddys patch does.
> >> 
> >> But I still don't understand why we have to clean instead of just
> >> invalidate?
> > 
> > The patch by Eddie Cai just does an invalidate_dcache_range on the
> > transfer
> > buffer, nothing else. Where do you see a "clean" (whatever that refers
> > to)?
> 
> In the armv8  invalidate_dcache_range() function.

The writeback does not happen, as the cacheline is not dirty. It should not 
even be in the cache after invalidate has been called once.

We have to make sure the buffers address range is not in the cache prior to 
starting the DMA. We can either use invalidate_dcache_range or 
flush_dcache_range to guarantee this. Which one we use does not matter here, 
although invalidate only is typically a little bit more lightweight.

Kind regards,

Stefan

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Simon Glass]

Hi Stefan,

On 3 April 2017 at 08:26, Brüns, Stefan <Stefan.Bruens@rwth-aachen.de> wrote:
> On Montag, 3. April 2017 01:23:17 CEST you wrote:
>> Hi Stefan,
>>
>> On 2 April 2017 at 15:34, Stefan Bruens <stefan.bruens@rwth-aachen.de>
> wrote:
>> > On Sonntag, 2. April 2017 17:43:38 CEST Simon Glass wrote:
>> >> Hi Stefan,
>> >>
>> >> On 2 April 2017 at 07:10, Stefan Bruens <stefan.bruens@rwth-aachen.de>
>> >
>> > wrote:
>> >> > On Sonntag, 2. April 2017 05:01:41 CEST Marek Vasut wrote:
>> >> >> On 04/02/2017 01:40 AM, Simon Glass wrote:
>> >> >> > Hi Marek,
>> >> >> >
>> >> >> > On 1 April 2017 at 14:15, Marek Vasut <marex@denx.de> wrote:
>> >> >> >> On 04/01/2017 08:05 PM, Simon Glass wrote:
>> >> >> >>> On Raspberry Pi 2 and 3 a problem was noticed when enabling driver
>> >> >> >>> model
>> >> >> >>> for USB: the cache invalidate after an incoming transfer does not
>> >> >> >>> seem
>> >> >> >>> to
>> >> >> >>> work correctly.
>> >> >> >>>
>> >> >> >>> This may be a problem with the underlying caching implementation
>> >> >> >>> on
>> >> >> >>> armv7
>> >> >> >>> and armv8 but this seems very unlikely. As a work-around, use
>> >> >> >>> separate
>> >> >> >>> buffers for input and output. This ensures that the input buffer
>> >> >> >>> will
>> >> >> >>> not
>> >> >> >>> hold dirty cache data.
>> >> >> >>
>> >> >> >> What do you think of this patch:
>> >> >> >> [U-Boot] usb: dwc2: invalidate the dcache before starting the DMA
>> >> >> >
>> >> >> > Yes that matches what I did as a hack. I didn't realise that the DMA
>> >> >> > would go through the cache. Thanks for the pointer.
>> >> >>
>> >> >> DMA should not go through the cache. I have yet to review that patch,
>> >> >> but IMO it's relevant to this problem you observe.
>> >> >
>> >> > DMA transfers not going through the cache is probably the problem here:
>> >> >
>> >> > Assume we have the aligned_buffer at address 0xdead0000
>> >> >
>> >> > 1. The cpu writes to address 0xdead0002. This is fine, as it is the
>> >> > current
>> >> > owner of the address. The cacheline is marked dirty.
>> >> > 2. The cpu no longer needs the corresponding address range, and it is
>> >> > reallocated (i.e. freed and then allocated from dwc2) or reused (i.e.
>> >> > formerly out buffer, now in buffer).
>> >> > 3. The CPU starts the DMA transfer
>> >> > 4. The DMA transfer writes to e.g. 0xdead0000-0xdead0200 in memory.
>> >> > 5. The CPU fetches an address aliasing with 0xdead0000. The dirty cache
>> >> > line is evicted, and the 0xdead0000-0xdead0040 memory contents are
>> >> > overwritten.
>> >>
>> >> This is the part I don't understand. This should be an invalidate, not
>> >> a clean and invalidate, so there should be not memory write.
>> >>
>> >> Also if the CPU fetches from cached 0xdead0000 without an invalidate,
>> >> it will not cause a cash clean. It will simple read the data from the
>> >> cache and ignore what the DMA wrote.
>> >
>> > The CPU does not fetch 0xdead0000, but from an address *aliasing* with
>> > 0xdead000. As 0xdead0000 is *dirty* (we have neither flushed (clears dirty
>> > bit) or invalidated (implicitly clears dirty for the address)), the cache
>> > controller has to write out the 0xdead0000 cache line to memory.
>>
>> That doesn't make any sense to me. Can you explain it a bit more?
>>
>> If the CPU fetches from a cache-alias of 0xdead0000, say 0xa11a5000
>> then I expect the cache line would contain the data for that alias,
>> not for 0xdead0000.
>
> The important part is the dirty flag in the 0xdead0000 cacheline. By reading
> an aliasing address, you are causing eviction of the current cache line
> contents, and writing back its contents to memory. Reading of an address may
> cause write of a different address. You can see it as an dcache_flush_range
> done by the cache controller.

OK so I think you are saying that reading from 0xa11a5000 causes dirty
data to be flushed from the cache to 0xdead0000. Makes perfect sense.
But why is there dirty data at 0xdead0000?

- If we did a write last time, then it would have been dirty until we
flushed the cache line, which we did before telling the DMA to start
up.

- If we did a read last time, then it is clean. We have read the data,
but not changed it.

What am I missing?

>
> I think you are assuming a write-through cache here, which leads to your
> confusion.

No that's a separate issue.

>
>> So a later invalidate of 0xdead0000 should at most
>> clean the cache line and write to memory at 0xa11a5000. If it were to
>> write cached data intended for 0xa11a5000 to memory at 0xdead0000,
>> then surely this would be a bug?
>
> After the cache line for 0xdead0000 has been evicted, any flush/invalidate
> operations are noops for that address.

OK good, so that's not the problem.

>
>> Therefore I cannot see the situation where the CPU should write to
>> 0xdead0000 when that address is invalidated.
>
> It is not the invalidation which causes the write, but eviction from the
> cache.
>
>  > >> On armv8 we appear not to suppose invalidate in the code, so it makes
>> >> sense for rpi_3.
>> >>
>> >> But for rpi_2 which seems to do a proper invalidate, I still don't see
>> >> the problem.
>> >
>> > Which part of the code is different between rpi2 and rpi3? The dwc2 code
>> > is
>> > identical, is the memory invalidated in some other place?
>>
>> It is the invalidate_dcache_range() function.
>
> Thats for pointing that out, for anyone not having read the code:
>
> ARMv7 has different operations for flush_dcache_range and
> invalidate_dcache_range, the former does a writeback of dirty cache lines and
> sets the invalid tags for the corresponding cache lines, the latter only does
> the invalidate part.
>
> ARMv8 does a writeback for both operations. I assume thats what you call
> "improper".

Yes, I believe it is wrong. Linux has a proper invalidate, why not U-Boot?

>
> The important part is, calling flush multiple times in a row is *exactly* the
> same thing as calling flush once. Calling flush instead of invalidate is the
> same thing *if* the dirty flag is not set, as the writeback part is skipped in
> that case.

Yes indeed.

>
>> >> > Obviously, the dirty cache line from (1.) has to be cleared at the
>> >> > beginning of (3.), as Eddys patch does.
>> >>
>> >> But I still don't understand why we have to clean instead of just
>> >> invalidate?
>> >
>> > The patch by Eddie Cai just does an invalidate_dcache_range on the
>> > transfer
>> > buffer, nothing else. Where do you see a "clean" (whatever that refers
>> > to)?
>>
>> In the armv8  invalidate_dcache_range() function.
>
> The writeback does not happen, as the cacheline is not dirty. It should not
> even be in the cache after invalidate has been called once.
>
> We have to make sure the buffers address range is not in the cache prior to
> starting the DMA. We can either use invalidate_dcache_range or
> flush_dcache_range to guarantee this. Which one we use does not matter here,
> although invalidate only is typically a little bit more lightweight.

Yes. Just to restate my assertion. It should be possible to:

- have some dirty data in the cache
- start up DMA
- invalidate that data
- read it

in that order. It should not be necessary to move the invalidate to
before the DMA start-up, right?

Regards,
Simon

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Brüns, Stefan]

On Montag, 3. April 2017 17:38:40 CEST you wrote:
> Hi Stefan,
> 
> On 3 April 2017 at 08:26, Brüns, Stefan <Stefan.Bruens@rwth-aachen.de> 
wrote:
> > On Montag, 3. April 2017 01:23:17 CEST you wrote:
> >> Hi Stefan,
> >> 
> >> On 2 April 2017 at 15:34, Stefan Bruens <stefan.bruens@rwth-aachen.de>
> > 
> > wrote:
> >> > On Sonntag, 2. April 2017 17:43:38 CEST Simon Glass wrote:
> >> >> Hi Stefan,
> >> >> 
> >> >> On 2 April 2017 at 07:10, Stefan Bruens <stefan.bruens@rwth-aachen.de>
> >> > 
> >> > wrote:
> >> >> > On Sonntag, 2. April 2017 05:01:41 CEST Marek Vasut wrote:
> >> >> >> On 04/02/2017 01:40 AM, Simon Glass wrote:
> >> >> >> > Hi Marek,
> >> >> >> > 
> >> >> >> > On 1 April 2017 at 14:15, Marek Vasut <marex@denx.de> wrote:
> >> >> >> >> On 04/01/2017 08:05 PM, Simon Glass wrote:
> >> >> >> >>> On Raspberry Pi 2 and 3 a problem was noticed when enabling
> >> >> >> >>> driver
> >> >> >> >>> model
> >> >> >> >>> for USB: the cache invalidate after an incoming transfer does
> >> >> >> >>> not
> >> >> >> >>> seem
> >> >> >> >>> to
> >> >> >> >>> work correctly.
> >> >> >> >>> 
> >> >> >> >>> This may be a problem with the underlying caching
> >> >> >> >>> implementation
> >> >> >> >>> on
> >> >> >> >>> armv7
> >> >> >> >>> and armv8 but this seems very unlikely. As a work-around, use
> >> >> >> >>> separate
> >> >> >> >>> buffers for input and output. This ensures that the input
> >> >> >> >>> buffer
> >> >> >> >>> will
> >> >> >> >>> not
> >> >> >> >>> hold dirty cache data.
> >> >> >> >> 
> >> >> >> >> What do you think of this patch:
> >> >> >> >> [U-Boot] usb: dwc2: invalidate the dcache before starting the
> >> >> >> >> DMA
> >> >> >> > 
> >> >> >> > Yes that matches what I did as a hack. I didn't realise that the
> >> >> >> > DMA
> >> >> >> > would go through the cache. Thanks for the pointer.
> >> >> >> 
> >> >> >> DMA should not go through the cache. I have yet to review that
> >> >> >> patch,
> >> >> >> but IMO it's relevant to this problem you observe.
> >> >> > 
> >> >> > DMA transfers not going through the cache is probably the problem
> >> >> > here:
> >> >> > 
> >> >> > Assume we have the aligned_buffer at address 0xdead0000
> >> >> > 
> >> >> > 1. The cpu writes to address 0xdead0002. This is fine, as it is the
> >> >> > current
> >> >> > owner of the address. The cacheline is marked dirty.
> >> >> > 2. The cpu no longer needs the corresponding address range, and it
> >> >> > is
> >> >> > reallocated (i.e. freed and then allocated from dwc2) or reused
> >> >> > (i.e.
> >> >> > formerly out buffer, now in buffer).
> >> >> > 3. The CPU starts the DMA transfer
> >> >> > 4. The DMA transfer writes to e.g. 0xdead0000-0xdead0200 in memory.
> >> >> > 5. The CPU fetches an address aliasing with 0xdead0000. The dirty
> >> >> > cache
> >> >> > line is evicted, and the 0xdead0000-0xdead0040 memory contents are
> >> >> > overwritten.
> >> >> 
> >> >> This is the part I don't understand. This should be an invalidate, not
> >> >> a clean and invalidate, so there should be not memory write.
> >> >> 
> >> >> Also if the CPU fetches from cached 0xdead0000 without an invalidate,
> >> >> it will not cause a cash clean. It will simple read the data from the
> >> >> cache and ignore what the DMA wrote.
> >> > 
> >> > The CPU does not fetch 0xdead0000, but from an address *aliasing* with
> >> > 0xdead000. As 0xdead0000 is *dirty* (we have neither flushed (clears
> >> > dirty
> >> > bit) or invalidated (implicitly clears dirty for the address)), the
> >> > cache
> >> > controller has to write out the 0xdead0000 cache line to memory.
> >> 
> >> That doesn't make any sense to me. Can you explain it a bit more?
> >> 
> >> If the CPU fetches from a cache-alias of 0xdead0000, say 0xa11a5000
> >> then I expect the cache line would contain the data for that alias,
> >> not for 0xdead0000.
> > 
> > The important part is the dirty flag in the 0xdead0000 cacheline. By
> > reading an aliasing address, you are causing eviction of the current
> > cache line contents, and writing back its contents to memory. Reading of
> > an address may cause write of a different address. You can see it as an
> > dcache_flush_range done by the cache controller.
> 
> OK so I think you are saying that reading from 0xa11a5000 causes dirty
> data to be flushed from the cache to 0xdead0000. Makes perfect sense.
> But why is there dirty data at 0xdead0000?
> 
> - If we did a write last time, then it would have been dirty until we
> flushed the cache line, which we did before telling the DMA to start
> up.
> 
> - If we did a read last time, then it is clean. We have read the data,
> but not changed it.
>
> What am I missing?

The following is a gross oversimplification, but might explain it:

1. Assume all of the 64kB of the aligned_buffer is dirty. (Likely, if the 
buffer is calloced.)
2. We are doing some transfers. All transfers are small, e.g. 64 byte.
3. In accordance with the two cases you mentioned above, the first 64 byte are 
no longer dirty, as the last out transfer has flushed the cacheline.
4. We are doing our first large in transfer (i.e. larger than 64 byte).
5. Bad Things (TM) may happen to any data at aligned_buffer[64] and beyond.

If this holds, a single invalidate_dcache_range(aligned_buffer, aligned_buffer
+65536,...) during the initialization of the controller would suffice.
 
> > I think you are assuming a write-through cache here, which leads to your
> > confusion.
> 
> No that's a separate issue.
> 
> >> So a later invalidate of 0xdead0000 should at most
> >> clean the cache line and write to memory at 0xa11a5000. If it were to
> >> write cached data intended for 0xa11a5000 to memory at 0xdead0000,
> >> then surely this would be a bug?
> > 
> > After the cache line for 0xdead0000 has been evicted, any flush/invalidate
> > operations are noops for that address.
> 
> OK good, so that's not the problem.
> 
> >> Therefore I cannot see the situation where the CPU should write to
> >> 0xdead0000 when that address is invalidated.
> > 
> > It is not the invalidation which causes the write, but eviction from the
> > cache.
> > 
> >  > >> On armv8 we appear not to suppose invalidate in the code, so it
> >  > >> makes
> >> >> 
> >> >> sense for rpi_3.
> >> >> 
> >> >> But for rpi_2 which seems to do a proper invalidate, I still don't see
> >> >> the problem.
> >> > 
> >> > Which part of the code is different between rpi2 and rpi3? The dwc2
> >> > code
> >> > is
> >> > identical, is the memory invalidated in some other place?
> >> 
> >> It is the invalidate_dcache_range() function.
> > 
> > Thats for pointing that out, for anyone not having read the code:
> > 
> > ARMv7 has different operations for flush_dcache_range and
> > invalidate_dcache_range, the former does a writeback of dirty cache lines
> > and sets the invalid tags for the corresponding cache lines, the latter
> > only does the invalidate part.
> > 
> > ARMv8 does a writeback for both operations. I assume thats what you call
> > "improper".
> 
> Yes, I believe it is wrong. Linux has a proper invalidate, why not U-Boot?

For why it does not exist for ARMv8 U-Boot, I can not answer.

The fact invalidate actually is a flush on ARMv8 makes the problem much more 
likely to happen. If the buffer, which is a member of dwc2_priv, is memset 
during the initialization it *will* be dirty. Any in transfer which is larger 
than any previous in or out transfer will cause a flush of the tail, up to 
xfer_len, of the transfer buffer.

On ARMv7 the problem will be only apparent if a cache eviction happens during 
the DMA.
 
> > The important part is, calling flush multiple times in a row is *exactly*
> > the same thing as calling flush once. Calling flush instead of invalidate
> > is the same thing *if* the dirty flag is not set, as the writeback part
> > is skipped in that case.
> 
> Yes indeed.
> 
> >> >> > Obviously, the dirty cache line from (1.) has to be cleared at the
> >> >> > beginning of (3.), as Eddys patch does.
> >> >> 
> >> >> But I still don't understand why we have to clean instead of just
> >> >> invalidate?
> >> > 
> >> > The patch by Eddie Cai just does an invalidate_dcache_range on the
> >> > transfer
> >> > buffer, nothing else. Where do you see a "clean" (whatever that refers
> >> > to)?
> >> 
> >> In the armv8  invalidate_dcache_range() function.
> > 
> > The writeback does not happen, as the cacheline is not dirty. It should
> > not
> > even be in the cache after invalidate has been called once.
> > 
> > We have to make sure the buffers address range is not in the cache prior
> > to
> > starting the DMA. We can either use invalidate_dcache_range or
> > flush_dcache_range to guarantee this. Which one we use does not matter
> > here, although invalidate only is typically a little bit more
> > lightweight.
> Yes. Just to restate my assertion. It should be possible to:
> 
> - have some dirty data in the cache
> - start up DMA
> - invalidate that data
> - read it
> 
> in that order. It should not be necessary to move the invalidate to
> before the DMA start-up, right?

Unfortunately that won't be enough. *If* there is some dirty data in the 
cache, it can be written back to main memory *any time*. If you are lucky, the 
writeback happens before the memory location is written by the DMA controller, 
but there is no guarantee until you flush and/or invalidate (either suffices).

Kind regards,

Stefan

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Brüns, Stefan]

On Montag, 3. April 2017 20:18:42 CEST Brüns, Stefan wrote:
> On Montag, 3. April 2017 17:38:40 CEST you wrote:
> > 
> > What am I missing?
> 
> The following is a gross oversimplification, but might explain it:
> 
> 1. Assume all of the 64kB of the aligned_buffer is dirty. (Likely, if the
> buffer is calloced.)
> 2. We are doing some transfers. All transfers are small, e.g. 64 byte.
> 3. In accordance with the two cases you mentioned above, the first 64 byte
> are no longer dirty, as the last out transfer has flushed the cacheline. 4.
> We are doing our first large in transfer (i.e. larger than 64 byte). 5. Bad
> Things (TM) may happen to any data at aligned_buffer[64] and beyond.
> 
> If this holds, a single invalidate_dcache_range(aligned_buffer,
> aligned_buffer +65536,...) during the initialization of the controller
> would suffice.

I just read "[U-Boot] [PATCH] usb: dwc3: gadget: make cache-maintenance on 
event buffers more robust" from Philipp Tomsich, which mentions the same 
problem:

The original code was doing the following (on AArch64, which
translates a 'flush' into a 'clean + invalidate'):
  # during initialisation:
      1. allocate buffers via memalign
         => buffers may still be modified (cached, dirty)
  # during interrupt processing
      2. clean + invalidate buffers
         => may commit stale data from a modified cacheline
      3. read from buffers

This could lead to garbage info being written to buffers before
reading them during even-processing.

To make the event processing more robust, we use the following sequence
for the cache-maintenance:
  # during initialisation:
      1. allocate buffers via memalign
      2. clean + invalidate buffers
         (we only need the 'invalidate' part, but dwc3_flush_cache()
          always performs a 'clean + invalidate')


Kind regards,

Stefan

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Simon Glass]

Hi Stefan,

On 3 April 2017 at 12:24, Brüns, Stefan <Stefan.Bruens@rwth-aachen.de> wrote:
> On Montag, 3. April 2017 20:18:42 CEST Brüns, Stefan wrote:
>> On Montag, 3. April 2017 17:38:40 CEST you wrote:
>> >
>> > What am I missing?
>>
>> The following is a gross oversimplification, but might explain it:
>>
>> 1. Assume all of the 64kB of the aligned_buffer is dirty. (Likely, if the
>> buffer is calloced.)
>> 2. We are doing some transfers. All transfers are small, e.g. 64 byte.
>> 3. In accordance with the two cases you mentioned above, the first 64 byte
>> are no longer dirty, as the last out transfer has flushed the cacheline. 4.
>> We are doing our first large in transfer (i.e. larger than 64 byte). 5. Bad
>> Things (TM) may happen to any data at aligned_buffer[64] and beyond.
>>
>> If this holds, a single invalidate_dcache_range(aligned_buffer,
>> aligned_buffer +65536,...) during the initialization of the controller
>> would suffice.
>
> I just read "[U-Boot] [PATCH] usb: dwc3: gadget: make cache-maintenance on
> event buffers more robust" from Philipp Tomsich, which mentions the same
> problem:
>
> The original code was doing the following (on AArch64, which
> translates a 'flush' into a 'clean + invalidate'):
>   # during initialisation:
>       1. allocate buffers via memalign
>          => buffers may still be modified (cached, dirty)
>   # during interrupt processing
>       2. clean + invalidate buffers
>          => may commit stale data from a modified cacheline
>       3. read from buffers
>
> This could lead to garbage info being written to buffers before
> reading them during even-processing.
>
> To make the event processing more robust, we use the following sequence
> for the cache-maintenance:
>   # during initialisation:
>       1. allocate buffers via memalign
>       2. clean + invalidate buffers
>          (we only need the 'invalidate' part, but dwc3_flush_cache()
>           always performs a 'clean + invalidate')

Yes that explains why the cache is dirty when the driver starts. I
hadn't thought of that. IMO changing the init sequence is a better
solution.

Regards,
Simon

[U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers

[Simon Glass]

Hi Stefan,

On 3 April 2017 at 12:18, Brüns, Stefan <Stefan.Bruens@rwth-aachen.de> wrote:
> On Montag, 3. April 2017 17:38:40 CEST you wrote:
>> Hi Stefan,
>>
>> On 3 April 2017 at 08:26, Brüns, Stefan <Stefan.Bruens@rwth-aachen.de>
> wrote:
>> > On Montag, 3. April 2017 01:23:17 CEST you wrote:
>> >> Hi Stefan,
>> >>
>> >> On 2 April 2017 at 15:34, Stefan Bruens <stefan.bruens@rwth-aachen.de>
>> >
>> > wrote:
>> >> > On Sonntag, 2. April 2017 17:43:38 CEST Simon Glass wrote:
>> >> >> Hi Stefan,
>> >> >>
>> >> >> On 2 April 2017 at 07:10, Stefan Bruens <stefan.bruens@rwth-aachen.de>
>> >> >
>> >> > wrote:
>> >> >> > On Sonntag, 2. April 2017 05:01:41 CEST Marek Vasut wrote:
>> >> >> >> On 04/02/2017 01:40 AM, Simon Glass wrote:
>> >> >> >> > Hi Marek,
>> >> >> >> >
>> >> >> >> > On 1 April 2017 at 14:15, Marek Vasut <marex@denx.de> wrote:
>> >> >> >> >> On 04/01/2017 08:05 PM, Simon Glass wrote:
>> >> >> >> >>> On Raspberry Pi 2 and 3 a problem was noticed when enabling
>> >> >> >> >>> driver
>> >> >> >> >>> model
>> >> >> >> >>> for USB: the cache invalidate after an incoming transfer does
>> >> >> >> >>> not
>> >> >> >> >>> seem
>> >> >> >> >>> to
>> >> >> >> >>> work correctly.
>> >> >> >> >>>
>> >> >> >> >>> This may be a problem with the underlying caching
>> >> >> >> >>> implementation
>> >> >> >> >>> on
>> >> >> >> >>> armv7
>> >> >> >> >>> and armv8 but this seems very unlikely. As a work-around, use
>> >> >> >> >>> separate
>> >> >> >> >>> buffers for input and output. This ensures that the input
>> >> >> >> >>> buffer
>> >> >> >> >>> will
>> >> >> >> >>> not
>> >> >> >> >>> hold dirty cache data.
>> >> >> >> >>
>> >> >> >> >> What do you think of this patch:
>> >> >> >> >> [U-Boot] usb: dwc2: invalidate the dcache before starting the
>> >> >> >> >> DMA
>> >> >> >> >
>> >> >> >> > Yes that matches what I did as a hack. I didn't realise that the
>> >> >> >> > DMA
>> >> >> >> > would go through the cache. Thanks for the pointer.
>> >> >> >>
>> >> >> >> DMA should not go through the cache. I have yet to review that
>> >> >> >> patch,
>> >> >> >> but IMO it's relevant to this problem you observe.
>> >> >> >
>> >> >> > DMA transfers not going through the cache is probably the problem
>> >> >> > here:
>> >> >> >
>> >> >> > Assume we have the aligned_buffer at address 0xdead0000
>> >> >> >
>> >> >> > 1. The cpu writes to address 0xdead0002. This is fine, as it is the
>> >> >> > current
>> >> >> > owner of the address. The cacheline is marked dirty.
>> >> >> > 2. The cpu no longer needs the corresponding address range, and it
>> >> >> > is
>> >> >> > reallocated (i.e. freed and then allocated from dwc2) or reused
>> >> >> > (i.e.
>> >> >> > formerly out buffer, now in buffer).
>> >> >> > 3. The CPU starts the DMA transfer
>> >> >> > 4. The DMA transfer writes to e.g. 0xdead0000-0xdead0200 in memory.
>> >> >> > 5. The CPU fetches an address aliasing with 0xdead0000. The dirty
>> >> >> > cache
>> >> >> > line is evicted, and the 0xdead0000-0xdead0040 memory contents are
>> >> >> > overwritten.
>> >> >>
>> >> >> This is the part I don't understand. This should be an invalidate, not
>> >> >> a clean and invalidate, so there should be not memory write.
>> >> >>
>> >> >> Also if the CPU fetches from cached 0xdead0000 without an invalidate,
>> >> >> it will not cause a cash clean. It will simple read the data from the
>> >> >> cache and ignore what the DMA wrote.
>> >> >
>> >> > The CPU does not fetch 0xdead0000, but from an address *aliasing* with
>> >> > 0xdead000. As 0xdead0000 is *dirty* (we have neither flushed (clears
>> >> > dirty
>> >> > bit) or invalidated (implicitly clears dirty for the address)), the
>> >> > cache
>> >> > controller has to write out the 0xdead0000 cache line to memory.
>> >>
>> >> That doesn't make any sense to me. Can you explain it a bit more?
>> >>
>> >> If the CPU fetches from a cache-alias of 0xdead0000, say 0xa11a5000
>> >> then I expect the cache line would contain the data for that alias,
>> >> not for 0xdead0000.
>> >
>> > The important part is the dirty flag in the 0xdead0000 cacheline. By
>> > reading an aliasing address, you are causing eviction of the current
>> > cache line contents, and writing back its contents to memory. Reading of
>> > an address may cause write of a different address. You can see it as an
>> > dcache_flush_range done by the cache controller.
>>
>> OK so I think you are saying that reading from 0xa11a5000 causes dirty
>> data to be flushed from the cache to 0xdead0000. Makes perfect sense.
>> But why is there dirty data at 0xdead0000?
>>
>> - If we did a write last time, then it would have been dirty until we
>> flushed the cache line, which we did before telling the DMA to start
>> up.
>>
>> - If we did a read last time, then it is clean. We have read the data,
>> but not changed it.
>>
>> What am I missing?
>
> The following is a gross oversimplification, but might explain it:
>
> 1. Assume all of the 64kB of the aligned_buffer is dirty. (Likely, if the
> buffer is calloced.)
> 2. We are doing some transfers. All transfers are small, e.g. 64 byte.
> 3. In accordance with the two cases you mentioned above, the first 64 byte are
> no longer dirty, as the last out transfer has flushed the cacheline.
> 4. We are doing our first large in transfer (i.e. larger than 64 byte).
> 5. Bad Things (TM) may happen to any data at aligned_buffer[64] and beyond.
>
> If this holds, a single invalidate_dcache_range(aligned_buffer, aligned_buffer
> +65536,...) during the initialization of the controller would suffice.

OK I can test that. It makes sense.

>
>> > I think you are assuming a write-through cache here, which leads to your
>> > confusion.
>>
>> No that's a separate issue.
>>
>> >> So a later invalidate of 0xdead0000 should at most
>> >> clean the cache line and write to memory at 0xa11a5000. If it were to
>> >> write cached data intended for 0xa11a5000 to memory at 0xdead0000,
>> >> then surely this would be a bug?
>> >
>> > After the cache line for 0xdead0000 has been evicted, any flush/invalidate
>> > operations are noops for that address.
>>
>> OK good, so that's not the problem.
>>
>> >> Therefore I cannot see the situation where the CPU should write to
>> >> 0xdead0000 when that address is invalidated.
>> >
>> > It is not the invalidation which causes the write, but eviction from the
>> > cache.
>> >
>> >  > >> On armv8 we appear not to suppose invalidate in the code, so it
>> >  > >> makes
>> >> >>
>> >> >> sense for rpi_3.
>> >> >>
>> >> >> But for rpi_2 which seems to do a proper invalidate, I still don't see
>> >> >> the problem.
>> >> >
>> >> > Which part of the code is different between rpi2 and rpi3? The dwc2
>> >> > code
>> >> > is
>> >> > identical, is the memory invalidated in some other place?
>> >>
>> >> It is the invalidate_dcache_range() function.
>> >
>> > Thats for pointing that out, for anyone not having read the code:
>> >
>> > ARMv7 has different operations for flush_dcache_range and
>> > invalidate_dcache_range, the former does a writeback of dirty cache lines
>> > and sets the invalid tags for the corresponding cache lines, the latter
>> > only does the invalidate part.
>> >
>> > ARMv8 does a writeback for both operations. I assume thats what you call
>> > "improper".
>>
>> Yes, I believe it is wrong. Linux has a proper invalidate, why not U-Boot?
>
> For why it does not exist for ARMv8 U-Boot, I can not answer.
>
> The fact invalidate actually is a flush on ARMv8 makes the problem much more
> likely to happen. If the buffer, which is a member of dwc2_priv, is memset
> during the initialization it *will* be dirty. Any in transfer which is larger
> than any previous in or out transfer will cause a flush of the tail, up to
> xfer_len, of the transfer buffer.
>
> On ARMv7 the problem will be only apparent if a cache eviction happens during
> the DMA.

OK now it makes sense. With a write-back cache the window of
opportunity for this is much larger. For me, this explains why we need
to invalidate before issuing the read to the DMA engine.

>
>> > The important part is, calling flush multiple times in a row is *exactly*
>> > the same thing as calling flush once. Calling flush instead of invalidate
>> > is the same thing *if* the dirty flag is not set, as the writeback part
>> > is skipped in that case.
>>
>> Yes indeed.
>>
>> >> >> > Obviously, the dirty cache line from (1.) has to be cleared at the
>> >> >> > beginning of (3.), as Eddys patch does.
>> >> >>
>> >> >> But I still don't understand why we have to clean instead of just
>> >> >> invalidate?
>> >> >
>> >> > The patch by Eddie Cai just does an invalidate_dcache_range on the
>> >> > transfer
>> >> > buffer, nothing else. Where do you see a "clean" (whatever that refers
>> >> > to)?
>> >>
>> >> In the armv8  invalidate_dcache_range() function.
>> >
>> > The writeback does not happen, as the cacheline is not dirty. It should
>> > not
>> > even be in the cache after invalidate has been called once.
>> >
>> > We have to make sure the buffers address range is not in the cache prior
>> > to
>> > starting the DMA. We can either use invalidate_dcache_range or
>> > flush_dcache_range to guarantee this. Which one we use does not matter
>> > here, although invalidate only is typically a little bit more
>> > lightweight.
>> Yes. Just to restate my assertion. It should be possible to:
>>
>> - have some dirty data in the cache
>> - start up DMA
>> - invalidate that data
>> - read it
>>
>> in that order. It should not be necessary to move the invalidate to
>> before the DMA start-up, right?
>
> Unfortunately that won't be enough. *If* there is some dirty data in the
> cache, it can be written back to main memory *any time*. If you are lucky, the
> writeback happens before the memory location is written by the DMA controller,
> but there is no guarantee until you flush and/or invalidate (either suffices).

OK this was the missing piece for me. And it makes sense since we
probably do memset() the data at startup (e.g. malloc() does this).
Also the problem only affects the first few blocks.

I'll do some more testing at some point now that I understand the issue.

Regards,
Simon

[U-Boot] [PATCH v5 03/19] dm: mmc: Set up the MMC device when controller is probed

[Simon Glass]

When an MMC controller is set up we should probe the device to find out
what it is. This includes finding its capacity and reading its partition
table.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 drivers/mmc/mmc-uclass.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index 5bb446bcc2..1e881e45e6 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -239,6 +239,15 @@ int mmc_unbind(struct udevice *dev)
 	return 0;
 }
 
+#ifdef CONFIG_BLK
+static int mmc_post_probe(struct udevice *dev)
+{
+	struct mmc *mmc = mmc_get_mmc_dev(dev);
+
+	return mmc_init(mmc);
+}
+#endif
+
 static int mmc_select_hwpart(struct udevice *bdev, int hwpart)
 {
 	struct udevice *mmc_dev = dev_get_parent(bdev);
@@ -292,4 +301,7 @@ UCLASS_DRIVER(mmc) = {
 	.name		= "mmc",
 	.flags		= DM_UC_FLAG_SEQ_ALIAS,
 	.per_device_auto_alloc_size = sizeof(struct mmc_uclass_priv),
+#ifdef CONFIG_BLK
+	.post_probe	= mmc_post_probe,
+#endif
 };
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 04/19] dm: video: Correct line clearing code

[Simon Glass]

At present we clear many more bytes than we should on 16bpp and 32bpp
displays. The number of pixels to clear is currently calculated as the
line length (in bytes) multiplied by the number of lines to clear. This
is only correct for 8bpp displays.

Correct the calculation to use the number of text columns multiplied by
the width of each character multiplied by the number of lines to clear.

Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Anatolij Gustschin <agust@denx.de>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 drivers/video/console_normal.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/console_normal.c b/drivers/video/console_normal.c
index 89a55dd11d..744345d58e 100644
--- a/drivers/video/console_normal.c
+++ b/drivers/video/console_normal.c
@@ -16,9 +16,10 @@
 
 static int console_normal_set_row(struct udevice *dev, uint row, int clr)
 {
+	struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
 	struct video_priv *vid_priv = dev_get_uclass_priv(dev->parent);
 	void *line;
-	int pixels = VIDEO_FONT_HEIGHT * vid_priv->line_length;
+	int pixels = VIDEO_FONT_HEIGHT * vc_priv->cols * VIDEO_FONT_WIDTH;
 	int i;
 
 	line = vid_priv->fb + row * VIDEO_FONT_HEIGHT * vid_priv->line_length;
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 05/19] string: Use memcpy() within memmove() when we can

[Simon Glass]

A common use of memmove() can be handled by memcpy(). Also memcpy()
includes an optimisation for large sizes: it copies a word at a time. So
we can get a speed-up by calling memcpy() to handle our move in this case.

Update memmove() to call memcpy() if the destination is before the source.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 lib/string.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index 67d5f6a421..6a75efb180 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -509,16 +509,9 @@ void * memmove(void * dest,const void *src,size_t count)
 {
 	char *tmp, *s;
 
-	if (src == dest)
-		return dest;
-
 	if (dest <= src) {
-		tmp = (char *) dest;
-		s = (char *) src;
-		while (count--)
-			*tmp++ = *s++;
-		}
-	else {
+		memcpy(dest, src, count);
+	} else {
 		tmp = (char *) dest + count;
 		s = (char *) src + count;
 		while (count--)
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 06/19] arm: rpi: Drop the GPIO device addresses

[Simon Glass]

We can rely on the device tree to provide this information.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 arch/arm/mach-bcm283x/include/mach/gpio.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/mach-bcm283x/include/mach/gpio.h b/arch/arm/mach-bcm283x/include/mach/gpio.h
index b2df75ad3f..daaee52f81 100644
--- a/arch/arm/mach-bcm283x/include/mach/gpio.h
+++ b/arch/arm/mach-bcm283x/include/mach/gpio.h
@@ -9,11 +9,6 @@
 #ifndef _BCM2835_GPIO_H_
 #define _BCM2835_GPIO_H_
 
-#ifndef CONFIG_BCM2835
-#define BCM2835_GPIO_BASE		0x3f200000
-#else
-#define BCM2835_GPIO_BASE		0x20200000
-#endif
 #define BCM2835_GPIO_COUNT		54
 
 #define BCM2835_GPIO_FSEL_MASK		0x7
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 07/19] arm: rpi: Drop CONFIG_CONS_INDEX

[Simon Glass]

This is not needed now that serial uses driver model.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 include/configs/rpi.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/configs/rpi.h b/include/configs/rpi.h
index 92eb792989..8d9a44c51d 100644
--- a/include/configs/rpi.h
+++ b/include/configs/rpi.h
@@ -98,7 +98,6 @@
 #else
 #define CONFIG_PL01X_SERIAL
 #endif
-#define CONFIG_CONS_INDEX		0
 
 /* Console configuration */
 #define CONFIG_SYS_CBSIZE		1024
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 08/19] dm: arm: rpi: Move to driver model for USB

[Simon Glass]

Start using driver model for USB on the Raspberry Pi. The dwc2 supports
this now so this is just a config change.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 configs/rpi_2_defconfig     | 1 +
 configs/rpi_3_32b_defconfig | 1 +
 configs/rpi_3_defconfig     | 1 +
 configs/rpi_defconfig       | 1 +
 include/configs/rpi.h       | 5 -----
 5 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/configs/rpi_2_defconfig b/configs/rpi_2_defconfig
index 9d669c92e4..4de1d206bc 100644
--- a/configs/rpi_2_defconfig
+++ b/configs/rpi_2_defconfig
@@ -17,6 +17,7 @@ CONFIG_OF_EMBED=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
 CONFIG_USB=y
+CONFIG_DM_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_KEYBOARD=y
 CONFIG_CONSOLE_SCROLL_LINES=10
diff --git a/configs/rpi_3_32b_defconfig b/configs/rpi_3_32b_defconfig
index d0f7beaa0b..746a6e2816 100644
--- a/configs/rpi_3_32b_defconfig
+++ b/configs/rpi_3_32b_defconfig
@@ -19,6 +19,7 @@ CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
 # CONFIG_REQUIRE_SERIAL_CONSOLE is not set
 CONFIG_USB=y
+CONFIG_DM_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_KEYBOARD=y
 CONFIG_CONSOLE_SCROLL_LINES=10
diff --git a/configs/rpi_3_defconfig b/configs/rpi_3_defconfig
index ce28c31283..b916ced65a 100644
--- a/configs/rpi_3_defconfig
+++ b/configs/rpi_3_defconfig
@@ -19,6 +19,7 @@ CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
 # CONFIG_REQUIRE_SERIAL_CONSOLE is not set
 CONFIG_USB=y
+CONFIG_DM_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_KEYBOARD=y
 CONFIG_CONSOLE_SCROLL_LINES=10
diff --git a/configs/rpi_defconfig b/configs/rpi_defconfig
index 4a90ca8348..514b29b5cd 100644
--- a/configs/rpi_defconfig
+++ b/configs/rpi_defconfig
@@ -17,6 +17,7 @@ CONFIG_OF_EMBED=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
 CONFIG_USB=y
+CONFIG_DM_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_KEYBOARD=y
 CONFIG_CONSOLE_SCROLL_LINES=10
diff --git a/include/configs/rpi.h b/include/configs/rpi.h
index 8d9a44c51d..f96ee46794 100644
--- a/include/configs/rpi.h
+++ b/include/configs/rpi.h
@@ -80,11 +80,6 @@
 
 #ifdef CONFIG_CMD_USB
 #define CONFIG_USB_DWC2
-#ifndef CONFIG_BCM2835
-#define CONFIG_USB_DWC2_REG_ADDR 0x3f980000
-#else
-#define CONFIG_USB_DWC2_REG_ADDR 0x20980000
-#endif
 #define CONFIG_USB_HOST_ETHER
 #define CONFIG_USB_ETHER_SMSC95XX
 #define CONFIG_TFTP_TSIZE
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 09/19] dm: arm: rpi: Use driver model for Ethernet

[Simon Glass]

Enable CONFIG_DM_ETH so that driver model is used for the USB Ethernet
device.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 configs/rpi_2_defconfig     | 1 +
 configs/rpi_3_32b_defconfig | 1 +
 configs/rpi_3_defconfig     | 1 +
 configs/rpi_defconfig       | 1 +
 4 files changed, 4 insertions(+)

diff --git a/configs/rpi_2_defconfig b/configs/rpi_2_defconfig
index 4de1d206bc..090a0b0714 100644
--- a/configs/rpi_2_defconfig
+++ b/configs/rpi_2_defconfig
@@ -16,6 +16,7 @@ CONFIG_CMD_GPIO=y
 CONFIG_OF_EMBED=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
+CONFIG_DM_ETH=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_STORAGE=y
diff --git a/configs/rpi_3_32b_defconfig b/configs/rpi_3_32b_defconfig
index 746a6e2816..785bf21e9a 100644
--- a/configs/rpi_3_32b_defconfig
+++ b/configs/rpi_3_32b_defconfig
@@ -17,6 +17,7 @@ CONFIG_CMD_GPIO=y
 CONFIG_OF_EMBED=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
+CONFIG_DM_ETH=y
 # CONFIG_REQUIRE_SERIAL_CONSOLE is not set
 CONFIG_USB=y
 CONFIG_DM_USB=y
diff --git a/configs/rpi_3_defconfig b/configs/rpi_3_defconfig
index b916ced65a..5ff8ed1cf2 100644
--- a/configs/rpi_3_defconfig
+++ b/configs/rpi_3_defconfig
@@ -17,6 +17,7 @@ CONFIG_CMD_GPIO=y
 CONFIG_OF_EMBED=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
+CONFIG_DM_ETH=y
 # CONFIG_REQUIRE_SERIAL_CONSOLE is not set
 CONFIG_USB=y
 CONFIG_DM_USB=y
diff --git a/configs/rpi_defconfig b/configs/rpi_defconfig
index 514b29b5cd..5d44f29726 100644
--- a/configs/rpi_defconfig
+++ b/configs/rpi_defconfig
@@ -16,6 +16,7 @@ CONFIG_CMD_GPIO=y
 CONFIG_OF_EMBED=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
+CONFIG_DM_ETH=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_STORAGE=y
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 10/19] arm: rpi: Add a file to handle messages

[Simon Glass]

The bcm283x chips provide a way for the ARM core to communicate with the
graphics processor, which is in charge of many things. This is handled by
way of a message prototcol.

At present the code for sending message (and receiving a reply) is spread
around U-Boot, primarily in the board file. This means that sending a
message from a driver requires duplicating the code.

Create a new message implementation with a function to support powering on
a subsystem as a starting point.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 arch/arm/mach-bcm283x/Makefile           |  2 +-
 arch/arm/mach-bcm283x/include/mach/msg.h | 18 +++++++++++++++
 arch/arm/mach-bcm283x/msg.c              | 39 ++++++++++++++++++++++++++++++++
 board/raspberrypi/rpi/rpi.c              | 35 +++-------------------------
 4 files changed, 61 insertions(+), 33 deletions(-)
 create mode 100644 arch/arm/mach-bcm283x/include/mach/msg.h
 create mode 100644 arch/arm/mach-bcm283x/msg.c

diff --git a/arch/arm/mach-bcm283x/Makefile b/arch/arm/mach-bcm283x/Makefile
index 5cb1b2fe94..b5f606ef0b 100644
--- a/arch/arm/mach-bcm283x/Makefile
+++ b/arch/arm/mach-bcm283x/Makefile
@@ -5,4 +5,4 @@
 #
 
 obj-$(CONFIG_BCM2835) += lowlevel_init.o
-obj-y	+= init.o reset.o mbox.o phys2bus.o
+obj-y	+= init.o reset.o mbox.o msg.o phys2bus.o
diff --git a/arch/arm/mach-bcm283x/include/mach/msg.h b/arch/arm/mach-bcm283x/include/mach/msg.h
new file mode 100644
index 0000000000..3bcb5db3e6
--- /dev/null
+++ b/arch/arm/mach-bcm283x/include/mach/msg.h
@@ -0,0 +1,18 @@
+/*
+ * (C) Copyright 2012,2015 Stephen Warren
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _BCM2835_MSG_H
+#define _BCM2835_MSG_H
+
+/**
+ * bcm2835_power_on_module() - power on an SoC module
+ *
+ * @module: ID of module to power on (BCM2835_MBOX_POWER_DEVID_...)
+ * @return 0 if OK, -EIO on error
+ */
+int bcm2835_power_on_module(u32 module);
+
+#endif
diff --git a/arch/arm/mach-bcm283x/msg.c b/arch/arm/mach-bcm283x/msg.c
new file mode 100644
index 0000000000..08b1beaa53
--- /dev/null
+++ b/arch/arm/mach-bcm283x/msg.c
@@ -0,0 +1,39 @@
+/*
+ * (C) Copyright 2012 Stephen Warren
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <memalign.h>
+#include <asm/arch/mbox.h>
+
+struct msg_set_power_state {
+	struct bcm2835_mbox_hdr hdr;
+	struct bcm2835_mbox_tag_set_power_state set_power_state;
+	u32 end_tag;
+};
+
+int bcm2835_power_on_module(u32 module)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(struct msg_set_power_state, msg_pwr, 1);
+	int ret;
+
+	BCM2835_MBOX_INIT_HDR(msg_pwr);
+	BCM2835_MBOX_INIT_TAG(&msg_pwr->set_power_state,
+			      SET_POWER_STATE);
+	msg_pwr->set_power_state.body.req.device_id = module;
+	msg_pwr->set_power_state.body.req.state =
+		BCM2835_MBOX_SET_POWER_STATE_REQ_ON |
+		BCM2835_MBOX_SET_POWER_STATE_REQ_WAIT;
+
+	ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN,
+				     &msg_pwr->hdr);
+	if (ret) {
+		printf("bcm2835: Could not set module %u power state\n",
+		       module);
+		return -EIO;
+	}
+
+	return 0;
+}
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 2146534b36..d0d9a9739d 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -16,6 +16,7 @@
 #include <mmc.h>
 #include <asm/gpio.h>
 #include <asm/arch/mbox.h>
+#include <asm/arch/msg.h>
 #include <asm/arch/sdhci.h>
 #include <asm/global_data.h>
 #include <dm/platform_data/serial_bcm283x_mu.h>
@@ -53,12 +54,6 @@ struct msg_get_mac_address {
 	u32 end_tag;
 };
 
-struct msg_set_power_state {
-	struct bcm2835_mbox_hdr hdr;
-	struct bcm2835_mbox_tag_set_power_state set_power_state;
-	u32 end_tag;
-};
-
 struct msg_get_clock_rate {
 	struct bcm2835_mbox_hdr hdr;
 	struct bcm2835_mbox_tag_get_clock_rate get_clock_rate;
@@ -365,30 +360,6 @@ int misc_init_r(void)
 	return 0;
 }
 
-static int power_on_module(u32 module)
-{
-	ALLOC_CACHE_ALIGN_BUFFER(struct msg_set_power_state, msg_pwr, 1);
-	int ret;
-
-	BCM2835_MBOX_INIT_HDR(msg_pwr);
-	BCM2835_MBOX_INIT_TAG(&msg_pwr->set_power_state,
-			      SET_POWER_STATE);
-	msg_pwr->set_power_state.body.req.device_id = module;
-	msg_pwr->set_power_state.body.req.state =
-		BCM2835_MBOX_SET_POWER_STATE_REQ_ON |
-		BCM2835_MBOX_SET_POWER_STATE_REQ_WAIT;
-
-	ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN,
-				     &msg_pwr->hdr);
-	if (ret) {
-		printf("bcm2835: Could not set module %u power state\n",
-		       module);
-		return -1;
-	}
-
-	return 0;
-}
-
 static void get_board_rev(void)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(struct msg_get_board_rev, msg, 1);
@@ -492,7 +463,7 @@ int board_init(void)
 
 	gd->bd->bi_boot_params = 0x100;
 
-	return power_on_module(BCM2835_MBOX_POWER_DEVID_USB_HCD);
+	return bcm2835_power_on_module(BCM2835_MBOX_POWER_DEVID_USB_HCD);
 }
 
 int board_mmc_init(bd_t *bis)
@@ -500,7 +471,7 @@ int board_mmc_init(bd_t *bis)
 	ALLOC_CACHE_ALIGN_BUFFER(struct msg_get_clock_rate, msg_clk, 1);
 	int ret;
 
-	power_on_module(BCM2835_MBOX_POWER_DEVID_SDHCI);
+	bcm2835_power_on_module(BCM2835_MBOX_POWER_DEVID_SDHCI);
 
 	BCM2835_MBOX_INIT_HDR(msg_clk);
 	BCM2835_MBOX_INIT_TAG(&msg_clk->get_clock_rate, GET_CLOCK_RATE);
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 11/19] arm: rpi: Add a function to obtain the MMC clock

[Simon Glass]

Move this code into the new message handler file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 arch/arm/mach-bcm283x/include/mach/msg.h |  7 +++++++
 arch/arm/mach-bcm283x/msg.c              | 28 ++++++++++++++++++++++++++++
 board/raspberrypi/rpi/rpi.c              | 16 ++++------------
 3 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-bcm283x/include/mach/msg.h b/arch/arm/mach-bcm283x/include/mach/msg.h
index 3bcb5db3e6..8fd4ace124 100644
--- a/arch/arm/mach-bcm283x/include/mach/msg.h
+++ b/arch/arm/mach-bcm283x/include/mach/msg.h
@@ -15,4 +15,11 @@
  */
 int bcm2835_power_on_module(u32 module);
 
+/**
+ * bcm2835_get_mmc_clock() - get the frequency of the MMC clock
+ *
+ * @return clock frequency, or -ve on error
+ */
+int bcm2835_get_mmc_clock(void);
+
 #endif
diff --git a/arch/arm/mach-bcm283x/msg.c b/arch/arm/mach-bcm283x/msg.c
index 08b1beaa53..facb916711 100644
--- a/arch/arm/mach-bcm283x/msg.c
+++ b/arch/arm/mach-bcm283x/msg.c
@@ -14,6 +14,12 @@ struct msg_set_power_state {
 	u32 end_tag;
 };
 
+struct msg_get_clock_rate {
+	struct bcm2835_mbox_hdr hdr;
+	struct bcm2835_mbox_tag_get_clock_rate get_clock_rate;
+	u32 end_tag;
+};
+
 int bcm2835_power_on_module(u32 module)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(struct msg_set_power_state, msg_pwr, 1);
@@ -37,3 +43,25 @@ int bcm2835_power_on_module(u32 module)
 
 	return 0;
 }
+
+int bcm2835_get_mmc_clock(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(struct msg_get_clock_rate, msg_clk, 1);
+	int ret;
+
+	ret = bcm2835_power_on_module(BCM2835_MBOX_POWER_DEVID_SDHCI);
+	if (ret)
+		return ret;
+
+	BCM2835_MBOX_INIT_HDR(msg_clk);
+	BCM2835_MBOX_INIT_TAG(&msg_clk->get_clock_rate, GET_CLOCK_RATE);
+	msg_clk->get_clock_rate.body.req.clock_id = BCM2835_MBOX_CLOCK_ID_EMMC;
+
+	ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &msg_clk->hdr);
+	if (ret) {
+		printf("bcm2835: Could not query eMMC clock rate\n");
+		return -EIO;
+	}
+
+	return msg_clk->get_clock_rate.body.resp.rate_hz;
+}
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index d0d9a9739d..2893f09bc5 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -468,23 +468,15 @@ int board_init(void)
 
 int board_mmc_init(bd_t *bis)
 {
-	ALLOC_CACHE_ALIGN_BUFFER(struct msg_get_clock_rate, msg_clk, 1);
 	int ret;
 
 	bcm2835_power_on_module(BCM2835_MBOX_POWER_DEVID_SDHCI);
 
-	BCM2835_MBOX_INIT_HDR(msg_clk);
-	BCM2835_MBOX_INIT_TAG(&msg_clk->get_clock_rate, GET_CLOCK_RATE);
-	msg_clk->get_clock_rate.body.req.clock_id = BCM2835_MBOX_CLOCK_ID_EMMC;
+	ret = bcm2835_get_mmc_clock();
+	if (ret)
+		return ret;
 
-	ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &msg_clk->hdr);
-	if (ret) {
-		printf("bcm2835: Could not query eMMC clock rate\n");
-		return -1;
-	}
-
-	return bcm2835_sdhci_init(BCM2835_SDHCI_BASE,
-				  msg_clk->get_clock_rate.body.resp.rate_hz);
+	return bcm2835_sdhci_init(BCM2835_SDHCI_BASE, ret);
 }
 
 int ft_board_setup(void *blob, bd_t *bd)
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 12/19] dm: mmc: rpi: Convert Raspberry Pi to driver model for MMC

[Simon Glass]

Convert the bcm2835 SDHCI driver over to support CONFIG_DM_MMC and move
all boards over. There is no need to keep the old code since there are no
other users.

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 board/raspberrypi/rpi/rpi.c | 13 --------
 configs/rpi_2_defconfig     |  1 +
 configs/rpi_3_32b_defconfig |  1 +
 configs/rpi_3_defconfig     |  1 +
 configs/rpi_defconfig       |  1 +
 drivers/mmc/bcm2835_sdhci.c | 81 ++++++++++++++++++++++++++++++++++++---------
 6 files changed, 69 insertions(+), 29 deletions(-)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 2893f09bc5..1fb7ba05ff 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -466,19 +466,6 @@ int board_init(void)
 	return bcm2835_power_on_module(BCM2835_MBOX_POWER_DEVID_USB_HCD);
 }
 
-int board_mmc_init(bd_t *bis)
-{
-	int ret;
-
-	bcm2835_power_on_module(BCM2835_MBOX_POWER_DEVID_SDHCI);
-
-	ret = bcm2835_get_mmc_clock();
-	if (ret)
-		return ret;
-
-	return bcm2835_sdhci_init(BCM2835_SDHCI_BASE, ret);
-}
-
 int ft_board_setup(void *blob, bd_t *bd)
 {
 	/*
diff --git a/configs/rpi_2_defconfig b/configs/rpi_2_defconfig
index 090a0b0714..3d6f2a3337 100644
--- a/configs/rpi_2_defconfig
+++ b/configs/rpi_2_defconfig
@@ -14,6 +14,7 @@ CONFIG_CMD_USB=y
 # CONFIG_CMD_FPGA is not set
 CONFIG_CMD_GPIO=y
 CONFIG_OF_EMBED=y
+CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
 CONFIG_DM_ETH=y
diff --git a/configs/rpi_3_32b_defconfig b/configs/rpi_3_32b_defconfig
index 785bf21e9a..ca0ef7afe7 100644
--- a/configs/rpi_3_32b_defconfig
+++ b/configs/rpi_3_32b_defconfig
@@ -15,6 +15,7 @@ CONFIG_CMD_USB=y
 # CONFIG_CMD_FPGA is not set
 CONFIG_CMD_GPIO=y
 CONFIG_OF_EMBED=y
+CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
 CONFIG_DM_ETH=y
diff --git a/configs/rpi_3_defconfig b/configs/rpi_3_defconfig
index 5ff8ed1cf2..185a56c280 100644
--- a/configs/rpi_3_defconfig
+++ b/configs/rpi_3_defconfig
@@ -15,6 +15,7 @@ CONFIG_CMD_USB=y
 # CONFIG_CMD_FPGA is not set
 CONFIG_CMD_GPIO=y
 CONFIG_OF_EMBED=y
+CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
 CONFIG_DM_ETH=y
diff --git a/configs/rpi_defconfig b/configs/rpi_defconfig
index 5d44f29726..89ecef8994 100644
--- a/configs/rpi_defconfig
+++ b/configs/rpi_defconfig
@@ -14,6 +14,7 @@ CONFIG_CMD_USB=y
 # CONFIG_CMD_FPGA is not set
 CONFIG_CMD_GPIO=y
 CONFIG_OF_EMBED=y
+CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
 CONFIG_DM_ETH=y
diff --git a/drivers/mmc/bcm2835_sdhci.c b/drivers/mmc/bcm2835_sdhci.c
index 29c2a85812..74ce573096 100644
--- a/drivers/mmc/bcm2835_sdhci.c
+++ b/drivers/mmc/bcm2835_sdhci.c
@@ -37,14 +37,23 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <malloc.h>
+#include <memalign.h>
 #include <sdhci.h>
-#include <mach/timer.h>
+#include <asm/arch/msg.h>
+#include <asm/arch/mbox.h>
 #include <mach/sdhci.h>
+#include <mach/timer.h>
 
 /* 400KHz is max freq for card ID etc. Use that as min */
 #define MIN_FREQ 400000
 
+struct bcm2835_sdhci_plat {
+	struct mmc_config cfg;
+	struct mmc mmc;
+};
+
 struct bcm2835_sdhci_host {
 	struct sdhci_host host;
 	uint twoticks_delay;
@@ -57,7 +66,7 @@ static inline struct bcm2835_sdhci_host *to_bcm(struct sdhci_host *host)
 }
 
 static inline void bcm2835_sdhci_raw_writel(struct sdhci_host *host, u32 val,
-						int reg)
+					    int reg)
 {
 	struct bcm2835_sdhci_host *bcm_host = to_bcm(host);
 
@@ -149,16 +158,33 @@ static const struct sdhci_ops bcm2835_ops = {
 	.read_b = bcm2835_sdhci_readb,
 };
 
-int bcm2835_sdhci_init(u32 regbase, u32 emmc_freq)
+static int bcm2835_sdhci_bind(struct udevice *dev)
 {
-	struct bcm2835_sdhci_host *bcm_host;
-	struct sdhci_host *host;
+	struct bcm2835_sdhci_plat *plat = dev_get_platdata(dev);
 
-	bcm_host = calloc(1, sizeof(*bcm_host));
-	if (!bcm_host) {
-		printf("sdhci_host calloc fail!\n");
-		return -ENOMEM;
+	return sdhci_bind(dev, &plat->mmc, &plat->cfg);
+}
+
+static int bcm2835_sdhci_probe(struct udevice *dev)
+{
+	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
+	struct bcm2835_sdhci_plat *plat = dev_get_platdata(dev);
+	struct bcm2835_sdhci_host *priv = dev_get_priv(dev);
+	struct sdhci_host *host = &priv->host;
+	fdt_addr_t base;
+	int emmc_freq;
+	int ret;
+
+	base = dev_get_addr(dev);
+	if (base == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	ret = bcm2835_get_mmc_clock();
+	if (ret < 0) {
+		debug("%s: Failed to set MMC clock (err=%d)\n", __func__, ret);
+		return ret;
 	}
+	emmc_freq = ret;
 
 	/*
 	 * See the comments in bcm2835_sdhci_raw_writel().
@@ -173,19 +199,42 @@ int bcm2835_sdhci_init(u32 regbase, u32 emmc_freq)
 	 * Multiply by 1000000 to get uS per two ticks.
 	 * +1 for hack rounding.
 	 */
-	bcm_host->twoticks_delay = ((2 * 1000000) / MIN_FREQ) + 1;
-	bcm_host->last_write = 0;
+	priv->twoticks_delay = ((2 * 1000000) / MIN_FREQ) + 1;
+	priv->last_write = 0;
 
-	host = &bcm_host->host;
-	host->name = "bcm2835_sdhci";
-	host->ioaddr = (void *)(unsigned long)regbase;
+	host->name = dev->name;
+	host->ioaddr = (void *)base;
 	host->quirks = SDHCI_QUIRK_BROKEN_VOLTAGE | SDHCI_QUIRK_BROKEN_R1B |
 		SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_NO_HISPD_BIT;
 	host->max_clk = emmc_freq;
 	host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
 	host->ops = &bcm2835_ops;
 
-	add_sdhci(host, 0, MIN_FREQ);
+	ret = sdhci_setup_cfg(&plat->cfg, host, emmc_freq, MIN_FREQ);
+	if (ret) {
+		debug("%s: Failed to setup SDHCI (err=%d)\n", __func__, ret);
+		return ret;
+	}
+
+	upriv->mmc = &plat->mmc;
+	host->mmc = &plat->mmc;
+	host->mmc->priv = host;
 
-	return 0;
+	return sdhci_probe(dev);
 }
+
+static const struct udevice_id bcm2835_sdhci_match[] = {
+	{ .compatible = "brcm,bcm2835-sdhci" },
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(sdhci_cdns) = {
+	.name = "sdhci-bcm2835",
+	.id = UCLASS_MMC,
+	.of_match = bcm2835_sdhci_match,
+	.bind = bcm2835_sdhci_bind,
+	.probe = bcm2835_sdhci_probe,
+	.priv_auto_alloc_size = sizeof(struct bcm2835_sdhci_host),
+	.platdata_auto_alloc_size = sizeof(struct bcm2835_sdhci_plat),
+	.ops = &sdhci_ops,
+};
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 13/19] dm: arm: rpi: Drop CONFIG_OF_EMBED

[Simon Glass]

We should not use an embedded device tree on a production board. There
does not seem to be any reason for it in commit 7670909. So drop this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 configs/rpi_2_defconfig     | 1 -
 configs/rpi_3_32b_defconfig | 1 -
 configs/rpi_3_defconfig     | 1 -
 configs/rpi_defconfig       | 1 -
 4 files changed, 4 deletions(-)

diff --git a/configs/rpi_2_defconfig b/configs/rpi_2_defconfig
index 3d6f2a3337..f8f123c4a6 100644
--- a/configs/rpi_2_defconfig
+++ b/configs/rpi_2_defconfig
@@ -13,7 +13,6 @@ CONFIG_CMD_MMC=y
 CONFIG_CMD_USB=y
 # CONFIG_CMD_FPGA is not set
 CONFIG_CMD_GPIO=y
-CONFIG_OF_EMBED=y
 CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
diff --git a/configs/rpi_3_32b_defconfig b/configs/rpi_3_32b_defconfig
index ca0ef7afe7..3ff932c361 100644
--- a/configs/rpi_3_32b_defconfig
+++ b/configs/rpi_3_32b_defconfig
@@ -14,7 +14,6 @@ CONFIG_CMD_MMC=y
 CONFIG_CMD_USB=y
 # CONFIG_CMD_FPGA is not set
 CONFIG_CMD_GPIO=y
-CONFIG_OF_EMBED=y
 CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
diff --git a/configs/rpi_3_defconfig b/configs/rpi_3_defconfig
index 185a56c280..98d7bd2c50 100644
--- a/configs/rpi_3_defconfig
+++ b/configs/rpi_3_defconfig
@@ -14,7 +14,6 @@ CONFIG_CMD_MMC=y
 CONFIG_CMD_USB=y
 # CONFIG_CMD_FPGA is not set
 CONFIG_CMD_GPIO=y
-CONFIG_OF_EMBED=y
 CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
diff --git a/configs/rpi_defconfig b/configs/rpi_defconfig
index 89ecef8994..23d540d129 100644
--- a/configs/rpi_defconfig
+++ b/configs/rpi_defconfig
@@ -13,7 +13,6 @@ CONFIG_CMD_MMC=y
 CONFIG_CMD_USB=y
 # CONFIG_CMD_FPGA is not set
 CONFIG_CMD_GPIO=y
-CONFIG_OF_EMBED=y
 CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 14/19] video: arm: rpi: Move the video query out of the driver

[Simon Glass]

Add a function to get the video size to the msg handler and remove it from
the video driver.

Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Anatolij Gustschin <agust@denx.de>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 arch/arm/mach-bcm283x/include/mach/msg.h |  9 +++++++++
 arch/arm/mach-bcm283x/msg.c              | 26 ++++++++++++++++++++++++++
 drivers/video/bcm2835.c                  | 20 +++-----------------
 3 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-bcm283x/include/mach/msg.h b/arch/arm/mach-bcm283x/include/mach/msg.h
index 8fd4ace124..c785c43a67 100644
--- a/arch/arm/mach-bcm283x/include/mach/msg.h
+++ b/arch/arm/mach-bcm283x/include/mach/msg.h
@@ -22,4 +22,13 @@ int bcm2835_power_on_module(u32 module);
  */
 int bcm2835_get_mmc_clock(void);
 
+/**
+ * bcm2835_get_video_size() - get the current display size
+ *
+ * @widthp: Returns the width in pixels
+ * @heightp: Returns the height in pixels
+ * @return 0 if OK, -ve on error
+ */
+int bcm2835_get_video_size(int *widthp, int *heightp);
+
 #endif
diff --git a/arch/arm/mach-bcm283x/msg.c b/arch/arm/mach-bcm283x/msg.c
index facb916711..17266c01a6 100644
--- a/arch/arm/mach-bcm283x/msg.c
+++ b/arch/arm/mach-bcm283x/msg.c
@@ -6,6 +6,7 @@
 
 #include <common.h>
 #include <memalign.h>
+#include <phys2bus.h>
 #include <asm/arch/mbox.h>
 
 struct msg_set_power_state {
@@ -20,6 +21,12 @@ struct msg_get_clock_rate {
 	u32 end_tag;
 };
 
+struct msg_query {
+	struct bcm2835_mbox_hdr hdr;
+	struct bcm2835_mbox_tag_physical_w_h physical_w_h;
+	u32 end_tag;
+};
+
 int bcm2835_power_on_module(u32 module)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(struct msg_set_power_state, msg_pwr, 1);
@@ -65,3 +72,22 @@ int bcm2835_get_mmc_clock(void)
 
 	return msg_clk->get_clock_rate.body.resp.rate_hz;
 }
+
+int bcm2835_get_video_size(int *widthp, int *heightp)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(struct msg_query, msg_query, 1);
+	int ret;
+
+	BCM2835_MBOX_INIT_HDR(msg_query);
+	BCM2835_MBOX_INIT_TAG_NO_REQ(&msg_query->physical_w_h,
+				     GET_PHYSICAL_W_H);
+	ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &msg_query->hdr);
+	if (ret) {
+		printf("bcm2835: Could not query display resolution\n");
+		return ret;
+	}
+	*widthp = msg_query->physical_w_h.body.resp.width;
+	*heightp = msg_query->physical_w_h.body.resp.height;
+
+	return 0;
+}
diff --git a/drivers/video/bcm2835.c b/drivers/video/bcm2835.c
index cc6454f10d..eb137be11c 100644
--- a/drivers/video/bcm2835.c
+++ b/drivers/video/bcm2835.c
@@ -9,6 +9,7 @@
 #include <memalign.h>
 #include <phys2bus.h>
 #include <asm/arch/mbox.h>
+#include <asm/arch/msg.h>
 #include <asm/global_data.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -18,12 +19,6 @@ vidinfo_t panel_info;
 
 static u32 bcm2835_pitch;
 
-struct msg_query {
-	struct bcm2835_mbox_hdr hdr;
-	struct bcm2835_mbox_tag_physical_w_h physical_w_h;
-	u32 end_tag;
-};
-
 struct msg_setup {
 	struct bcm2835_mbox_hdr hdr;
 	struct bcm2835_mbox_tag_physical_w_h physical_w_h;
@@ -40,27 +35,18 @@ struct msg_setup {
 
 void lcd_ctrl_init(void *lcdbase)
 {
-	ALLOC_CACHE_ALIGN_BUFFER(struct msg_query, msg_query, 1);
 	ALLOC_CACHE_ALIGN_BUFFER(struct msg_setup, msg_setup, 1);
 	int ret;
-	u32 w, h;
+	int w, h;
 	u32 fb_start, fb_end;
 
 	debug("bcm2835: Query resolution...\n");
-
-	BCM2835_MBOX_INIT_HDR(msg_query);
-	BCM2835_MBOX_INIT_TAG_NO_REQ(&msg_query->physical_w_h,
-					GET_PHYSICAL_W_H);
-	ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &msg_query->hdr);
+	ret = bcm2835_get_video_size(&w, &h);
 	if (ret) {
-		printf("bcm2835: Could not query display resolution\n");
 		/* FIXME: How to disable the LCD to prevent errors? hang()? */
 		return;
 	}
 
-	w = msg_query->physical_w_h.body.resp.width;
-	h = msg_query->physical_w_h.body.resp.height;
-
 	debug("bcm2835: Setting up display for %d x %d\n", w, h);
 
 	BCM2835_MBOX_INIT_HDR(msg_setup);
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 15/19] video: arm: rpi: Move the video settings out of the driver

[Simon Glass]

Add a function to set the video parameters to the msg handler and remove
it from the video driver.

Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Anatolij Gustschin <agust@denx.de>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 arch/arm/mach-bcm283x/include/mach/msg.h | 17 ++++++++
 arch/arm/mach-bcm283x/msg.c              | 61 ++++++++++++++++++++++++++++
 drivers/video/bcm2835.c                  | 68 +++++---------------------------
 3 files changed, 88 insertions(+), 58 deletions(-)

diff --git a/arch/arm/mach-bcm283x/include/mach/msg.h b/arch/arm/mach-bcm283x/include/mach/msg.h
index c785c43a67..478b1f1c50 100644
--- a/arch/arm/mach-bcm283x/include/mach/msg.h
+++ b/arch/arm/mach-bcm283x/include/mach/msg.h
@@ -31,4 +31,21 @@ int bcm2835_get_mmc_clock(void);
  */
 int bcm2835_get_video_size(int *widthp, int *heightp);
 
+/**
+ * bcm2835_set_video_params() - set the video parameters
+ *
+ * @widthp: Video width to request (returns the actual width selected)
+ * @heightp: Video height to request (returns the actual height selected)
+ * @depth_bpp: Requested bit depth
+ * @pixel_order: Pixel order to use (BCM2835_MBOX_PIXEL_ORDER_...)
+ * @alpha_mode: Alpha transparency mode to use (BCM2835_MBOX_ALPHA_MODE_...)
+ * @fb_basep: Returns base address of frame buffer
+ * @fb_sizep: Returns size of frame buffer
+ * @pitchp: Returns number of bytes in each frame buffer line
+ * @return 0 if OK, -ve on error
+ */
+int bcm2835_set_video_params(int *widthp, int *heightp, int depth_bpp,
+			     int pixel_order, int alpha_mode, ulong *fb_basep,
+			     ulong *fb_sizep, int *pitchp);
+
 #endif
diff --git a/arch/arm/mach-bcm283x/msg.c b/arch/arm/mach-bcm283x/msg.c
index 17266c01a6..92e93ad9e5 100644
--- a/arch/arm/mach-bcm283x/msg.c
+++ b/arch/arm/mach-bcm283x/msg.c
@@ -27,6 +27,20 @@ struct msg_query {
 	u32 end_tag;
 };
 
+struct msg_setup {
+	struct bcm2835_mbox_hdr hdr;
+	struct bcm2835_mbox_tag_physical_w_h physical_w_h;
+	struct bcm2835_mbox_tag_virtual_w_h virtual_w_h;
+	struct bcm2835_mbox_tag_depth depth;
+	struct bcm2835_mbox_tag_pixel_order pixel_order;
+	struct bcm2835_mbox_tag_alpha_mode alpha_mode;
+	struct bcm2835_mbox_tag_virtual_offset virtual_offset;
+	struct bcm2835_mbox_tag_overscan overscan;
+	struct bcm2835_mbox_tag_allocate_buffer allocate_buffer;
+	struct bcm2835_mbox_tag_pitch pitch;
+	u32 end_tag;
+};
+
 int bcm2835_power_on_module(u32 module)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(struct msg_set_power_state, msg_pwr, 1);
@@ -91,3 +105,50 @@ int bcm2835_get_video_size(int *widthp, int *heightp)
 
 	return 0;
 }
+
+int bcm2835_set_video_params(int *widthp, int *heightp, int depth_bpp,
+			     int pixel_order, int alpha_mode, ulong *fb_basep,
+			     ulong *fb_sizep, int *pitchp)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(struct msg_setup, msg_setup, 1);
+	int ret;
+
+	BCM2835_MBOX_INIT_HDR(msg_setup);
+	BCM2835_MBOX_INIT_TAG(&msg_setup->physical_w_h, SET_PHYSICAL_W_H);
+	msg_setup->physical_w_h.body.req.width = *widthp;
+	msg_setup->physical_w_h.body.req.height = *heightp;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->virtual_w_h, SET_VIRTUAL_W_H);
+	msg_setup->virtual_w_h.body.req.width = *widthp;
+	msg_setup->virtual_w_h.body.req.height = *heightp;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->depth, SET_DEPTH);
+	msg_setup->depth.body.req.bpp = 32;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->pixel_order, SET_PIXEL_ORDER);
+	msg_setup->pixel_order.body.req.order = pixel_order;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->alpha_mode, SET_ALPHA_MODE);
+	msg_setup->alpha_mode.body.req.alpha = alpha_mode;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->virtual_offset, SET_VIRTUAL_OFFSET);
+	msg_setup->virtual_offset.body.req.x = 0;
+	msg_setup->virtual_offset.body.req.y = 0;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->overscan, SET_OVERSCAN);
+	msg_setup->overscan.body.req.top = 0;
+	msg_setup->overscan.body.req.bottom = 0;
+	msg_setup->overscan.body.req.left = 0;
+	msg_setup->overscan.body.req.right = 0;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->allocate_buffer, ALLOCATE_BUFFER);
+	msg_setup->allocate_buffer.body.req.alignment = 0x100;
+	BCM2835_MBOX_INIT_TAG_NO_REQ(&msg_setup->pitch, GET_PITCH);
+
+	ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &msg_setup->hdr);
+	if (ret) {
+		printf("bcm2835: Could not configure display\n");
+		return ret;
+	}
+	*widthp = msg_setup->physical_w_h.body.resp.width;
+	*heightp = msg_setup->physical_w_h.body.resp.height;
+	*pitchp = msg_setup->pitch.body.resp.pitch;
+	*fb_basep = bus_to_phys(
+			msg_setup->allocate_buffer.body.resp.fb_address);
+	*fb_sizep = msg_setup->allocate_buffer.body.resp.fb_size;
+
+	return 0;
+}
diff --git a/drivers/video/bcm2835.c b/drivers/video/bcm2835.c
index eb137be11c..cd15f7f32a 100644
--- a/drivers/video/bcm2835.c
+++ b/drivers/video/bcm2835.c
@@ -17,28 +17,13 @@ DECLARE_GLOBAL_DATA_PTR;
 /* Global variables that lcd.c expects to exist */
 vidinfo_t panel_info;
 
-static u32 bcm2835_pitch;
-
-struct msg_setup {
-	struct bcm2835_mbox_hdr hdr;
-	struct bcm2835_mbox_tag_physical_w_h physical_w_h;
-	struct bcm2835_mbox_tag_virtual_w_h virtual_w_h;
-	struct bcm2835_mbox_tag_depth depth;
-	struct bcm2835_mbox_tag_pixel_order pixel_order;
-	struct bcm2835_mbox_tag_alpha_mode alpha_mode;
-	struct bcm2835_mbox_tag_virtual_offset virtual_offset;
-	struct bcm2835_mbox_tag_overscan overscan;
-	struct bcm2835_mbox_tag_allocate_buffer allocate_buffer;
-	struct bcm2835_mbox_tag_pitch pitch;
-	u32 end_tag;
-};
+static int bcm2835_pitch;
 
 void lcd_ctrl_init(void *lcdbase)
 {
-	ALLOC_CACHE_ALIGN_BUFFER(struct msg_setup, msg_setup, 1);
 	int ret;
 	int w, h;
-	u32 fb_start, fb_end;
+	ulong fb_base, fb_size, fb_start, fb_end;
 
 	debug("bcm2835: Query resolution...\n");
 	ret = bcm2835_get_video_size(&w, &h);
@@ -48,42 +33,9 @@ void lcd_ctrl_init(void *lcdbase)
 	}
 
 	debug("bcm2835: Setting up display for %d x %d\n", w, h);
-
-	BCM2835_MBOX_INIT_HDR(msg_setup);
-	BCM2835_MBOX_INIT_TAG(&msg_setup->physical_w_h, SET_PHYSICAL_W_H);
-	msg_setup->physical_w_h.body.req.width = w;
-	msg_setup->physical_w_h.body.req.height = h;
-	BCM2835_MBOX_INIT_TAG(&msg_setup->virtual_w_h, SET_VIRTUAL_W_H);
-	msg_setup->virtual_w_h.body.req.width = w;
-	msg_setup->virtual_w_h.body.req.height = h;
-	BCM2835_MBOX_INIT_TAG(&msg_setup->depth, SET_DEPTH);
-	msg_setup->depth.body.req.bpp = 32;
-	BCM2835_MBOX_INIT_TAG(&msg_setup->pixel_order, SET_PIXEL_ORDER);
-	msg_setup->pixel_order.body.req.order = BCM2835_MBOX_PIXEL_ORDER_RGB;
-	BCM2835_MBOX_INIT_TAG(&msg_setup->alpha_mode, SET_ALPHA_MODE);
-	msg_setup->alpha_mode.body.req.alpha = BCM2835_MBOX_ALPHA_MODE_IGNORED;
-	BCM2835_MBOX_INIT_TAG(&msg_setup->virtual_offset, SET_VIRTUAL_OFFSET);
-	msg_setup->virtual_offset.body.req.x = 0;
-	msg_setup->virtual_offset.body.req.y = 0;
-	BCM2835_MBOX_INIT_TAG(&msg_setup->overscan, SET_OVERSCAN);
-	msg_setup->overscan.body.req.top = 0;
-	msg_setup->overscan.body.req.bottom = 0;
-	msg_setup->overscan.body.req.left = 0;
-	msg_setup->overscan.body.req.right = 0;
-	BCM2835_MBOX_INIT_TAG(&msg_setup->allocate_buffer, ALLOCATE_BUFFER);
-	msg_setup->allocate_buffer.body.req.alignment = 0x100;
-	BCM2835_MBOX_INIT_TAG_NO_REQ(&msg_setup->pitch, GET_PITCH);
-
-	ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &msg_setup->hdr);
-	if (ret) {
-		printf("bcm2835: Could not configure display\n");
-		/* FIXME: How to disable the LCD to prevent errors? hang()? */
-		return;
-	}
-
-	w = msg_setup->physical_w_h.body.resp.width;
-	h = msg_setup->physical_w_h.body.resp.height;
-	bcm2835_pitch = msg_setup->pitch.body.resp.pitch;
+	ret = bcm2835_set_video_params(&w, &h, 32, BCM2835_MBOX_PIXEL_ORDER_RGB,
+				       BCM2835_MBOX_ALPHA_MODE_IGNORED,
+				       &fb_base, &fb_size, &bcm2835_pitch);
 
 	debug("bcm2835: Final resolution is %d x %d\n", w, h);
 
@@ -91,15 +43,14 @@ void lcd_ctrl_init(void *lcdbase)
 	panel_info.vl_row = h;
 	panel_info.vl_bpix = LCD_COLOR32;
 
-	gd->fb_base = bus_to_phys(
-		msg_setup->allocate_buffer.body.resp.fb_address);
+	gd->fb_base = fb_base;
 
 	/* Enable dcache for the frame buffer */
-	fb_start = gd->fb_base & ~(MMU_SECTION_SIZE - 1);
-	fb_end = gd->fb_base + msg_setup->allocate_buffer.body.resp.fb_size;
+	fb_start = fb_base & ~(MMU_SECTION_SIZE - 1);
+	fb_end = fb_base + fb_size;
 	fb_end = ALIGN(fb_end, 1 << MMU_SECTION_SHIFT);
 	mmu_set_region_dcache_behaviour(fb_start, fb_end - fb_start,
-		DCACHE_WRITEBACK);
+					DCACHE_WRITEBACK);
 	lcd_set_flush_dcache(1);
 }
 
@@ -110,5 +61,6 @@ void lcd_enable(void)
 int lcd_get_size(int *line_length)
 {
 	*line_length = bcm2835_pitch;
+
 	return *line_length * panel_info.vl_row;
 }
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 16/19] dm: video: Refactor lcd_simplefb to prepare for driver model

[Simon Glass]

Adjust this function so that we can convert it to support CONFIG_DM_VIDEO
without a lot of code duplication.

Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Anatolij Gustschin <agust@denx.de>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 common/lcd_simplefb.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/common/lcd_simplefb.c b/common/lcd_simplefb.c
index 2ba00f6d34..e479b492b8 100644
--- a/common/lcd_simplefb.c
+++ b/common/lcd_simplefb.c
@@ -16,17 +16,28 @@ DECLARE_GLOBAL_DATA_PTR;
 
 static int lcd_dt_simplefb_configure_node(void *blob, int off)
 {
-	int vl_col = lcd_get_pixel_width();
-	int vl_row = lcd_get_pixel_height();
-#if LCD_BPP == LCD_COLOR16
-	return fdt_setup_simplefb_node(blob, off, gd->fb_base, vl_col, vl_row,
-				       vl_col * 2, "r5g6b5");
-#elif LCD_BPP == LCD_COLOR32
-	return fdt_setup_simplefb_node(blob, off, gd->fb_base, vl_col, vl_row,
-				       vl_col * 4, "a8r8g8b8");
-#else
-	return -1;
-#endif
+	int xsize, ysize;
+	int bpix; /* log2 of bits per pixel */
+	const char *name;
+	ulong fb_base;
+
+	xsize = lcd_get_pixel_width();
+	ysize = lcd_get_pixel_height();
+	bpix = LCD_BPP;
+	fb_base = gd->fb_base;
+	switch (bpix) {
+	case 4: /* VIDEO_BPP16 */
+		name = "r5g6b5";
+		break;
+	case 5: /* VIDEO_BPP32 */
+		name = "a8r8g8b8";
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return fdt_setup_simplefb_node(blob, off, fb_base, xsize, ysize,
+				       xsize * (1 << bpix) / 8, name);
 }
 
 int lcd_dt_simplefb_add_node(void *blob)
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 17/19] dm: video: Add driver-model support to lcd_simplefb

[Simon Glass]

Allow this to work with CONFIG_DM_VIDEO enabled.

Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Anatolij Gustschin <agust@denx.de>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 common/lcd_simplefb.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/common/lcd_simplefb.c b/common/lcd_simplefb.c
index e479b492b8..d7e9fc9f65 100644
--- a/common/lcd_simplefb.c
+++ b/common/lcd_simplefb.c
@@ -8,9 +8,11 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <lcd.h>
 #include <fdt_support.h>
 #include <libfdt.h>
+#include <video.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -20,11 +22,27 @@ static int lcd_dt_simplefb_configure_node(void *blob, int off)
 	int bpix; /* log2 of bits per pixel */
 	const char *name;
 	ulong fb_base;
+#ifdef CONFIG_DM_VIDEO
+	struct video_uc_platdata *plat;
+	struct video_priv *uc_priv;
+	struct udevice *dev;
+	int ret;
 
+	ret = uclass_first_device_err(UCLASS_VIDEO, &dev);
+	if (ret)
+		return ret;
+	uc_priv = dev_get_uclass_priv(dev);
+	plat = dev_get_uclass_platdata(dev);
+	xsize = uc_priv->xsize;
+	ysize = uc_priv->ysize;
+	bpix = uc_priv->bpix;
+	fb_base = plat->base;
+#else
 	xsize = lcd_get_pixel_width();
 	ysize = lcd_get_pixel_height();
 	bpix = LCD_BPP;
 	fb_base = gd->fb_base;
+#endif
 	switch (bpix) {
 	case 4: /* VIDEO_BPP16 */
 		name = "r5g6b5";
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 18/19] dm: video: arm: rpi: Convert to use driver model for video

[Simon Glass]

Adjust the video driver to work with driver model and move over existing
baords. There is no need to keep the old code.

We can also drop setting of CONFIG_FB_ADDR since driver model doesn't have
this problem.

Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Anatolij Gustschin <agust@denx.de>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 configs/rpi_2_defconfig     |  2 +-
 configs/rpi_3_32b_defconfig |  2 +-
 configs/rpi_3_defconfig     |  2 +-
 configs/rpi_defconfig       |  2 +-
 drivers/video/bcm2835.c     | 62 ++++++++++++++++++++-------------------------
 include/configs/rpi.h       | 11 ++------
 6 files changed, 34 insertions(+), 47 deletions(-)

diff --git a/configs/rpi_2_defconfig b/configs/rpi_2_defconfig
index f8f123c4a6..e492468a84 100644
--- a/configs/rpi_2_defconfig
+++ b/configs/rpi_2_defconfig
@@ -21,6 +21,6 @@ CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_KEYBOARD=y
+CONFIG_DM_VIDEO=y
 CONFIG_CONSOLE_SCROLL_LINES=10
-CONFIG_LCD=y
 CONFIG_PHYS_TO_BUS=y
diff --git a/configs/rpi_3_32b_defconfig b/configs/rpi_3_32b_defconfig
index 3ff932c361..efcc23e5f4 100644
--- a/configs/rpi_3_32b_defconfig
+++ b/configs/rpi_3_32b_defconfig
@@ -23,6 +23,6 @@ CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_KEYBOARD=y
+CONFIG_DM_VIDEO=y
 CONFIG_CONSOLE_SCROLL_LINES=10
-CONFIG_LCD=y
 CONFIG_PHYS_TO_BUS=y
diff --git a/configs/rpi_3_defconfig b/configs/rpi_3_defconfig
index 98d7bd2c50..8f39d32cd6 100644
--- a/configs/rpi_3_defconfig
+++ b/configs/rpi_3_defconfig
@@ -23,6 +23,6 @@ CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_KEYBOARD=y
+CONFIG_DM_VIDEO=y
 CONFIG_CONSOLE_SCROLL_LINES=10
-CONFIG_LCD=y
 CONFIG_PHYS_TO_BUS=y
diff --git a/configs/rpi_defconfig b/configs/rpi_defconfig
index 23d540d129..62a1d981e9 100644
--- a/configs/rpi_defconfig
+++ b/configs/rpi_defconfig
@@ -21,6 +21,6 @@ CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_KEYBOARD=y
+CONFIG_DM_VIDEO=y
 CONFIG_CONSOLE_SCROLL_LINES=10
-CONFIG_LCD=y
 CONFIG_PHYS_TO_BUS=y
diff --git a/drivers/video/bcm2835.c b/drivers/video/bcm2835.c
index cd15f7f32a..952ef59661 100644
--- a/drivers/video/bcm2835.c
+++ b/drivers/video/bcm2835.c
@@ -5,62 +5,56 @@
  */
 
 #include <common.h>
-#include <lcd.h>
-#include <memalign.h>
-#include <phys2bus.h>
+#include <dm.h>
+#include <video.h>
 #include <asm/arch/mbox.h>
 #include <asm/arch/msg.h>
-#include <asm/global_data.h>
 
-DECLARE_GLOBAL_DATA_PTR;
-
-/* Global variables that lcd.c expects to exist */
-vidinfo_t panel_info;
-
-static int bcm2835_pitch;
-
-void lcd_ctrl_init(void *lcdbase)
+static int bcm2835_video_probe(struct udevice *dev)
 {
+	struct video_uc_platdata *plat = dev_get_uclass_platdata(dev);
+	struct video_priv *uc_priv = dev_get_uclass_priv(dev);
 	int ret;
-	int w, h;
+	int w, h, pitch;
 	ulong fb_base, fb_size, fb_start, fb_end;
 
 	debug("bcm2835: Query resolution...\n");
 	ret = bcm2835_get_video_size(&w, &h);
-	if (ret) {
-		/* FIXME: How to disable the LCD to prevent errors? hang()? */
-		return;
-	}
+	if (ret)
+		return -EIO;
 
 	debug("bcm2835: Setting up display for %d x %d\n", w, h);
 	ret = bcm2835_set_video_params(&w, &h, 32, BCM2835_MBOX_PIXEL_ORDER_RGB,
 				       BCM2835_MBOX_ALPHA_MODE_IGNORED,
-				       &fb_base, &fb_size, &bcm2835_pitch);
+				       &fb_base, &fb_size, &pitch);
 
 	debug("bcm2835: Final resolution is %d x %d\n", w, h);
 
-	panel_info.vl_col = w;
-	panel_info.vl_row = h;
-	panel_info.vl_bpix = LCD_COLOR32;
-
-	gd->fb_base = fb_base;
-
 	/* Enable dcache for the frame buffer */
 	fb_start = fb_base & ~(MMU_SECTION_SIZE - 1);
 	fb_end = fb_base + fb_size;
 	fb_end = ALIGN(fb_end, 1 << MMU_SECTION_SHIFT);
 	mmu_set_region_dcache_behaviour(fb_start, fb_end - fb_start,
 					DCACHE_WRITEBACK);
-	lcd_set_flush_dcache(1);
-}
+	video_set_flush_dcache(dev, true);
 
-void lcd_enable(void)
-{
-}
+	uc_priv->xsize = w;
+	uc_priv->ysize = h;
+	uc_priv->bpix = VIDEO_BPP32;
+	plat->base = fb_base;
+	plat->size = fb_size;
 
-int lcd_get_size(int *line_length)
-{
-	*line_length = bcm2835_pitch;
-
-	return *line_length * panel_info.vl_row;
+	return 0;
 }
+
+static const struct udevice_id bcm2835_video_ids[] = {
+	{ .compatible = "brcm,bcm2835-hdmi" },
+	{ }
+};
+
+U_BOOT_DRIVER(bcm2835_video) = {
+	.name	= "bcm2835_video",
+	.id	= UCLASS_VIDEO,
+	.of_match = bcm2835_video_ids,
+	.probe	= bcm2835_video_probe,
+};
diff --git a/include/configs/rpi.h b/include/configs/rpi.h
index f96ee46794..bebb79897b 100644
--- a/include/configs/rpi.h
+++ b/include/configs/rpi.h
@@ -68,13 +68,6 @@
 #define CONFIG_BCM2835_GPIO
 /* LCD */
 #define CONFIG_LCD_DT_SIMPLEFB
-#define LCD_BPP				LCD_COLOR32
-/*
- * Prevent allocation of RAM for FB; the real FB address is queried
- * dynamically from the VideoCore co-processor, and comes from RAM
- * not owned by the ARM CPU.
- */
-#define CONFIG_FB_ADDR			0
 #define CONFIG_VIDEO_BCM2835
 #define CONFIG_SYS_WHITE_ON_BLACK
 
@@ -125,8 +118,8 @@
 #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
 #define ENV_DEVICE_SETTINGS \
 	"stdin=serial,usbkbd\0" \
-	"stdout=serial,lcd\0" \
-	"stderr=serial,lcd\0"
+	"stdout=serial,vidconsole\0" \
+	"stderr=serial,vidconsole\0"
 
 /*
  * Memory layout for where various images get loaded by boot scripts:
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 19/19] arm: rpi: Add a TODO to move all messages into the msg handler

[Simon Glass]

The board code should all move into msg.c for consistency. Add a TODO for
this.

Signed-off-by: Simon Glass <sjg@chromium.org>

---

Changes in v5:
- Rebase to master

Changes in v4:
- Add patches to convert video and MMC to driver model also
- Rebase to master

Changes in v3:
- Drop applied patches from series
- Drop patch to introduce usbethaddr for driver model

 board/raspberrypi/rpi/rpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 1fb7ba05ff..c99beedc49 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -29,7 +29,7 @@ DECLARE_GLOBAL_DATA_PTR;
 /* From lowlevel_init.S */
 extern unsigned long fw_dtb_pointer;
 
-
+/* TODO(sjg at chromium.org): Move these to the msg.c file */
 struct msg_get_arm_mem {
 	struct bcm2835_mbox_hdr hdr;
 	struct bcm2835_mbox_tag_get_arm_mem get_arm_mem;
-- 
2.12.2.564.g063fe858b8-goog

[U-Boot] [PATCH v5 00/19] arm: rpi: Enable USB, Ethernet, MMC, Video driver model on Raspberry Pi

[Stefan Bruens]

On Samstag, 1. April 2017 20:05:37 CEST Simon Glass wrote:
> Raspberry Pi uses a DWC2 USB controller and a SMSC USB Ethernet adaptor.
> Driver model support for these is available.
> 
> This series does the following:
> - Enable CONFIG_DM_ETH and CONFIG_DM_USB on Raspberry Pi
> - Convert the MMC driver to driver model
> - Convert the video driver to driver model
> - Fixes a driver model video bug which accessed beyond the frame buffer
> - Fixes start-up of MMC with driver model (e.g. at present it does not
>     support env_fat)
> - Clean up a few loose ends
> 
> With Ethernet active the device list looks something like this:
> 
> U-Boot> dm tree
>  Class       Probed   Name
> ----------------------------------------
>  root        [ + ]    root_driver
>  simple_bus  [ + ]    |-- soc
>  gpio        [ + ]    |   |-- gpio at 7e200000
>  serial      [ + ]    |   |-- serial at 7e215040
>  mmc         [ + ]    |   |-- sdhci at 7e300000
>  blk         [ + ]    |   |   `-- sdhci at 7e300000.blk
>  video       [ + ]    |   |-- hdmi at 7e902000
>  vidconsole0 [ + ]    |   |   `-- hdmi at 7e902000.vidconsole0
>  usb         [ + ]    |   `-- usb at 7e980000
>  usb_hub     [ + ]    |       `-- usb_hub
>  usb_hub     [ + ]    |           `-- usb_hub
>  eth         [ + ]    |               `-- smsc95xx_eth
>  simple_bus  [   ]    `-- clocks
> 
> With version 4 a problem was discovered where tftpboot does not correctly
> read a file on rpi_2 and rpi_3. The cause is unknown but for now this
> series includes a work-around in the dwc2 driver. See here for details:
> 
> https://lists.denx.de/pipermail/u-boot/2017-April/285451.html

Have you seen https://lists.denx.de/pipermail/u-boot/2017-April/285546.html ?

This may fix the problem seen.

Kind regards,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019
work: +49 2405 49936-424