mbox series

[RFC,00/40] Cleanup pci-keystone.c and Add AM654 PCIe Support

Message ID 20180921102155.22839-1-kishon@ti.com
Headers show
Series Cleanup pci-keystone.c and Add AM654 PCIe Support | expand

Message

Kishon Vijay Abraham I Sept. 21, 2018, 10:21 a.m. UTC
Add PCIe RC support for TI's AM654 SoC. The PCIe controller in AM654
uses Synopsys core revision 4.90a and uses the same TI wrapper as used
in keystone2 with certain modification. Hence AM654 will use the same
pci wrapper driver pci-keystone.c

The initial support for AM654 was merged recently [1]

[1] -> https://lore.kernel.org/lkml/20180626162615.19194-1-nm@ti.com/

Patch series includes the following
 *) Merge pci-keystone-dw.c into pci-keystone.c so that we have single
    file for PCIe keystone driver.
 *) Cleanup the pci-keystone driver. In certain cases the DT binding is
    also modified since PCIe in keystone has never worked in mainline
    due to lack of PHY support.
 *) Included the PHY driver here for completeness (though the driver
    might go via linux-phy tree)
 *) Included the device tree patches here. Once this series is reviewed
    it'll be sent to be merged via Tony's tree.
 *) Patch to fix ATU identification for designware version >= 4.80 in
    designware core is also included here.

TODO:
 *) Add Endpoint Support for AM654
 *) Send a patch to fix the MRRS after the correct value is identified.

Once this series is reviewed I'll split the series and send to
corresponding subsytem Maintainers after removing RFC in subject.

Thanks
Kishon

Benoit Parrot (1):
  arm64: dts: k3-am6: Add Main System Control Module node

