U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Mattijs Korpershoek <mkorpershoek@kernel.org>
To: Levi Tomes <levi@userdevice.net>, Marek Vasut <marex@denx.de>
Cc: Lukasz Majewski <lukma@denx.de>, Tom Rini <trini@konsulko.com>,
	u-boot@lists.u-boot-project.org
Subject: Re: [PATCH] usb: gadget: dwc2_udc_otg: translate DMA addresses via dev_phys_to_bus()
Date: Mon, 07 Sep 2026 10:13:24 +0200	[thread overview]
Message-ID: <87tso1tr6j.fsf@kernel.org> (raw)
In-Reply-To: <CAG7BTaqzj4Z=LM8E6U+e8Rz531Pf0BvFEB8nnSWvz9BE2e3tRw@mail.gmail.com>

Hi Levi,

Thank you for contributing to U-Boot.

On Fri, Sep 04, 2026 at 12:12, Levi Tomes <levi@userdevice.net> wrote:

> The bcm283x phys_to_bus() helper applies the 0xC0000000 VideoCore alias
> unconditionally. On BCM2712 the OTG controller sits under the axi bus
> with identity dma-ranges, so the alias points DMA at nothing and the
> host sees 'device descriptor read/64, error -71' during enumeration.
> Use the DT-aware dev_phys_to_bus() like macb/sdhci/nvme already do; on
> BCM2711 the soc node's dma-ranges still yields the alias.
>
> Tested on a Raspberry Pi CM5 Lite (BCM2712): the CDC-ACM gadget
> enumerates and works as a U-Boot console. Also tested on a Compute
> Module 4 (BCM2711) to confirm no regression: the gadget still
> enumerates cleanly there.
>
> This is my first contribution to u-boot, let me know if I am not following
> process and I can adjust. My hope would be to do any testing you
> need on this patch so it can land, and I can stop patching 20 machines
>  in my talos cluster with my own sketchy patch.

Please move this section below, with the "Notes for reviewers". We don't
need this part to be part of the git log.

As Marek mentioned, this patch is not patching the checkpatch.pl script:

