Re: [U-Boot] [PATCH 8/9] pci: layerscape: rewrite pci driver based on DM
Hi Bin, Thanks for your suggestion. Please see my comments inline. Thanks, Minghuan > -Original Message- > From: Bin Meng [mailto:bmeng...@gmail.com] > Sent: Monday, October 10, 2016 7:49 PM > To: M.H. Lian ; Simon Glass > Cc: U-Boot Mailing List ; Mingkai Hu > ; Leo Li > Subject: Re: [U-Boot] [PATCH 8/9] pci: layerscape: rewrite pci driver based on > DM > > Hi Minghuan, > > On Mon, Oct 10, 2016 at 4:47 PM, Minghuan Lian > wrote: > > There are more than five kinds of Layerscape SoCs. unfortunately, PCIe > > controller of each SoC is a little bit different. In order to avoid > > too many macro definitions, the patch re-implement PCIe driver based > > on DM. PCIe dts node is to describe the difference. > > > > Signed-off-by: Minghuan Lian > > --- > > .../include/asm/arch-fsl-layerscape/immap_lsch3.h | 8 - > > drivers/pci/Kconfig| 8 + > > drivers/pci/pcie_layerscape.c | 958 > > +++-- > > include/configs/ls1012a_common.h | 12 + > > include/configs/ls1012aqds.h | 24 - > > include/configs/ls1012ardb.h | 24 - > > include/configs/ls1021aqds.h | 18 +- > > include/configs/ls1021atwr.h | 18 +- > > include/configs/ls1043a_common.h | 23 +- > > include/configs/ls2080a_common.h | 27 +- > > include/configs/ls2080aqds.h | 8 - > > include/configs/ls2080ardb.h | 8 - > > These header file changes should not be put in the same commit of the > layerscape PCIe driver conversion. They should be in a separate commit. So > you are likely to have 3 commits: firstly add DM codes with #ifdef > CONFIG_DM_PCI #endif in the layerscape PCIe driver, without breaking the > existing board support. 2nd commit to update the boards configuration files > (defconfig and the header), and 3rd commit to remove the #ifdef > CONFIG_DM_PCI #endif, only leaving the DM version codes. [Minghuan Lian] Your suggestion is excellent. I will change the patch. For the header and defconfig files, do I need to separate them to several patches for different SoC? I mean a patch is for ls1012, a patch is for ls1043 ... > > > 12 files changed, 526 insertions(+), 610 deletions(-) > > > > diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > > b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > > index 7acba27..bd07808 100644 > > --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > > +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > > @@ -104,14 +104,6 @@ > > #define CONFIG_SYS_PCIE2_PHYS_ADDR 0x12ULL > > #define CONFIG_SYS_PCIE3_PHYS_ADDR 0x14ULL > > #define CONFIG_SYS_PCIE4_PHYS_ADDR 0x16ULL > > -/* LUT registers */ > > -#define PCIE_LUT_BASE 0x8 > > -#define PCIE_LUT_LCTRL00x7F8 > > -#define PCIE_LUT_DBG 0x7FC > > -#define PCIE_LUT_UDR(n) (0x800 + (n) * 8) > > -#define PCIE_LUT_LDR(n) (0x804 + (n) * 8) > > -#define PCIE_LUT_ENABLE (1 << 31) > > -#define PCIE_LUT_ENTRY_COUNT32 > > > > [snip] > > Regards, > Bin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 8/9] pci: layerscape: rewrite pci driver based on DM
Hi Minghuan, On Tue, Oct 11, 2016 at 3:21 PM, M.H. Lian wrote: > Hi Bin, > > Thanks for your suggestion. > Please see my comments inline. > > Thanks, > Minghuan > >> -Original Message- >> From: Bin Meng [mailto:bmeng...@gmail.com] >> Sent: Monday, October 10, 2016 7:49 PM >> To: M.H. Lian ; Simon Glass >> Cc: U-Boot Mailing List ; Mingkai Hu >> ; Leo Li >> Subject: Re: [U-Boot] [PATCH 8/9] pci: layerscape: rewrite pci driver based >> on >> DM >> >> Hi Minghuan, >> >> On Mon, Oct 10, 2016 at 4:47 PM, Minghuan Lian >> wrote: >> > There are more than five kinds of Layerscape SoCs. unfortunately, PCIe >> > controller of each SoC is a little bit different. In order to avoid >> > too many macro definitions, the patch re-implement PCIe driver based >> > on DM. PCIe dts node is to describe the difference. >> > >> > Signed-off-by: Minghuan Lian >> > --- >> > .../include/asm/arch-fsl-layerscape/immap_lsch3.h | 8 - >> > drivers/pci/Kconfig| 8 + >> > drivers/pci/pcie_layerscape.c | 958 >> > +++-- >> > include/configs/ls1012a_common.h | 12 + >> > include/configs/ls1012aqds.h | 24 - >> > include/configs/ls1012ardb.h | 24 - >> > include/configs/ls1021aqds.h | 18 +- >> > include/configs/ls1021atwr.h | 18 +- >> > include/configs/ls1043a_common.h | 23 +- >> > include/configs/ls2080a_common.h | 27 +- >> > include/configs/ls2080aqds.h | 8 - >> > include/configs/ls2080ardb.h | 8 - >> >> These header file changes should not be put in the same commit of the >> layerscape PCIe driver conversion. They should be in a separate commit. So >> you are likely to have 3 commits: firstly add DM codes with #ifdef >> CONFIG_DM_PCI #endif in the layerscape PCIe driver, without breaking the >> existing board support. 2nd commit to update the boards configuration files >> (defconfig and the header), and 3rd commit to remove the #ifdef >> CONFIG_DM_PCI #endif, only leaving the DM version codes. > > [Minghuan Lian] Your suggestion is excellent. I will change the patch. > For the header and defconfig files, do I need to separate them to several > patches for different SoC? > I mean a patch is for ls1012, a patch is for ls1043 ... It depends on how you organize your patch series. We should make every commit bisectable, eg: the commit itself does not break build or any functionality. If all SoC header files need to change all together, it should be in one patch. Otherwise, you can put them into separate patch. Regards, Bin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 8/9] pci: layerscape: rewrite pci driver based on DM
Hi Minghuan, On Mon, Oct 10, 2016 at 4:47 PM, Minghuan Lian wrote: > There are more than five kinds of Layerscape SoCs. unfortunately, > PCIe controller of each SoC is a little bit different. In order > to avoid too many macro definitions, the patch re-implement PCIe > driver based on DM. PCIe dts node is to describe the difference. > > Signed-off-by: Minghuan Lian > --- > .../include/asm/arch-fsl-layerscape/immap_lsch3.h | 8 - > drivers/pci/Kconfig| 8 + > drivers/pci/pcie_layerscape.c | 958 > +++-- > include/configs/ls1012a_common.h | 12 + > include/configs/ls1012aqds.h | 24 - > include/configs/ls1012ardb.h | 24 - > include/configs/ls1021aqds.h | 18 +- > include/configs/ls1021atwr.h | 18 +- > include/configs/ls1043a_common.h | 23 +- > include/configs/ls2080a_common.h | 27 +- > include/configs/ls2080aqds.h | 8 - > include/configs/ls2080ardb.h | 8 - These header file changes should not be put in the same commit of the layerscape PCIe driver conversion. They should be in a separate commit. So you are likely to have 3 commits: firstly add DM codes with #ifdef CONFIG_DM_PCI #endif in the layerscape PCIe driver, without breaking the existing board support. 2nd commit to update the boards configuration files (defconfig and the header), and 3rd commit to remove the #ifdef CONFIG_DM_PCI #endif, only leaving the DM version codes. > 12 files changed, 526 insertions(+), 610 deletions(-) > > diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > index 7acba27..bd07808 100644 > --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > @@ -104,14 +104,6 @@ > #define CONFIG_SYS_PCIE2_PHYS_ADDR 0x12ULL > #define CONFIG_SYS_PCIE3_PHYS_ADDR 0x14ULL > #define CONFIG_SYS_PCIE4_PHYS_ADDR 0x16ULL > -/* LUT registers */ > -#define PCIE_LUT_BASE 0x8 > -#define PCIE_LUT_LCTRL00x7F8 > -#define PCIE_LUT_DBG 0x7FC > -#define PCIE_LUT_UDR(n) (0x800 + (n) * 8) > -#define PCIE_LUT_LDR(n) (0x804 + (n) * 8) > -#define PCIE_LUT_ENABLE (1 << 31) > -#define PCIE_LUT_ENTRY_COUNT32 > [snip] Regards, Bin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