Kishon Vijay Abraham I (39):
  PCI: keystone: Use quirk to limit MRRS for K2G
  PCI: keystone: Use quirk to set MRRS for PCI host bridge
  PCI: keystone: Move dw_pcie_setup_rc out of ks_pcie_establish_link()
  PCI: keystone: Do not initiate link training multiple times
  PCI: keystone: Remove unused argument from ks_dw_pcie_host_init()
  PCI: keystone: Add start_link/stop_link dw_pcie_ops
  PCI: keystone: Merge pci-keystone-dw.c and pci-keystone.c
  PCI: keystone: Cleanup MSI/legacy interrupt configuration and handling
  PCI: keystone: Remove redundant platform_set_drvdata
  PCI: keystone: Use uniform function naming convention
  dt-bindings: PCI: keystone: Add bindings to get device control module
  PCI: keystone: Use syscon APIs to get device id from control module
  dt-bindings: PCI: keystone: Add "reg-names" binding information
  PCI: keystone: Use platform_get_resource_byname to get memory
    resources
  PCI: keystone: Cleanup PHY handling
  PCI: keystone: Invoke pm_runtime APIs to enable clock
  PCI: keystone: Cleanup configuration space access
  PCI: keystone: Get number of OB windows from DT and cleanup MEM space
    configuration
  PCI: keystone: Cleanup set_dbi_mode and get_dbi_mode
  PCI: keystone: Cleanup ks_pcie_link_up()
  PCI: keystone: Add debug error message for all errors
  PCI: keystone: Reorder header file in alphabetical order
  PCI: keystone: Cleanup macros defined in pci-keystone.c
  PCI: keystone: Move initializations to appropriate places
  dt-bindings: PCI: Add dt-binding to configure PCIe mode
  PCI: keystone: Explicitly set the PCIe mode
  dt-bindings: PCI: Document "atu" reg-names
  PCI: dwc: Fix ATU identification for designware version >= 4.80
  PCI: keystone: Prevent ARM32 specific code to be compiled for ARM64
  dt-bindings: PCI: Add PCI RC dt binding documentation for AM654
  PCI: keystone: Add support for PCIe in AM654x Platforms
  phy: core: Invoke pm_runtime_get_*/pm_runtime_put_* before invoking
    reset callback
  dt-bindings: phy: ti: Add dt binding documentation for SERDES in
    AM654x SoC
  phy: ti: Add a new SERDES driver for TI's AM654x SoC
  ARM: dts: keystone-k2e: Use the updated binding to describe PCIe in
    k2e
  arm64: dts: k3-am6: Add "socionext,synquacer-pre-its" property to
    gic_its
  arm64: dts: k3-am6: Add mux-controller dt node required for muxing
    SERDES
  arm64: dts: k3-am6: Add SERDES DT node
  arm64: dts: k3-am6: Add Root Complex PCIe dt node

 .../bindings/pci/designware-pcie.txt          |    7 +-
 .../devicetree/bindings/pci/pci-keystone.txt  |   17 +-
 .../devicetree/bindings/phy/ti-phy.txt        |   77 ++
 MAINTAINERS                                   |    2 +-
 arch/arm/boot/dts/keystone-k2e.dtsi           |   15 +-
 arch/arm/boot/dts/keystone.dtsi               |   18 +-
 arch/arm64/boot/dts/ti/k3-am65-main.dtsi      |  142 +++
 arch/arm64/boot/dts/ti/k3-am65.dtsi           |    1 +
 drivers/pci/controller/dwc/Kconfig            |    2 +-
 drivers/pci/controller/dwc/Makefile           |    2 +-
 drivers/pci/controller/dwc/pci-keystone-dw.c  |  484 --------
 drivers/pci/controller/dwc/pci-keystone.c     | 1059 +++++++++++++----
 drivers/pci/controller/dwc/pci-keystone.h     |   57 -
 .../pci/controller/dwc/pcie-designware-host.c |   16 -
 drivers/pci/controller/dwc/pcie-designware.c  |   50 +-
 drivers/pci/controller/dwc/pcie-designware.h  |   13 +-
 drivers/phy/phy-core.c                        |    6 +
 drivers/phy/ti/Kconfig                        |   11 +
 drivers/phy/ti/Makefile                       |    1 +
 drivers/phy/ti/phy-am654-serdes.c             |  513 ++++++++
 include/dt-bindings/phy/phy-am654-serdes.h    |   13 +
 21 files changed, 1690 insertions(+), 816 deletions(-)
 delete mode 100644 drivers/pci/controller/dwc/pci-keystone-dw.c
 delete mode 100644 drivers/pci/controller/dwc/pci-keystone.h
 create mode 100644 drivers/phy/ti/phy-am654-serdes.c
 create mode 100644 include/dt-bindings/phy/phy-am654-serdes.h

Comments

Nishanth Menon Sept. 21, 2018, 7:42 p.m. UTC | #1
On 10:21-20180921, Kishon Vijay Abraham I wrote:
> GIC_ITS used in AM65x platform has the same configuration as that of
> GIC_ITS used in Socionext SoCs. Add "socionext,synquacer-pre-its"
> property to get PCI MSI working.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> index adcd6341e40c..2df4acb198bd 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> @@ -24,6 +24,7 @@
>  		gic_its: gic-its@18200000 {
>  			compatible = "arm,gic-v3-its";
>  			reg = <0x00 0x01820000 0x00 0x10000>;
> +			socionext,synquacer-pre-its = <0x1000000 0x400000>;
>  			msi-controller;
>  			#msi-cells = <1>;
>  		};
> -- 
> 2.17.1
> 


