| Message ID | 1535096165-45827-2-git-send-email-hanjie.lin@amlogic.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | add the Amlogic Meson PCIe controller driver. | expand |
On Fri, 2018-08-24 at 15:36 +0800, Hanjie Lin wrote: > From: Yue Wang <yue.wang@amlogic.com> > > The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare > PCI core. This patch adds documentation for the DT bindings in Meson PCIe > controller. > > Signed-off-by: Yue Wang <yue.wang@amlogic.com> > Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com> > --- > .../devicetree/bindings/pci/amlogic,meson-pcie.txt | 63 ++++++++++++++++++++++ > 1 file changed, 63 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt > > diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt > new file mode 100644 > index 0000000..8a831d1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt > @@ -0,0 +1,63 @@ > +Amlogic Meson AXG DWC PCIE SoC controller > + > +Amlogic Meson PCIe host controller is based on the Synopsys DesignWare PCI core. > +It shares common functions with the PCIe DesignWare core driver and > +inherits common properties defined in > +Documentation/devicetree/bindings/pci/designware-pci.txt. > + > +Additional properties are described here: > + > +Required properties: > +- compatible: > + should contain "amlogic,axg-pcie" to identify the core. > +- reg: > + Should contain the configuration address space. > +- reg-names: Must be > + - "elbi" External local bus interface registers > + - "cfg" Meson specific registers > + - "config" PCIe configuration space > +- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal. > +- clocks: Must contain an entry for each entry in clock-names. > +- clock-names: Must include the following entries: > + - "pclk" PCIe GEN 100M PLL clock > + - "port" PCIe_x(A or B) RC clock gate > + - "general" PCIe Phy clock > + - "mipi" PCIe_x(A or B) 100M ref clock gate > +- resets: phandle to the reset lines. > +- reset-names: must contain "phy" and "peripheral" > + - "port" Port A or B reset > + - "apb" APB reset The above description is not coherent (phy <=> port) > + > +Example configuration: > + > + pcie: pcie@f9800000 { > + compatible = "amlogic,axg-pcie", "snps,dw-pcie"; > + reg = <0x0 0xf9800000 0x0 0x400000 > + 0x0 0xff646000 0x0 0x2000 > + 0x0 0xf9f00000 0x0 0x100000>; > + reg-names = "elbi", "cfg", "config"; > + reset-gpios = <&gpio GPIOX_19 GPIO_ACTIVE_HIGH>; > + interrupts = <GIC_SPI 177 IRQ_TYPE_EDGE_RISING>; > + #interrupt-cells = <1>; > + interrupt-map-mask = <0 0 0 0>; > + interrupt-map = <0 0 0 0 &gic GIC_SPI 179 IRQ_TYPE_EDGE_RISING>; > + bus-range = <0x0 0xff>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; Not described above - is it even used ? > + phys = <&pcie_phy>; Not documented and not necessary. Please remove this. > + ranges = <0x82000000 0 0 0x0 0xf9c00000 0 0x00300000>; > + > + clocks = <&clkc CLKID_USB > + &clkc CLKID_MIPI_ENABLE > + &clkc CLKID_PCIE_A > + &clkc CLKID_PCIE_CML_EN0>; > + clock-names = "general", > + "mipi", > + "pclk", > + "port"; > + resets = <&reset RESET_PCIE_A>, > + <&reset RESET_PCIE_APB>; > + reset-names = "port", > + "apb"; > + };
On 2018/8/24 16:22, Jerome Brunet wrote: > On Fri, 2018-08-24 at 15:36 +0800, Hanjie Lin wrote: >> From: Yue Wang <yue.wang@amlogic.com> >> >> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare >> PCI core. This patch adds documentation for the DT bindings in Meson PCIe >> controller. >> >> Signed-off-by: Yue Wang <yue.wang@amlogic.com> >> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com> >> --- >> .../devicetree/bindings/pci/amlogic,meson-pcie.txt | 63 ++++++++++++++++++++++ >> 1 file changed, 63 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt >> >> diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt >> new file mode 100644 >> index 0000000..8a831d1 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt >> @@ -0,0 +1,63 @@ >> +Amlogic Meson AXG DWC PCIE SoC controller >> + >> +Amlogic Meson PCIe host controller is based on the Synopsys DesignWare PCI core. >> +It shares common functions with the PCIe DesignWare core driver and >> +inherits common properties defined in >> +Documentation/devicetree/bindings/pci/designware-pci.txt. >> + >> +Additional properties are described here: >> + >> +Required properties: >> +- compatible: >> + should contain "amlogic,axg-pcie" to identify the core. >> +- reg: >> + Should contain the configuration address space. >> +- reg-names: Must be >> + - "elbi" External local bus interface registers >> + - "cfg" Meson specific registers >> + - "config" PCIe configuration space >> +- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal. >> +- clocks: Must contain an entry for each entry in clock-names. >> +- clock-names: Must include the following entries: >> + - "pclk" PCIe GEN 100M PLL clock >> + - "port" PCIe_x(A or B) RC clock gate >> + - "general" PCIe Phy clock >> + - "mipi" PCIe_x(A or B) 100M ref clock gate >> +- resets: phandle to the reset lines. >> +- reset-names: must contain "phy" and "peripheral" >> + - "port" Port A or B reset >> + - "apb" APB reset > > The above description is not coherent (phy <=> port) > Yes, this should be port and apb here. We'll integrate phy driver into ctrl driver, and move phy reset to here also. >> + >> +Example configuration: >> + >> + pcie: pcie@f9800000 { >> + compatible = "amlogic,axg-pcie", "snps,dw-pcie"; >> + reg = <0x0 0xf9800000 0x0 0x400000 >> + 0x0 0xff646000 0x0 0x2000 >> + 0x0 0xf9f00000 0x0 0x100000>; >> + reg-names = "elbi", "cfg", "config"; >> + reset-gpios = <&gpio GPIOX_19 GPIO_ACTIVE_HIGH>; >> + interrupts = <GIC_SPI 177 IRQ_TYPE_EDGE_RISING>; >> + #interrupt-cells = <1>; >> + interrupt-map-mask = <0 0 0 0>; >> + interrupt-map = <0 0 0 0 &gic GIC_SPI 179 IRQ_TYPE_EDGE_RISING>; >> + bus-range = <0x0 0xff>; >> + #address-cells = <3>; >> + #size-cells = <2>; >> + device_type = "pci"; > > Not described above - is it even used ? > It's necessary, specified in designware-pcie.txt: - device_type: Usage: required Value type: <string> Definition: Should be "pci". >> + phys = <&pcie_phy>; > > Not documented and not necessary. Please remove this. > We'll remove phy driver and this also. >> + ranges = <0x82000000 0 0 0x0 0xf9c00000 0 0x00300000>; >> + >> + clocks = <&clkc CLKID_USB >> + &clkc CLKID_MIPI_ENABLE >> + &clkc CLKID_PCIE_A >> + &clkc CLKID_PCIE_CML_EN0>; >> + clock-names = "general", >> + "mipi", >> + "pclk", >> + "port"; >> + resets = <&reset RESET_PCIE_A>, >> + <&reset RESET_PCIE_APB>; >> + reset-names = "port", >> + "apb"; >> + }; > > > . >
On Mon, Aug 27, 2018 at 04:55:20PM +0800, Hanjie Lin wrote: > > > On 2018/8/24 16:22, Jerome Brunet wrote: > > On Fri, 2018-08-24 at 15:36 +0800, Hanjie Lin wrote: > >> From: Yue Wang <yue.wang@amlogic.com> > >> > >> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare > >> PCI core. This patch adds documentation for the DT bindings in Meson PCIe > >> controller. > >> > >> Signed-off-by: Yue Wang <yue.wang@amlogic.com> > >> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com> > >> --- > >> .../devicetree/bindings/pci/amlogic,meson-pcie.txt | 63 ++++++++++++++++++++++ > >> 1 file changed, 63 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt > >> > >> diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt > >> new file mode 100644 > >> index 0000000..8a831d1 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt > >> @@ -0,0 +1,63 @@ > >> +Amlogic Meson AXG DWC PCIE SoC controller > >> + > >> +Amlogic Meson PCIe host controller is based on the Synopsys DesignWare PCI core. > >> +It shares common functions with the PCIe DesignWare core driver and > >> +inherits common properties defined in > >> +Documentation/devicetree/bindings/pci/designware-pci.txt. > >> + > >> +Additional properties are described here: > >> + > >> +Required properties: > >> +- compatible: > >> + should contain "amlogic,axg-pcie" to identify the core. > >> +- reg: > >> + Should contain the configuration address space. > >> +- reg-names: Must be > >> + - "elbi" External local bus interface registers > >> + - "cfg" Meson specific registers > >> + - "config" PCIe configuration space > >> +- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal. > >> +- clocks: Must contain an entry for each entry in clock-names. > >> +- clock-names: Must include the following entries: > >> + - "pclk" PCIe GEN 100M PLL clock > >> + - "port" PCIe_x(A or B) RC clock gate > >> + - "general" PCIe Phy clock > >> + - "mipi" PCIe_x(A or B) 100M ref clock gate > >> +- resets: phandle to the reset lines. > >> +- reset-names: must contain "phy" and "peripheral" > >> + - "port" Port A or B reset > >> + - "apb" APB reset > > > > The above description is not coherent (phy <=> port) > > > > Yes, this should be port and apb here. > We'll integrate phy driver into ctrl driver, and move phy reset to here also. Why? That's the wrong thing to do if they are separate h/w blocks. You can do whatever you like in the drivers, but the DT should reflect the h/w. Rob
On 2018/8/29 8:41, Rob Herring wrote: > On Mon, Aug 27, 2018 at 04:55:20PM +0800, Hanjie Lin wrote: >> >> >> On 2018/8/24 16:22, Jerome Brunet wrote: >>> On Fri, 2018-08-24 at 15:36 +0800, Hanjie Lin wrote: >>>> From: Yue Wang <yue.wang@amlogic.com> >>>> >>>> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare >>>> PCI core. This patch adds documentation for the DT bindings in Meson PCIe >>>> controller. >>>> >>>> Signed-off-by: Yue Wang <yue.wang@amlogic.com> >>>> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com> >>>> --- >>>> .../devicetree/bindings/pci/amlogic,meson-pcie.txt | 63 ++++++++++++++++++++++ >>>> 1 file changed, 63 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt >>>> new file mode 100644 >>>> index 0000000..8a831d1 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt >>>> @@ -0,0 +1,63 @@ >>>> +Amlogic Meson AXG DWC PCIE SoC controller >>>> + >>>> +Amlogic Meson PCIe host controller is based on the Synopsys DesignWare PCI core. >>>> +It shares common functions with the PCIe DesignWare core driver and >>>> +inherits common properties defined in >>>> +Documentation/devicetree/bindings/pci/designware-pci.txt. >>>> + >>>> +Additional properties are described here: >>>> + >>>> +Required properties: >>>> +- compatible: >>>> + should contain "amlogic,axg-pcie" to identify the core. >>>> +- reg: >>>> + Should contain the configuration address space. >>>> +- reg-names: Must be >>>> + - "elbi" External local bus interface registers >>>> + - "cfg" Meson specific registers >>>> + - "config" PCIe configuration space >>>> +- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal. >>>> +- clocks: Must contain an entry for each entry in clock-names. >>>> +- clock-names: Must include the following entries: >>>> + - "pclk" PCIe GEN 100M PLL clock >>>> + - "port" PCIe_x(A or B) RC clock gate >>>> + - "general" PCIe Phy clock >>>> + - "mipi" PCIe_x(A or B) 100M ref clock gate >>>> +- resets: phandle to the reset lines. >>>> +- reset-names: must contain "phy" and "peripheral" >>>> + - "port" Port A or B reset >>>> + - "apb" APB reset >>> >>> The above description is not coherent (phy <=> port) >>> >> >> Yes, this should be port and apb here. >> We'll integrate phy driver into ctrl driver, and move phy reset to here also. > > Why? That's the wrong thing to do if they are separate h/w blocks. You > can do whatever you like in the drivers, but the DT should reflect the > h/w. > > Rob > > . > We have the dedicated phy driver which only process reset job, and we consider that it's too overkill to do just these things . So we will integrate phy reset job into the controller driver int the next version. thanks.
On Thu, 2018-08-30 at 15:37 +0800, Hanjie Lin wrote: > > On 2018/8/29 8:41, Rob Herring wrote: > > On Mon, Aug 27, 2018 at 04:55:20PM +0800, Hanjie Lin wrote: > > > > > > > > > On 2018/8/24 16:22, Jerome Brunet wrote: > > > > On Fri, 2018-08-24 at 15:36 +0800, Hanjie Lin wrote: > > > > > From: Yue Wang <yue.wang@amlogic.com> > > > > > > > > > > The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare > > > > > PCI core. This patch adds documentation for the DT bindings in Meson PCIe > > > > > controller. > > > > > > > > > > Signed-off-by: Yue Wang <yue.wang@amlogic.com> > > > > > Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com> > > > > > --- > > > > > .../devicetree/bindings/pci/amlogic,meson-pcie.txt | 63 ++++++++++++++++++++++ > > > > > 1 file changed, 63 insertions(+) > > > > > create mode 100644 Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt > > > > > new file mode 100644 > > > > > index 0000000..8a831d1 > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt > > > > > @@ -0,0 +1,63 @@ > > > > > +Amlogic Meson AXG DWC PCIE SoC controller > > > > > + > > > > > +Amlogic Meson PCIe host controller is based on the Synopsys DesignWare PCI core. > > > > > +It shares common functions with the PCIe DesignWare core driver and > > > > > +inherits common properties defined in > > > > > +Documentation/devicetree/bindings/pci/designware-pci.txt. > > > > > + > > > > > +Additional properties are described here: > > > > > + > > > > > +Required properties: > > > > > +- compatible: > > > > > + should contain "amlogic,axg-pcie" to identify the core. > > > > > +- reg: > > > > > + Should contain the configuration address space. > > > > > +- reg-names: Must be > > > > > + - "elbi" External local bus interface registers > > > > > + - "cfg" Meson specific registers > > > > > + - "config" PCIe configuration space > > > > > +- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal. > > > > > +- clocks: Must contain an entry for each entry in clock-names. > > > > > +- clock-names: Must include the following entries: > > > > > + - "pclk" PCIe GEN 100M PLL clock > > > > > + - "port" PCIe_x(A or B) RC clock gate > > > > > + - "general" PCIe Phy clock > > > > > + - "mipi" PCIe_x(A or B) 100M ref clock gate > > > > > +- resets: phandle to the reset lines. > > > > > +- reset-names: must contain "phy" and "peripheral" > > > > > + - "port" Port A or B reset > > > > > + - "apb" APB reset > > > > > > > > The above description is not coherent (phy <=> port) > > > > > > > > > > Yes, this should be port and apb here. > > > We'll integrate phy driver into ctrl driver, and move phy reset to here also. > > > > Why? That's the wrong thing to do if they are separate h/w blocks. You > > can do whatever you like in the drivers, but the DT should reflect the > > h/w. > > > > Rob > > > > . > > > > We have the dedicated phy driver which only process reset job, > and we consider that it's too overkill to do just these things . > So we will integrate phy reset job into the controller driver int the next version. Rob has a point there. Even if overkill, it does model the HW as it is. + I spotted in your v2 that there is also a register access, so not only the reset > > thanks.
On Thu, Aug 30, 2018 at 2:37 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote: > > > > On 2018/8/29 8:41, Rob Herring wrote: > > On Mon, Aug 27, 2018 at 04:55:20PM +0800, Hanjie Lin wrote: > >> > >> > >> On 2018/8/24 16:22, Jerome Brunet wrote: > >>> On Fri, 2018-08-24 at 15:36 +0800, Hanjie Lin wrote: > >>>> From: Yue Wang <yue.wang@amlogic.com> > >>>> > >>>> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare > >>>> PCI core. This patch adds documentation for the DT bindings in Meson PCIe > >>>> controller. > >>>> > >>>> Signed-off-by: Yue Wang <yue.wang@amlogic.com> > >>>> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com> > >>>> --- > >>>> .../devicetree/bindings/pci/amlogic,meson-pcie.txt | 63 ++++++++++++++++++++++ > >>>> 1 file changed, 63 insertions(+) > >>>> create mode 100644 Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt > >>>> new file mode 100644 > >>>> index 0000000..8a831d1 > >>>> --- /dev/null > >>>> +++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt > >>>> @@ -0,0 +1,63 @@ > >>>> +Amlogic Meson AXG DWC PCIE SoC controller > >>>> + > >>>> +Amlogic Meson PCIe host controller is based on the Synopsys DesignWare PCI core. > >>>> +It shares common functions with the PCIe DesignWare core driver and > >>>> +inherits common properties defined in > >>>> +Documentation/devicetree/bindings/pci/designware-pci.txt. > >>>> + > >>>> +Additional properties are described here: > >>>> + > >>>> +Required properties: > >>>> +- compatible: > >>>> + should contain "amlogic,axg-pcie" to identify the core. > >>>> +- reg: > >>>> + Should contain the configuration address space. > >>>> +- reg-names: Must be > >>>> + - "elbi" External local bus interface registers > >>>> + - "cfg" Meson specific registers > >>>> + - "config" PCIe configuration space > >>>> +- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal. > >>>> +- clocks: Must contain an entry for each entry in clock-names. > >>>> +- clock-names: Must include the following entries: > >>>> + - "pclk" PCIe GEN 100M PLL clock > >>>> + - "port" PCIe_x(A or B) RC clock gate > >>>> + - "general" PCIe Phy clock > >>>> + - "mipi" PCIe_x(A or B) 100M ref clock gate > >>>> +- resets: phandle to the reset lines. > >>>> +- reset-names: must contain "phy" and "peripheral" > >>>> + - "port" Port A or B reset > >>>> + - "apb" APB reset > >>> > >>> The above description is not coherent (phy <=> port) > >>> > >> > >> Yes, this should be port and apb here. > >> We'll integrate phy driver into ctrl driver, and move phy reset to here also. > > > > Why? That's the wrong thing to do if they are separate h/w blocks. You > > can do whatever you like in the drivers, but the DT should reflect the > > h/w. > > > > Rob > > > > . > > > > We have the dedicated phy driver which only process reset job, > and we consider that it's too overkill to do just these things . > So we will integrate phy reset job into the controller driver int the next version. What's in the separate register space you had for the phy? Rob
On 2018/8/30 21:59, Rob Herring wrote: > On Thu, Aug 30, 2018 at 2:37 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote: >> >> >> >> On 2018/8/29 8:41, Rob Herring wrote: >>> On Mon, Aug 27, 2018 at 04:55:20PM +0800, Hanjie Lin wrote: >>>> >>>> >>>> On 2018/8/24 16:22, Jerome Brunet wrote: >>>>> On Fri, 2018-08-24 at 15:36 +0800, Hanjie Lin wrote: >>>>>> From: Yue Wang <yue.wang@amlogic.com> >>>>>> >>>>>> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare >>>>>> PCI core. This patch adds documentation for the DT bindings in Meson PCIe >>>>>> controller. >>>>>> >>>>>> Signed-off-by: Yue Wang <yue.wang@amlogic.com> >>>>>> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com> >>>>>> --- >>>>>> .../devicetree/bindings/pci/amlogic,meson-pcie.txt | 63 ++++++++++++++++++++++ >>>>>> 1 file changed, 63 insertions(+) >>>>>> create mode 100644 Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt >>>>>> new file mode 100644 >>>>>> index 0000000..8a831d1 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt >>>>>> @@ -0,0 +1,63 @@ >>>>>> +Amlogic Meson AXG DWC PCIE SoC controller >>>>>> + >>>>>> +Amlogic Meson PCIe host controller is based on the Synopsys DesignWare PCI core. >>>>>> +It shares common functions with the PCIe DesignWare core driver and >>>>>> +inherits common properties defined in >>>>>> +Documentation/devicetree/bindings/pci/designware-pci.txt. >>>>>> + >>>>>> +Additional properties are described here: >>>>>> + >>>>>> +Required properties: >>>>>> +- compatible: >>>>>> + should contain "amlogic,axg-pcie" to identify the core. >>>>>> +- reg: >>>>>> + Should contain the configuration address space. >>>>>> +- reg-names: Must be >>>>>> + - "elbi" External local bus interface registers >>>>>> + - "cfg" Meson specific registers >>>>>> + - "config" PCIe configuration space >>>>>> +- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal. >>>>>> +- clocks: Must contain an entry for each entry in clock-names. >>>>>> +- clock-names: Must include the following entries: >>>>>> + - "pclk" PCIe GEN 100M PLL clock >>>>>> + - "port" PCIe_x(A or B) RC clock gate >>>>>> + - "general" PCIe Phy clock >>>>>> + - "mipi" PCIe_x(A or B) 100M ref clock gate >>>>>> +- resets: phandle to the reset lines. >>>>>> +- reset-names: must contain "phy" and "peripheral" >>>>>> + - "port" Port A or B reset >>>>>> + - "apb" APB reset >>>>> >>>>> The above description is not coherent (phy <=> port) >>>>> >>>> >>>> Yes, this should be port and apb here. >>>> We'll integrate phy driver into ctrl driver, and move phy reset to here also. >>> >>> Why? That's the wrong thing to do if they are separate h/w blocks. You >>> can do whatever you like in the drivers, but the DT should reflect the >>> h/w. >>> >>> Rob >>> >>> . >>> >> >> We have the dedicated phy driver which only process reset job, >> and we consider that it's too overkill to do just these things . >> So we will integrate phy reset job into the controller driver int the next version. > > What's in the separate register space you had for the phy? > > Rob > > . > As described with 'phy' reg in ctrl patch v3 thread [0] - reg-names: Must be - "elbi" External local bus interface registers - "cfg" Meson specific registers - "phy" Meson PCIE PHY registers - "config" PCIe configuration space When each controller driver probe, we powerup phy by write phy register like below: writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base); [0] https://lkml.kernel.org/r/1535616829-167936-1-git-send-email-hanjie.lin@amlogic.com
diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt new file mode 100644 index 0000000..8a831d1 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt @@ -0,0 +1,63 @@ +Amlogic Meson AXG DWC PCIE SoC controller + +Amlogic Meson PCIe host controller is based on the Synopsys DesignWare PCI core. +It shares common functions with the PCIe DesignWare core driver and +inherits common properties defined in +Documentation/devicetree/bindings/pci/designware-pci.txt. + +Additional properties are described here: + +Required properties: +- compatible: + should contain "amlogic,axg-pcie" to identify the core. +- reg: + Should contain the configuration address space. +- reg-names: Must be + - "elbi" External local bus interface registers + - "cfg" Meson specific registers + - "config" PCIe configuration space +- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal. +- clocks: Must contain an entry for each entry in clock-names. +- clock-names: Must include the following entries: + - "pclk" PCIe GEN 100M PLL clock + - "port" PCIe_x(A or B) RC clock gate + - "general" PCIe Phy clock + - "mipi" PCIe_x(A or B) 100M ref clock gate +- resets: phandle to the reset lines. +- reset-names: must contain "phy" and "peripheral" + - "port" Port A or B reset + - "apb" APB reset + +Example configuration: + + pcie: pcie@f9800000 { + compatible = "amlogic,axg-pcie", "snps,dw-pcie"; + reg = <0x0 0xf9800000 0x0 0x400000 + 0x0 0xff646000 0x0 0x2000 + 0x0 0xf9f00000 0x0 0x100000>; + reg-names = "elbi", "cfg", "config"; + reset-gpios = <&gpio GPIOX_19 GPIO_ACTIVE_HIGH>; + interrupts = <GIC_SPI 177 IRQ_TYPE_EDGE_RISING>; + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0>; + interrupt-map = <0 0 0 0 &gic GIC_SPI 179 IRQ_TYPE_EDGE_RISING>; + bus-range = <0x0 0xff>; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + phys = <&pcie_phy>; + ranges = <0x82000000 0 0 0x0 0xf9c00000 0 0x00300000>; + + clocks = <&clkc CLKID_USB + &clkc CLKID_MIPI_ENABLE + &clkc CLKID_PCIE_A + &clkc CLKID_PCIE_CML_EN0>; + clock-names = "general", + "mipi", + "pclk", + "port"; + resets = <&reset RESET_PCIE_A>, + <&reset RESET_PCIE_APB>; + reset-names = "port", + "apb"; + };