$ ./scripts/checkpatch.pl usb-gadget-dwc2_udc_otg-translate-DMA-addresses-via-dev_phys_to_bus.patch
WARNING: It's generally not useful to have the filename in the file
#271: FILE: :52:
+ writel(dwc2_phys_to_bus((unsigned long)usb_ctrl_dma_addr),

WARNING: It's generally not useful to have the filename in the file
#280: FILE: :75:
+ writel(dwc2_phys_to_bus((unsigned long)usb_ctrl_dma_addr),

ERROR: patch seems to be corrupt (line wrapped?)
#286: FILE: :106:
dwc2_request *req)

See:
https://docs.u-boot-project.org/en/latest/develop/sending_patches.html#general-patch-submission-rules

Also consider using b4 for helping to send patches in the proper format:
https://docs.u-boot-project.org/en/latest/develop/sending_patches.html#using-b4

Thanks
Mattijs

>
> Signed-off-by: Levi Tomes <levi@userdevice.net>
> ---
>
> Notes for reviewers.
>
> How this was tested:
>
>  - BCM2712 (CM5 Lite): with the fix, the CDC-ACM gadget enumerates on
>    the OTG port and serves a working U-Boot console. Without it,
>    enumeration fails with "device descriptor read/64, error -71" as
>    described above.
>
>  - BCM2711 (CM4): regression check. U-Boot built from rpi_4_defconfig
>    plus USB_GADGET_DWC2_OTG/USB_FUNCTION_ACM, loaded over USB with
>    rpiboot; the gadget enumerates cleanly (0525:a4a7, cdc_acm bound, no
>    descriptor errors). rpi_4_defconfig also builds clean with the
>    change.
>
> Note on the BCM2712 side: Raspberry Pi 5 / CM5 support is not upstream
> yet, so that testing used an out-of-tree Pi5 patch stack. The change
> itself is independent of Pi5 support -- it only swaps phys_to_bus() for
> the DT-aware dev_phys_to_bus() in the dwc2 gadget, the same pattern
> macb, sdhci and nvme already use.
>
>  drivers/usb/gadget/dwc2_udc_otg.c          | 10 ++++++++++
>  drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 14 ++++++++------
>  2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/gadget/dwc2_udc_otg.c
> b/drivers/usb/gadget/dwc2_udc_otg.c
> index e475b14b9a..3835244cca 100644
> --- a/drivers/usb/gadget/dwc2_udc_otg.c
> +++ b/drivers/usb/gadget/dwc2_udc_otg.c
> @@ -168,6 +168,15 @@ __weak void otg_phy_off(struct dwc2_udc *dev) {}
>
>  /***********************************************************/
>
> +/* udevice of the probed controller, for DT-aware DMA address translation */
> +static struct udevice *dwc2_udc_udev;
> +static inline unsigned long dwc2_phys_to_bus(unsigned long addr)
> +{
> + if (dwc2_udc_udev)
> + return dev_phys_to_bus(dwc2_udc_udev, addr);
> + return phys_to_bus(addr);
> +}
> +
>  #include "dwc2_udc_otg_xfer_dma.c"
>
>  /*
> @@ -1073,6 +1082,7 @@ static int dwc2_udc_otg_clk_init(struct udevice *dev,
>
>  static int dwc2_udc_otg_probe(struct udevice *dev)
>  {
> + dwc2_udc_udev = dev;
>   struct dwc2_plat_otg_data *plat = dev_get_plat(dev);
>   struct dwc2_priv_data *priv = dev_get_priv(dev);
>   struct dwc2_core_regs *usbotg_reg =
> diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
> b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
> index 5a7f50ebaa..04c0485ae9 100644
> --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
> +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
> @@ -31,7 +31,7 @@ int clear_feature_flag;
>
>  static inline void dwc2_udc_ep0_zlp(struct dwc2_udc *dev)
>  {
> - writel(phys_to_bus((unsigned long)usb_ctrl_dma_addr),
> + writel(dwc2_phys_to_bus((unsigned long)usb_ctrl_dma_addr),
>         &reg->device_regs.in_endp[EP0_CON].diepdma);
>   writel(FIELD_PREP(DXEPTSIZ_PKTCNT_MASK, 1),
> &reg->device_regs.in_endp[EP0_CON].dieptsiz);
>
> @@ -49,7 +49,7 @@ static void dwc2_udc_pre_setup(void)
>
>   writel(FIELD_PREP(DXEPTSIZ_PKTCNT_MASK, 1) | sizeof(struct usb_ctrlrequest),
>         &reg->device_regs.out_endp[EP0_CON].doeptsiz);
> - writel(phys_to_bus((unsigned long)usb_ctrl_dma_addr),
> + writel(dwc2_phys_to_bus((unsigned long)usb_ctrl_dma_addr),
>         &reg->device_regs.out_endp[EP0_CON].doepdma);
>
>   setbits_le32(&reg->device_regs.out_endp[EP0_CON].doepctl, DXEPCTL_EPENA);
> @@ -72,7 +72,7 @@ static inline void dwc2_ep0_complete_out(void)
>
>   writel(FIELD_PREP(DXEPTSIZ_PKTCNT_MASK, 1) | sizeof(struct usb_ctrlrequest),
>         &reg->device_regs.out_endp[EP0_CON].doeptsiz);
> - writel(phys_to_bus((unsigned long)usb_ctrl_dma_addr),
> + writel(dwc2_phys_to_bus((unsigned long)usb_ctrl_dma_addr),
>         &reg->device_regs.out_endp[EP0_CON].doepdma);
>
>   setbits_le32(&reg->device_regs.out_endp[EP0_CON].doepctl,
> DXEPCTL_EPENA | DXEPCTL_CNAK);
> @@ -107,7 +107,8 @@ static int setdma_rx(struct dwc2_ep *ep, struct
> dwc2_request *req)
>   (unsigned long) ep->dma_buf +
>   ROUND(ep->len, CONFIG_SYS_CACHELINE_SIZE));
>
> - writel(phys_to_bus((unsigned long)ep->dma_buf),
> &reg->device_regs.out_endp[ep_num].doepdma);
> + writel(dwc2_phys_to_bus((unsigned long)ep->dma_buf),
> +       &reg->device_regs.out_endp[ep_num].doepdma);
>   writel(FIELD_PREP(DXEPTSIZ_PKTCNT_MASK, pktcnt) |
>         FIELD_PREP(DXEPTSIZ_XFERSIZE_MASK, length),
>         &reg->device_regs.out_endp[ep_num].doeptsiz);
> @@ -152,7 +153,8 @@ static int setdma_tx(struct dwc2_ep *ep, struct
> dwc2_request *req)
>   /* Flush the endpoint's Tx FIFO */
>   dwc2_flush_tx_fifo(reg, ep->fifo_num);
>
> - writel(phys_to_bus((unsigned long)ep->dma_buf),
> &reg->device_regs.in_endp[ep_num].diepdma);
> + writel(dwc2_phys_to_bus((unsigned long)ep->dma_buf),
> +       &reg->device_regs.in_endp[ep_num].diepdma);
>   writel(FIELD_PREP(DXEPTSIZ_PKTCNT_MASK, pktcnt) |
>         FIELD_PREP(DXEPTSIZ_XFERSIZE_MASK, length),
>         &reg->device_regs.in_endp[ep_num].dieptsiz);
> @@ -913,7 +915,7 @@ static int dwc2_udc_get_status(struct dwc2_udc *dev,
>     (unsigned long) usb_ctrl +
>     ROUND(sizeof(g_status), CONFIG_SYS_CACHELINE_SIZE));
>
> - writel(phys_to_bus(usb_ctrl_dma_addr),
> &reg->device_regs.in_endp[EP0_CON].diepdma);
> + writel(dwc2_phys_to_bus(usb_ctrl_dma_addr),
> &reg->device_regs.in_endp[EP0_CON].diepdma);
>   writel(FIELD_PREP(DXEPTSIZ_PKTCNT_MASK, 1) |
> FIELD_PREP(DXEPTSIZ_XFERSIZE_MASK, 2),
>         &reg->device_regs.in_endp[EP0_CON].dieptsiz);
>
> --
> 2.54.0

      parent reply	other threads:[~2026-09-07  8:13 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-04 17:12 [PATCH] usb: gadget: dwc2_udc_otg: translate DMA addresses via dev_phys_to_bus() Levi Tomes
2026-09-04 17:33 ` Marek Vasut
2026-09-07  8:13 ` Mattijs Korpershoek [this message]

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=87tso1tr6j.fsf@kernel.org \
    --to=mkorpershoek@kernel.org \
    --cc=levi@userdevice.net \
    --cc=lukma@denx.de \
    --cc=marex@denx.de \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.u-boot-project.org \
    /path/to/YOUR_REPLY

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

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