Please post the DTS series separately. This specific patch is a GIC ITS
patch, and does'nt need to depend on rest of the PCI series. Also looks
like you missed the v4.20-rc1 bandwagon.
Rob Herring (Arm) Sept. 24, 2018, 11 p.m. UTC | #2
On Fri, Sep 21, 2018 at 03:51:28PM +0530, Kishon Vijay Abraham I wrote:
> Add "reg-names" binding information in order for device tree node
> to be populated with the correct register strings.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  Documentation/devicetree/bindings/pci/pci-keystone.txt | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
> index 2030ee0dc4f9..3a551687cfa2 100644
> --- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
> +++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
> @@ -12,8 +12,10 @@ described here as well as properties that are not applicable.
>  Required Properties:-
>  
>  compatibility: "ti,keystone-pcie"
> -reg:	index 1 is the base address and length of DW application registers.
> -	index 2 is the base address and length of PCI device ID register.
> +reg: Three register ranges as listed in the reg-names property
> +reg-names: "dbics" for the DesignWare PCIe registers, "app" for the
> +	   TI specific application registers, "config" for the
> +	   configuration space address

This doesn't doesn't look like a compatible change.

>  
>  pcie_msi_intc : Interrupt controller device node for MSI IRQ chip
>  	interrupt-cells: should be set to 1
> -- 
> 2.17.1
>
Rob Herring (Arm) Sept. 25, 2018, 11:25 a.m. UTC | #3
On Fri, 21 Sep 2018 15:51:26 +0530, Kishon Vijay Abraham I wrote:
> Add bindings to get device control module which has the device id and
> vendor id to be configured in the keystone PCIe controller.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  Documentation/devicetree/bindings/pci/pci-keystone.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Lorenzo Pieralisi Oct. 12, 2018, 1:47 p.m. UTC | #4
On Fri, Sep 21, 2018 at 03:51:15PM +0530, Kishon Vijay Abraham I wrote:
> Add PCIe RC support for TI's AM654 SoC. The PCIe controller in AM654
> uses Synopsys core revision 4.90a and uses the same TI wrapper as used
> in keystone2 with certain modification. Hence AM654 will use the same
> pci wrapper driver pci-keystone.c
> 
> The initial support for AM654 was merged recently [1]
> 
> [1] -> https://lore.kernel.org/lkml/20180626162615.19194-1-nm@ti.com/
> 
> Patch series includes the following
>  *) Merge pci-keystone-dw.c into pci-keystone.c so that we have single
>     file for PCIe keystone driver.
>  *) Cleanup the pci-keystone driver. In certain cases the DT binding is
>     also modified since PCIe in keystone has never worked in mainline
>     due to lack of PHY support.
>  *) Included the PHY driver here for completeness (though the driver
>     might go via linux-phy tree)
>  *) Included the device tree patches here. Once this series is reviewed
>     it'll be sent to be merged via Tony's tree.
>  *) Patch to fix ATU identification for designware version >= 4.80 in
>     designware core is also included here.
> 
> TODO:
>  *) Add Endpoint Support for AM654
>  *) Send a patch to fix the MRRS after the correct value is identified.
> 
> Once this series is reviewed I'll split the series and send to
> corresponding subsytem Maintainers after removing RFC in subject.

Hi Kishon,

I started reviewing the series, I noticed that some patches are
clean-ups that I would like to queue to cut the delta so that you
can rebase on top of them, do you have time to post the clean-up
patches (no functional changes) stand-alone so that I can queue
them up please ?

I think you should drop the RFC tag from this series.

Thanks,
Lorenzo
Kishon Vijay Abraham I Nov. 2, 2018, 7:19 a.m. UTC | #5
Hi Rob,

On 25/09/18 4:30 AM, Rob Herring wrote:
> On Fri, Sep 21, 2018 at 03:51:28PM +0530, Kishon Vijay Abraham I wrote:
>> Add "reg-names" binding information in order for device tree node
>> to be populated with the correct register strings.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  Documentation/devicetree/bindings/pci/pci-keystone.txt | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
>> index 2030ee0dc4f9..3a551687cfa2 100644
>> --- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
>> +++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
>> @@ -12,8 +12,10 @@ described here as well as properties that are not applicable.
>>  Required Properties:-
>>  
>>  compatibility: "ti,keystone-pcie"
>> -reg:	index 1 is the base address and length of DW application registers.
>> -	index 2 is the base address and length of PCI device ID register.
>> +reg: Three register ranges as listed in the reg-names property
>> +reg-names: "dbics" for the DesignWare PCIe registers, "app" for the
>> +	   TI specific application registers, "config" for the
>> +	   configuration space address
> 
> This doesn't doesn't look like a compatible change.

