Message ID | 20230306153222.157667-1-manivannan.sadhasivam@linaro.org |
---|---|
Headers | show |
Series | Qcom PCIe cleanups and improvements | expand |
On 06/03/2023 16:32, Manivannan Sadhasivam wrote: > As per Qualcomm's internal documentation, the name of the region is "mhi" > and not "mmio". So let's rename it to follow the convention. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > > @@ -477,16 +477,16 @@ static int qcom_pcie_ep_get_io_resources(struct platform_device *pdev, > if (IS_ERR(pcie_ep->elbi)) > return PTR_ERR(pcie_ep->elbi); > > - pcie_ep->mmio_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, > - "mmio"); > - if (!pcie_ep->mmio_res) { > - dev_err(dev, "Failed to get mmio resource\n"); > + pcie_ep->mhi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, > + "mhi"); That's an ABI break. Patchset is also non-bisectable. Best regards, Krzysztof
On 06/03/2023 16:32, Manivannan Sadhasivam wrote: > The "mhi" region contains the debug registers that could be used to monitor > the PCIe link transitions. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 479859bd8ab3..0104e77dd8d5 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -2280,10 +2280,11 @@ opp-4 { > pcie0: pci@1c00000 { > compatible = "qcom,pcie-sdm845"; > reg = <0 0x01c00000 0 0x2000>, > + <0 0x01c07000 0 0x1000>, > <0 0x60000000 0 0xf1d>, > <0 0x60000f20 0 0xa8>, > <0 0x60100000 0 0x100000>; > - reg-names = "parf", "dbi", "elbi", "config"; > + reg-names = "parf", "mhi", "dbi", "elbi", "config"; Indexes are fixed, thus this breaks other users of DTS. Best regards, Krzysztof
On Tue, Mar 07, 2023 at 09:19:29AM +0100, Krzysztof Kozlowski wrote: > On 06/03/2023 16:32, Manivannan Sadhasivam wrote: > > As per Qualcomm's internal documentation, the name of the region is "mhi" > > and not "mmio". So let's rename it to follow the convention. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > > > > > > @@ -477,16 +477,16 @@ static int qcom_pcie_ep_get_io_resources(struct platform_device *pdev, > > if (IS_ERR(pcie_ep->elbi)) > > return PTR_ERR(pcie_ep->elbi); > > > > - pcie_ep->mmio_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, > > - "mmio"); > > - if (!pcie_ep->mmio_res) { > > - dev_err(dev, "Failed to get mmio resource\n"); > > + pcie_ep->mhi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, > > + "mhi"); > > That's an ABI break. Patchset is also non-bisectable. > As agreed, I will drop this patch in next revision. Thanks, Mani > Best regards, > Krzysztof >
On Tue, Mar 07, 2023 at 09:20:23AM +0100, Krzysztof Kozlowski wrote: > On 06/03/2023 16:32, Manivannan Sadhasivam wrote: > > The "mhi" region contains the debug registers that could be used to monitor > > the PCIe link transitions. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > index 479859bd8ab3..0104e77dd8d5 100644 > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > @@ -2280,10 +2280,11 @@ opp-4 { > > pcie0: pci@1c00000 { > > compatible = "qcom,pcie-sdm845"; > > reg = <0 0x01c00000 0 0x2000>, > > + <0 0x01c07000 0 0x1000>, > > <0 0x60000000 0 0xf1d>, > > <0 0x60000f20 0 0xa8>, > > <0 0x60100000 0 0x100000>; > > - reg-names = "parf", "dbi", "elbi", "config"; > > + reg-names = "parf", "mhi", "dbi", "elbi", "config"; > > Indexes are fixed, thus this breaks other users of DTS. > Are you suggesting to move the "mhi" to the end and do not care about sorting? Thanks, Mani > Best regards, > Krzysztof >
On 08/03/2023 09:31, Manivannan Sadhasivam wrote: > On Tue, Mar 07, 2023 at 09:20:23AM +0100, Krzysztof Kozlowski wrote: >> On 06/03/2023 16:32, Manivannan Sadhasivam wrote: >>> The "mhi" region contains the debug registers that could be used to monitor >>> the PCIe link transitions. >>> >>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >>> --- >>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi >>> index 479859bd8ab3..0104e77dd8d5 100644 >>> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi >>> @@ -2280,10 +2280,11 @@ opp-4 { >>> pcie0: pci@1c00000 { >>> compatible = "qcom,pcie-sdm845"; >>> reg = <0 0x01c00000 0 0x2000>, >>> + <0 0x01c07000 0 0x1000>, >>> <0 0x60000000 0 0xf1d>, >>> <0 0x60000f20 0 0xa8>, >>> <0 0x60100000 0 0x100000>; >>> - reg-names = "parf", "dbi", "elbi", "config"; >>> + reg-names = "parf", "mhi", "dbi", "elbi", "config"; >> >> Indexes are fixed, thus this breaks other users of DTS. >> > > Are you suggesting to move the "mhi" to the end and do not care about sorting? Yes, any new entry must be added at the end. What sorting do you mean? Entries are not sorted. Best regards, Krzysztof