Linux-mediatek Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH RFC 05/12] usb: hub: Power on connected M.2 E-key connectors
From: Chen-Yu Tsai @ 2026-05-18  9:13 UTC (permalink / raw)
  To: Alan Stern
  Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, linux-pm, linux-usb, devicetree,
	linux-mediatek, linux-arm-kernel, linux-kernel,
	Manivannan Sadhasivam
In-Reply-To: <41260a6d-46fa-4a45-9906-e1bc5e5dd83a@rowland.harvard.edu>

On Fri, May 15, 2026 at 10:39 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Fri, May 15, 2026 at 05:01:41PM +0800, Chen-Yu Tsai wrote:
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 90ea597d42ae..4165f71e212b 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -31,7 +31,9 @@
> >  #include <linux/minmax.h>
> >  #include <linux/mutex.h>
> >  #include <linux/random.h>
> > +#include <linux/of_graph.h>
> >  #include <linux/pm_qos.h>
> > +#include <linux/pwrseq/consumer.h>
> >  #include <linux/kobject.h>
> >
> >  #include <linux/bitfield.h>
> > @@ -888,13 +890,25 @@ int usb_hub_set_port_power(struct usb_device *hdev, struct usb_hub *hub,
> >  {
> >       int ret;
> >
> > +     if (set)
> > +             ret = pwrseq_power_on(hub->ports[port1 - 1]->pwrseq);
> > +     else
> > +             ret = pwrseq_power_off(hub->ports[port1 - 1]->pwrseq);
> > +     if (ret)
> > +             return ret;
> > +
> >       if (set)
> >               ret = set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
> >       else
> >               ret = usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
> >
> > -     if (ret)
> > +     if (ret) {
> > +             if (set)
> > +                     pwrseq_power_off(hub->ports[port1 - 1]->pwrseq);
> > +             else
> > +                     pwrseq_power_on(hub->ports[port1 - 1]->pwrseq);
> >               return ret;
> > +     }
> >
> >       if (set)
> >               set_bit(port1, hub->power_bits);
> > @@ -1867,6 +1881,7 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
> >       struct usb_host_interface *desc;
> >       struct usb_device *hdev;
> >       struct usb_hub *hub;
> > +     int ret;
> >
> >       desc = intf->cur_altsetting;
> >       hdev = interface_to_usbdev(intf);
>
> This change is totally useless.  Didn't you get a warning from the
> compiler when you built it?

Apologies. This should have been part of the previous patch.

> > diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> > index 9ebc5ef54a32..6039e5f5dcd7 100644
> > --- a/drivers/usb/core/hub.h
> > +++ b/drivers/usb/core/hub.h
> > @@ -85,6 +85,7 @@ struct usb_hub {
> >   * @port_owner: port's owner
> >   * @peer: related usb2 and usb3 ports (share the same connector)
> >   * @connector: USB Type-C connector
> > + * @pwrseq: power sequencing descriptor for the port
> >   * @req: default pm qos request for hubs without port power control
> >   * @connect_type: port's connect type
> >   * @state: device state of the usb device attached to the port
> > @@ -104,6 +105,7 @@ struct usb_port {
> >       struct usb_dev_state *port_owner;
> >       struct usb_port *peer;
> >       struct typec_connector *connector;
> > +     struct pwrseq_desc *pwrseq;
> >       struct dev_pm_qos_request *req;
> >       enum usb_port_connect_type connect_type;
> >       enum usb_device_state state;
>
> The fact that hub.h uses struct pwrseq_desc indicates that it ought to
> #include <linux/pwrseq/consumer.h>, instead of making the .c files do
> so themselves.  Then you wouldn't have to add the #include lines to
> hub.c and port.c.

I couldn't tell if the existing pattern in this file was to include
the headers or not, as it's missing a whole bunch.

Regardless of whether this header file includes linux/pwrseq/consumer.h
or has a forward declaration or nothing, I think that if the .c files
use the API, then they should include the corresponding header file
directly.

> > diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> > index b1364f0c384c..2d09037fee93 100644
> > --- a/drivers/usb/core/port.c
> > +++ b/drivers/usb/core/port.c
> > @@ -7,11 +7,14 @@
> >   * Author: Lan Tianyu <tianyu.lan@intel.com>
> >   */
> >
> > +#include <linux/cleanup.h>
>
> Why is this needed?

For the __free() in usb_hub_port_pwrseq_get() below:

    struct device_node *np __free(device_node) = NULL;


Thanks
ChenYu


> >  #include <linux/kstrtox.h>
> >  #include <linux/slab.h>
> >  #include <linux/string_choices.h>
> >  #include <linux/sysfs.h>
> > +#include <linux/of_graph.h>
> >  #include <linux/pm_qos.h>
> > +#include <linux/pwrseq/consumer.h>
> >  #include <linux/component.h>
> >  #include <linux/usb/of.h>
> >
>
> Alan Stern


^ permalink raw reply

* Re: [PATCH] dt-bindings: PCI: mediatek-gen3: Allow memory-region for restricted DMA buffer
From: Chen-Yu Tsai @ 2026-05-18  9:02 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Ryder Lee,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
	Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, devicetree,
	linux-pci, linux-mediatek, linux-kernel
In-Reply-To: <2tq4zy3zhcrz25rl2qhdsxvoedcry4z3v2lmmropesxqgzhtlp@fn6htdl74ogu>

On Fri, May 15, 2026 at 8:34 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Fri, May 15, 2026 at 05:16:19PM +0800, Chen-Yu Tsai wrote:
> > On Thu, May 14, 2026 at 7:48 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> > >
> > > On Thu, May 14, 2026 at 03:54:29PM +0800, Chen-Yu Tsai wrote:
> > > > On Thu, May 14, 2026 at 1:23 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> > > > >
> > > > > On Fri, May 08, 2026 at 02:36:32PM +0800, Chen-Yu Tsai wrote:
> > > > > > On some SoCs without an IOMMU behind the PCIe controller, the PCIe
> > > > > > controller memory access could be limited to a small region by the
> > > > > > firmware configuring a memory protection unit. This memory region
> > > > > > must be assigned to the PCIe controller so that the OS knows to
> > > > > > use that region. Otherwise PCIe devices would not work properly.
> > > > > >
> > > > >
> > > > > So this means, the PCIe devices can only access a specific carveout memory
> > > > > configured by MPU for DMA? If so, you should use 'dma-ranges' as suggested by
> > > > > Rob.
> > > > >
> > > > > 'memory-region' also serves the purpose, but for PCI, we have the dedicated
> > > > > 'dma-ranges' property.
> > > >
> > > > I think I need some sort of guide on writing the 'dma-ranges' property,
> > > > because it is not working for me.
> > > >
> > > > I'm adding
> > > >
> > > >     dma-ranges = <0x42000000 0 0x00000000 0 0xc0000000 0 0x4000000>;
> > > >
> > >
> > > So the device DMA address start from 0x0? Isn't it a 1:1 mapping?
> >
> > I actually don't know. But
> >
> > >         dma-ranges = <0x42000000 0 0xc0000000 0 0xc0000000 0 0x4000000>;
> >
> > this didn't work either.
>
>
> Hmm. Can you print the DMA address programmed to the device? i.e., the address
> returned by dma_map_single() in the driver.

On a working system still using the restricted-dma-pool memory region,
it gives something like 0x00000000c0009000, so indeed it is 1:1 mapping?
These are for the RX/TX descriptors [1][2].

When using dma-ranges, the failure is from dma_alloc_coherent() [3][4],
which is the descriptor ring. On a working system, this is something
like 0x00000000c0c9d000, so again 1:1.

[1] https://elixir.bootlin.com/linux/v7.0.8/source/drivers/net/wireless/realtek/rtw88/pci.c#L221
[2] https://elixir.bootlin.com/linux/v7.0.8/source/drivers/net/wireless/realtek/rtw88/pci.c#L829
[3] https://elixir.bootlin.com/linux/v7.0.8/source/drivers/net/wireless/realtek/rtw88/pci.c#L192
[4] https://elixir.bootlin.com/linux/v7.0.8/source/drivers/net/wireless/realtek/rtw88/pci.c#L265

> Also, using prefetchable flag is not correct for DMA memory. You should use:
>
>         dma-ranges = <0x02000000 0 0xc0000000 0 0xc0000000 0 0x4000000>;

This didn't work either. What exactly is supposed to handle dma-ranges?
I see some code parsing it in the PCI core, but it just saves it to a list.


Here's a function graph trace for the dma_alloc_coherent() call:

funcgraph_entry:                   |  dma_alloc_attrs() {
funcgraph_entry:        6.538 us   |    dma_alloc_from_dev_coherent(); (ret=0x0)
funcgraph_entry:                   |    dma_direct_alloc() {
funcgraph_entry:                   |      __dma_direct_alloc_pages.isra.0() {
funcgraph_entry:        4.846 us   |        dma_alloc_contiguous(); (ret=0x0)
funcgraph_entry:                   |        __alloc_pages_noprof() {
funcgraph_entry:                   |          __alloc_frozen_pages_noprof() {
funcgraph_entry:        5.539 us   |            fs_reclaim_acquire();
(ret=0xffffff80c7dcd580)
funcgraph_entry:        5.077 us   |            fs_reclaim_release();
(ret=0xffffff80c7dcd580)
funcgraph_entry:                   |            __might_sleep() {
funcgraph_entry:        5.153 us   |              __might_resched(); (ret=0x0)
funcgraph_exit:       + 16.230 us  |            } (ret=0x0)
funcgraph_entry:        5.077 us   |
__next_zones_zonelist(); (ret=0xffffffd055d598e0)
funcgraph_entry:                   |            get_page_from_freelist() {
funcgraph_entry:                   |              _raw_spin_trylock() {
funcgraph_entry:        5.385 us   |
do_raw_spin_trylock(); (ret=0x1)
funcgraph_exit:       + 16.923 us  |              } (ret=0x1)
funcgraph_entry:                   |              _raw_spin_unlock() {
funcgraph_entry:        5.077 us   |
do_raw_spin_unlock(); (ret=0x1)
funcgraph_exit:       + 16.538 us  |              } (ret=0x100000001)
funcgraph_exit:       + 54.231 us  |            } (ret=0xfffffffec051c540)
funcgraph_exit:       ! 123.462 us |          } (ret=0xfffffffec051c540)
funcgraph_exit:       ! 134.692 us |        } (ret=0xfffffffec051c540)
funcgraph_entry:                   |        __free_pages() {
funcgraph_entry:                   |          ___free_pages() {
funcgraph_entry:                   |            __free_frozen_pages() {
funcgraph_entry:        5.538 us   |
__get_pfnblock_flags_mask.isra.0(); (ret=0x0)
funcgraph_entry:                   |              _raw_spin_trylock() {
funcgraph_entry:        5.077 us   |
do_raw_spin_trylock(); (ret=0x1)
funcgraph_exit:       + 16.538 us  |              } (ret=0x1)
funcgraph_entry:        5.385 us   |
free_frozen_page_commit(); (ret=0x1)
funcgraph_entry:                   |              _raw_spin_unlock() {
funcgraph_entry:        5.077 us   |
do_raw_spin_unlock(); (ret=0x1)
funcgraph_exit:       + 16.385 us  |              } (ret=0x100000001)
funcgraph_exit:       + 75.000 us  |            } (ret=0x0)
funcgraph_exit:       + 86.230 us  |          } (ret=0x0)
funcgraph_exit:       + 97.384 us  |        } (ret=0x0)
funcgraph_exit:       ! 262.846 us |      } (ret=0x0)
funcgraph_exit:       ! 274.538 us |    } (ret=0x0)
funcgraph_exit:       ! 309.077 us |  } (ret=0x0)


And here are kernel logs for all the system's memory regions:

Reserved memory: created DMA memory pool at 0x000000013ff00000, size 1 MiB
OF: reserved mem: initialized node audio-dma-pool, compatible id shared-dma-pool
OF: reserved mem: 0x000000013ff00000..0x000000013fffffff (1024 KiB)
nomap non-reusable audio-dma-pool
OF: reserved mem: 0x00000000ffe65000..0x00000000fff64fff (1024 KiB)
map non-reusable ramoops
Reserved memory: created DMA memory pool at 0x0000000050000000, size 41 MiB
OF: reserved mem: initialized node scp@50000000, compatible id shared-dma-pool
OF: reserved mem: 0x0000000050000000..0x00000000528fffff (41984 KiB)
nomap non-reusable scp@50000000
cma: Reserved 16 MiB at 0x00000000c3000000

Zone ranges:
  DMA      [mem 0x0000000040000000-0x00000000c3ffffff]
  DMA32    [mem 0x00000000c4000000-0x00000000ffffffff]
  Normal   [mem 0x0000000100000000-0x000000013fffffff]

Early memory node ranges
  node   0: [mem 0x0000000040000000-0x000000004fffffff]
  node   0: [mem 0x0000000050000000-0x00000000528fffff]
  node   0: [mem 0x0000000052900000-0x00000000545fffff]
  node   0: [mem 0x0000000054700000-0x00000000ffdfffff]
  node   0: [mem 0x0000000100000000-0x000000013fefffff]
  node   0: [mem 0x000000013ff00000-0x000000013fffffff]

software IO TLB: area num 8.
software IO TLB: mapped [mem 0x00000000bf000000-0x00000000c3000000] (64MB)


So I think it could be that the usable memory has all been given away to
other bits? But then dma_alloc_contiguous() returned NULL.


ChenYu


^ permalink raw reply

* Re: [PATCH] drm/mediatek: Convert legacy DRM logging to drm_* helpers in mtk_dsi.c
From: CK Hu (胡俊光) @ 2026-05-18  8:29 UTC (permalink / raw)
  To: p.zabel@pengutronix.de, AngeloGioacchino Del Regno,
	abhiraj21put@gmail.com, chunkuang.hu@kernel.org,
	airlied@gmail.com, matthias.bgg@gmail.com, simona@ffwll.ch
  Cc: dri-devel@lists.freedesktop.org,
	linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20260420052008.5417-1-abhiraj21put@gmail.com>

On Mon, 2026-04-20 at 10:50 +0530, Abhishek Rajput wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> Replace DRM_INFO(), DRM_WARN() and DRM_ERROR() calls in
> drivers/gpu/drm/mediatek/mtk_dsi.c with the corresponding
> drm_info(), drm_warn() and drm_err() helpers.
> 
> The drm_*() logging helpers take a struct drm_device * argument,
> allowing the DRM core to prefix log messages with the correct device
> name and instance. This is required to correctly distinguish log
> messages on systems with multiple GPUs.
> 
> This change aligns the radeon driver with the DRM TODO item:
> "Convert logging to drm_* functions with drm_device parameter".

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Signed-off-by: Abhishek Rajput <abhiraj21put@gmail.com>
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 0e2bcd5f67b7..a67ad575f5f0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -510,6 +510,7 @@ static void mtk_dsi_config_vdo_timing_per_line_lp(struct mtk_dsi *dsi)
>         u32 delta;
>         struct mtk_phy_timing *timing = &dsi->phy_timing;
>         struct videomode *vm = &dsi->vm;
> +       struct drm_device *drm = dsi->bridge.dev;
> 
>         if (dsi->format == MIPI_DSI_FMT_RGB565)
>                 dsi_tmp_buf_bpp = 2;
> @@ -543,7 +544,7 @@ static void mtk_dsi_config_vdo_timing_per_line_lp(struct mtk_dsi *dsi)
>                                              horizontal_backporch_byte /
>                                              horizontal_front_back_byte;
>         } else {
> -               DRM_WARN("HFP + HBP less than d-phy, FPS will under 60Hz\n");
> +               drm_warn(drm, "HFP + HBP less than d-phy, FPS will under 60Hz\n");
>         }
> 
>         if ((dsi->mode_flags & MIPI_DSI_HS_PKT_END_ALIGNED) &&
> @@ -623,12 +624,13 @@ static s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
>  {
>         s32 ret = 0;
>         unsigned long jiffies = msecs_to_jiffies(timeout);
> +       struct drm_device *drm = dsi->bridge.dev;
> 
>         ret = wait_event_interruptible_timeout(dsi->irq_wait_queue,
>                                                dsi->irq_data & irq_flag,
>                                                jiffies);
>         if (ret == 0) {
> -               DRM_WARN("Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
> +               drm_warn(drm, "Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
> 
>                 mtk_dsi_enable(dsi);
>                 mtk_dsi_reset_engine(dsi);
> @@ -663,9 +665,10 @@ static s32 mtk_dsi_switch_to_cmd_mode(struct mtk_dsi *dsi, u8 irq_flag, u32 t)
>  {
>         mtk_dsi_irq_data_clear(dsi, irq_flag);
>         mtk_dsi_set_cmd_mode(dsi);
> +       struct drm_device *drm = dsi->bridge.dev;
> 
>         if (!mtk_dsi_wait_for_irq_done(dsi, irq_flag, t)) {
> -               DRM_ERROR("failed to switch cmd mode\n");
> +               drm_err(drm, "failed to switch cmd mode\n");
>                 return -ETIME;
>         } else {
>                 return 0;
> @@ -849,11 +852,12 @@ static void mtk_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
>                                              struct drm_atomic_state *state)
>  {
>         struct mtk_dsi *dsi = bridge_to_dsi(bridge);
> +       struct drm_device *drm = bridge->dev;
>         int ret;
> 
>         ret = mtk_dsi_poweron(dsi);
>         if (ret < 0)
> -               DRM_ERROR("failed to power on dsi\n");
> +               drm_err(drm, "failed to power on dsi\n");
>  }
> 
>  static void mtk_dsi_bridge_atomic_post_disable(struct drm_bridge *bridge,
> @@ -916,7 +920,7 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi)
>         ret = drm_simple_encoder_init(drm, &dsi->encoder,
>                                       DRM_MODE_ENCODER_DSI);
>         if (ret) {
> -               DRM_ERROR("Failed to encoder init to drm\n");
> +               drm_err(drm, "Failed to encoder init to drm\n");
>                 return ret;
>         }
> 
> @@ -932,7 +936,7 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi)
> 
>         dsi->connector = drm_bridge_connector_init(drm, &dsi->encoder);
>         if (IS_ERR(dsi->connector)) {
> -               DRM_ERROR("Unable to create bridge connector\n");
> +               drm_err(drm, "Unable to create bridge connector\n");
>                 ret = PTR_ERR(dsi->connector);
>                 goto err_cleanup_encoder;
>         }
> @@ -985,6 +989,7 @@ static int mtk_dsi_host_attach(struct mipi_dsi_host *host,
>  {
>         struct mtk_dsi *dsi = host_to_dsi(host);
>         struct device *dev = host->dev;
> +       struct drm_device *drm = dsi->bridge.dev;
>         int ret;
> 
>         dsi->lanes = device->lanes;
> @@ -1012,7 +1017,7 @@ static int mtk_dsi_host_attach(struct mipi_dsi_host *host,
> 
>         ret = component_add(host->dev, &mtk_dsi_component_ops);
>         if (ret) {
> -               DRM_ERROR("failed to add dsi_host component: %d\n", ret);
> +               drm_err(drm, "failed to add dsi_host component: %d\n", ret);
>                 drm_bridge_remove(&dsi->bridge);
>                 return ret;
>         }
> @@ -1034,11 +1039,12 @@ static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi)
>  {
>         int ret;
>         u32 val;
> +       struct drm_device *drm = dsi->bridge.dev;
> 
>         ret = readl_poll_timeout(dsi->regs + DSI_INTSTA, val, !(val & DSI_BUSY),
>                                  4, 2000000);
>         if (ret) {
> -               DRM_WARN("polling dsi wait not busy timeout!\n");
> +               drm_warn(drm, "polling dsi wait not busy timeout!\n");
> 
>                 mtk_dsi_enable(dsi);
>                 mtk_dsi_reset_engine(dsi);
> @@ -1123,6 +1129,7 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
>                                      const struct mipi_dsi_msg *msg)
>  {
>         struct mtk_dsi *dsi = host_to_dsi(host);
> +       struct drm_device *drm = dsi->bridge.dev;
>         ssize_t recv_cnt;
>         u8 read_data[16];
>         void *src_addr;
> @@ -1153,7 +1160,7 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
>         }
> 
>         if (!msg->rx_buf) {
> -               DRM_ERROR("dsi receive buffer size may be NULL\n");
> +               drm_err(drm, "dsi receive buffer size may be NULL\n");
>                 ret = -EINVAL;
>                 goto restore_dsi_mode;
>         }
> @@ -1177,7 +1184,7 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
>         if (recv_cnt)
>                 memcpy(msg->rx_buf, src_addr, recv_cnt);
> 
> -       DRM_INFO("dsi get %zd byte data from the panel address(0x%x)\n",
> +       drm_info(drm, "dsi get %zd byte data from the panel address(0x%x)\n",
>                  recv_cnt, *((u8 *)(msg->tx_buf)));
> 
>  restore_dsi_mode:
> --
> 2.43.0
> 


^ permalink raw reply

* Re: [PATCH 2/3] memory: mtk-smi: Add a flag skip_rpm
From: Xueqi Zhang (张雪琦) @ 2026-05-18  7:41 UTC (permalink / raw)
  To: robh@kernel.org, matthias.bgg@gmail.com,
	Yong Wu (吴勇), AngeloGioacchino Del Regno,
	krzk@kernel.org, conor+dt@kernel.org
  Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	devicetree@vger.kernel.org,
	Xueqi Zhang (张雪琦),
	Project_Global_Chrome_Upstream_Group,
	Wendy-ST Lin (林詩庭),
	linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev
In-Reply-To: <1571510c-e51c-40c6-a702-214bf77f9f4d@kernel.org>

hi Krzysztof

My apologies for the multiple emails! I've been having some technical
issues with my mail client's plain-text settings and thought my
previous attempts had failed.Thanks for the reminder!

thanks
xueqi

On Mon, 2026-05-18 at 09:18 +0200, Krzysztof Kozlowski wrote:
> On 18/05/2026 09:16, Xueqi Zhang (张雪琦) wrote:
> > Hi Angelo,
> > 
> > First of all, please accept my apologies for the delayed response.
> > I 
> > have been deeply occupied with MT8196 Aluminium pKVM SMMU and SMI 
> > related tasks recently.
> > 
> 
> 
> How may times are you going to send it? I counted FOUR already!
> 
> 
https://lore.kernel.org/all/?q=%22Re%3A+%5BPATCH+2%2F3%5D+memory%3A+mtk-smi%3A+Add+a+flag+skip_rpm%22
>  
> 
> Best regards,
> Krzysztof

^ permalink raw reply

* Re: [PATCH 2/3] memory: mtk-smi: Add a flag skip_rpm
From: Krzysztof Kozlowski @ 2026-05-18  7:18 UTC (permalink / raw)
  To: Xueqi Zhang (张雪琦), robh@kernel.org,
	matthias.bgg@gmail.com, Yong Wu (吴勇),
	AngeloGioacchino Del Regno, conor+dt@kernel.org
  Cc: Wendy-ST Lin (林詩庭),
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
	Project_Global_Chrome_Upstream_Group, devicetree@vger.kernel.org
In-Reply-To: <198865fb3184926d0b1b4e4855b5f863ad0d6a20.camel@mediatek.com>

On 18/05/2026 09:16, Xueqi Zhang (张雪琦) wrote:
> Hi Angelo,
> 
> First of all, please accept my apologies for the delayed response. I 
> have been deeply occupied with MT8196 Aluminium pKVM SMMU and SMI 
> related tasks recently.
> 


How may times are you going to send it? I counted FOUR already!

https://lore.kernel.org/all/?q=%22Re%3A+%5BPATCH+2%2F3%5D+memory%3A+mtk-smi%3A+Add+a+flag+skip_rpm%22

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH 4/8] drm/panthor: Add support for protected memory allocation in panthor
From: Boris Brezillon @ 2026-05-18  7:16 UTC (permalink / raw)
  To: Chia-I Wu
  Cc: Liviu Dudau, Marcin Ślusarz, Ketil Johnsen, David Airlie,
	Simona Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
	Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
	Christian König, Steven Price, Daniel Almeida, Alice Ryhl,
	Matthias Brugger, AngeloGioacchino Del Regno, dri-devel,
	linux-doc, linux-kernel, linux-media, linaro-mm-sig,
	linux-arm-kernel, linux-mediatek, Florent Tomasin, nd
In-Reply-To: <CAPaKu7QC7FdjL6m_OSb+E5aYKs6bmT-9DAHc5PC=XctCmRph2Q@mail.gmail.com>

On Wed, 13 May 2026 12:31:32 -0700
Chia-I Wu <olvaffe@gmail.com> wrote:

> On Tue, May 12, 2026 at 8:39 AM Liviu Dudau <liviu.dudau@arm.com> wrote:
> >
> > On Tue, May 12, 2026 at 04:11:11PM +0200, Boris Brezillon wrote:  
> > > On Tue, 12 May 2026 14:47:27 +0100
> > > Liviu Dudau <liviu.dudau@arm.com> wrote:
> > >  
> > > > On Thu, May 07, 2026 at 01:53:56PM +0200, Boris Brezillon wrote:  
> > > > > On Thu, 7 May 2026 11:02:26 +0200
> > > > > Marcin Ślusarz <marcin.slusarz@arm.com> wrote:
> > > > >  
> > > > > > On Tue, May 05, 2026 at 06:15:23PM +0200, Boris Brezillon wrote:  
> > > > > > > > @@ -277,9 +286,21 @@ int panthor_device_init(struct panthor_device *ptdev)
> > > > > > > >                     return ret;
> > > > > > > >     }
> > > > > > > >
> > > > > > > > +   /* If a protected heap name is specified but not found, defer the probe until created */
> > > > > > > > +   if (protected_heap_name && strlen(protected_heap_name)) {  
> > > > > > >
> > > > > > > Do we really need this strlen() > 0? Won't dma_heap_find() fail is the
> > > > > > > name is "" already?  
> > > > > >
> > > > > > If dma_heap_find() will fail, then the whole probe with fail too.
> > > > > > This check prevents that.  
> > > > >
> > > > > Yeah, that's also a questionable design choice. I mean, we can
> > > > > currently probe and boot the FW even though we never setup the
> > > > > protected FW sections, so why should we defer the probe here? Can't we
> > > > > just retry the next time a group with the protected bit is created and
> > > > > fail if we can find a protected heap?  
> > > >
> > > > The problem we have with the current firmware is that it does a number of setup steps at "boot"
> > > > time only. One of the steps is preparing its internal structures for when it enters protected
> > > > mode and it stores them in the buffer passed in at firmware loading. We cannot later run the
> > > > process when we have a group with protected mode set.  
> > >
> > > No, but we can force a full/slow reset and have that thing
> > > re-initialized, can't we? I mean, that's basically what we do when a
> > > fast reset fails: we re-initialize all the sections and reset again, at
> > > which point the FW should start from a fresh state, and be able to
> > > properly initialize the protected-related stuff if protected sections
> > > are populated. Am I missing something?  
> >
> > Right, we can do that. For some reason I keep associating the reset with the
> > error handling and not with "normal" operations.  
> I kind of hope we end up with either
> 
>  - panthor knows the exact heap to use and fails with EPROBE_DEFER if
> the heap is missing, or
>  - panthor gets a dma-buf from userspace and does the full reset
>    - userspace also needs to provide a dma-buf for each protected
> group for the suspend buffer
> 
> than something in-between. The latter is more ad-hoc and basically
> kicks the issue to the userspace.

Indeed, the second option is more ad-hoc, but when you think about it,
userspace has to have this knowledge, because it needs to know the
dma-heap to use for buffer allocation that cross a device boundary
anyway. Think about frames produced by a video decoder, and composited
by the GPU into a protected scanout buffer that's passed to the KMS
device. Why would the GPU driver be source of truth when it comes to
choosing the heap to use to allocate protected buffers for the video
decoder or those used for the display?

> 
> For the former, expressing the relation in DT seems to be the best,
> but only if possible :-). Otherwise, a kconfig option (instead of
> module param) should be easier to work with.
> 
> Looking at the userspace implementation, can we also have an panthor
> ioctl to return the heap to userspace?

Yes, it's something we can add, but again, I'm questioning the
usefulness of this: how can we ensure the heap used by panthor to
allocate its protected FW buffers is suitable for scanout buffers
(buffers that can be used by display drivers). There needs to be a glue
leaving in usersland and taking the decision, and I'm not too sure
trusting any of the component in the chain (vdec, gpu, display) is the
right thing to do.


^ permalink raw reply

* Re: [PATCH 2/3] memory: mtk-smi: Add a flag skip_rpm
From: Xueqi Zhang (张雪琦) @ 2026-05-18  7:16 UTC (permalink / raw)
  To: robh@kernel.org, matthias.bgg@gmail.com,
	Yong Wu (吴勇), AngeloGioacchino Del Regno,
	krzk@kernel.org, conor+dt@kernel.org
  Cc: Wendy-ST Lin (林詩庭),
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
	Project_Global_Chrome_Upstream_Group, devicetree@vger.kernel.org
In-Reply-To: <46e0e1f1-e094-40f9-99f9-22678bb40d39@collabora.com>

Hi Angelo,

First of all, please accept my apologies for the delayed response. I 
have been deeply occupied with MT8196 Aluminium pKVM SMMU and SMI 
related tasks recently.

Regarding your question, my previous description in the patch was not 
accurate enough and may have caused some confusion. In fact, not 
all SMI commons have their backup/restore handled by the RTFF 
hardware. The SMI commons are distributed across various subsystems 
(e.g., mminfra, venc, display, cam, etc.). Currently, only the SMI 
common under the mminfra subsystem is backed up and restored by 
the RTFF hardware.

Therefore, I believe adding a specific 'skip_rpm' flag is more 
appropriate here. If we were to differentiate this based on a new 
MTK_SMI_GEN3 type, it would imply that all SMI common modules of 
that generation would skip the RPM operations, which is not the 
intended behavior.

To make this clearer, I plan to update the commit message in the 
next version as follows:

Subject: memory: mtk-smi: Add skip_rpm flag for certain MT8196 SMI
commons

Body:
On MT8196, certain SMI commons are backed up and restored by the RTFF 
hardware rather than by software. 

For these specific SMI commons, software-controlled register backup 
and restore in the runtime callback is no longer necessary. Therefore, 
introduce a 'skip_rpm' flag to bypass these redundant RPMoperations
for these SMI commons.

What do you think about this approach?

Thanks,
Xueqi

On Thu, 2025-03-20 at 13:11 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 20/03/25 08:36, Xueqi Zhang ha scritto:
> > MT8196 SMI commons is backed up/restored by RTFF HW.
> > It doesn't need SW control the register backup/store
> > in the runtime callback.Therefore, add a flag skip_rpm
> > to help skip RPM operations for SMI commons.
> > 
> > Signed-off-by: Xueqi Zhang <xueqi.zhang@mediatek.com>
> 
> So the MT8196 SMI common doesn't require any clocks?
> 
> That's fine for me, but this looks bloody similar to MT6989's SMI
> common, which
> is SMI GEN3 and not GEN2....
> 
> ....so, are you sure that you need a `skip_rpm` flag and not new
> MTK_SMI_GEN3 and
> MTK_SMI_GEN3_SUB_COMM types? :-)
> 
> Regards,
> Angelo
> 
> > ---
> >   drivers/memory/mtk-smi.c | 11 ++++++++---
> >   1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index a8f5467d6b31..b9affa3c3185 100644
> > --- a/drivers/memory/mtk-smi.c
> > +++ b/drivers/memory/mtk-smi.c
> > @@ -123,6 +123,7 @@ static const char * const mtk_smi_common_clks[]
> > = {"apb", "smi", "gals0", "gals1
> >   struct mtk_smi_common_plat {
> >       enum mtk_smi_type       type;
> >       bool                    has_gals;
> > +     bool                    skip_rpm;
> >       u32                     bus_sel; /* Balance some larbs to
> > enter mmu0 or mmu1 */
> > 
> >       const struct mtk_smi_reg_pair   *init;
> > @@ -547,6 +548,9 @@ static int mtk_smi_dts_clk_init(struct device
> > *dev, struct mtk_smi *smi,
> >   {
> >       int i, ret;
> > 
> > +     if (smi->plat->skip_rpm)
> > +             return 0;
> > +
> >       for (i = 0; i < clk_nr_required; i++)
> >               smi->clks[i].id = clks[i];
> >       ret = devm_clk_bulk_get(dev, clk_nr_required, smi->clks);
> > @@ -783,7 +787,7 @@ static int mtk_smi_common_probe(struct
> > platform_device *pdev)
> >       common->dev = dev;
> >       common->plat = of_device_get_match_data(dev);
> > 
> > -     if (common->plat->has_gals) {
> > +     if (!common->plat->skip_rpm && common->plat->has_gals) {
> >               if (common->plat->type == MTK_SMI_GEN2)
> >                       clk_required = MTK_SMI_COM_GALS_REQ_CLK_NR;
> >               else if (common->plat->type == MTK_SMI_GEN2_SUB_COMM)
> > @@ -814,13 +818,14 @@ static int mtk_smi_common_probe(struct
> > platform_device *pdev)
> >       }
> > 
> >       /* link its smi-common if this is smi-sub-common */
> > -     if (common->plat->type == MTK_SMI_GEN2_SUB_COMM) {
> > +     if (common->plat->type == MTK_SMI_GEN2_SUB_COMM && !common-
> > >plat->skip_rpm) {
> >               ret = mtk_smi_device_link_common(dev, &common-
> > >smi_common_dev);
> >               if (ret < 0)
> >                       return ret;
> >       }
> > 
> > -     pm_runtime_enable(dev);
> > +     if (!common->plat->skip_rpm)
> > +             pm_runtime_enable(dev);
> >       platform_set_drvdata(pdev, common);
> >       return 0;
> >   }
> 
> 

^ permalink raw reply

* Re: [PATCH 2/3] memory: mtk-smi: Add a flag skip_rpm
From: Xueqi Zhang (张雪琦) @ 2026-05-18  7:13 UTC (permalink / raw)
  To: robh@kernel.org, matthias.bgg@gmail.com,
	Yong Wu (吴勇), AngeloGioacchino Del Regno,
	krzk@kernel.org, conor+dt@kernel.org
  Cc: Wendy-ST Lin (林詩庭),
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
	Project_Global_Chrome_Upstream_Group, devicetree@vger.kernel.org
In-Reply-To: <46e0e1f1-e094-40f9-99f9-22678bb40d39@collabora.com>

Hi Angelo,

First of all, please accept my apologies for the delayed response. I 
have been deeply occupied with MT8196 Aluminium pKVM SMMU and SMI 
related tasks recently.

Regarding your question, my previous description in the patch was not 
accurate enough and may have caused some confusion. In fact, not 
all SMI commons have their backup/restore handled by the RTFF 
hardware. The SMI commons are distributed across various subsystems 
(e.g., mminfra, venc, display, cam, etc.). Currently, only the SMI 
common under the mminfra subsystem is backed up and restored by 
the RTFF hardware.

Therefore, I believe adding a specific 'skip_rpm' flag is more 
appropriate here. If we were to differentiate this based on a new 
MTK_SMI_GEN3 type, it would imply that all SMI common modules of 
that generation would skip the RPM operations, which is not the 
intended behavior.

To make this clearer, I plan to update the commit message in the 
next version as follows:

Subject: memory: mtk-smi: Add skip_rpm flag for certain MT8196 SMI
commons

Body:
On MT8196, certain SMI commons are backed up and restored by the RTFF 
hardware rather than by software. 

For these specific SMI commons, software-controlled register backup 
and restore in the runtime callback is no longer necessary. Therefore, 
introduce a 'skip_rpm' flag to bypass these redundant RPMoperations
for these SMI commons.

What do you think about this approach?

For reference, here is the link to the patch series on Patchwork:

https://patchwork.kernel.org/project/linux-mediatek/list/?series=945811&archive=both

Thanks,
Xueqi

On Thu, 2025-03-20 at 13:11 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 20/03/25 08:36, Xueqi Zhang ha scritto:
> > MT8196 SMI commons is backed up/restored by RTFF HW.
> > It doesn't need SW control the register backup/store
> > in the runtime callback.Therefore, add a flag skip_rpm
> > to help skip RPM operations for SMI commons.
> > 
> > Signed-off-by: Xueqi Zhang <xueqi.zhang@mediatek.com>
> 
> So the MT8196 SMI common doesn't require any clocks?
> 
> That's fine for me, but this looks bloody similar to MT6989's SMI
> common, which
> is SMI GEN3 and not GEN2....
> 
> ....so, are you sure that you need a `skip_rpm` flag and not new
> MTK_SMI_GEN3 and
> MTK_SMI_GEN3_SUB_COMM types? :-)
> 
> Regards,
> Angelo
> 
> > ---
> >   drivers/memory/mtk-smi.c | 11 ++++++++---
> >   1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index a8f5467d6b31..b9affa3c3185 100644
> > --- a/drivers/memory/mtk-smi.c
> > +++ b/drivers/memory/mtk-smi.c
> > @@ -123,6 +123,7 @@ static const char * const mtk_smi_common_clks[]
> > = {"apb", "smi", "gals0", "gals1
> >   struct mtk_smi_common_plat {
> >       enum mtk_smi_type       type;
> >       bool                    has_gals;
> > +     bool                    skip_rpm;
> >       u32                     bus_sel; /* Balance some larbs to
> > enter mmu0 or mmu1 */
> > 
> >       const struct mtk_smi_reg_pair   *init;
> > @@ -547,6 +548,9 @@ static int mtk_smi_dts_clk_init(struct device
> > *dev, struct mtk_smi *smi,
> >   {
> >       int i, ret;
> > 
> > +     if (smi->plat->skip_rpm)
> > +             return 0;
> > +
> >       for (i = 0; i < clk_nr_required; i++)
> >               smi->clks[i].id = clks[i];
> >       ret = devm_clk_bulk_get(dev, clk_nr_required, smi->clks);
> > @@ -783,7 +787,7 @@ static int mtk_smi_common_probe(struct
> > platform_device *pdev)
> >       common->dev = dev;
> >       common->plat = of_device_get_match_data(dev);
> > 
> > -     if (common->plat->has_gals) {
> > +     if (!common->plat->skip_rpm && common->plat->has_gals) {
> >               if (common->plat->type == MTK_SMI_GEN2)
> >                       clk_required = MTK_SMI_COM_GALS_REQ_CLK_NR;
> >               else if (common->plat->type == MTK_SMI_GEN2_SUB_COMM)
> > @@ -814,13 +818,14 @@ static int mtk_smi_common_probe(struct
> > platform_device *pdev)
> >       }
> > 
> >       /* link its smi-common if this is smi-sub-common */
> > -     if (common->plat->type == MTK_SMI_GEN2_SUB_COMM) {
> > +     if (common->plat->type == MTK_SMI_GEN2_SUB_COMM && !common-
> > >plat->skip_rpm) {
> >               ret = mtk_smi_device_link_common(dev, &common-
> > >smi_common_dev);
> >               if (ret < 0)
> >                       return ret;
> >       }
> > 
> > -     pm_runtime_enable(dev);
> > +     if (!common->plat->skip_rpm)
> > +             pm_runtime_enable(dev);
> >       platform_set_drvdata(pdev, common);
> >       return 0;
> >   }
> 
> 

^ permalink raw reply

* Re: [PATCH 2/3] memory: mtk-smi: Add a flag skip_rpm
From: Xueqi Zhang (张雪琦) @ 2026-05-18  6:51 UTC (permalink / raw)
  To: robh@kernel.org, matthias.bgg@gmail.com,
	Yong Wu (吴勇), AngeloGioacchino Del Regno,
	krzk@kernel.org, conor+dt@kernel.org
  Cc: Wendy-ST Lin (林詩庭),
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
	Project_Global_Chrome_Upstream_Group, devicetree@vger.kernel.org
In-Reply-To: <46e0e1f1-e094-40f9-99f9-22678bb40d39@collabora.com>

Hi Angelo

First of all, please accept my apologies for the delayed response. I
have been deeply occupied with MT8196 Aluminium pKVM SMMU and SMI
related tasks recently.

Regarding your question, my previous description in the patch was not
accurate enough and may have caused some confusion. In fact, not all
SMI commons have their backup/restore handled by the RTFF hardware. The
SMI commons are distributed across various subsystems (e.g., mminfra,
venc, display, cam, etc.). Currently, only the SMI common under the
mminfra subsystem is backed up and restored by the RTFF hardware.

Therefore, I believe adding a specific skip_rpm flag is more
appropriate here. If we were to differentiate this based on a new
MTK_SMI_GEN3 type, it would imply that all SMI common modules of that
generation would skip the RPM operations, which is not the intended
behavior.

To make this clearer, I plan to update the commit message in the next
version as follows:

Subject: memory: mtk-smi: Add skip_rpm flag for certain MT8196 SMI
commons

Body:
On MT8196, certain SMI commons are backed up and restored by the RTFF
hardware rather than by software.

For these specific SMI commons, software-controlled register backup and
restore in the runtime callback is no longer necessary. Therefore,
introduce a skip_rpm flag to bypass these redundant RPM operations for
these SMI commons.

What do you think about this approach?

Thanks,
Xueqi

On Thu, 2025-03-20 at 13:11 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 20/03/25 08:36, Xueqi Zhang ha scritto:
> > MT8196 SMI commons is backed up/restored by RTFF HW.
> > It doesn't need SW control the register backup/store
> > in the runtime callback.Therefore, add a flag skip_rpm
> > to help skip RPM operations for SMI commons.
> > 
> > Signed-off-by: Xueqi Zhang <xueqi.zhang@mediatek.com>
> 
> So the MT8196 SMI common doesn't require any clocks?
> 
> That's fine for me, but this looks bloody similar to MT6989's SMI
> common, which
> is SMI GEN3 and not GEN2....
> 
> ....so, are you sure that you need a `skip_rpm` flag and not new
> MTK_SMI_GEN3 and
> MTK_SMI_GEN3_SUB_COMM types? :-)
> 
> Regards,
> Angelo
> 
> > ---
> >   drivers/memory/mtk-smi.c | 11 ++++++++---
> >   1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index a8f5467d6b31..b9affa3c3185 100644
> > --- a/drivers/memory/mtk-smi.c
> > +++ b/drivers/memory/mtk-smi.c
> > @@ -123,6 +123,7 @@ static const char * const mtk_smi_common_clks[]
> > = {"apb", "smi", "gals0", "gals1
> >   struct mtk_smi_common_plat {
> >       enum mtk_smi_type       type;
> >       bool                    has_gals;
> > +     bool                    skip_rpm;
> >       u32                     bus_sel; /* Balance some larbs to
> > enter mmu0 or mmu1 */
> > 
> >       const struct mtk_smi_reg_pair   *init;
> > @@ -547,6 +548,9 @@ static int mtk_smi_dts_clk_init(struct device
> > *dev, struct mtk_smi *smi,
> >   {
> >       int i, ret;
> > 
> > +     if (smi->plat->skip_rpm)
> > +             return 0;
> > +
> >       for (i = 0; i < clk_nr_required; i++)
> >               smi->clks[i].id = clks[i];
> >       ret = devm_clk_bulk_get(dev, clk_nr_required, smi->clks);
> > @@ -783,7 +787,7 @@ static int mtk_smi_common_probe(struct
> > platform_device *pdev)
> >       common->dev = dev;
> >       common->plat = of_device_get_match_data(dev);
> > 
> > -     if (common->plat->has_gals) {
> > +     if (!common->plat->skip_rpm && common->plat->has_gals) {
> >               if (common->plat->type == MTK_SMI_GEN2)
> >                       clk_required = MTK_SMI_COM_GALS_REQ_CLK_NR;
> >               else if (common->plat->type == MTK_SMI_GEN2_SUB_COMM)
> > @@ -814,13 +818,14 @@ static int mtk_smi_common_probe(struct
> > platform_device *pdev)
> >       }
> > 
> >       /* link its smi-common if this is smi-sub-common */
> > -     if (common->plat->type == MTK_SMI_GEN2_SUB_COMM) {
> > +     if (common->plat->type == MTK_SMI_GEN2_SUB_COMM && !common-
> > >plat->skip_rpm) {
> >               ret = mtk_smi_device_link_common(dev, &common-
> > >smi_common_dev);
> >               if (ret < 0)
> >                       return ret;
> >       }
> > 
> > -     pm_runtime_enable(dev);
> > +     if (!common->plat->skip_rpm)
> > +             pm_runtime_enable(dev);
> >       platform_set_drvdata(pdev, common);
> >       return 0;
> >   }
> 
> 

^ permalink raw reply

* Re: [PATCH 2/3] memory: mtk-smi: Add a flag skip_rpm
From: Xueqi Zhang (张雪琦) @ 2026-05-18  6:42 UTC (permalink / raw)
  To: robh@kernel.org, matthias.bgg@gmail.com,
	Yong Wu (吴勇), AngeloGioacchino Del Regno,
	krzk@kernel.org, conor+dt@kernel.org
  Cc: Wendy-ST Lin (林詩庭),
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
	Project_Global_Chrome_Upstream_Group, devicetree@vger.kernel.org
In-Reply-To: <46e0e1f1-e094-40f9-99f9-22678bb40d39@collabora.com>

hi Angelo

First of all, please accept my apologies for the delayed response. I
have been deeply occupied with MT8196 Aluminium pKVM SMMU and SMI
related tasks recently.

Regarding your question, my previous description in the patch was not
accurate enough and may have caused some confusion. In fact, not all
SMI commons have their backup/restore handled by the RTFF hardware. The
SMI commons are distributed across various subsystems (e.g., mminfra,
venc, display, cam, etc.). Currently, only the SMI common under the
mminfra subsystem is backed up and restored by the RTFF hardware.

Therefore, I believe adding a specific skip_rpm flag is more
appropriate here. If we were to differentiate this based on a new
MTK_SMI_GEN3 type, it would imply that all SMI common modules of that
generation would skip the RPM operations, which is not the intended
behavior.

To make this clearer, I plan to update the commit message in the next
version as follows:

Subject: memory: mtk-smi: Add skip_rpm flag for certain MT8196 SMI
commons

Body:
On MT8196, certain SMI commons are backed up and restored by the RTFF
(Real-Time Function Fix) hardware rather than by software.

For these specific SMI commons, software-controlled register backup and
restore in the runtime callback is no longer necessary. Therefore,
introduce a skip_rpm flag to bypass these redundant RPM operations for
these SMI commons.

What do you think about this approach?

For reference, here is the link to the patch series on Patchwork:

https://patchwork.kernel.org/project/linux-mediatek/list/?series=945811&archive=both

Thanks,
Xueqi

On Thu, 2025-03-20 at 13:11 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 20/03/25 08:36, Xueqi Zhang ha scritto:
> > MT8196 SMI commons is backed up/restored by RTFF HW.
> > It doesn't need SW control the register backup/store
> > in the runtime callback.Therefore, add a flag skip_rpm
> > to help skip RPM operations for SMI commons.
> > 
> > Signed-off-by: Xueqi Zhang <xueqi.zhang@mediatek.com>
> 
> So the MT8196 SMI common doesn't require any clocks?
> 
> That's fine for me, but this looks bloody similar to MT6989's SMI
> common, which
> is SMI GEN3 and not GEN2....
> 
> ....so, are you sure that you need a `skip_rpm` flag and not new
> MTK_SMI_GEN3 and
> MTK_SMI_GEN3_SUB_COMM types? :-)
> 
> Regards,
> Angelo
> 
> > ---
> >   drivers/memory/mtk-smi.c | 11 ++++++++---
> >   1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index a8f5467d6b31..b9affa3c3185 100644
> > --- a/drivers/memory/mtk-smi.c
> > +++ b/drivers/memory/mtk-smi.c
> > @@ -123,6 +123,7 @@ static const char * const mtk_smi_common_clks[]
> > = {"apb", "smi", "gals0", "gals1
> >   struct mtk_smi_common_plat {
> >       enum mtk_smi_type       type;
> >       bool                    has_gals;
> > +     bool                    skip_rpm;
> >       u32                     bus_sel; /* Balance some larbs to
> > enter mmu0 or mmu1 */
> > 
> >       const struct mtk_smi_reg_pair   *init;
> > @@ -547,6 +548,9 @@ static int mtk_smi_dts_clk_init(struct device
> > *dev, struct mtk_smi *smi,
> >   {
> >       int i, ret;
> > 
> > +     if (smi->plat->skip_rpm)
> > +             return 0;
> > +
> >       for (i = 0; i < clk_nr_required; i++)
> >               smi->clks[i].id = clks[i];
> >       ret = devm_clk_bulk_get(dev, clk_nr_required, smi->clks);
> > @@ -783,7 +787,7 @@ static int mtk_smi_common_probe(struct
> > platform_device *pdev)
> >       common->dev = dev;
> >       common->plat = of_device_get_match_data(dev);
> > 
> > -     if (common->plat->has_gals) {
> > +     if (!common->plat->skip_rpm && common->plat->has_gals) {
> >               if (common->plat->type == MTK_SMI_GEN2)
> >                       clk_required = MTK_SMI_COM_GALS_REQ_CLK_NR;
> >               else if (common->plat->type == MTK_SMI_GEN2_SUB_COMM)
> > @@ -814,13 +818,14 @@ static int mtk_smi_common_probe(struct
> > platform_device *pdev)
> >       }
> > 
> >       /* link its smi-common if this is smi-sub-common */
> > -     if (common->plat->type == MTK_SMI_GEN2_SUB_COMM) {
> > +     if (common->plat->type == MTK_SMI_GEN2_SUB_COMM && !common-
> > >plat->skip_rpm) {
> >               ret = mtk_smi_device_link_common(dev, &common-
> > >smi_common_dev);
> >               if (ret < 0)
> >                       return ret;
> >       }
> > 
> > -     pm_runtime_enable(dev);
> > +     if (!common->plat->skip_rpm)
> > +             pm_runtime_enable(dev);
> >       platform_set_drvdata(pdev, common);
> >       return 0;
> >   }
> 
> 

^ permalink raw reply

* Re: [REGRESSION] Bluetooth: btmtk: MT7922 "Failed to send wmt func ctrl (-22)" after 634a4408c0615c ("validate WMT event SKB length before struct access")
From: Thorsten Leemhuis @ 2026-05-18  4:39 UTC (permalink / raw)
  To: Brandon Arnold, linux-bluetooth
  Cc: luiz.von.dentz, tristan, chris.lu, linux-mediatek, gregkh, stable,
	regressions
In-Reply-To: <CAM07e=vYJE8khJmbsn75SOYye44o8YV8TZsRyF1mFRuUTMCgUw@mail.gmail.com>

On 5/18/26 01:25, Brandon Arnold wrote:
> #regzbot introduced: 634a4408c0615c206885e60ea05f489c426f64b6
> #regzbot title: MT7922 BT controller never registers (wmt func ctrl
> -22) after btmtk WMT SKB-length validation

Thx for the reports, there are quite a few similar ones already; the
problem is known and the fix (see the link below) should be heading to
mainline this week and from there go to stable.

Ciao, Thorsten

#regzbot dup:
https://lore.kernel.org/linux-bluetooth/770d36b07311bf88210c187923f243fb9f126f04.1777058551.git.pav@iki.fi/

> Hello there.
> 
> I wanted to report the below information about a commit that I
> confirmed broke my MediaTek MT7922 adapter. The commit is also built
> into the main Arch Linux package, so it affected me during a routine
> update (see the BBS link in the report below). This adapter is the one
> that came with my Framework Laptop 16 system.
> 
> Apologies for Claude's verbosity and language but I can confirm I
> attended the bisect and revert of the offending diff. Thank you!
> 
> Workaround in use: pinned to the Arch Linux pre-7.0.7 kernel.
> 
> Thanks,
> Brandon Arnold
> 
> Commit 634a4408c0615c ("Bluetooth: btmtk: validate WMT event SKB
> length before struct access"), backported to 7.0.x stable and shipped
> in v7.0.7, breaks Bluetooth on the MediaTek MT7922: the WMT function
> control step fails with -EINVAL and the HCI controller never
> registers. WiFi on the same chip (mt7921e) is unaffected.
> 
> This is NOT a v6.19->v7.0 mainline regression -- mainline v7.0 is
> GOOD. The regression is the above commit specifically; it is absent
> from v7.0 and v7.0.6 and present in v7.0.7.
> 
> Scope / affected versions
> -------------------------
> - GOOD: mainline v7.0, linux-stable v7.0.6 (commit absent)
> - BAD: linux-stable v7.0.7 (commit present)
> - Independently reported on Arch (different machine -- Lenovo
> IdeaPad, MT7922 USB [0489:e0d8]): BROKEN on linux 7.0.7.arch2-1,
> WORKING on 7.0.6.arch1-1, identical symptom.
> https://bbs.archlinux.org/viewtopic.php?id=313561
> 
> Hardware / firmware (primary reporter)
> --------------------------------------
> - Framework Laptop 16 (AMD Ryzen AI 300 Series), board FRANMHCP09 A9
> - BIOS: INSYDE/Framework 03.04, 2025-11-06
> - MT7922, PCI [14c3:0616] subsys [14c3:e616]; BT side USB, driver
> btusb/btmtk; WiFi side mt7921e (works)
> - linux-firmware 20260410; BT firmware BT_RAM_CODE_MT7922_1_1,
> HW/SW Version 0x008a008a, Build Time 20260224103448
> - bluez 5.86
> 
> Symptom
> -------
> The firmware download and version handshake still succeed (identical
> HW/SW version line before and after), then:
> 
> Bluetooth: hci0: HW/SW Version: 0x008a008a, Build Time: 20260224103448
> Bluetooth: hci0: Failed to send wmt func ctrl (-22)
> 
> No "Device setup in N usecs" follows; BlueZ reports "No default
> controller available". hci0 exists in /sys/class/bluetooth and is not
> rfkill-blocked. Reproducible deterministically on cold boot,
> `modprobe -r btusb; modprobe btusb`, and full USB unbind/rebind. It
> fails even with btusb enable_autosuspend=0, so this is distinct from
> the known func-ctrl/autosuspend race.
> 
> Analysis
> --------
> 634a4408c0615c reworks btmtk_usb_hci_wmt_sync() to validate length
> with skb_pull_data() before casting the WMT event:
> 
> - wmt_evt = (struct btmtk_hci_wmt_evt *)data->evt_skb->data;
> + wmt_evt = skb_pull_data(data->evt_skb, sizeof(*wmt_evt));
> + if (!wmt_evt) { ... err = -EINVAL; goto err_free_skb; }
> ...
> case BTMTK_WMT_FUNC_CTRL:
> + if (!skb_pull_data(data->evt_skb,
> + sizeof(wmt_evt_funcc->status))) {
> + err = -EINVAL; goto err_free_skb;
> + }
> wmt_evt_funcc = (struct btmtk_hci_wmt_evt_funcc *)wmt_evt;
> 
> For the FUNC_CTRL response from the MT7922, one of these
> skb_pull_data() calls returns NULL and the function returns -EINVAL
> (surfaced as "Failed to send wmt func ctrl (-22)"). In effect the
> length the driver now requires
> (sizeof(struct btmtk_hci_wmt_evt) + sizeof(status)) exceeds what this
> controller actually sends for FUNC_CTRL, so a valid handshake is now
> rejected. The pre-patch code cast and read the same bytes without the
> strict length gate and worked on this hardware.
> 
> Empirical confirmation (mainline, no distro patches)
> ----------------------------------------------------
> Built faithfully from the running Arch config (only LOCALVERSION
> added); no out-of-tree changes.
> 
> - mainline v7.0 : GOOD (commit absent)
> - linux-stable v7.0.7 : BAD (kernel 7.0.7-bisect; WMT func ctrl -22
> count = 1; hci0 device setup never completes; bluetoothctl shows 0
> controllers)
> - v7.0.7 + revert of
> 70d37a8b9229 (the 7.0.7
> backport of 634a4408c0615c): GOOD (kernel
> 7.0.7-bisect-00001-ge33bfb5d7480; WMT func ctrl -22 count = 0; hci0
> device setup OK; bluetoothctl shows 1 controller)
> 
> The reverted tree's drivers/bluetooth/btmtk.c is byte-identical to
> v7.0.6 (git diff v7.0.6 -- drivers/bluetooth/btmtk.c is empty), so the
> v7.0.7-vs-revert pair isolates exactly this one commit.
> 
> Reproduction
> ------------
> 1. Boot a kernel >= v7.0.7-equivalent on MT7922 hardware.
> 2. `bluetoothctl list` -> no controller.
> 3. `journalctl -k -b | grep 'wmt func ctrl'` -> -22.
> 4. Revert 634a4408c0615c (or boot v7.0.6) -> controller registers.
> 
> A straight revert restores operation and is offered as the candidate
> fix; alternatively the length checks should be relaxed to match the
> MT7922's actual FUNC_CTRL WMT event length. I can test patches, or
> provide full bad/good kernel logs, btmon/hci traces, and the exact
> short SKB length on request.



^ permalink raw reply

* [PATCH] Input: mtk-pmic-keys - use of_device_get_match_data()
From: Rosen Penev @ 2026-05-18  4:20 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Matthias Brugger, AngeloGioacchino Del Regno,
	open list:ARM/Mediatek SoC support,
	moderated list:ARM/Mediatek SoC support,
	moderated list:ARM/Mediatek SoC support

Use of_device_get_match_data() to fetch the PMIC key register data directly instead of open-coding an of_match_device() lookup.

This also lets the driver drop the of_device.h include.

Assisted-by: Codex:GPT-5.5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/input/keyboard/mtk-pmic-keys.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
index c78d9f6d97c4..fd684ac16938 100644
--- a/drivers/input/keyboard/mtk-pmic-keys.c
+++ b/drivers/input/keyboard/mtk-pmic-keys.c
@@ -16,7 +16,6 @@
 #include <linux/mfd/mt6397/core.h>
 #include <linux/mfd/mt6397/registers.h>
 #include <linux/module.h>
-#include <linux/of_device.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -333,8 +332,6 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
 	struct mtk_pmic_keys *keys;
 	const struct mtk_pmic_regs *mtk_pmic_regs;
 	struct input_dev *input_dev;
-	const struct of_device_id *of_id =
-		of_match_device(of_mtk_pmic_keys_match_tbl, &pdev->dev);
 
 	keys = devm_kzalloc(&pdev->dev, sizeof(*keys), GFP_KERNEL);
 	if (!keys)
@@ -342,7 +339,7 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
 
 	keys->dev = &pdev->dev;
 	keys->regmap = pmic_chip->regmap;
-	mtk_pmic_regs = of_id->data;
+	mtk_pmic_regs = of_device_get_match_data(&pdev->dev);
 
 	keys->input_dev = input_dev = devm_input_allocate_device(keys->dev);
 	if (!input_dev) {
-- 
2.54.0



^ permalink raw reply related

* RE: [PATCH v4 2/7] PCI: cadence: Add post-link delay for LGA and j721e glue driver
From: Manikandan Karunakaran Pillai @ 2026-05-18  3:17 UTC (permalink / raw)
  To: Hans Zhang, bhelgaas@google.com, lpieralisi@kernel.org,
	kwilczynski@kernel.org, mani@kernel.org, vigneshr@ti.com,
	jingoohan1@gmail.com, thomas.petazzoni@bootlin.com,
	ryder.lee@mediatek.com, claudiu.beznea.uj@bp.renesas.com
  Cc: robh@kernel.org, s-vadapalli@ti.com, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, claudiu.beznea@tuxon.dev,
	linux-mediatek@lists.infradead.org,
	linux-renesas-soc@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <e7143078-0c0a-4c15-a192-081f2a5a5ff8@163.com>

>>>>>
>>>>> #include "pcie-cadence.h"
>>>>> #include "pcie-cadence-host-common.h"
>>>>> +#include "../pci-host-common.h"
>>>>>
>>>>> #define LINK_RETRAIN_TIMEOUT HZ
>>>>>
>>>>> @@ -115,6 +116,9 @@ int cdns_pcie_host_start_link(struct cdns_pcie_rc
>>> *rc,
>>>>> 	if (!ret && rc->quirk_retrain_flag)
>>>>> 		ret = cdns_pcie_retrain(pcie, pcie_link_up);
>>>>>
>>>>> +	if (!ret)
>>>>> +		pci_host_common_link_train_delay(pcie->max_link_speed);
>>>>> +
>>>>> 	return ret;
>>>>> }
>>>>> EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
>>>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c
>>>>> b/drivers/pci/controller/cadence/pcie-cadence-host.c
>>>>> index 0bc9e6e90e0e..058e4e619654 100644
>>>>> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
>>>>> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
>>>>> @@ -13,6 +13,7 @@
>>>>>
>>>>> #include "pcie-cadence.h"
>>>>> #include "pcie-cadence-host-common.h"
>>>>> +#include "../../pci.h"
>>>>>
>>>>> static u8 bar_aperture_mask[] = {
>>>>> 	[RP_BAR0] = 0x1F,
>>>>> @@ -397,6 +398,9 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
>>>>> 	rc->device_id = 0xffff;
>>>>> 	of_property_read_u32(np, "device-id", &rc->device_id);
>>>>>
>>>>> +	if (pcie->max_link_speed < 1)
>>>>> +		pcie->max_link_speed = of_pci_get_max_link_speed(np);
>>>>> +
>>>> Why is the conditional if required here as during cdns_pcie_host_setup(),
>the
>>> value of
>>>> max_link_speed is expected to be '0', unless specifically initialized by the
>>> platform code separately.
>>>>
>>>> What happens if the max_link_speed is not defined in the corresponding
>dts
>>> ? Would not the -EINVAL returned from the function create issues ?
>>>
>>> Hi Manikandan,
>>>
>>> Please see:
>>>
>>> https://urldefense.com/v3/__https://github.com/torvalds/linux/blob/v7.1-
>>> rc4/drivers/pci/controller/dwc/pcie-
>>>
>designware.c*L191__;Iw!!EHscmS1ygiU1lA!EDHVakD3QN0gGza3V1__qzHgDG9
>>> RZlq7LzC5AFsYLV2i5FcoveNFsjWORRgRdHCAmOI-LizY5cJvGIWBOFJG$
>>>
>>>
>>> Best regards,
>>> Hans
>>>
>> That is how Designware has implemented it but that does not answer my
>query. Becos both these implementations do
>> not take care of the error returned, and it could well be the case for many of
>the current implementations.
>
>Hi Manikandan,
>
>If "max-link-speed" is not defined in the DT, then:
>
>of_pci_get_max_link_speed
>   of_property_read_u32
>     of_property_read_u32_array
>       of_property_read_variable_u32_array
>         return -EINVAL;
>
>
>For patch 0001, no actions will be executed. I wonder if this answers
>your question?
>
Yes, got it. Thanks Hans.

>Best regards,
>Hans
>
>
>>
>>>>
>>>>> 	pcie->reg_base = devm_platform_ioremap_resource_byname(pdev,
>>>>> "reg");
>>>>> 	if (IS_ERR(pcie->reg_base)) {
>>>>> 		dev_err(dev, "missing \"reg\"\n");
>>>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h
>>>>> b/drivers/pci/controller/cadence/pcie-cadence.h
>>>>> index 574e9cf4d003..042a4c49bb9a 100644
>>>>> --- a/drivers/pci/controller/cadence/pcie-cadence.h
>>>>> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
>>>>> @@ -86,6 +86,7 @@ struct cdns_plat_pcie_of_data {
>>>>>    * @ops: Platform-specific ops to control various inputs from Cadence
>PCIe
>>>>>    *       wrapper
>>>>>    * @cdns_pcie_reg_offsets: Register bank offsets for different SoC
>>>>> + * @max_link_speed: Maximum supported link speed
>>>>>    */
>>>>> struct cdns_pcie {
>>>>> 	void __iomem		             *reg_base;
>>>>> @@ -98,6 +99,7 @@ struct cdns_pcie {
>>>>> 	struct device_link	             **link;
>>>>> 	const  struct cdns_pcie_ops          *ops;
>>>>> 	const  struct cdns_plat_pcie_of_data *cdns_pcie_reg_offsets;
>>>>> +	int				     max_link_speed;
>>>>> };
>>>>>
>>>>> /**
>>>>> --
>>>>> 2.43.0
>>


^ permalink raw reply

* Re: [PATCH v4 2/7] PCI: cadence: Add post-link delay for LGA and j721e glue driver
From: Hans Zhang @ 2026-05-18  3:03 UTC (permalink / raw)
  To: Manikandan Karunakaran Pillai, bhelgaas@google.com,
	lpieralisi@kernel.org, kwilczynski@kernel.org, mani@kernel.org,
	vigneshr@ti.com, jingoohan1@gmail.com,
	thomas.petazzoni@bootlin.com, ryder.lee@mediatek.com,
	claudiu.beznea.uj@bp.renesas.com
  Cc: robh@kernel.org, s-vadapalli@ti.com, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, claudiu.beznea@tuxon.dev,
	linux-mediatek@lists.infradead.org,
	linux-renesas-soc@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <DM6PR07MB6716CC48016E6D44EC5FCAB3A2032@DM6PR07MB6716.namprd07.prod.outlook.com>



On 5/18/26 10:38, Manikandan Karunakaran Pillai wrote:
> 
> 
>> EXTERNAL MAIL
>>
>>
>>
>>
>> On 5/18/26 10:12, Manikandan Karunakaran Pillai wrote:
>>>
>>>
>>>> EXTERNAL MAIL
>>>>
>>>>
>>>> The Cadence LGA (Legacy Architecture IP) PCIe host controller currently
>>>> lacks the mandatory 100 ms delay after link training completes for speeds
>>>>> 5.0 GT/s, as required by PCIe r6.0 sec 6.6.1.
>>>>
>>>> Add a 'max_link_speed' field to struct cdns_pcie. In the common host
>>>> layer function cdns_pcie_host_start_link(), after the link has been
>>>> successfully established, call pci_host_common_link_train_delay() to
>>>> insert the required delay.
>>>>
>>>> For the j721e glue driver, set cdns_pcie.max_link_speed from the existing
>>>> link speed logic. For other LGA-based glue drivers (sky1, sg2042), the
>>>> common LGA host setup (pcie-cadence-host.c) provides a fallback reading
>>>> of the device tree property "max-link-speed" when available. This ensures
>>>> that the delay is not missed on those platforms once they enable the
>>>> property.
>>>>
>>>> Signed-off-by: Hans Zhang <18255117159@163.com>
>>>> ---
>>>> drivers/pci/controller/cadence/pci-j721e.c                | 1 +
>>>> drivers/pci/controller/cadence/pcie-cadence-host-common.c | 4 ++++
>>>> drivers/pci/controller/cadence/pcie-cadence-host.c        | 4 ++++
>>>> drivers/pci/controller/cadence/pcie-cadence.h             | 2 ++
>>>> 4 files changed, 11 insertions(+)
>>>>
>>>> diff --git a/drivers/pci/controller/cadence/pci-j721e.c
>>>> b/drivers/pci/controller/cadence/pci-j721e.c
>>>> index bfdfe98d5aba..ae916e7b1927 100644
>>>> --- a/drivers/pci/controller/cadence/pci-j721e.c
>>>> +++ b/drivers/pci/controller/cadence/pci-j721e.c
>>>> @@ -206,6 +206,7 @@ static int j721e_pcie_set_link_speed(struct
>> j721e_pcie
>>>> *pcie,
>>>> 	    (pcie_get_link_speed(link_speed) == PCI_SPEED_UNKNOWN))
>>>> 		link_speed = 2;
>>>>
>>>> +	pcie->cdns_pcie->max_link_speed = link_speed;
>>>> 	val = link_speed - 1;
>>>> 	ret = regmap_update_bits(syscon, offset, GENERATION_SEL_MASK,
>>>> val);
>>>> 	if (ret)
>>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>>>> b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>>>> index 2b0211870f02..18e4b6c760b5 100644
>>>> --- a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>>>> +++ b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>>>> @@ -14,6 +14,7 @@
>>>>
>>>> #include "pcie-cadence.h"
>>>> #include "pcie-cadence-host-common.h"
>>>> +#include "../pci-host-common.h"
>>>>
>>>> #define LINK_RETRAIN_TIMEOUT HZ
>>>>
>>>> @@ -115,6 +116,9 @@ int cdns_pcie_host_start_link(struct cdns_pcie_rc
>> *rc,
>>>> 	if (!ret && rc->quirk_retrain_flag)
>>>> 		ret = cdns_pcie_retrain(pcie, pcie_link_up);
>>>>
>>>> +	if (!ret)
>>>> +		pci_host_common_link_train_delay(pcie->max_link_speed);
>>>> +
>>>> 	return ret;
>>>> }
>>>> EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
>>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c
>>>> b/drivers/pci/controller/cadence/pcie-cadence-host.c
>>>> index 0bc9e6e90e0e..058e4e619654 100644
>>>> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
>>>> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
>>>> @@ -13,6 +13,7 @@
>>>>
>>>> #include "pcie-cadence.h"
>>>> #include "pcie-cadence-host-common.h"
>>>> +#include "../../pci.h"
>>>>
>>>> static u8 bar_aperture_mask[] = {
>>>> 	[RP_BAR0] = 0x1F,
>>>> @@ -397,6 +398,9 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
>>>> 	rc->device_id = 0xffff;
>>>> 	of_property_read_u32(np, "device-id", &rc->device_id);
>>>>
>>>> +	if (pcie->max_link_speed < 1)
>>>> +		pcie->max_link_speed = of_pci_get_max_link_speed(np);
>>>> +
>>> Why is the conditional if required here as during cdns_pcie_host_setup(), the
>> value of
>>> max_link_speed is expected to be '0', unless specifically initialized by the
>> platform code separately.
>>>
>>> What happens if the max_link_speed is not defined in the corresponding dts
>> ? Would not the -EINVAL returned from the function create issues ?
>>
>> Hi Manikandan,
>>
>> Please see:
>>
>> https://urldefense.com/v3/__https://github.com/torvalds/linux/blob/v7.1-
>> rc4/drivers/pci/controller/dwc/pcie-
>> designware.c*L191__;Iw!!EHscmS1ygiU1lA!EDHVakD3QN0gGza3V1__qzHgDG9
>> RZlq7LzC5AFsYLV2i5FcoveNFsjWORRgRdHCAmOI-LizY5cJvGIWBOFJG$
>>
>>
>> Best regards,
>> Hans
>>
> That is how Designware has implemented it but that does not answer my query. Becos both these implementations do
> not take care of the error returned, and it could well be the case for many of the current implementations.

Hi Manikandan,

If "max-link-speed" is not defined in the DT, then:

of_pci_get_max_link_speed
   of_property_read_u32
     of_property_read_u32_array
       of_property_read_variable_u32_array
         return -EINVAL;


For patch 0001, no actions will be executed. I wonder if this answers 
your question?

Best regards,
Hans


> 
>>>
>>>> 	pcie->reg_base = devm_platform_ioremap_resource_byname(pdev,
>>>> "reg");
>>>> 	if (IS_ERR(pcie->reg_base)) {
>>>> 		dev_err(dev, "missing \"reg\"\n");
>>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h
>>>> b/drivers/pci/controller/cadence/pcie-cadence.h
>>>> index 574e9cf4d003..042a4c49bb9a 100644
>>>> --- a/drivers/pci/controller/cadence/pcie-cadence.h
>>>> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
>>>> @@ -86,6 +86,7 @@ struct cdns_plat_pcie_of_data {
>>>>    * @ops: Platform-specific ops to control various inputs from Cadence PCIe
>>>>    *       wrapper
>>>>    * @cdns_pcie_reg_offsets: Register bank offsets for different SoC
>>>> + * @max_link_speed: Maximum supported link speed
>>>>    */
>>>> struct cdns_pcie {
>>>> 	void __iomem		             *reg_base;
>>>> @@ -98,6 +99,7 @@ struct cdns_pcie {
>>>> 	struct device_link	             **link;
>>>> 	const  struct cdns_pcie_ops          *ops;
>>>> 	const  struct cdns_plat_pcie_of_data *cdns_pcie_reg_offsets;
>>>> +	int				     max_link_speed;
>>>> };
>>>>
>>>> /**
>>>> --
>>>> 2.43.0
> 



^ permalink raw reply

* [PATCH] ASoC: mediatek: mt8196: Fix probe resource cleanup
From: Cássio Gabriel @ 2026-05-18  2:41 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Takashi Iwai, Jaroslav Kysela,
	Matthias Brugger, AngeloGioacchino Del Regno, Darren Ye,
	Cyril Chao
  Cc: linux-sound, linux-kernel, linux-arm-kernel, linux-mediatek,
	Cássio Gabriel

The MT8196 AFE probe assigns reserved memory with
of_reserved_mem_device_init(), but never releases it.
This leaks the reserved memory assignment on driver
removal and on later probe failures.

The same probe path also uses unchecked pm_runtime_get_sync() calls.
A failure while resuming the device can leave the runtime PM usage
count in an unexpected state.

The regmap error path returns directly while the device is still
runtime active, and the remove path drops a runtime PM reference even
though successful probe has already released its temporary reference.

Register a devm cleanup action for the reserved memory assignment,
use pm_runtime_resume_and_get(), and only drop runtime PM references
on paths where they are actually held.

Fixes: 57513aabfe5b ("ASoC: mediatek: mt8196: add platform driver")
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
---
 sound/soc/mediatek/mt8196/mt8196-afe-pcm.c | 44 ++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/sound/soc/mediatek/mt8196/mt8196-afe-pcm.c b/sound/soc/mediatek/mt8196/mt8196-afe-pcm.c
index 511e888567be..a1ae8322d8b6 100644
--- a/sound/soc/mediatek/mt8196/mt8196-afe-pcm.c
+++ b/sound/soc/mediatek/mt8196/mt8196-afe-pcm.c
@@ -2242,13 +2242,16 @@ static int mt8196_afe_runtime_resume(struct device *dev)
 static int mt8196_afe_component_probe(struct snd_soc_component *component)
 {
 	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
+	int ret;
+
+	/* enable clock for regcache get default value from hw */
+	ret = pm_runtime_resume_and_get(afe->dev);
+	if (ret)
+		return dev_err_probe(afe->dev, ret, "failed to resume device\n");
+
+	mtk_afe_add_sub_dai_control(component);
+	pm_runtime_put_sync(afe->dev);
 
-	if (component) {
-		/* enable clock for regcache get default value from hw */
-		pm_runtime_get_sync(afe->dev);
-		mtk_afe_add_sub_dai_control(component);
-		pm_runtime_put_sync(afe->dev);
-	}
 	return 0;
 }
 
@@ -2306,6 +2309,11 @@ static const struct reg_sequence mt8196_cg_patch[] = {
 	{ AUDIO_TOP_CON4, 0x361c },
 };
 
+static void mt8196_afe_release_reserved_mem(void *data)
+{
+	of_reserved_mem_device_release(data);
+}
+
 static int mt8196_afe_pcm_dev_probe(struct platform_device *pdev)
 {
 	int ret, i;
@@ -2320,8 +2328,13 @@ static int mt8196_afe_pcm_dev_probe(struct platform_device *pdev)
 		return ret;
 
 	ret = of_reserved_mem_device_init(dev);
-	if (ret)
+	if (ret) {
 		dev_err(dev, "failed to assign memory region: %d\n", ret);
+	} else {
+		ret = devm_add_action_or_reset(dev, mt8196_afe_release_reserved_mem, dev);
+		if (ret)
+			return ret;
+	}
 
 	afe = devm_kzalloc(dev, sizeof(*afe), GFP_KERNEL);
 	if (!afe)
@@ -2422,18 +2435,22 @@ static int mt8196_afe_pcm_dev_probe(struct platform_device *pdev)
 	dev_pm_syscore_device(dev, true);
 
 	/* enable clock for regcache get default value from hw */
-	pm_runtime_get_sync(dev);
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to resume device\n");
 
 	afe->regmap = devm_regmap_init_mmio(dev, afe->base_addr,
 					    &mt8196_afe_regmap_config);
-	if (IS_ERR(afe->regmap))
-		return PTR_ERR(afe->regmap);
+	if (IS_ERR(afe->regmap)) {
+		ret = PTR_ERR(afe->regmap);
+		goto err_pm_put;
+	}
 
 	ret = regmap_register_patch(afe->regmap, mt8196_cg_patch,
 				    ARRAY_SIZE(mt8196_cg_patch));
 	if (ret < 0) {
 		dev_err(dev, "Failed to apply cg patch\n");
-		goto err_pm_disable;
+		goto err_pm_put;
 	}
 
 	regmap_read(afe->regmap, AFE_IRQ_MCU_EN, &tmp_reg);
@@ -2452,12 +2469,12 @@ static int mt8196_afe_pcm_dev_probe(struct platform_device *pdev)
 					      afe->num_dai_drivers);
 	if (ret) {
 		dev_err(dev, "afe component err\n");
-		goto err_pm_disable;
+		return ret;
 	}
 
 	return 0;
 
-err_pm_disable:
+err_pm_put:
 	pm_runtime_put_sync(dev);
 	return ret;
 }
@@ -2467,7 +2484,6 @@ static void mt8196_afe_pcm_dev_remove(struct platform_device *pdev)
 	struct mtk_base_afe *afe = platform_get_drvdata(pdev);
 	struct device *dev = &pdev->dev;
 
-	pm_runtime_put_sync(dev);
 	if (!pm_runtime_status_suspended(dev))
 		mt8196_afe_runtime_suspend(dev);
 

---
base-commit: 2be19ed9535043fe6abd7ccfc9aac6b7ecaac842
change-id: 20260507-asoc-mt8196-probe-cleanup-222142d1000c

Best regards,
--  
Cássio Gabriel <cassiogabrielcontato@gmail.com>



^ permalink raw reply related

* RE: [PATCH v4 2/7] PCI: cadence: Add post-link delay for LGA and j721e glue driver
From: Manikandan Karunakaran Pillai @ 2026-05-18  2:38 UTC (permalink / raw)
  To: Hans Zhang, bhelgaas@google.com, lpieralisi@kernel.org,
	kwilczynski@kernel.org, mani@kernel.org, vigneshr@ti.com,
	jingoohan1@gmail.com, thomas.petazzoni@bootlin.com,
	ryder.lee@mediatek.com, claudiu.beznea.uj@bp.renesas.com
  Cc: robh@kernel.org, s-vadapalli@ti.com, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, claudiu.beznea@tuxon.dev,
	linux-mediatek@lists.infradead.org,
	linux-renesas-soc@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <c909a890-65cb-444f-9b4f-9482d2f71c6d@163.com>



>EXTERNAL MAIL
>
>
>
>
>On 5/18/26 10:12, Manikandan Karunakaran Pillai wrote:
>>
>>
>>> EXTERNAL MAIL
>>>
>>>
>>> The Cadence LGA (Legacy Architecture IP) PCIe host controller currently
>>> lacks the mandatory 100 ms delay after link training completes for speeds
>>>> 5.0 GT/s, as required by PCIe r6.0 sec 6.6.1.
>>>
>>> Add a 'max_link_speed' field to struct cdns_pcie. In the common host
>>> layer function cdns_pcie_host_start_link(), after the link has been
>>> successfully established, call pci_host_common_link_train_delay() to
>>> insert the required delay.
>>>
>>> For the j721e glue driver, set cdns_pcie.max_link_speed from the existing
>>> link speed logic. For other LGA-based glue drivers (sky1, sg2042), the
>>> common LGA host setup (pcie-cadence-host.c) provides a fallback reading
>>> of the device tree property "max-link-speed" when available. This ensures
>>> that the delay is not missed on those platforms once they enable the
>>> property.
>>>
>>> Signed-off-by: Hans Zhang <18255117159@163.com>
>>> ---
>>> drivers/pci/controller/cadence/pci-j721e.c                | 1 +
>>> drivers/pci/controller/cadence/pcie-cadence-host-common.c | 4 ++++
>>> drivers/pci/controller/cadence/pcie-cadence-host.c        | 4 ++++
>>> drivers/pci/controller/cadence/pcie-cadence.h             | 2 ++
>>> 4 files changed, 11 insertions(+)
>>>
>>> diff --git a/drivers/pci/controller/cadence/pci-j721e.c
>>> b/drivers/pci/controller/cadence/pci-j721e.c
>>> index bfdfe98d5aba..ae916e7b1927 100644
>>> --- a/drivers/pci/controller/cadence/pci-j721e.c
>>> +++ b/drivers/pci/controller/cadence/pci-j721e.c
>>> @@ -206,6 +206,7 @@ static int j721e_pcie_set_link_speed(struct
>j721e_pcie
>>> *pcie,
>>> 	    (pcie_get_link_speed(link_speed) == PCI_SPEED_UNKNOWN))
>>> 		link_speed = 2;
>>>
>>> +	pcie->cdns_pcie->max_link_speed = link_speed;
>>> 	val = link_speed - 1;
>>> 	ret = regmap_update_bits(syscon, offset, GENERATION_SEL_MASK,
>>> val);
>>> 	if (ret)
>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>>> b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>>> index 2b0211870f02..18e4b6c760b5 100644
>>> --- a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>>> +++ b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>>> @@ -14,6 +14,7 @@
>>>
>>> #include "pcie-cadence.h"
>>> #include "pcie-cadence-host-common.h"
>>> +#include "../pci-host-common.h"
>>>
>>> #define LINK_RETRAIN_TIMEOUT HZ
>>>
>>> @@ -115,6 +116,9 @@ int cdns_pcie_host_start_link(struct cdns_pcie_rc
>*rc,
>>> 	if (!ret && rc->quirk_retrain_flag)
>>> 		ret = cdns_pcie_retrain(pcie, pcie_link_up);
>>>
>>> +	if (!ret)
>>> +		pci_host_common_link_train_delay(pcie->max_link_speed);
>>> +
>>> 	return ret;
>>> }
>>> EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c
>>> b/drivers/pci/controller/cadence/pcie-cadence-host.c
>>> index 0bc9e6e90e0e..058e4e619654 100644
>>> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
>>> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
>>> @@ -13,6 +13,7 @@
>>>
>>> #include "pcie-cadence.h"
>>> #include "pcie-cadence-host-common.h"
>>> +#include "../../pci.h"
>>>
>>> static u8 bar_aperture_mask[] = {
>>> 	[RP_BAR0] = 0x1F,
>>> @@ -397,6 +398,9 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
>>> 	rc->device_id = 0xffff;
>>> 	of_property_read_u32(np, "device-id", &rc->device_id);
>>>
>>> +	if (pcie->max_link_speed < 1)
>>> +		pcie->max_link_speed = of_pci_get_max_link_speed(np);
>>> +
>> Why is the conditional if required here as during cdns_pcie_host_setup(), the
>value of
>> max_link_speed is expected to be '0', unless specifically initialized by the
>platform code separately.
>>
>> What happens if the max_link_speed is not defined in the corresponding dts
>? Would not the -EINVAL returned from the function create issues ?
>
>Hi Manikandan,
>
>Please see:
>
>https://urldefense.com/v3/__https://github.com/torvalds/linux/blob/v7.1-
>rc4/drivers/pci/controller/dwc/pcie-
>designware.c*L191__;Iw!!EHscmS1ygiU1lA!EDHVakD3QN0gGza3V1__qzHgDG9
>RZlq7LzC5AFsYLV2i5FcoveNFsjWORRgRdHCAmOI-LizY5cJvGIWBOFJG$
>
>
>Best regards,
>Hans
>
That is how Designware has implemented it but that does not answer my query. Becos both these implementations do 
not take care of the error returned, and it could well be the case for many of the current implementations.

>>
>>> 	pcie->reg_base = devm_platform_ioremap_resource_byname(pdev,
>>> "reg");
>>> 	if (IS_ERR(pcie->reg_base)) {
>>> 		dev_err(dev, "missing \"reg\"\n");
>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h
>>> b/drivers/pci/controller/cadence/pcie-cadence.h
>>> index 574e9cf4d003..042a4c49bb9a 100644
>>> --- a/drivers/pci/controller/cadence/pcie-cadence.h
>>> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
>>> @@ -86,6 +86,7 @@ struct cdns_plat_pcie_of_data {
>>>   * @ops: Platform-specific ops to control various inputs from Cadence PCIe
>>>   *       wrapper
>>>   * @cdns_pcie_reg_offsets: Register bank offsets for different SoC
>>> + * @max_link_speed: Maximum supported link speed
>>>   */
>>> struct cdns_pcie {
>>> 	void __iomem		             *reg_base;
>>> @@ -98,6 +99,7 @@ struct cdns_pcie {
>>> 	struct device_link	             **link;
>>> 	const  struct cdns_pcie_ops          *ops;
>>> 	const  struct cdns_plat_pcie_of_data *cdns_pcie_reg_offsets;
>>> +	int				     max_link_speed;
>>> };
>>>
>>> /**
>>> --
>>> 2.43.0


^ permalink raw reply

* Re: [PATCH v4 3/7] PCI: cadence: HPA: Add post-link delay
From: Hans Zhang @ 2026-05-18  2:27 UTC (permalink / raw)
  To: Manikandan Karunakaran Pillai, bhelgaas@google.com,
	lpieralisi@kernel.org, kwilczynski@kernel.org, mani@kernel.org,
	vigneshr@ti.com, jingoohan1@gmail.com,
	thomas.petazzoni@bootlin.com, ryder.lee@mediatek.com,
	claudiu.beznea.uj@bp.renesas.com
  Cc: robh@kernel.org, s-vadapalli@ti.com, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, claudiu.beznea@tuxon.dev,
	linux-mediatek@lists.infradead.org,
	linux-renesas-soc@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <DM6PR07MB671635EE7FF28BD2FA5DDE36A2032@DM6PR07MB6716.namprd07.prod.outlook.com>



On 5/18/26 10:16, Manikandan Karunakaran Pillai wrote:
> 
> 
>> EXTERNAL MAIL
>>
>>
>> The Cadence HPA (High Performance Architecture IP) specific link setup
>> function cdns_pcie_hpa_host_link_setup() waits for the link to come up
>> but does not implement the required 100 ms delay after link training
>> completes for speeds > 5.0 GT/s (PCIe r6.0 sec 6.6.1).
>>
>> Add a call to pci_host_common_link_train_delay() immediately after the
>> link is confirmed to be up, using the max_link_speed field. Also, in the
>> HPA host setup function, read the device tree property "max-link-speed"
>> to initialize max_link_speed if not already set by a glue driver.
>>
>> This ensures compliance for HPA-based platforms.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>> drivers/pci/controller/cadence/pcie-cadence-host-hpa.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
>> b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
>> index 0f540bed58e8..8ef58ed01daa 100644
>> --- a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
>> +++ b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
>> @@ -15,6 +15,8 @@
>>
>> #include "pcie-cadence.h"
>> #include "pcie-cadence-host-common.h"
>> +#include "../pci-host-common.h"
>> +#include "../../pci.h"
>>
>> static u8 bar_aperture_mask[] = {
>> 	[RP_BAR0] = 0x3F,
>> @@ -304,6 +306,8 @@ int cdns_pcie_hpa_host_link_setup(struct cdns_pcie_rc
>> *rc)
>> 	ret = cdns_pcie_host_wait_for_link(pcie, cdns_pcie_hpa_link_up);
>> 	if (ret)
>> 		dev_dbg(dev, "PCIe link never came up\n");
>> +	else
>> +		pci_host_common_link_train_delay(pcie->max_link_speed);
>>
>> 	return ret;
>> }
>> @@ -313,6 +317,7 @@ int cdns_pcie_hpa_host_setup(struct cdns_pcie_rc *rc)
>> {
>> 	struct device *dev = rc->pcie.dev;
>> 	struct platform_device *pdev = to_platform_device(dev);
>> +	struct device_node *np = dev->of_node;
>> 	struct pci_host_bridge *bridge;
>> 	enum   cdns_pcie_rp_bar bar;
>> 	struct cdns_pcie *pcie;
>> @@ -343,6 +348,9 @@ int cdns_pcie_hpa_host_setup(struct cdns_pcie_rc *rc)
>> 		rc->cfg_res = res;
>> 	}
>>
>> +	if (pcie->max_link_speed < 1)
>> +		pcie->max_link_speed = of_pci_get_max_link_speed(np);
>> +
> 
> Similar queries as for Cadence LGA controllers. Why do you need the max_link_speed check for "<1" and
> What would be the consequences of not defining the max-link-speed in dts ?

Hi Manikandan,

It has been replied in patch 0002.

Best regards,
Hans

> 
>> 	/* Put EROM Bar aperture to 0 */
>> 	cdns_pcie_hpa_writel(pcie, REG_BANK_IP_CFG_CTRL_REG,
>> CDNS_PCIE_EROM, 0x0);
>>
>> --
>> 2.43.0



^ permalink raw reply

* Re: [PATCH v4 2/7] PCI: cadence: Add post-link delay for LGA and j721e glue driver
From: Hans Zhang @ 2026-05-18  2:26 UTC (permalink / raw)
  To: Manikandan Karunakaran Pillai, bhelgaas@google.com,
	lpieralisi@kernel.org, kwilczynski@kernel.org, mani@kernel.org,
	vigneshr@ti.com, jingoohan1@gmail.com,
	thomas.petazzoni@bootlin.com, ryder.lee@mediatek.com,
	claudiu.beznea.uj@bp.renesas.com
  Cc: robh@kernel.org, s-vadapalli@ti.com, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, claudiu.beznea@tuxon.dev,
	linux-mediatek@lists.infradead.org,
	linux-renesas-soc@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <DM6PR07MB67169B65E6B826E24A8E64AEA2032@DM6PR07MB6716.namprd07.prod.outlook.com>



On 5/18/26 10:12, Manikandan Karunakaran Pillai wrote:
> 
> 
>> EXTERNAL MAIL
>>
>>
>> The Cadence LGA (Legacy Architecture IP) PCIe host controller currently
>> lacks the mandatory 100 ms delay after link training completes for speeds
>>> 5.0 GT/s, as required by PCIe r6.0 sec 6.6.1.
>>
>> Add a 'max_link_speed' field to struct cdns_pcie. In the common host
>> layer function cdns_pcie_host_start_link(), after the link has been
>> successfully established, call pci_host_common_link_train_delay() to
>> insert the required delay.
>>
>> For the j721e glue driver, set cdns_pcie.max_link_speed from the existing
>> link speed logic. For other LGA-based glue drivers (sky1, sg2042), the
>> common LGA host setup (pcie-cadence-host.c) provides a fallback reading
>> of the device tree property "max-link-speed" when available. This ensures
>> that the delay is not missed on those platforms once they enable the
>> property.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>> drivers/pci/controller/cadence/pci-j721e.c                | 1 +
>> drivers/pci/controller/cadence/pcie-cadence-host-common.c | 4 ++++
>> drivers/pci/controller/cadence/pcie-cadence-host.c        | 4 ++++
>> drivers/pci/controller/cadence/pcie-cadence.h             | 2 ++
>> 4 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/pci/controller/cadence/pci-j721e.c
>> b/drivers/pci/controller/cadence/pci-j721e.c
>> index bfdfe98d5aba..ae916e7b1927 100644
>> --- a/drivers/pci/controller/cadence/pci-j721e.c
>> +++ b/drivers/pci/controller/cadence/pci-j721e.c
>> @@ -206,6 +206,7 @@ static int j721e_pcie_set_link_speed(struct j721e_pcie
>> *pcie,
>> 	    (pcie_get_link_speed(link_speed) == PCI_SPEED_UNKNOWN))
>> 		link_speed = 2;
>>
>> +	pcie->cdns_pcie->max_link_speed = link_speed;
>> 	val = link_speed - 1;
>> 	ret = regmap_update_bits(syscon, offset, GENERATION_SEL_MASK,
>> val);
>> 	if (ret)
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>> b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>> index 2b0211870f02..18e4b6c760b5 100644
>> --- a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>> +++ b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>> @@ -14,6 +14,7 @@
>>
>> #include "pcie-cadence.h"
>> #include "pcie-cadence-host-common.h"
>> +#include "../pci-host-common.h"
>>
>> #define LINK_RETRAIN_TIMEOUT HZ
>>
>> @@ -115,6 +116,9 @@ int cdns_pcie_host_start_link(struct cdns_pcie_rc *rc,
>> 	if (!ret && rc->quirk_retrain_flag)
>> 		ret = cdns_pcie_retrain(pcie, pcie_link_up);
>>
>> +	if (!ret)
>> +		pci_host_common_link_train_delay(pcie->max_link_speed);
>> +
>> 	return ret;
>> }
>> EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c
>> b/drivers/pci/controller/cadence/pcie-cadence-host.c
>> index 0bc9e6e90e0e..058e4e619654 100644
>> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
>> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
>> @@ -13,6 +13,7 @@
>>
>> #include "pcie-cadence.h"
>> #include "pcie-cadence-host-common.h"
>> +#include "../../pci.h"
>>
>> static u8 bar_aperture_mask[] = {
>> 	[RP_BAR0] = 0x1F,
>> @@ -397,6 +398,9 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
>> 	rc->device_id = 0xffff;
>> 	of_property_read_u32(np, "device-id", &rc->device_id);
>>
>> +	if (pcie->max_link_speed < 1)
>> +		pcie->max_link_speed = of_pci_get_max_link_speed(np);
>> +
> Why is the conditional if required here as during cdns_pcie_host_setup(), the value of
> max_link_speed is expected to be '0', unless specifically initialized by the platform code separately.
> 
> What happens if the max_link_speed is not defined in the corresponding dts ? Would not the -EINVAL returned from the function create issues ?

Hi Manikandan,

Please see:

https://github.com/torvalds/linux/blob/v7.1-rc4/drivers/pci/controller/dwc/pcie-designware.c#L191


Best regards,
Hans

> 
>> 	pcie->reg_base = devm_platform_ioremap_resource_byname(pdev,
>> "reg");
>> 	if (IS_ERR(pcie->reg_base)) {
>> 		dev_err(dev, "missing \"reg\"\n");
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h
>> b/drivers/pci/controller/cadence/pcie-cadence.h
>> index 574e9cf4d003..042a4c49bb9a 100644
>> --- a/drivers/pci/controller/cadence/pcie-cadence.h
>> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
>> @@ -86,6 +86,7 @@ struct cdns_plat_pcie_of_data {
>>   * @ops: Platform-specific ops to control various inputs from Cadence PCIe
>>   *       wrapper
>>   * @cdns_pcie_reg_offsets: Register bank offsets for different SoC
>> + * @max_link_speed: Maximum supported link speed
>>   */
>> struct cdns_pcie {
>> 	void __iomem		             *reg_base;
>> @@ -98,6 +99,7 @@ struct cdns_pcie {
>> 	struct device_link	             **link;
>> 	const  struct cdns_pcie_ops          *ops;
>> 	const  struct cdns_plat_pcie_of_data *cdns_pcie_reg_offsets;
>> +	int				     max_link_speed;
>> };
>>
>> /**
>> --
>> 2.43.0



^ permalink raw reply

* [PATCH] Bluetooth: btmtk: fix urb->setup_packet leak in error paths
From: Jiajia Liu @ 2026-05-18  2:24 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz, Matthias Brugger,
	AngeloGioacchino Del Regno, Sean Wang
  Cc: linux-bluetooth, linux-kernel, linux-arm-kernel, linux-mediatek,
	Jiajia Liu

The setup_packet of control urb is not freed if usb_submit_urb fails or
the submitted urb is killed. Add free in these two paths.

Fixes: a1c49c434e150 ("Bluetooth: btusb: Add protocol support for MediaTek MT7668U USB devices")
Signed-off-by: Jiajia Liu <liujiajia@kylinos.cn>
---
 drivers/bluetooth/btmtk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index f70c1b0f8990..5330f06f000a 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -537,6 +537,7 @@ static void btmtk_usb_wmt_recv(struct urb *urb)
 		return;
 	} else if (urb->status == -ENOENT) {
 		/* Avoid suspend failed when usb_kill_urb */
+		kfree(urb->setup_packet);
 		return;
 	}
 
@@ -610,6 +611,7 @@ static int btmtk_usb_submit_wmt_recv_urb(struct hci_dev *hdev)
 		if (err != -EPERM && err != -ENODEV)
 			bt_dev_err(hdev, "urb %p submission failed (%d)",
 				   urb, -err);
+		kfree(dr);
 		usb_unanchor_urb(urb);
 	}
 
-- 
2.53.0



^ permalink raw reply related

* RE: [PATCH v4 3/7] PCI: cadence: HPA: Add post-link delay
From: Manikandan Karunakaran Pillai @ 2026-05-18  2:16 UTC (permalink / raw)
  To: Hans Zhang, bhelgaas@google.com, lpieralisi@kernel.org,
	kwilczynski@kernel.org, mani@kernel.org, vigneshr@ti.com,
	jingoohan1@gmail.com, thomas.petazzoni@bootlin.com,
	ryder.lee@mediatek.com, claudiu.beznea.uj@bp.renesas.com
  Cc: robh@kernel.org, s-vadapalli@ti.com, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, claudiu.beznea@tuxon.dev,
	linux-mediatek@lists.infradead.org,
	linux-renesas-soc@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20260518004246.1384532-4-18255117159@163.com>



>EXTERNAL MAIL
>
>
>The Cadence HPA (High Performance Architecture IP) specific link setup
>function cdns_pcie_hpa_host_link_setup() waits for the link to come up
>but does not implement the required 100 ms delay after link training
>completes for speeds > 5.0 GT/s (PCIe r6.0 sec 6.6.1).
>
>Add a call to pci_host_common_link_train_delay() immediately after the
>link is confirmed to be up, using the max_link_speed field. Also, in the
>HPA host setup function, read the device tree property "max-link-speed"
>to initialize max_link_speed if not already set by a glue driver.
>
>This ensures compliance for HPA-based platforms.
>
>Signed-off-by: Hans Zhang <18255117159@163.com>
>---
> drivers/pci/controller/cadence/pcie-cadence-host-hpa.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
>diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
>b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
>index 0f540bed58e8..8ef58ed01daa 100644
>--- a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
>+++ b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
>@@ -15,6 +15,8 @@
>
> #include "pcie-cadence.h"
> #include "pcie-cadence-host-common.h"
>+#include "../pci-host-common.h"
>+#include "../../pci.h"
>
> static u8 bar_aperture_mask[] = {
> 	[RP_BAR0] = 0x3F,
>@@ -304,6 +306,8 @@ int cdns_pcie_hpa_host_link_setup(struct cdns_pcie_rc
>*rc)
> 	ret = cdns_pcie_host_wait_for_link(pcie, cdns_pcie_hpa_link_up);
> 	if (ret)
> 		dev_dbg(dev, "PCIe link never came up\n");
>+	else
>+		pci_host_common_link_train_delay(pcie->max_link_speed);
>
> 	return ret;
> }
>@@ -313,6 +317,7 @@ int cdns_pcie_hpa_host_setup(struct cdns_pcie_rc *rc)
> {
> 	struct device *dev = rc->pcie.dev;
> 	struct platform_device *pdev = to_platform_device(dev);
>+	struct device_node *np = dev->of_node;
> 	struct pci_host_bridge *bridge;
> 	enum   cdns_pcie_rp_bar bar;
> 	struct cdns_pcie *pcie;
>@@ -343,6 +348,9 @@ int cdns_pcie_hpa_host_setup(struct cdns_pcie_rc *rc)
> 		rc->cfg_res = res;
> 	}
>
>+	if (pcie->max_link_speed < 1)
>+		pcie->max_link_speed = of_pci_get_max_link_speed(np);
>+

Similar queries as for Cadence LGA controllers. Why do you need the max_link_speed check for "<1" and
What would be the consequences of not defining the max-link-speed in dts ?

> 	/* Put EROM Bar aperture to 0 */
> 	cdns_pcie_hpa_writel(pcie, REG_BANK_IP_CFG_CTRL_REG,
>CDNS_PCIE_EROM, 0x0);
>
>--
>2.43.0



^ permalink raw reply

* RE: [PATCH v4 2/7] PCI: cadence: Add post-link delay for LGA and j721e glue driver
From: Manikandan Karunakaran Pillai @ 2026-05-18  2:12 UTC (permalink / raw)
  To: Hans Zhang, bhelgaas@google.com, lpieralisi@kernel.org,
	kwilczynski@kernel.org, mani@kernel.org, vigneshr@ti.com,
	jingoohan1@gmail.com, thomas.petazzoni@bootlin.com,
	ryder.lee@mediatek.com, claudiu.beznea.uj@bp.renesas.com
  Cc: robh@kernel.org, s-vadapalli@ti.com, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, claudiu.beznea@tuxon.dev,
	linux-mediatek@lists.infradead.org,
	linux-renesas-soc@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20260518004246.1384532-3-18255117159@163.com>



>EXTERNAL MAIL
>
>
>The Cadence LGA (Legacy Architecture IP) PCIe host controller currently
>lacks the mandatory 100 ms delay after link training completes for speeds
>> 5.0 GT/s, as required by PCIe r6.0 sec 6.6.1.
>
>Add a 'max_link_speed' field to struct cdns_pcie. In the common host
>layer function cdns_pcie_host_start_link(), after the link has been
>successfully established, call pci_host_common_link_train_delay() to
>insert the required delay.
>
>For the j721e glue driver, set cdns_pcie.max_link_speed from the existing
>link speed logic. For other LGA-based glue drivers (sky1, sg2042), the
>common LGA host setup (pcie-cadence-host.c) provides a fallback reading
>of the device tree property "max-link-speed" when available. This ensures
>that the delay is not missed on those platforms once they enable the
>property.
>
>Signed-off-by: Hans Zhang <18255117159@163.com>
>---
> drivers/pci/controller/cadence/pci-j721e.c                | 1 +
> drivers/pci/controller/cadence/pcie-cadence-host-common.c | 4 ++++
> drivers/pci/controller/cadence/pcie-cadence-host.c        | 4 ++++
> drivers/pci/controller/cadence/pcie-cadence.h             | 2 ++
> 4 files changed, 11 insertions(+)
>
>diff --git a/drivers/pci/controller/cadence/pci-j721e.c
>b/drivers/pci/controller/cadence/pci-j721e.c
>index bfdfe98d5aba..ae916e7b1927 100644
>--- a/drivers/pci/controller/cadence/pci-j721e.c
>+++ b/drivers/pci/controller/cadence/pci-j721e.c
>@@ -206,6 +206,7 @@ static int j721e_pcie_set_link_speed(struct j721e_pcie
>*pcie,
> 	    (pcie_get_link_speed(link_speed) == PCI_SPEED_UNKNOWN))
> 		link_speed = 2;
>
>+	pcie->cdns_pcie->max_link_speed = link_speed;
> 	val = link_speed - 1;
> 	ret = regmap_update_bits(syscon, offset, GENERATION_SEL_MASK,
>val);
> 	if (ret)
>diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>index 2b0211870f02..18e4b6c760b5 100644
>--- a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>+++ b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>@@ -14,6 +14,7 @@
>
> #include "pcie-cadence.h"
> #include "pcie-cadence-host-common.h"
>+#include "../pci-host-common.h"
>
> #define LINK_RETRAIN_TIMEOUT HZ
>
>@@ -115,6 +116,9 @@ int cdns_pcie_host_start_link(struct cdns_pcie_rc *rc,
> 	if (!ret && rc->quirk_retrain_flag)
> 		ret = cdns_pcie_retrain(pcie, pcie_link_up);
>
>+	if (!ret)
>+		pci_host_common_link_train_delay(pcie->max_link_speed);
>+
> 	return ret;
> }
> EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
>diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c
>b/drivers/pci/controller/cadence/pcie-cadence-host.c
>index 0bc9e6e90e0e..058e4e619654 100644
>--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
>+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
>@@ -13,6 +13,7 @@
>
> #include "pcie-cadence.h"
> #include "pcie-cadence-host-common.h"
>+#include "../../pci.h"
>
> static u8 bar_aperture_mask[] = {
> 	[RP_BAR0] = 0x1F,
>@@ -397,6 +398,9 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
> 	rc->device_id = 0xffff;
> 	of_property_read_u32(np, "device-id", &rc->device_id);
>
>+	if (pcie->max_link_speed < 1)
>+		pcie->max_link_speed = of_pci_get_max_link_speed(np);
>+
Why is the conditional if required here as during cdns_pcie_host_setup(), the value of
max_link_speed is expected to be '0', unless specifically initialized by the platform code separately.

What happens if the max_link_speed is not defined in the corresponding dts ? Would not the -EINVAL returned from the function create issues ?

> 	pcie->reg_base = devm_platform_ioremap_resource_byname(pdev,
>"reg");
> 	if (IS_ERR(pcie->reg_base)) {
> 		dev_err(dev, "missing \"reg\"\n");
>diff --git a/drivers/pci/controller/cadence/pcie-cadence.h
>b/drivers/pci/controller/cadence/pcie-cadence.h
>index 574e9cf4d003..042a4c49bb9a 100644
>--- a/drivers/pci/controller/cadence/pcie-cadence.h
>+++ b/drivers/pci/controller/cadence/pcie-cadence.h
>@@ -86,6 +86,7 @@ struct cdns_plat_pcie_of_data {
>  * @ops: Platform-specific ops to control various inputs from Cadence PCIe
>  *       wrapper
>  * @cdns_pcie_reg_offsets: Register bank offsets for different SoC
>+ * @max_link_speed: Maximum supported link speed
>  */
> struct cdns_pcie {
> 	void __iomem		             *reg_base;
>@@ -98,6 +99,7 @@ struct cdns_pcie {
> 	struct device_link	             **link;
> 	const  struct cdns_pcie_ops          *ops;
> 	const  struct cdns_plat_pcie_of_data *cdns_pcie_reg_offsets;
>+	int				     max_link_speed;
> };
>
> /**
>--
>2.43.0



^ permalink raw reply

* [PATCH v4 4/7] PCI: dwc: Use common pci_host_common_link_train_delay() helper
From: Hans Zhang @ 2026-05-18  0:42 UTC (permalink / raw)
  To: bhelgaas, lpieralisi, kwilczynski, mani, vigneshr, jingoohan1,
	thomas.petazzoni, ryder.lee, claudiu.beznea.uj, mpillai
  Cc: robh, s-vadapalli, linux-omap, linux-arm-kernel, claudiu.beznea,
	linux-mediatek, linux-renesas-soc, linux-pci, linux-kernel,
	Hans Zhang
In-Reply-To: <20260518004246.1384532-1-18255117159@163.com>

The DWC driver already implements the 100 ms delay required by PCIe
r6.0 sec 6.6.1 by checking pci->max_link_speed and calling msleep(100).

Replace the open-coded msleep() with the new common helper
pci_host_common_link_train_delay() to reduce code duplication and
improve maintainability. No functional change intended.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/dwc/pcie-designware.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index c11cf61b8319..7021d21bb601 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -22,6 +22,7 @@
 #include <linux/sizes.h>
 #include <linux/types.h>
 
+#include "../pci-host-common.h"
 #include "../../pci.h"
 #include "pcie-designware.h"
 
@@ -799,13 +800,7 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
 		return -ETIMEDOUT;
 	}
 
-	/*
-	 * As per PCIe r6.0, sec 6.6.1, a Downstream Port that supports Link
-	 * speeds greater than 5.0 GT/s, software must wait a minimum of 100 ms
-	 * after Link training completes before sending a Configuration Request.
-	 */
-	if (pci->max_link_speed > 2)
-		msleep(PCIE_RESET_CONFIG_WAIT_MS);
+	pci_host_common_link_train_delay(pci->max_link_speed);
 
 	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
 	val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
-- 
2.43.0



^ permalink raw reply related

* [PATCH v4 3/7] PCI: cadence: HPA: Add post-link delay
From: Hans Zhang @ 2026-05-18  0:42 UTC (permalink / raw)
  To: bhelgaas, lpieralisi, kwilczynski, mani, vigneshr, jingoohan1,
	thomas.petazzoni, ryder.lee, claudiu.beznea.uj, mpillai
  Cc: robh, s-vadapalli, linux-omap, linux-arm-kernel, claudiu.beznea,
	linux-mediatek, linux-renesas-soc, linux-pci, linux-kernel,
	Hans Zhang
In-Reply-To: <20260518004246.1384532-1-18255117159@163.com>

The Cadence HPA (High Performance Architecture IP) specific link setup
function cdns_pcie_hpa_host_link_setup() waits for the link to come up
but does not implement the required 100 ms delay after link training
completes for speeds > 5.0 GT/s (PCIe r6.0 sec 6.6.1).

Add a call to pci_host_common_link_train_delay() immediately after the
link is confirmed to be up, using the max_link_speed field. Also, in the
HPA host setup function, read the device tree property "max-link-speed"
to initialize max_link_speed if not already set by a glue driver.

This ensures compliance for HPA-based platforms.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/cadence/pcie-cadence-host-hpa.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
index 0f540bed58e8..8ef58ed01daa 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
@@ -15,6 +15,8 @@
 
 #include "pcie-cadence.h"
 #include "pcie-cadence-host-common.h"
+#include "../pci-host-common.h"
+#include "../../pci.h"
 
 static u8 bar_aperture_mask[] = {
 	[RP_BAR0] = 0x3F,
@@ -304,6 +306,8 @@ int cdns_pcie_hpa_host_link_setup(struct cdns_pcie_rc *rc)
 	ret = cdns_pcie_host_wait_for_link(pcie, cdns_pcie_hpa_link_up);
 	if (ret)
 		dev_dbg(dev, "PCIe link never came up\n");
+	else
+		pci_host_common_link_train_delay(pcie->max_link_speed);
 
 	return ret;
 }
@@ -313,6 +317,7 @@ int cdns_pcie_hpa_host_setup(struct cdns_pcie_rc *rc)
 {
 	struct device *dev = rc->pcie.dev;
 	struct platform_device *pdev = to_platform_device(dev);
+	struct device_node *np = dev->of_node;
 	struct pci_host_bridge *bridge;
 	enum   cdns_pcie_rp_bar bar;
 	struct cdns_pcie *pcie;
@@ -343,6 +348,9 @@ int cdns_pcie_hpa_host_setup(struct cdns_pcie_rc *rc)
 		rc->cfg_res = res;
 	}
 
+	if (pcie->max_link_speed < 1)
+		pcie->max_link_speed = of_pci_get_max_link_speed(np);
+
 	/* Put EROM Bar aperture to 0 */
 	cdns_pcie_hpa_writel(pcie, REG_BANK_IP_CFG_CTRL_REG, CDNS_PCIE_EROM, 0x0);
 
-- 
2.43.0



^ permalink raw reply related

* [PATCH v4 5/7] PCI: aardvark: Add 100 ms delay after link training
From: Hans Zhang @ 2026-05-18  0:42 UTC (permalink / raw)
  To: bhelgaas, lpieralisi, kwilczynski, mani, vigneshr, jingoohan1,
	thomas.petazzoni, ryder.lee, claudiu.beznea.uj, mpillai
  Cc: robh, s-vadapalli, linux-omap, linux-arm-kernel, claudiu.beznea,
	linux-mediatek, linux-renesas-soc, linux-pci, linux-kernel,
	Hans Zhang
In-Reply-To: <20260518004246.1384532-1-18255117159@163.com>

The Aardvark PCIe controller driver waits for the link to come up but
does not implement the mandatory 100 ms delay after link training
completes for speeds greater than 5.0 GT/s (PCIe r6.0 sec 6.6.1).

The driver already maintains a 'link_gen' field that holds the negotiated
link speed. Use it together with pci_host_common_link_train_delay() to
insert the required delay immediately after confirming that the link
is up.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/pci-aardvark.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index e34bea1ff0ac..fd9c7d53e8a7 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -26,6 +26,7 @@
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
 
+#include "pci-host-common.h"
 #include "../pci.h"
 #include "../pci-bridge-emul.h"
 
@@ -350,8 +351,10 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
 
 	/* check if the link is up or not */
 	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
-		if (advk_pcie_link_up(pcie))
+		if (advk_pcie_link_up(pcie)) {
+			pci_host_common_link_train_delay(pcie->link_gen);
 			return 0;
+		}
 
 		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
 	}
-- 
2.43.0



^ permalink raw reply related

* [PATCH v4 1/7] PCI: Add pci_host_common_link_train_delay() helper
From: Hans Zhang @ 2026-05-18  0:42 UTC (permalink / raw)
  To: bhelgaas, lpieralisi, kwilczynski, mani, vigneshr, jingoohan1,
	thomas.petazzoni, ryder.lee, claudiu.beznea.uj, mpillai
  Cc: robh, s-vadapalli, linux-omap, linux-arm-kernel, claudiu.beznea,
	linux-mediatek, linux-renesas-soc, linux-pci, linux-kernel,
	Hans Zhang
In-Reply-To: <20260518004246.1384532-1-18255117159@163.com>

PCIe r6.0, sec 6.6.1 (Conventional Reset) requires that for a Downstream
Port supporting Link speeds greater than 5.0 GT/s, software must wait a
minimum of 100 ms after Link training completes before sending any
Configuration Request.

Introduce a static inline helper pci_host_common_link_train_delay() that
checks the given max_link_speed (2 = 5.0 GT/s, 3 = 8.0 GT/s, etc.) and
calls msleep(100) only when the speed is greater than 5.0 GT/s.

This allows multiple host controller drivers to share the same mandatory
delay without duplicating the logic.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/pci-host-common.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
index b5075d4bd7eb..d709f7e3e11a 100644
--- a/drivers/pci/controller/pci-host-common.h
+++ b/drivers/pci/controller/pci-host-common.h
@@ -10,6 +10,9 @@
 #ifndef _PCI_HOST_COMMON_H
 #define _PCI_HOST_COMMON_H
 
+#include <linux/delay.h>
+#include "../pci.h"
+
 struct pci_ecam_ops;
 
 int pci_host_common_probe(struct platform_device *pdev);
@@ -20,4 +23,18 @@ void pci_host_common_remove(struct platform_device *pdev);
 
 struct pci_config_window *pci_host_common_ecam_create(struct device *dev,
 	struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
+
+/**
+ * pci_host_common_link_train_delay - Wait 100 ms if link speed > 5 GT/s
+ * @max_link_speed: the maximum link speed (2 = 5.0 GT/s, 3 = 8.0 GT/s, ...)
+ *
+ * Must be called after Link training completes and before the first
+ * Configuration Request is sent.
+ */
+static inline void pci_host_common_link_train_delay(int max_link_speed)
+{
+	if (max_link_speed > 2)
+		msleep(PCIE_RESET_CONFIG_WAIT_MS);
+}
+
 #endif
-- 
2.43.0



^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox