| Message ID | 20170705161333.9315-1-gregory.clement@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 05, 2017 at 06:13:33PM +0200, Gregory CLEMENT wrote: > Enable the PCIe interface on the MACCHIATOBin board. It is located on > CON12 and is 4 lanes capable. > > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Why do you folk at free-electrons like doing half a job all the friggin time? You know I have complete patches for mcbin, but you pointedly won't look at them at all - except when you have a problem and want to test my tree. And even then, you ignore my work (despite testing that it works), and you still recreate my patches. This is really frustrating and insane behaviour on your part. Here's what I have: +&cpm_pcie0 { + pinctrl-names = "default"; + pinctrl-0 = <&cpm_pcie_pins>; + num-lanes = <4>; + reset-gpio = <&cpm_gpio1 20 GPIO_ACTIVE_LOW>; + status = "okay"; +}; + + cpm_pcie_pins: pcie-pins { + marvell,pins = "mpp52"; + marvell,function = "gpio"; + }; Since you have merged GPIO and pinmux support for the v4.13 window, there's absolutely no reason not to include the GPIO bits. In fact, there's no reason not to consider using my bloody patches. Except your stupid idiotic NiH problem that you seem to have. I know that your behaviour in regard of this has been discussed within Marvell, and people are getting unhappy with free electron's attitude over this. You need to change, and start working _with_ people instead of constantly screwing people over. So, NAK on your patch. Once v4.13-rc1 is out, I'll update my patch series for the screw-over free-electrons has already done, and post some patches. I can't do it sooner, your work is scattered all over the place which makes it impossible to build upon, and afaics you've not published a consolidated tree. Maybe the whole plan here is to "screw rmk" - that's exactly the message that I'm getting from you guys. You've no interest in working with others, you seem to want to out-right own everything Marvell and sod everyone else.
On 5 July 2017 at 18:16, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Wed, Jul 05, 2017 at 06:13:33PM +0200, Gregory CLEMENT wrote: >> Enable the PCIe interface on the MACCHIATOBin board. It is located on >> CON12 and is 4 lanes capable. >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > > Why do you folk at free-electrons like doing half a job all the friggin > time? > > You know I have complete patches for mcbin, but you pointedly won't look > at them at all - except when you have a problem and want to test my tree. > And even then, you ignore my work (despite testing that it works), and > you still recreate my patches. > > This is really frustrating and insane behaviour on your part. > > Here's what I have: > > +&cpm_pcie0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&cpm_pcie_pins>; > + num-lanes = <4>; > + reset-gpio = <&cpm_gpio1 20 GPIO_ACTIVE_LOW>; > + status = "okay"; This needs 'num-viewport = <8>' as well, or the crazy Synopsys DWC PCIe driver will happily reconfigure the I/O window at runtime to perform config space accesses, without any locking whatsoever.
On Wed, Jul 05, 2017 at 06:36:53PM +0100, Ard Biesheuvel wrote: > On 5 July 2017 at 18:16, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > > On Wed, Jul 05, 2017 at 06:13:33PM +0200, Gregory CLEMENT wrote: > >> Enable the PCIe interface on the MACCHIATOBin board. It is located on > >> CON12 and is 4 lanes capable. > >> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > > > > Why do you folk at free-electrons like doing half a job all the friggin > > time? > > > > You know I have complete patches for mcbin, but you pointedly won't look > > at them at all - except when you have a problem and want to test my tree. > > And even then, you ignore my work (despite testing that it works), and > > you still recreate my patches. > > > > This is really frustrating and insane behaviour on your part. > > > > Here's what I have: > > > > +&cpm_pcie0 { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&cpm_pcie_pins>; > > + num-lanes = <4>; > > + reset-gpio = <&cpm_gpio1 20 GPIO_ACTIVE_LOW>; > > + status = "okay"; > > This needs 'num-viewport = <8>' as well, or the crazy Synopsys DWC > PCIe driver will happily reconfigure the I/O window at runtime to > perform config space accesses, without any locking whatsoever. Thanks for the feedback, I'll integrate that change into my patch, so it's ready for when I rebase after -rc1.
On Wed, 5 Jul 2017 18:44:03 +0100 Russell King - ARM Linux wrote: > On Wed, Jul 05, 2017 at 06:36:53PM +0100, Ard Biesheuvel wrote: > > On 5 July 2017 at 18:16, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > > > On Wed, Jul 05, 2017 at 06:13:33PM +0200, Gregory CLEMENT wrote: > > >> Enable the PCIe interface on the MACCHIATOBin board. It is located on > > >> CON12 and is 4 lanes capable. > > >> > > >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > > > > > > Why do you folk at free-electrons like doing half a job all the friggin > > > time? > > > > > > You know I have complete patches for mcbin, but you pointedly won't look > > > at them at all - except when you have a problem and want to test my tree. > > > And even then, you ignore my work (despite testing that it works), and > > > you still recreate my patches. > > > > > > This is really frustrating and insane behaviour on your part. > > > > > > Here's what I have: > > > > > > +&cpm_pcie0 { > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&cpm_pcie_pins>; > > > + num-lanes = <4>; > > > + reset-gpio = <&cpm_gpio1 20 GPIO_ACTIVE_LOW>; > > > + status = "okay"; > > > > This needs 'num-viewport = <8>' as well, or the crazy Synopsys DWC IMHO, maybe putting this property into dtsi is better. Thanks, Jisheng > > PCIe driver will happily reconfigure the I/O window at runtime to > > perform config space accesses, without any locking whatsoever. > > Thanks for the feedback, I'll integrate that change into my patch, so > it's ready for when I rebase after -rc1. >
Hi Russell King, On mer., juil. 05 2017, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Wed, Jul 05, 2017 at 06:13:33PM +0200, Gregory CLEMENT wrote: >> Enable the PCIe interface on the MACCHIATOBin board. It is located on >> CON12 and is 4 lanes capable. >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > > Why do you folk at free-electrons like doing half a job all the friggin > time? > > You know I have complete patches for mcbin, but you pointedly won't look > at them at all - except when you have a problem and want to test my tree. > And even then, you ignore my work (despite testing that it works), and > you still recreate my patches. > > This is really frustrating and insane behaviour on your part. Sorry for this, I wrongly assumed that if you had the PCIe part you would have already submitted it. Also I didn't expect there was a gpio dependency with PCIe because until now I never saw the reset-gpio for PCIe with the controller I use. > > Here's what I have: > > +&cpm_pcie0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&cpm_pcie_pins>; > + num-lanes = <4>; > + reset-gpio = <&cpm_gpio1 20 GPIO_ACTIVE_LOW>; > + status = "okay"; > +}; > + > > + cpm_pcie_pins: pcie-pins { > + marvell,pins = "mpp52"; > + marvell,function = "gpio"; > + }; > > Since you have merged GPIO and pinmux support for the v4.13 window, > there's absolutely no reason not to include the GPIO bits. In fact, > there's no reason not to consider using my bloody patches. > > Except your stupid idiotic NiH problem that you seem to have. > > I know that your behaviour in regard of this has been discussed within > Marvell, and people are getting unhappy with free electron's attitude > over this. You need to change, and start working _with_ people instead > of constantly screwing people over. > > So, NAK on your patch. > > Once v4.13-rc1 is out, I'll update my patch series for the screw-over > free-electrons has already done, and post some patches. I can't do it > sooner, your work is scattered all over the place which makes it Actually everything is available in the linux-next branch since one or two weeks. So you can rebase and send your series right now. Gregory
On 6 July 2017 at 07:31, Jisheng Zhang <jszhang@marvell.com> wrote: > On Wed, 5 Jul 2017 18:44:03 +0100 Russell King - ARM Linux wrote: > >> On Wed, Jul 05, 2017 at 06:36:53PM +0100, Ard Biesheuvel wrote: >> > On 5 July 2017 at 18:16, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: >> > > On Wed, Jul 05, 2017 at 06:13:33PM +0200, Gregory CLEMENT wrote: >> > >> Enable the PCIe interface on the MACCHIATOBin board. It is located on >> > >> CON12 and is 4 lanes capable. >> > >> >> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >> > > >> > > Why do you folk at free-electrons like doing half a job all the friggin >> > > time? >> > > >> > > You know I have complete patches for mcbin, but you pointedly won't look >> > > at them at all - except when you have a problem and want to test my tree. >> > > And even then, you ignore my work (despite testing that it works), and >> > > you still recreate my patches. >> > > >> > > This is really frustrating and insane behaviour on your part. >> > > >> > > Here's what I have: >> > > >> > > +&cpm_pcie0 { >> > > + pinctrl-names = "default"; >> > > + pinctrl-0 = <&cpm_pcie_pins>; >> > > + num-lanes = <4>; >> > > + reset-gpio = <&cpm_gpio1 20 GPIO_ACTIVE_LOW>; >> > > + status = "okay"; >> > >> > This needs 'num-viewport = <8>' as well, or the crazy Synopsys DWC > > IMHO, maybe putting this property into dtsi is better. > Good point. Do all instances of this IP that live on the SoC have 8 viewports?
On Thu, Jul 06, 2017 at 09:18:25AM +0200, Gregory CLEMENT wrote: > Hi Russell King, > > Actually everything is available in the linux-next branch since one or > two weeks. So you can rebase and send your series right now. As there are multiple trees, just specifying a branch is not helpful. The Xenon SDHCI and ethernet changes were being published through the MISL tree, but that tree has no for-next branch. There is no git tree specified in MAINTAINERS either. So, sorry, I've no idea which tree you're referring to. And no, I do _not_ want to grab a copy of linux-next to rebase my changes on and publish back to SolidRun for their use, that would be insane.
Hi Russell King, On jeu., juil. 06 2017, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Thu, Jul 06, 2017 at 09:18:25AM +0200, Gregory CLEMENT wrote: >> Hi Russell King, >> >> Actually everything is available in the linux-next branch since one or >> two weeks. So you can rebase and send your series right now. > > As there are multiple trees, just specifying a branch is not helpful. > The Xenon SDHCI and ethernet changes were being published through the > MISL tree, but that tree has no for-next branch. > > There is no git tree specified in MAINTAINERS either. > > So, sorry, I've no idea which tree you're referring to. > > And no, I do _not_ want to grab a copy of linux-next to rebase my Well for upstreaming I don't see the problem to base on a linux-net tags such as next-20170704. We do it often when we submit patches to various subsystem (and if it is not a next tag we merge various branch by ourselves). > changes on and publish back to SolidRun for their use, that would be > insane. We also deliver stable branch for various customers and in this case we just have a different branch or tree. However, I think we have exactly what meets your requirements: https://github.com/MISL-EBU-System-SW/mainline-public/tree/4.12-rc6/backports It is a stable branch which gathers most of the patch submitted for Armada 37xx, 7K and 8K. Gregory > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net.
Hi Ard, On jeu., juil. 06 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 6 July 2017 at 07:31, Jisheng Zhang <jszhang@marvell.com> wrote: >> On Wed, 5 Jul 2017 18:44:03 +0100 Russell King - ARM Linux wrote: >> >>> On Wed, Jul 05, 2017 at 06:36:53PM +0100, Ard Biesheuvel wrote: >>> > On 5 July 2017 at 18:16, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: >>> > > On Wed, Jul 05, 2017 at 06:13:33PM +0200, Gregory CLEMENT wrote: >>> > >> Enable the PCIe interface on the MACCHIATOBin board. It is located on >>> > >> CON12 and is 4 lanes capable. >>> > >> >>> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >>> > > >>> > > Why do you folk at free-electrons like doing half a job all the friggin >>> > > time? >>> > > >>> > > You know I have complete patches for mcbin, but you pointedly won't look >>> > > at them at all - except when you have a problem and want to test my tree. >>> > > And even then, you ignore my work (despite testing that it works), and >>> > > you still recreate my patches. >>> > > >>> > > This is really frustrating and insane behaviour on your part. >>> > > >>> > > Here's what I have: >>> > > >>> > > +&cpm_pcie0 { >>> > > + pinctrl-names = "default"; >>> > > + pinctrl-0 = <&cpm_pcie_pins>; >>> > > + num-lanes = <4>; >>> > > + reset-gpio = <&cpm_gpio1 20 GPIO_ACTIVE_LOW>; >>> > > + status = "okay"; >>> > >>> > This needs 'num-viewport = <8>' as well, or the crazy Synopsys DWC >> >> IMHO, maybe putting this property into dtsi is better. >> > > Good point. Do all instances of this IP that live on the SoC have 8 > viewports? In the Armada 80x0 datasheet there is no mention of viewport. However 2 days ago you said that "What I do know is that boards like the Marvell 8040 based MacchiatoBin uses this IP in RC mode, and exposes config, MMIO and IO space windows using only 2 viewports. Note that this is essentially a bug in the DT description, given that its version of the IP supports 8 viewports."[1] So you seem to know that the version of the IP used support the 8 viewports! What I can also say is that the in the datasheet there is no mention of difference between the instance. The only thing that can differ is the number of lane for each PCIe Port. Gregory [1]: https://www.spinics.net/lists/arm-kernel/msg592091.html
On 6 July 2017 at 13:10, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > Hi Ard, > > On jeu., juil. 06 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > >> On 6 July 2017 at 07:31, Jisheng Zhang <jszhang@marvell.com> wrote: >>> On Wed, 5 Jul 2017 18:44:03 +0100 Russell King - ARM Linux wrote: >>> >>>> On Wed, Jul 05, 2017 at 06:36:53PM +0100, Ard Biesheuvel wrote: >>>> > On 5 July 2017 at 18:16, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: >>>> > > On Wed, Jul 05, 2017 at 06:13:33PM +0200, Gregory CLEMENT wrote: >>>> > >> Enable the PCIe interface on the MACCHIATOBin board. It is located on >>>> > >> CON12 and is 4 lanes capable. >>>> > >> >>>> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >>>> > > >>>> > > Why do you folk at free-electrons like doing half a job all the friggin >>>> > > time? >>>> > > >>>> > > You know I have complete patches for mcbin, but you pointedly won't look >>>> > > at them at all - except when you have a problem and want to test my tree. >>>> > > And even then, you ignore my work (despite testing that it works), and >>>> > > you still recreate my patches. >>>> > > >>>> > > This is really frustrating and insane behaviour on your part. >>>> > > >>>> > > Here's what I have: >>>> > > >>>> > > +&cpm_pcie0 { >>>> > > + pinctrl-names = "default"; >>>> > > + pinctrl-0 = <&cpm_pcie_pins>; >>>> > > + num-lanes = <4>; >>>> > > + reset-gpio = <&cpm_gpio1 20 GPIO_ACTIVE_LOW>; >>>> > > + status = "okay"; >>>> > >>>> > This needs 'num-viewport = <8>' as well, or the crazy Synopsys DWC >>> >>> IMHO, maybe putting this property into dtsi is better. >>> >> >> Good point. Do all instances of this IP that live on the SoC have 8 >> viewports? > > In the Armada 80x0 datasheet there is no mention of viewport. However 2 > days ago you said that "What I do know is that boards like the Marvell > 8040 based MacchiatoBin uses this IP in RC mode, and exposes config, > MMIO and IO space windows using only 2 viewports. Note that this is > essentially a bug in the DT description, given that its version of the > IP supports 8 viewports."[1] > > So you seem to know that the version of the IP used support the 8 > viewports! > > What I can also say is that the in the datasheet there is no mention of > difference between the instance. The only thing that can differ is the > number of lane for each PCIe Port. > I am sure I found it in the datasheet somewhere. It is the number of ATU windows. In any case, I implemented ACPI firmware for the MacchiatoBin, which also involves programming the PCIe RC in a mode that is compatible with ECAM. This turns out to be impossible (which means you cannot implement a SBSA compatible platform with this SoC), but in attempting to do so, I used five different windows, for type 0 config cycles, type 1 config cycles, IO space, MMIO32 space and MMIO64 space (which the Synopsys DW PCIe controller does not even implement, unfortunately) Note that this also involved reprogramming all the memory translation windows from the secure firmware, to free up the region 0xc000_0000 - 0xefff_ffff for use by PCIe (config and MMIO32 space), and assigning a window 0x8_0000_0000 - 0x8_ffff_ffff for 64-bit MMIO BARs.
Hi Ard, On jeu., juil. 06 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 6 July 2017 at 13:10, Gregory CLEMENT > <gregory.clement@free-electrons.com> wrote: >> Hi Ard, >> >> On jeu., juil. 06 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> >>> On 6 July 2017 at 07:31, Jisheng Zhang <jszhang@marvell.com> wrote: >>>> On Wed, 5 Jul 2017 18:44:03 +0100 Russell King - ARM Linux wrote: >>>> >>>>> On Wed, Jul 05, 2017 at 06:36:53PM +0100, Ard Biesheuvel wrote: >>>>> > On 5 July 2017 at 18:16, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: >>>>> > > On Wed, Jul 05, 2017 at 06:13:33PM +0200, Gregory CLEMENT wrote: >>>>> > >> Enable the PCIe interface on the MACCHIATOBin board. It is located on >>>>> > >> CON12 and is 4 lanes capable. >>>>> > >> >>>>> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >>>>> > > >>>>> > > Why do you folk at free-electrons like doing half a job all the friggin >>>>> > > time? >>>>> > > >>>>> > > You know I have complete patches for mcbin, but you pointedly won't look >>>>> > > at them at all - except when you have a problem and want to test my tree. >>>>> > > And even then, you ignore my work (despite testing that it works), and >>>>> > > you still recreate my patches. >>>>> > > >>>>> > > This is really frustrating and insane behaviour on your part. >>>>> > > >>>>> > > Here's what I have: >>>>> > > >>>>> > > +&cpm_pcie0 { >>>>> > > + pinctrl-names = "default"; >>>>> > > + pinctrl-0 = <&cpm_pcie_pins>; >>>>> > > + num-lanes = <4>; >>>>> > > + reset-gpio = <&cpm_gpio1 20 GPIO_ACTIVE_LOW>; >>>>> > > + status = "okay"; >>>>> > >>>>> > This needs 'num-viewport = <8>' as well, or the crazy Synopsys DWC >>>> >>>> IMHO, maybe putting this property into dtsi is better. >>>> >>> >>> Good point. Do all instances of this IP that live on the SoC have 8 >>> viewports? >> >> In the Armada 80x0 datasheet there is no mention of viewport. However 2 >> days ago you said that "What I do know is that boards like the Marvell >> 8040 based MacchiatoBin uses this IP in RC mode, and exposes config, >> MMIO and IO space windows using only 2 viewports. Note that this is >> essentially a bug in the DT description, given that its version of the >> IP supports 8 viewports."[1] >> >> So you seem to know that the version of the IP used support the 8 >> viewports! >> >> What I can also say is that the in the datasheet there is no mention of >> difference between the instance. The only thing that can differ is the >> number of lane for each PCIe Port. >> > > I am sure I found it in the datasheet somewhere. It is the number of > ATU windows. Under the "Outbound iATU Features" and "Inbound iATU Features" I have the following point: "Up to 8 address regions programmable for location and size." Is that what you think about ? > > In any case, I implemented ACPI firmware for the MacchiatoBin, which > also involves programming the PCIe RC in a mode that is compatible > with ECAM. This turns out to be impossible (which means you cannot > implement a SBSA compatible platform with this SoC), but in attempting > to do so, I used five different windows, for type 0 config cycles, > type 1 config cycles, IO space, MMIO32 space and MMIO64 space (which > the Synopsys DW PCIe controller does not even implement, > unfortunately) > > Note that this also involved reprogramming all the memory translation > windows from the secure firmware, to free up the region 0xc000_0000 - > 0xefff_ffff for use by PCIe (config and MMIO32 space), and assigning a > window 0x8_0000_0000 - 0x8_ffff_ffff for 64-bit MMIO BARs. Currently for the driver the only important information is having a num-viewport greater than 2. So given all this we are OK if we add the 'num-viewport = <8>' in all the PCIe port node in the dtsi file. Gregory
On 6 July 2017 at 13:55, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > Hi Ard, > > On jeu., juil. 06 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > >> On 6 July 2017 at 13:10, Gregory CLEMENT >> <gregory.clement@free-electrons.com> wrote: >>> Hi Ard, >>> >>> On jeu., juil. 06 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >>> >>>> On 6 July 2017 at 07:31, Jisheng Zhang <jszhang@marvell.com> wrote: >>>>> On Wed, 5 Jul 2017 18:44:03 +0100 Russell King - ARM Linux wrote: >>>>> >>>>>> On Wed, Jul 05, 2017 at 06:36:53PM +0100, Ard Biesheuvel wrote: >>>>>> > On 5 July 2017 at 18:16, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: >>>>>> > > On Wed, Jul 05, 2017 at 06:13:33PM +0200, Gregory CLEMENT wrote: >>>>>> > >> Enable the PCIe interface on the MACCHIATOBin board. It is located on >>>>>> > >> CON12 and is 4 lanes capable. >>>>>> > >> >>>>>> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >>>>>> > > >>>>>> > > Why do you folk at free-electrons like doing half a job all the friggin >>>>>> > > time? >>>>>> > > >>>>>> > > You know I have complete patches for mcbin, but you pointedly won't look >>>>>> > > at them at all - except when you have a problem and want to test my tree. >>>>>> > > And even then, you ignore my work (despite testing that it works), and >>>>>> > > you still recreate my patches. >>>>>> > > >>>>>> > > This is really frustrating and insane behaviour on your part. >>>>>> > > >>>>>> > > Here's what I have: >>>>>> > > >>>>>> > > +&cpm_pcie0 { >>>>>> > > + pinctrl-names = "default"; >>>>>> > > + pinctrl-0 = <&cpm_pcie_pins>; >>>>>> > > + num-lanes = <4>; >>>>>> > > + reset-gpio = <&cpm_gpio1 20 GPIO_ACTIVE_LOW>; >>>>>> > > + status = "okay"; >>>>>> > >>>>>> > This needs 'num-viewport = <8>' as well, or the crazy Synopsys DWC >>>>> >>>>> IMHO, maybe putting this property into dtsi is better. >>>>> >>>> >>>> Good point. Do all instances of this IP that live on the SoC have 8 >>>> viewports? >>> >>> In the Armada 80x0 datasheet there is no mention of viewport. However 2 >>> days ago you said that "What I do know is that boards like the Marvell >>> 8040 based MacchiatoBin uses this IP in RC mode, and exposes config, >>> MMIO and IO space windows using only 2 viewports. Note that this is >>> essentially a bug in the DT description, given that its version of the >>> IP supports 8 viewports."[1] >>> >>> So you seem to know that the version of the IP used support the 8 >>> viewports! >>> >>> What I can also say is that the in the datasheet there is no mention of >>> difference between the instance. The only thing that can differ is the >>> number of lane for each PCIe Port. >>> >> >> I am sure I found it in the datasheet somewhere. It is the number of >> ATU windows. > > Under the "Outbound iATU Features" and "Inbound iATU Features" I have > the following point: > "Up to 8 address regions programmable for location and size." > > Is that what you think about ? > Yes. >> >> In any case, I implemented ACPI firmware for the MacchiatoBin, which >> also involves programming the PCIe RC in a mode that is compatible >> with ECAM. This turns out to be impossible (which means you cannot >> implement a SBSA compatible platform with this SoC), but in attempting >> to do so, I used five different windows, for type 0 config cycles, >> type 1 config cycles, IO space, MMIO32 space and MMIO64 space (which >> the Synopsys DW PCIe controller does not even implement, >> unfortunately) >> >> Note that this also involved reprogramming all the memory translation >> windows from the secure firmware, to free up the region 0xc000_0000 - >> 0xefff_ffff for use by PCIe (config and MMIO32 space), and assigning a >> window 0x8_0000_0000 - 0x8_ffff_ffff for 64-bit MMIO BARs. > > Currently for the driver the only important information is having a > num-viewport greater than 2. So given all this we are OK if we add the > 'num-viewport = <8>' in all the PCIe port node in the dtsi file. > OK. If the x4 and the x1 versions of the description in the manual both contain the sentence about 8 the address regions, then I think it is safe to include it for all 6 instances of this IP.
diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts index 4968e731de61..e445c80213ab 100644 --- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts +++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts @@ -136,6 +136,12 @@ vqmmc-supply = <&v_3_3>; }; +&cpm_pcie0 { + /* CON 12 */ + num-lanes = <4>; + status = "okay"; +}; + &cpm_usb3_0 { /* J38? - USB2.0 only */ status = "okay";
Enable the PCIe interface on the MACCHIATOBin board. It is located on CON12 and is 4 lanes capable. Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 6 ++++++ 1 file changed, 6 insertions(+)