The pcie-keystone driver hasn't worked in mainline because of lack of serdes
support. Since the same driver is used for am654 SoC (which has serdes support
included in this series), I'm trying to cleanup the binding.

Thanks
Kishon
Krzysztof Wilczy��ski July 3, 2021, 9:01 p.m. UTC | #6
Hi Kishon,

> Now that all PCI keystone functionality has been moved to pci-keystone.c,
> cleanup MSI/legacy interrupt configuration and handling.
>  *) Cleanup macros
>  *) Remove unnecessary structure variables (required when 2 files are
>     used)
>  *) Remove ks_dw_pcie_legacy_irq_chip and use dummy_irq_chip
>  *) Move request_irq of error irq from ks_add_pcie_port to ks_pcie_probe
>     as error_irq is common to both host mode and device mode
[...]

While looking at some small clean-ups for Bjorn, I stumbled upon this
series, and it seems a lot of your work here cover what Bjorn wanted to
do, thus I need to ask - do you recall, and I appreciate it's been
a while (three years actually), what happened and/or if you ever had the
time to work on this series?

Would it be possible to resurrect this?  Do you need any help?

Thank you in advance!

	Krzysztof
Kishon Vijay Abraham I July 5, 2021, 8:21 a.m. UTC | #7
Hi Krzysztof,

On 04/07/21 2:31 am, Krzysztof Wilczyński wrote:
> Hi Kishon,
> 
>> Now that all PCI keystone functionality has been moved to pci-keystone.c,
>> cleanup MSI/legacy interrupt configuration and handling.
>>  *) Cleanup macros
>>  *) Remove unnecessary structure variables (required when 2 files are
>>     used)
>>  *) Remove ks_dw_pcie_legacy_irq_chip and use dummy_irq_chip
>>  *) Move request_irq of error irq from ks_add_pcie_port to ks_pcie_probe
>>     as error_irq is common to both host mode and device mode
> [...]
> 
> While looking at some small clean-ups for Bjorn, I stumbled upon this
> series, and it seems a lot of your work here cover what Bjorn wanted to
> do, thus I need to ask - do you recall, and I appreciate it's been
> a while (three years actually), what happened and/or if you ever had the
> time to work on this series?
> 
> Would it be possible to resurrect this?  Do you need any help?

A lot of patches in this series should already be merged (after
splitting into smaller ones)
http://patchwork.ozlabs.org/project/linux-pci/list/?series=71185

https://patchwork.kernel.org/project/linux-arm-kernel/cover/20190321095927.7058-1-kishon@ti.com/

The following series is still pending and is in my TODO list
https://lore.kernel.org/r/20210325090026.8843-1-kishon@ti.com

Are there any other clean-ups you are looking into?

Thanks and Regards
Kishon
Krzysztof Wilczy��ski July 6, 2021, 8:34 p.m. UTC | #8
Hi Kishon,

[...]
> > Would it be possible to resurrect this?  Do you need any help?
> 
> A lot of patches in this series should already be merged (after
> splitting into smaller ones)
> http://patchwork.ozlabs.org/project/linux-pci/list/?series=71185
> 
> https://patchwork.kernel.org/project/linux-arm-kernel/cover/20190321095927.7058-1-kishon@ti.com/

Ah!  Nice!

[...]
> Are there any other clean-ups you are looking into?

Bjorn was looking recently at struct keystone_pcie and suggested that
perhaps things such as for example the legacy_host_irqs member could be
refactored - in this particular case it seems to only store a single
item in the array, etc.

And this series of patches I found is refactoring a lot of the elements
of the driver and thus the struct keystone_pcie too in due process.

I suppose, it would be just better to wait for you to complete all the
work you have planned?  What do you think?

	Krzysztof