mbox series

[00/16] Qcom: Fix PCI I/O range defined in devicetree

Message ID 20230228164752.55682-1-manivannan.sadhasivam@linaro.org
Headers
Series Qcom: Fix PCI I/O range defined in devicetree |

Message

Manivannan Sadhasivam Feb. 28, 2023, 4:47 p.m. UTC 
  Hi,

This series fixes the issue with PCI I/O ranges defined in devicetree of
Qualcomm SoCs as reported by Arnd [1]. Most of the Qualcomm SoCs define
identical mapping for the PCI I/O range. But the PCI device I/O ports
are usually located between 0x0 to 64KiB/1MiB. So the defined PCI addresses are
mostly bogus. The lack of bug report on this issue indicates that no one really
tested legacy PCI devices with these SoCs.

This series also contains a couple of cleanup patches that aligns the entries of
ranges property.

Thanks,
Mani

[1] https://lore.kernel.org/linux-arm-msm/7c5dfa87-41df-4ba7-b0e4-72c8386402a8@app.fastmail.com/

Manivannan Sadhasivam (16):
  arm64: dts: qcom: sdm845: Fix the PCI I/O port range
  arm64: dts: qcom: msm8998: Fix the PCI I/O port range
  arm64: dts: qcom: sc7280: Fix the PCI I/O port range
  arm64: dts: qcom: sm8550: Fix the PCI I/O port range
  arm64: dts: qcom: ipq8074: Fix the PCI I/O port range
  arm64: dts: qcom: ipq6018: Fix the PCI I/O port range
  arm64: dts: qcom: msm8996: Fix the PCI I/O port range
  arm64: dts: qcom: sm8250: Fix the PCI I/O port range
  arm64: dts: qcom: qcs404: Use 0x prefix for the PCI I/O and MEM ranges
  arm64: dts: qcom: sc8280xp: Fix the PCI I/O port range
  arm64: dts: qcom: sm8150: Fix the PCI I/O port range
  arm64: dts: qcom: sm8450: Fix the PCI I/O port range
  arm64: dts: qcom: sm8350: Fix the PCI I/O port range
  ARM: dts: qcom: apq8064: Use 0x prefix for the PCI I/O and MEM ranges
  ARM: dts: qcom: ipq4019: Fix the PCI I/O port range
  ARM: dts: qcom: ipq8064: Fix the PCI I/O port range

 arch/arm/boot/dts/qcom-apq8064.dtsi    |  4 ++--
 arch/arm/boot/dts/qcom-ipq4019.dtsi    |  4 ++--
 arch/arm/boot/dts/qcom-ipq8064.dtsi    | 12 ++++++------
 arch/arm64/boot/dts/qcom/ipq6018.dtsi  |  4 ++--
 arch/arm64/boot/dts/qcom/ipq8074.dtsi  | 12 ++++--------
 arch/arm64/boot/dts/qcom/msm8996.dtsi  | 12 ++++++------
 arch/arm64/boot/dts/qcom/msm8998.dtsi  |  2 +-
 arch/arm64/boot/dts/qcom/qcs404.dtsi   |  4 ++--
 arch/arm64/boot/dts/qcom/sc7280.dtsi   |  2 +-
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 10 +++++-----
 arch/arm64/boot/dts/qcom/sdm845.dtsi   |  6 +++---
 arch/arm64/boot/dts/qcom/sm8150.dtsi   |  6 +++---
 arch/arm64/boot/dts/qcom/sm8250.dtsi   |  8 ++++----
 arch/arm64/boot/dts/qcom/sm8350.dtsi   |  8 ++++----
 arch/arm64/boot/dts/qcom/sm8450.dtsi   |  8 ++++----
 arch/arm64/boot/dts/qcom/sm8550.dtsi   |  8 ++++----
 16 files changed, 53 insertions(+), 57 deletions(-)

Comments

