| Message ID | 1527754134-164985-6-git-send-email-tien.fong.chee@intel.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Add DMA driver for DMA330 controller | expand |
On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote: > From: Tien Fong Chee <tien.fong.chee@intel.com> > > Update pdma properties for Stratix 10 > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com> > --- > arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org> Is there a DT binding file for this somewhere?
On Fri, 2018-06-01 at 08:25 -0600, Simon Glass wrote: > On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote: > > > > From: Tien Fong Chee <tien.fong.chee@intel.com> > > > > Update pdma properties for Stratix 10 > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com> > > --- > > arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > Reviewed-by: Simon Glass <sjg@chromium.org> > > Is there a DT binding file for this somewhere? Yeah, it is in linux/Documentation https://github.com/torvalds/linux/blob/master/Documentation/devicetree/ bindings/dma/arm-pl330.txt I can port over to U-boot.
On 05/31/2018 03:08 AM, tien.fong.chee@intel.com wrote: > From: Tien Fong Chee <tien.fong.chee@intel.com> > > Update pdma properties for Stratix 10 > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com> > --- > arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/arch/arm/dts/socfpga_stratix10.dtsi b/arch/arm/dts/socfpga_stratix10.dtsi > index ccd3f32..311ba09 100644 > --- a/arch/arm/dts/socfpga_stratix10.dtsi > +++ b/arch/arm/dts/socfpga_stratix10.dtsi > @@ -82,6 +82,26 @@ > ranges = <0 0 0 0xffffffff>; > u-boot,dm-pre-reloc; > > + amba { > + u-boot,dm-pre-reloc; > + compatible = "arm,amba-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + pdma: pdma@ffda0000 { > + u-boot,dm-pre-reloc; > + compatible = "arm,pl330", "arm,dma330"; I think you got "arm,dma330" binding wrong. I don't see any binding with that name. Dinh
On 07/09/2018 08:03 PM, Dinh Nguyen wrote: > > > On 05/31/2018 03:08 AM, tien.fong.chee@intel.com wrote: >> From: Tien Fong Chee <tien.fong.chee@intel.com> >> >> Update pdma properties for Stratix 10 >> >> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com> >> --- >> arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/arch/arm/dts/socfpga_stratix10.dtsi b/arch/arm/dts/socfpga_stratix10.dtsi >> index ccd3f32..311ba09 100644 >> --- a/arch/arm/dts/socfpga_stratix10.dtsi >> +++ b/arch/arm/dts/socfpga_stratix10.dtsi >> @@ -82,6 +82,26 @@ >> ranges = <0 0 0 0xffffffff>; >> u-boot,dm-pre-reloc; >> >> + amba { >> + u-boot,dm-pre-reloc; >> + compatible = "arm,amba-bus"; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges; >> + >> + pdma: pdma@ffda0000 { >> + u-boot,dm-pre-reloc; >> + compatible = "arm,pl330", "arm,dma330"; > > I think you got "arm,dma330" binding wrong. I don't see any binding with > that name. I think the whole idea of using pl330 to scrub ECC DRAM is wrong. It adds massive amount of code while a CPU can do the same and faster, cfr arria10.
On Mon, 2018-07-09 at 22:28 +0200, Marek Vasut wrote: > On 07/09/2018 08:03 PM, Dinh Nguyen wrote: > > > > > > > > On 05/31/2018 03:08 AM, tien.fong.chee@intel.com wrote: > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com> > > > > > > Update pdma properties for Stratix 10 > > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com> > > > --- > > > arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++ > > > 1 file changed, 20 insertions(+) > > > > > > diff --git a/arch/arm/dts/socfpga_stratix10.dtsi > > > b/arch/arm/dts/socfpga_stratix10.dtsi > > > index ccd3f32..311ba09 100644 > > > --- a/arch/arm/dts/socfpga_stratix10.dtsi > > > +++ b/arch/arm/dts/socfpga_stratix10.dtsi > > > @@ -82,6 +82,26 @@ > > > ranges = <0 0 0 0xffffffff>; > > > u-boot,dm-pre-reloc; > > > > > > + amba { > > > + u-boot,dm-pre-reloc; > > > + compatible = "arm,amba-bus"; > > > + #address-cells = <1>; > > > + #size-cells = <1>; > > > + ranges; > > > + > > > + pdma: pdma@ffda0000 { > > > + u-boot,dm-pre-reloc; > > > + compatible = > > > "arm,pl330", "arm,dma330"; > > I think you got "arm,dma330" binding wrong. I don't see any binding > > with > > that name. Here https://patchwork.ozlabs.org/patch/923234/ . > I think the whole idea of using pl330 to scrub ECC DRAM is wrong. It > adds massive amount of code while a CPU can do the same and faster, > cfr > arria10. > I just measured the performance of initializing DRAM based on custodian arria10_sdmmc, which is around 16sec with 1GB. Using DMA to init the DDR, which is around 3-4sec for 2GB. I attached screenshot for the print out based on arria10_sdmmc custodian.
On 07/10/2018 08:11 AM, Chee, Tien Fong wrote: > On Mon, 2018-07-09 at 22:28 +0200, Marek Vasut wrote: >> On 07/09/2018 08:03 PM, Dinh Nguyen wrote: >>> >>> >>> >>> On 05/31/2018 03:08 AM, tien.fong.chee@intel.com wrote: >>>> >>>> From: Tien Fong Chee <tien.fong.chee@intel.com> >>>> >>>> Update pdma properties for Stratix 10 >>>> >>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com> >>>> --- >>>> arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++ >>>> 1 file changed, 20 insertions(+) >>>> >>>> diff --git a/arch/arm/dts/socfpga_stratix10.dtsi >>>> b/arch/arm/dts/socfpga_stratix10.dtsi >>>> index ccd3f32..311ba09 100644 >>>> --- a/arch/arm/dts/socfpga_stratix10.dtsi >>>> +++ b/arch/arm/dts/socfpga_stratix10.dtsi >>>> @@ -82,6 +82,26 @@ >>>> ranges = <0 0 0 0xffffffff>; >>>> u-boot,dm-pre-reloc; >>>> >>>> + amba { >>>> + u-boot,dm-pre-reloc; >>>> + compatible = "arm,amba-bus"; >>>> + #address-cells = <1>; >>>> + #size-cells = <1>; >>>> + ranges; >>>> + >>>> + pdma: pdma@ffda0000 { >>>> + u-boot,dm-pre-reloc; >>>> + compatible = >>>> "arm,pl330", "arm,dma330"; >>> I think you got "arm,dma330" binding wrong. I don't see any binding >>> with >>> that name. > Here https://patchwork.ozlabs.org/patch/923234/ . Oh okay...why do you need to add the additional binding "arm,dma330"? Dinh
On 07/10/2018 03:11 PM, Chee, Tien Fong wrote: > On Mon, 2018-07-09 at 22:28 +0200, Marek Vasut wrote: >> On 07/09/2018 08:03 PM, Dinh Nguyen wrote: >>> >>> >>> >>> On 05/31/2018 03:08 AM, tien.fong.chee@intel.com wrote: >>>> >>>> From: Tien Fong Chee <tien.fong.chee@intel.com> >>>> >>>> Update pdma properties for Stratix 10 >>>> >>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com> >>>> --- >>>> arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++ >>>> 1 file changed, 20 insertions(+) >>>> >>>> diff --git a/arch/arm/dts/socfpga_stratix10.dtsi >>>> b/arch/arm/dts/socfpga_stratix10.dtsi >>>> index ccd3f32..311ba09 100644 >>>> --- a/arch/arm/dts/socfpga_stratix10.dtsi >>>> +++ b/arch/arm/dts/socfpga_stratix10.dtsi >>>> @@ -82,6 +82,26 @@ >>>> ranges = <0 0 0 0xffffffff>; >>>> u-boot,dm-pre-reloc; >>>> >>>> + amba { >>>> + u-boot,dm-pre-reloc; >>>> + compatible = "arm,amba-bus"; >>>> + #address-cells = <1>; >>>> + #size-cells = <1>; >>>> + ranges; >>>> + >>>> + pdma: pdma@ffda0000 { >>>> + u-boot,dm-pre-reloc; >>>> + compatible = >>>> "arm,pl330", "arm,dma330"; >>> I think you got "arm,dma330" binding wrong. I don't see any binding >>> with >>> that name. > Here https://patchwork.ozlabs.org/patch/923234/ . >> I think the whole idea of using pl330 to scrub ECC DRAM is wrong. It >> adds massive amount of code while a CPU can do the same and faster, >> cfr >> arria10. >> > I just measured the performance of initializing DRAM based on custodian > arria10_sdmmc, which is around 16sec with 1GB. Then you're doing something very wrong, it should be around 1 second. See ie. "SoCFPGA PL330 DMA driver and ECC scrubbing" thread > Using DMA to init the DDR, which is around 3-4sec for 2GB. I attached > screenshot for the print out based on arria10_sdmmc custodian. Well sure, if you do it wrong, it'll take longer for the CPU.
On Tue, 2018-07-10 at 08:37 -0500, Dinh Nguyen wrote: > > On 07/10/2018 08:11 AM, Chee, Tien Fong wrote: > > > > On Mon, 2018-07-09 at 22:28 +0200, Marek Vasut wrote: > > > > > > On 07/09/2018 08:03 PM, Dinh Nguyen wrote: > > > > > > > > > > > > > > > > > > > > On 05/31/2018 03:08 AM, tien.fong.chee@intel.com wrote: > > > > > > > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com> > > > > > > > > > > Update pdma properties for Stratix 10 > > > > > > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com> > > > > > --- > > > > > arch/arm/dts/socfpga_stratix10.dtsi | 20 > > > > > ++++++++++++++++++++ > > > > > 1 file changed, 20 insertions(+) > > > > > > > > > > diff --git a/arch/arm/dts/socfpga_stratix10.dtsi > > > > > b/arch/arm/dts/socfpga_stratix10.dtsi > > > > > index ccd3f32..311ba09 100644 > > > > > --- a/arch/arm/dts/socfpga_stratix10.dtsi > > > > > +++ b/arch/arm/dts/socfpga_stratix10.dtsi > > > > > @@ -82,6 +82,26 @@ > > > > > ranges = <0 0 0 0xffffffff>; > > > > > u-boot,dm-pre-reloc; > > > > > > > > > > + amba { > > > > > + u-boot,dm-pre-reloc; > > > > > + compatible = "arm,amba-bus"; > > > > > + #address-cells = <1>; > > > > > + #size-cells = <1>; > > > > > + ranges; > > > > > + > > > > > + pdma: pdma@ffda0000 { > > > > > + u-boot,dm-pre-reloc; > > > > > + compatible = > > > > > "arm,pl330", "arm,dma330"; > > > > I think you got "arm,dma330" binding wrong. I don't see any > > > > binding > > > > with > > > > that name. > > Here https://patchwork.ozlabs.org/patch/923234/ . > Oh okay...why do you need to add the additional binding "arm,dma330"? pl330 is old name, now arm using dma330. > > Dinh
diff --git a/arch/arm/dts/socfpga_stratix10.dtsi b/arch/arm/dts/socfpga_stratix10.dtsi index ccd3f32..311ba09 100644 --- a/arch/arm/dts/socfpga_stratix10.dtsi +++ b/arch/arm/dts/socfpga_stratix10.dtsi @@ -82,6 +82,26 @@ ranges = <0 0 0 0xffffffff>; u-boot,dm-pre-reloc; + amba { + u-boot,dm-pre-reloc; + compatible = "arm,amba-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + pdma: pdma@ffda0000 { + u-boot,dm-pre-reloc; + compatible = "arm,pl330", "arm,dma330"; + reg = <0xffda0000 0x2000>; + #dma-cells = <1>; + #dma-channels = <8>; + #dma-requests = <32>; + dma-max-burst-size = <2>; + dma-burst-size = <2>; + dma-burst-length = <16>; + }; + }; + clkmgr@ffd1000 { compatible = "altr,clk-mgr"; reg = <0xffd10000 0x1000>;