| Message ID | 20191102182608.30624-2-michael@walle.cc |
|---|---|
| State | Superseded |
| Delegated to: | Priyanka Jain |
| Headers | show |
| Series | [U-Boot,v2,1/2] spi: nxp_fspi: new driver for the FlexSPI controller | expand |
Hi Michael, > -----Original Message----- > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Michael Walle > Sent: Saturday, November 2, 2019 11:56 PM > To: u-boot@lists.denx.de > Subject: [EXT] [U-Boot] [PATCH v2 2/2] arm: ls1028a: use the new flexspi > driver > > Caution: EXT Email > > Also align the fspi node with the kernel one. There is actually no driver which > would match "nxp,dn-fspi". > > Signed-off-by: Michael Walle <michael@walle.cc> > --- > changes since v1: > - none > > arch/arm/dts/fsl-ls1028a.dtsi | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/dts/fsl-ls1028a.dtsi b/arch/arm/dts/fsl-ls1028a.dtsi index > 43a154e8e7..774e477542 100644 > --- a/arch/arm/dts/fsl-ls1028a.dtsi > +++ b/arch/arm/dts/fsl-ls1028a.dtsi > @@ -49,14 +49,16 @@ > <1 10 0x8>; /* Hypervisor PPI, active-low */ > }; > > - fspi: flexspi@20C0000 { > - compatible = "nxp,dn-fspi"; > + fspi: flexspi@20c0000 { > + compatible = "nxp,lx2160a-fspi"; > #address-cells = <1>; > #size-cells = <0>; > - reg = <0x0 0x20C0000 0x0 0x10000>, > - <0x0 0x20000000 0x0 0x10000000>; /*64MB flash*/ > - reg-names = "FSPI", "FSPI-memory"; > - num-cs = <1>; > + reg = <0x0 0x20c0000 0x0 0x10000>, > + <0x0 0x20000000 0x0 0x10000000>; > + reg-names = "fspi_base", "fspi_mmap"; > + clocks = <&clockgen 4 3>, <&clockgen 4 3>; > + clock-names = "fspi_en", "fspi"; > + interrupts = <0 25 0x4>; Please change the interrupts to "<GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>" as it avoids magic numbers. Reviewed-by: Kuldeep Singh <kuldeep.singh@nxp.com> > status = "disabled"; > }; > > -- > 2.20.1
Hi Singh, Am 2019-11-05 07:40, schrieb Kuldeep Singh: > Hi Michael, > >> -----Original Message----- >> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Michael Walle >> Sent: Saturday, November 2, 2019 11:56 PM >> To: u-boot@lists.denx.de >> Subject: [EXT] [U-Boot] [PATCH v2 2/2] arm: ls1028a: use the new >> flexspi >> driver >> >> Caution: EXT Email >> >> Also align the fspi node with the kernel one. There is actually no >> driver which >> would match "nxp,dn-fspi". >> >> Signed-off-by: Michael Walle <michael@walle.cc> >> --- >> changes since v1: >> - none >> >> arch/arm/dts/fsl-ls1028a.dtsi | 14 ++++++++------ >> 1 file changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/arch/arm/dts/fsl-ls1028a.dtsi >> b/arch/arm/dts/fsl-ls1028a.dtsi index >> 43a154e8e7..774e477542 100644 >> --- a/arch/arm/dts/fsl-ls1028a.dtsi >> +++ b/arch/arm/dts/fsl-ls1028a.dtsi >> @@ -49,14 +49,16 @@ >> <1 10 0x8>; /* Hypervisor PPI, active-low >> */ >> }; >> >> - fspi: flexspi@20C0000 { >> - compatible = "nxp,dn-fspi"; >> + fspi: flexspi@20c0000 { >> + compatible = "nxp,lx2160a-fspi"; >> #address-cells = <1>; >> #size-cells = <0>; >> - reg = <0x0 0x20C0000 0x0 0x10000>, >> - <0x0 0x20000000 0x0 0x10000000>; /*64MB >> flash*/ >> - reg-names = "FSPI", "FSPI-memory"; >> - num-cs = <1>; >> + reg = <0x0 0x20c0000 0x0 0x10000>, >> + <0x0 0x20000000 0x0 0x10000000>; >> + reg-names = "fspi_base", "fspi_mmap"; >> + clocks = <&clockgen 4 3>, <&clockgen 4 3>; >> + clock-names = "fspi_en", "fspi"; >> + interrupts = <0 25 0x4>; > > Please change the interrupts to "<GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>" as > it avoids magic numbers. This is consistent with the other interrupt properties, which doesn't have these constants either. IMHO another commit where all these magic numbers are removed would be better. > Reviewed-by: Kuldeep Singh <kuldeep.singh@nxp.com> Thanks. -michael
Hi Michael, > -----Original Message----- > From: Michael Walle <michael@walle.cc> > Sent: Tuesday, November 5, 2019 3:07 PM > To: Kuldeep Singh <kuldeep.singh@nxp.com> > Cc: u-boot@lists.denx.de > Subject: Re: [EXT] [U-Boot] [PATCH v2 2/2] arm: ls1028a: use the new flexspi > driver > > Caution: EXT Email > > Hi Singh, > > Am 2019-11-05 07:40, schrieb Kuldeep Singh: > > Hi Michael, > > > >> -----Original Message----- > >> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Michael > >> Walle > >> Sent: Saturday, November 2, 2019 11:56 PM > >> To: u-boot@lists.denx.de > >> Subject: [EXT] [U-Boot] [PATCH v2 2/2] arm: ls1028a: use the new > >> flexspi driver > >> > >> Caution: EXT Email > >> > >> Also align the fspi node with the kernel one. There is actually no > >> driver which would match "nxp,dn-fspi". > >> > >> Signed-off-by: Michael Walle <michael@walle.cc> > >> --- > >> changes since v1: > >> - none > >> > >> arch/arm/dts/fsl-ls1028a.dtsi | 14 ++++++++------ > >> 1 file changed, 8 insertions(+), 6 deletions(-) > >> > >> diff --git a/arch/arm/dts/fsl-ls1028a.dtsi > >> b/arch/arm/dts/fsl-ls1028a.dtsi index > >> 43a154e8e7..774e477542 100644 > >> --- a/arch/arm/dts/fsl-ls1028a.dtsi > >> +++ b/arch/arm/dts/fsl-ls1028a.dtsi > >> @@ -49,14 +49,16 @@ > >> <1 10 0x8>; /* Hypervisor PPI, > >> active-low */ > >> }; > >> > >> - fspi: flexspi@20C0000 { > >> - compatible = "nxp,dn-fspi"; > >> + fspi: flexspi@20c0000 { > >> + compatible = "nxp,lx2160a-fspi"; > >> #address-cells = <1>; > >> #size-cells = <0>; > >> - reg = <0x0 0x20C0000 0x0 0x10000>, > >> - <0x0 0x20000000 0x0 0x10000000>; /*64MB > >> flash*/ > >> - reg-names = "FSPI", "FSPI-memory"; > >> - num-cs = <1>; > >> + reg = <0x0 0x20c0000 0x0 0x10000>, > >> + <0x0 0x20000000 0x0 0x10000000>; > >> + reg-names = "fspi_base", "fspi_mmap"; > >> + clocks = <&clockgen 4 3>, <&clockgen 4 3>; > >> + clock-names = "fspi_en", "fspi"; > >> + interrupts = <0 25 0x4>; > > > > Please change the interrupts to "<GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>" as > > it avoids magic numbers. > > This is consistent with the other interrupt properties, which doesn't have these > constants either. IMHO another commit where all these magic numbers are > removed would be better. > I just checked and found no interrupt handler in spi/nxp_fspi.c driver. So, it's better to remove this line altogether. What do you think? -Kuldeep > > > Reviewed-by: Kuldeep Singh <kuldeep.singh@nxp.com> > Thanks. > > -michael
Am 2019-11-05 10:53, schrieb Kuldeep Singh: Hi Kuldeep, >> >> diff --git a/arch/arm/dts/fsl-ls1028a.dtsi >> >> b/arch/arm/dts/fsl-ls1028a.dtsi index >> >> 43a154e8e7..774e477542 100644 >> >> --- a/arch/arm/dts/fsl-ls1028a.dtsi >> >> +++ b/arch/arm/dts/fsl-ls1028a.dtsi >> >> @@ -49,14 +49,16 @@ >> >> <1 10 0x8>; /* Hypervisor PPI, >> >> active-low */ >> >> }; >> >> >> >> - fspi: flexspi@20C0000 { >> >> - compatible = "nxp,dn-fspi"; >> >> + fspi: flexspi@20c0000 { >> >> + compatible = "nxp,lx2160a-fspi"; >> >> #address-cells = <1>; >> >> #size-cells = <0>; >> >> - reg = <0x0 0x20C0000 0x0 0x10000>, >> >> - <0x0 0x20000000 0x0 0x10000000>; /*64MB >> >> flash*/ >> >> - reg-names = "FSPI", "FSPI-memory"; >> >> - num-cs = <1>; >> >> + reg = <0x0 0x20c0000 0x0 0x10000>, >> >> + <0x0 0x20000000 0x0 0x10000000>; >> >> + reg-names = "fspi_base", "fspi_mmap"; >> >> + clocks = <&clockgen 4 3>, <&clockgen 4 3>; >> >> + clock-names = "fspi_en", "fspi"; >> >> + interrupts = <0 25 0x4>; >> > >> > Please change the interrupts to "<GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>" as >> > it avoids magic numbers. >> >> This is consistent with the other interrupt properties, which doesn't >> have these >> constants either. IMHO another commit where all these magic numbers >> are >> removed would be better. >> > > I just checked and found no interrupt handler in spi/nxp_fspi.c > driver. So, it's better to remove this line altogether. What do you > think? For the sake of consistency (with all other nodes and with the linux device tree) and if someone would like to sync the device tree with the linux one, I'd keep that. Like I said before, I'd do another patch were all interrupt magic numbers are replaced with the correct constants. -michael
diff --git a/arch/arm/dts/fsl-ls1028a.dtsi b/arch/arm/dts/fsl-ls1028a.dtsi index 43a154e8e7..774e477542 100644 --- a/arch/arm/dts/fsl-ls1028a.dtsi +++ b/arch/arm/dts/fsl-ls1028a.dtsi @@ -49,14 +49,16 @@ <1 10 0x8>; /* Hypervisor PPI, active-low */ }; - fspi: flexspi@20C0000 { - compatible = "nxp,dn-fspi"; + fspi: flexspi@20c0000 { + compatible = "nxp,lx2160a-fspi"; #address-cells = <1>; #size-cells = <0>; - reg = <0x0 0x20C0000 0x0 0x10000>, - <0x0 0x20000000 0x0 0x10000000>; /*64MB flash*/ - reg-names = "FSPI", "FSPI-memory"; - num-cs = <1>; + reg = <0x0 0x20c0000 0x0 0x10000>, + <0x0 0x20000000 0x0 0x10000000>; + reg-names = "fspi_base", "fspi_mmap"; + clocks = <&clockgen 4 3>, <&clockgen 4 3>; + clock-names = "fspi_en", "fspi"; + interrupts = <0 25 0x4>; status = "disabled"; };
Also align the fspi node with the kernel one. There is actually no driver which would match "nxp,dn-fspi". Signed-off-by: Michael Walle <michael@walle.cc> --- changes since v1: - none arch/arm/dts/fsl-ls1028a.dtsi | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)