Arnd Bergmann Feb. 28, 2023, 4:58 p.m. UTC | #1 
On Tue, Feb 28, 2023, at 17:47, Manivannan Sadhasivam wrote:
> Hi,
>
> This series fixes the issue with PCI I/O ranges defined in devicetree of
> Qualcomm SoCs as reported by Arnd [1]. Most of the Qualcomm SoCs define
> identical mapping for the PCI I/O range. But the PCI device I/O ports
> are usually located between 0x0 to 64KiB/1MiB. So the defined PCI addresses are
> mostly bogus. The lack of bug report on this issue indicates that no one really
> tested legacy PCI devices with these SoCs.
>
> This series also contains a couple of cleanup patches that aligns the entries of
> ranges property.

Looks good to me. I already commented that we may also want to use
64KB everywhere instead of 1MB for the per-host window size. Regardless
of that, please add

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

I would also prefer to do this in fewer patches, maybe one to
change all the prefixes, and another one to change the location,
or whichever way Bjorn prefers.

     Arnd
Andrew Halaney Feb. 28, 2023, 5:29 p.m. UTC | #2 
On Tue, Feb 28, 2023 at 10:17:36PM +0530, Manivannan Sadhasivam wrote:
> Hi,
> 
> This series fixes the issue with PCI I/O ranges defined in devicetree of
> Qualcomm SoCs as reported by Arnd [1]. Most of the Qualcomm SoCs define
> identical mapping for the PCI I/O range. But the PCI device I/O ports
> are usually located between 0x0 to 64KiB/1MiB. So the defined PCI addresses are
> mostly bogus. The lack of bug report on this issue indicates that no one really
> tested legacy PCI devices with these SoCs.
> 
> This series also contains a couple of cleanup patches that aligns the entries of
> ranges property.
> 
> Thanks,
> Mani
> 
> [1] https://lore.kernel.org/linux-arm-msm/7c5dfa87-41df-4ba7-b0e4-72c8386402a8@app.fastmail.com/
> 
> Manivannan Sadhasivam (16):
>   arm64: dts: qcom: sdm845: Fix the PCI I/O port range
>   arm64: dts: qcom: msm8998: Fix the PCI I/O port range
>   arm64: dts: qcom: sc7280: Fix the PCI I/O port range
>   arm64: dts: qcom: sm8550: Fix the PCI I/O port range
>   arm64: dts: qcom: ipq8074: Fix the PCI I/O port range
>   arm64: dts: qcom: ipq6018: Fix the PCI I/O port range
>   arm64: dts: qcom: msm8996: Fix the PCI I/O port range
>   arm64: dts: qcom: sm8250: Fix the PCI I/O port range
>   arm64: dts: qcom: qcs404: Use 0x prefix for the PCI I/O and MEM ranges
>   arm64: dts: qcom: sc8280xp: Fix the PCI I/O port range
>   arm64: dts: qcom: sm8150: Fix the PCI I/O port range
>   arm64: dts: qcom: sm8450: Fix the PCI I/O port range
>   arm64: dts: qcom: sm8350: Fix the PCI I/O port range
>   ARM: dts: qcom: apq8064: Use 0x prefix for the PCI I/O and MEM ranges
>   ARM: dts: qcom: ipq4019: Fix the PCI I/O port range
>   ARM: dts: qcom: ipq8064: Fix the PCI I/O port range
> 
>  arch/arm/boot/dts/qcom-apq8064.dtsi    |  4 ++--
>  arch/arm/boot/dts/qcom-ipq4019.dtsi    |  4 ++--
>  arch/arm/boot/dts/qcom-ipq8064.dtsi    | 12 ++++++------
>  arch/arm64/boot/dts/qcom/ipq6018.dtsi  |  4 ++--
>  arch/arm64/boot/dts/qcom/ipq8074.dtsi  | 12 ++++--------
>  arch/arm64/boot/dts/qcom/msm8996.dtsi  | 12 ++++++------
>  arch/arm64/boot/dts/qcom/msm8998.dtsi  |  2 +-
>  arch/arm64/boot/dts/qcom/qcs404.dtsi   |  4 ++--

Would you mind giving sa8540p.dtsi to similiar treatment? I will admit
I know next to nothing about PCI techically, so I can't even comment
with confidence that this is needed there, but it looks similar to other
descriptions modified in this patch series.

    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sa8540p.dtsi#n197

