On Fri, 2025-01-03 at 10:29 +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 03/01/25 07:00, Jianjun Wang ha scritto: > > There are some circumstances where the EP device will not respond > > to > > non-posted access from the root port (e.g., MMIO read). In such > > cases, > > the root port will reply with an AXI slave error, which will be > > treated > > as a System Error (SError), causing a kernel panic and preventing > > us > > from obtaining any useful information for further debugging. > > > > We have added a new bit in the PCIE_AXI_IF_CTRL_REG register to > > prevent > > PCIe AXI0 from replying with a slave error. Setting this bit on an > > older > > platform that does not support this feature will have no effect. > > > > By preventing AXI0 from replying with a slave error, we can keep > > the > > kernel alive and debug using the information from AER. > > > > Signed-off-by: Jianjun Wang <jianjun.wang@xxxxxxxxxxxx> > > --- > > drivers/pci/controller/pcie-mediatek-gen3.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c > > b/drivers/pci/controller/pcie-mediatek-gen3.c > > index 4bd3b39eebe2..48f83c2d91f7 100644 > > --- a/drivers/pci/controller/pcie-mediatek-gen3.c > > +++ b/drivers/pci/controller/pcie-mediatek-gen3.c > > @@ -87,6 +87,9 @@ > > #define PCIE_LOW_POWER_CTRL_REG 0x194 > > #define PCIE_FORCE_DIS_L0S BIT(8) > > > > +#define PCIE_AXI_IF_CTRL_REG 0x1a8 > > +#define PCIE_AXI0_SLV_RESP_MASK BIT(12) > > + > > #define PCIE_PIPE4_PIE8_REG 0x338 > > #define PCIE_K_FINETUNE_MAX GENMASK(5, 0) > > #define PCIE_K_FINETUNE_ERR GENMASK(7, 6) > > @@ -469,6 +472,15 @@ static int mtk_pcie_startup_port(struct > > mtk_gen3_pcie *pcie) > > val |= PCIE_FORCE_DIS_L0S; > > writel_relaxed(val, pcie->base + PCIE_LOW_POWER_CTRL_REG); > > > > + /* > > + * Prevent PCIe AXI0 from replying a slave error, as it will > > cause kernel panic > > + * and prevent us from getting useful information. > > + * Keep the kernel alive and debug using the information from > > AER. > > + */ > > Isn't it safer if we set this bit at the beginning of > mtk_pcie_startup_port() > instead? Agree, I'll move it to the beginning of mtk_pcie_startup_port(). Thanks. > > Cheers, > Angelo > > > + val = readl_relaxed(pcie->base + PCIE_AXI_IF_CTRL_REG); > > + val |= PCIE_AXI0_SLV_RESP_MASK; > > + writel_relaxed(val, pcie->base + PCIE_AXI_IF_CTRL_REG); > > + > > /* Disable DVFSRC voltage request */ > > val = readl_relaxed(pcie->base + PCIE_MISC_CTRL_REG); > > val |= PCIE_DISABLE_DVFSRC_VLT_REQ; > > >