Thanks,
Andrew
Manivannan Sadhasivam March 1, 2023, 11:29 a.m. UTC | #3 
On Tue, Feb 28, 2023 at 05:58:37PM +0100, Arnd Bergmann wrote:
> On Tue, Feb 28, 2023, at 17:47, Manivannan Sadhasivam wrote:
> > Hi,
> >
> > This series fixes the issue with PCI I/O ranges defined in devicetree of
> > Qualcomm SoCs as reported by Arnd [1]. Most of the Qualcomm SoCs define
> > identical mapping for the PCI I/O range. But the PCI device I/O ports
> > are usually located between 0x0 to 64KiB/1MiB. So the defined PCI addresses are
> > mostly bogus. The lack of bug report on this issue indicates that no one really
> > tested legacy PCI devices with these SoCs.
> >
> > This series also contains a couple of cleanup patches that aligns the entries of
> > ranges property.
> 
> Looks good to me. I already commented that we may also want to use
> 64KB everywhere instead of 1MB for the per-host window size.

I also spotted this discrepancy while working on this series, but the size
seems to be not universal across SoCs from many vendors. So I settled with
whatever range that was used before.

>  Regardless of that, please add
> 
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> 
> I would also prefer to do this in fewer patches, maybe one to
> change all the prefixes, and another one to change the location,
> or whichever way Bjorn prefers.
> 

Well, the only intention of doing a per-patch change is to backport them if
needed. But I'll defer it to Bjorn.

Thanks,
Mani

>      Arnd
Manivannan Sadhasivam March 1, 2023, 11:32 a.m. UTC | #4 
On Tue, Feb 28, 2023 at 11:29:52AM -0600, Andrew Halaney wrote:
> On Tue, Feb 28, 2023 at 10:17:36PM +0530, Manivannan Sadhasivam wrote:
> > Hi,
> > 
> > This series fixes the issue with PCI I/O ranges defined in devicetree of
> > Qualcomm SoCs as reported by Arnd [1]. Most of the Qualcomm SoCs define
> > identical mapping for the PCI I/O range. But the PCI device I/O ports
> > are usually located between 0x0 to 64KiB/1MiB. So the defined PCI addresses are
> > mostly bogus. The lack of bug report on this issue indicates that no one really
> > tested legacy PCI devices with these SoCs.
> > 
> > This series also contains a couple of cleanup patches that aligns the entries of
> > ranges property.
> > 
> > Thanks,
> > Mani
> > 
> > [1] https://lore.kernel.org/linux-arm-msm/7c5dfa87-41df-4ba7-b0e4-72c8386402a8@app.fastmail.com/
> > 
> > Manivannan Sadhasivam (16):
> >   arm64: dts: qcom: sdm845: Fix the PCI I/O port range
> >   arm64: dts: qcom: msm8998: Fix the PCI I/O port range
> >   arm64: dts: qcom: sc7280: Fix the PCI I/O port range
> >   arm64: dts: qcom: sm8550: Fix the PCI I/O port range
> >   arm64: dts: qcom: ipq8074: Fix the PCI I/O port range
> >   arm64: dts: qcom: ipq6018: Fix the PCI I/O port range
> >   arm64: dts: qcom: msm8996: Fix the PCI I/O port range
> >   arm64: dts: qcom: sm8250: Fix the PCI I/O port range
> >   arm64: dts: qcom: qcs404: Use 0x prefix for the PCI I/O and MEM ranges
> >   arm64: dts: qcom: sc8280xp: Fix the PCI I/O port range
> >   arm64: dts: qcom: sm8150: Fix the PCI I/O port range
> >   arm64: dts: qcom: sm8450: Fix the PCI I/O port range
> >   arm64: dts: qcom: sm8350: Fix the PCI I/O port range
> >   ARM: dts: qcom: apq8064: Use 0x prefix for the PCI I/O and MEM ranges
> >   ARM: dts: qcom: ipq4019: Fix the PCI I/O port range
> >   ARM: dts: qcom: ipq8064: Fix the PCI I/O port range
> > 
> >  arch/arm/boot/dts/qcom-apq8064.dtsi    |  4 ++--
> >  arch/arm/boot/dts/qcom-ipq4019.dtsi    |  4 ++--
> >  arch/arm/boot/dts/qcom-ipq8064.dtsi    | 12 ++++++------
> >  arch/arm64/boot/dts/qcom/ipq6018.dtsi  |  4 ++--
> >  arch/arm64/boot/dts/qcom/ipq8074.dtsi  | 12 ++++--------
> >  arch/arm64/boot/dts/qcom/msm8996.dtsi  | 12 ++++++------
> >  arch/arm64/boot/dts/qcom/msm8998.dtsi  |  2 +-
> >  arch/arm64/boot/dts/qcom/qcs404.dtsi   |  4 ++--
> 
> Would you mind giving sa8540p.dtsi to similiar treatment? I will admit
> I know next to nothing about PCI techically, so I can't even comment
> with confidence that this is needed there, but it looks similar to other
> descriptions modified in this patch series.
> 
>     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sa8540p.dtsi#n197

Ah, I didn't spot the separate ranges used for this SoC. Will fix it in next
revision.

Thanks,
Mani

> 
> Thanks,
> Andrew
>
Arnd Bergmann March 1, 2023, 11:57 a.m. UTC | #5 
On Wed, Mar 1, 2023, at 12:29, Manivannan Sadhasivam wrote:
> On Tue, Feb 28, 2023 at 05:58:37PM +0100, Arnd Bergmann wrote:
>> On Tue, Feb 28, 2023, at 17:47, Manivannan Sadhasivam wrote:
>> > Hi,
>> >
>> > This series fixes the issue with PCI I/O ranges defined in devicetree of
>> > Qualcomm SoCs as reported by Arnd [1]. Most of the Qualcomm SoCs define
>> > identical mapping for the PCI I/O range. But the PCI device I/O ports
>> > are usually located between 0x0 to 64KiB/1MiB. So the defined PCI addresses are
>> > mostly bogus. The lack of bug report on this issue indicates that no one really
>> > tested legacy PCI devices with these SoCs.
>> >
>> > This series also contains a couple of cleanup patches that aligns the entries of
>> > ranges property.
>> 
>> Looks good to me. I already commented that we may also want to use
>> 64KB everywhere instead of 1MB for the per-host window size.
>
> I also spotted this discrepancy while working on this series, but the size
> seems to be not universal across SoCs from many vendors. So I settled with
> whatever range that was used before.

Makes sense. We could add another patch if necessary of course, and
it probably doesn't matter much. OTOH I don't think there is anything
SoC specific in this and we used to just truncate this to 64KB per
domain. It's only really a problem if the total size of the I/O ports
for all domains in a system exceeds the 16MB of virtual memory area.

     Arnd
Bjorn Andersson March 16, 2023, 3:20 a.m. UTC | #6 
On Tue, 28 Feb 2023 22:17:36 +0530, Manivannan Sadhasivam wrote:
> This series fixes the issue with PCI I/O ranges defined in devicetree of
> Qualcomm SoCs as reported by Arnd [1]. Most of the Qualcomm SoCs define
> identical mapping for the PCI I/O range. But the PCI device I/O ports
> are usually located between 0x0 to 64KiB/1MiB. So the defined PCI addresses are
> mostly bogus. The lack of bug report on this issue indicates that no one really
> tested legacy PCI devices with these SoCs.
> 
> [...]

Applied, thanks!

[14/16] ARM: dts: qcom: apq8064: Use 0x prefix for the PCI I/O and MEM ranges
        commit: 84160da56dd0ce48dd8eed56237cc8be45bd55dc
[15/16] ARM: dts: qcom: ipq4019: Fix the PCI I/O port range
        commit: 2540279e9a9e74fc880d1e4c83754ecfcbe290a0
[16/16] ARM: dts: qcom: ipq8064: Fix the PCI I/O port range
        commit: 0b16b34e491629016109e56747ad64588074194b

Best regards,