RE: [PATCH v2 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
Hi Frieder,
> -Original Message-
> From: Frieder Schrempf [mailto:frieder.schre...@exceet.de]
> Sent: Tuesday, September 18, 2018 1:52 PM
> To: Boris Brezillon ; Yogesh Narayan Gaur
>
> Cc: linux-...@lists.infradead.org; marek.va...@gmail.com; linux-
> s...@vger.kernel.org; devicet...@vger.kernel.org; r...@kernel.org;
> mark.rutl...@arm.com; shawn...@kernel.org; linux-arm-
> ker...@lists.infradead.org; computersforpe...@gmail.com; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH v2 1/5] spi: spi-mem: Add driver for NXP FlexSPI
> controller
>
> Hi Boris, Yogesh,
>
> On 17.09.2018 13:37, Boris Brezillon wrote:
> > Hi Yogesh,
> >
> > On Mon, 17 Sep 2018 15:18:26 +0530
> > Yogesh Gaur wrote:
> >
> >> +
> >> + /*
> >> + * R/W functions for big- or little-endian registers:
> >> + * The FSPI controller's endianness is independent of
> >> + * the CPU core's endianness. So far, although the CPU
> >> + * core is little-endian the FSPI controller can use
> >> + * big-endian or little-endian.
> >> + */
> >> + if (of_property_read_bool(np, "big-endian")) {
> >> + f->write = fspi_writel_be;
> >> + f->read = fspi_readl_be;
> >> + } else {
> >> + f->write = fspi_writel;
> >> + f->read = fspi_readl;
> >> + }
> >
> > Hm, isn't it something you can extract from the compatible string? I'd
> > rather not allow users to set that in their DT if it's not something
> > you can change.
>
> This was copied from the QSPI driver, but I think Boris is right. This seems
> to be a
> fixed SOC-specific setting and we shouldn't set it in the DT. This applies to
> QSPI
> and FSPI alike.
>
Yes, it can be modified. Instead of reading endianness property from device
tree can be moved to the platform structure nxp_fspi_devtype_data which is
linked with the compatible string
{ .compatible = "nxp,lx2160a-fspi", .data = (void *)_data, }
Thanks.
Would send the changes in next version.
--
Regards,
Yogesh Gaur.
> Regards,
> Frieder
>
> >
> > Regards,
> >
> > Boris
> >
RE: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Thursday, September 6, 2018 5:14 PM > To: Yogesh Narayan Gaur > Cc: Frieder Schrempf ; linux- > m...@lists.infradead.org; marek.va...@gmail.com; linux-...@vger.kernel.org; > devicet...@vger.kernel.org; r...@kernel.org; mark.rutl...@arm.com; > shawn...@kernel.org; linux-arm-ker...@lists.infradead.org; > computersforpe...@gmail.com; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI controller > > On Thu, 6 Sep 2018 11:35:13 + > Yogesh Narayan Gaur wrote: > > > Hi Frieder, > > > > > -Original Message- > > > From: Frieder Schrempf [mailto:frieder.schre...@exceet.de] > > > Sent: Thursday, September 6, 2018 1:56 PM > > > To: Yogesh Narayan Gaur ; Boris > > > Brezillon > > > Cc: linux-...@lists.infradead.org; marek.va...@gmail.com; linux- > > > s...@vger.kernel.org; devicet...@vger.kernel.org; r...@kernel.org; > > > mark.rutl...@arm.com; shawn...@kernel.org; linux-arm- > > > ker...@lists.infradead.org; computersforpe...@gmail.com; linux- > > > ker...@vger.kernel.org > > > Subject: Re: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI > > > controller > > > >> Hi Yogesh, > > > >> > > > >> On Fri, 31 Aug 2018 16:00:00 +0530 Yogesh Gaur > > > >> wrote: > > > >> > > > >>> - Add a driver for NXP FlexSPI host controller > > > >>> > > > >> > > > >> Yep, I had a quick look at the code and they really look similar. > > > >> Why not extending the existing driver instead of creating a new > > > >> one from scratch? > > > > > > > > FlexSPI is different controller not related to the QuadSPI > > > > controller in its working behavior Both these controller are > > > > having > > > > * Different registers name, registers address, registers > > > > functionality etc, section 30.5.2[1] > > > > * Different programming sequence for initialization of the > > > > controller, section 30.8.1[1] > > > > * Different programming sequence for performing Read and Write > > > > operation using IP, section 30.7.9[1] and AHB mode > > > > * Different programming sequence for checking controller current > > > > state like busy or not > > > > * New mechanism to program for the connected slave device i.e. > > > > flash access mode and flash memory map 30.7.4[1] and 30.7.5[1] > > > > * New entries added for FlexSPI controller for LUT_XX mode for > > > > various commands, section 30.7.8[1] > > > > > > > > There are few similarities between these two like LUT programming > > > > logic is same i.e. LUT needs to be programmed in same sequence of > > > > 'INSTR > > > PAD OPCODE', but again LUT register address and LUT command mode > > > values are different. > > > > > > > > Creating common driver for both FlexSPI and QuadSPI controller, > > > > would > > > involve creation of one more layer between driver/spi/spi-xxx and > > > the actual controller driver, this layer would going to have less > > > functionality like doing LUT creation programming and then would > > > re-direct calls to the respective controller driver functionality to > > > perform desired programming sequence. > > > > > > > >>> > > > >>> (1) The FlexSPI controller is driven by the LUT(Look-up Table) > > > >>> registers. > > > >>> The LUT registers are a look-up-table for sequences of > > > >>> instructions. A valid sequence consists of four LUT registers. > > > >>> Maximum 32 LUT sequences can be programmed simultaneously. > > > >>> > > > >> > > > >> Do we really want to have this level of details in the commit > > > >> message? I mean, this is something you can document in the > > > >> driver, but not here. > > > >> > > > >> BTW, you might want to have a look at [1]. I think we can get rid > > > >> of the ->size field you're adding if this driver implements the > > > >> dirmap hooks. > > > > > > > > I need size information for the connected device to program the > > > > controller > > > register FLSHXXCRO as Flash Chip select is determined
RE: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Thursday, September 6, 2018 5:14 PM > To: Yogesh Narayan Gaur > Cc: Frieder Schrempf ; linux- > m...@lists.infradead.org; marek.va...@gmail.com; linux-...@vger.kernel.org; > devicet...@vger.kernel.org; r...@kernel.org; mark.rutl...@arm.com; > shawn...@kernel.org; linux-arm-ker...@lists.infradead.org; > computersforpe...@gmail.com; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI controller > > On Thu, 6 Sep 2018 11:35:13 + > Yogesh Narayan Gaur wrote: > > > Hi Frieder, > > > > > -Original Message- > > > From: Frieder Schrempf [mailto:frieder.schre...@exceet.de] > > > Sent: Thursday, September 6, 2018 1:56 PM > > > To: Yogesh Narayan Gaur ; Boris > > > Brezillon > > > Cc: linux-...@lists.infradead.org; marek.va...@gmail.com; linux- > > > s...@vger.kernel.org; devicet...@vger.kernel.org; r...@kernel.org; > > > mark.rutl...@arm.com; shawn...@kernel.org; linux-arm- > > > ker...@lists.infradead.org; computersforpe...@gmail.com; linux- > > > ker...@vger.kernel.org > > > Subject: Re: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI > > > controller > > > >> Hi Yogesh, > > > >> > > > >> On Fri, 31 Aug 2018 16:00:00 +0530 Yogesh Gaur > > > >> wrote: > > > >> > > > >>> - Add a driver for NXP FlexSPI host controller > > > >>> > > > >> > > > >> Yep, I had a quick look at the code and they really look similar. > > > >> Why not extending the existing driver instead of creating a new > > > >> one from scratch? > > > > > > > > FlexSPI is different controller not related to the QuadSPI > > > > controller in its working behavior Both these controller are > > > > having > > > > * Different registers name, registers address, registers > > > > functionality etc, section 30.5.2[1] > > > > * Different programming sequence for initialization of the > > > > controller, section 30.8.1[1] > > > > * Different programming sequence for performing Read and Write > > > > operation using IP, section 30.7.9[1] and AHB mode > > > > * Different programming sequence for checking controller current > > > > state like busy or not > > > > * New mechanism to program for the connected slave device i.e. > > > > flash access mode and flash memory map 30.7.4[1] and 30.7.5[1] > > > > * New entries added for FlexSPI controller for LUT_XX mode for > > > > various commands, section 30.7.8[1] > > > > > > > > There are few similarities between these two like LUT programming > > > > logic is same i.e. LUT needs to be programmed in same sequence of > > > > 'INSTR > > > PAD OPCODE', but again LUT register address and LUT command mode > > > values are different. > > > > > > > > Creating common driver for both FlexSPI and QuadSPI controller, > > > > would > > > involve creation of one more layer between driver/spi/spi-xxx and > > > the actual controller driver, this layer would going to have less > > > functionality like doing LUT creation programming and then would > > > re-direct calls to the respective controller driver functionality to > > > perform desired programming sequence. > > > > > > > >>> > > > >>> (1) The FlexSPI controller is driven by the LUT(Look-up Table) > > > >>> registers. > > > >>> The LUT registers are a look-up-table for sequences of > > > >>> instructions. A valid sequence consists of four LUT registers. > > > >>> Maximum 32 LUT sequences can be programmed simultaneously. > > > >>> > > > >> > > > >> Do we really want to have this level of details in the commit > > > >> message? I mean, this is something you can document in the > > > >> driver, but not here. > > > >> > > > >> BTW, you might want to have a look at [1]. I think we can get rid > > > >> of the ->size field you're adding if this driver implements the > > > >> dirmap hooks. > > > > > > > > I need size information for the connected device to program the > > > > controller > > > register FLSHXXCRO as Flash Chip select is determined
RE: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI controller
Hi Frieder, > -Original Message- > From: Frieder Schrempf [mailto:frieder.schre...@exceet.de] > Sent: Thursday, September 6, 2018 1:56 PM > To: Yogesh Narayan Gaur ; Boris Brezillon > > Cc: linux-...@lists.infradead.org; marek.va...@gmail.com; linux- > s...@vger.kernel.org; devicet...@vger.kernel.org; r...@kernel.org; > mark.rutl...@arm.com; shawn...@kernel.org; linux-arm- > ker...@lists.infradead.org; computersforpe...@gmail.com; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI controller > > >> Hi Yogesh, > >> > >> On Fri, 31 Aug 2018 16:00:00 +0530 > >> Yogesh Gaur wrote: > >> > >>> - Add a driver for NXP FlexSPI host controller > >>> > >> > >> Yep, I had a quick look at the code and they really look similar. Why > >> not extending the existing driver instead of creating a new one from > >> scratch? > >> > > > > FlexSPI is different controller not related to the QuadSPI controller > > in its working behavior Both these controller are having > > * Different registers name, registers address, registers functionality > > etc, section 30.5.2[1] > > * Different programming sequence for initialization of the controller, > > section 30.8.1[1] > > * Different programming sequence for performing Read and Write > > operation using IP, section 30.7.9[1] and AHB mode > > * Different programming sequence for checking controller current state > > like busy or not > > * New mechanism to program for the connected slave device i.e. flash > > access mode and flash memory map 30.7.4[1] and 30.7.5[1] > > * New entries added for FlexSPI controller for LUT_XX mode for various > > commands, section 30.7.8[1] > > > > There are few similarities between these two like LUT programming > > logic is same i.e. LUT needs to be programmed in same sequence of 'INSTR > PAD OPCODE', but again LUT register address and LUT command mode values > are different. > > > > Creating common driver for both FlexSPI and QuadSPI controller, would > involve creation of one more layer between driver/spi/spi-xxx and the actual > controller driver, this layer would going to have less functionality like > doing LUT > creation programming and then would re-direct calls to the respective > controller > driver functionality to perform desired programming sequence. > > > >>> > >>> (1) The FlexSPI controller is driven by the LUT(Look-up Table) > >>> registers. > >>> The LUT registers are a look-up-table for sequences of instructions. > >>> A valid sequence consists of four LUT registers. > >>> Maximum 32 LUT sequences can be programmed simultaneously. > >>> > >> > >> Do we really want to have this level of details in the commit message? > >> I mean, this is something you can document in the driver, but not here. > >> > >> BTW, you might want to have a look at [1]. I think we can get rid of > >> the ->size field you're adding if this driver implements the dirmap hooks. > >> > > > > I need size information for the connected device to program the controller > register FLSHXXCRO as Flash Chip select is determined by flash access address > and Flash size setting in register FLSHXXCR0[FLSHz], section 30.7.9[1]. > > It's the same situation we had with the QSPI driver before. We decided to > **not** pass information about flash size directly to the controller for now. > That's why we currently don't support mapping the flash device in the > implementation of the QSPI driver. > > I think we should not start this discussion all over again for the FlexSPI > driver, but > stick to what we decided for QSPI. > There is difference between FlexSPI and QuadSPI controller functionality in detecting the current CS. As per table-10.32[1] for QuadSPI controller, access to flash is being assigned as per the address values provided i.e. it would be CS0 if address is between TOP_ADDR_MEMXX and QSPI_AMBA_BASE and CS1 if access is in between TOP_ADDR_MEMA2 and TOP_ADDR_MEMA1. But for case of FlexSPI controller, section 30.7.5[2], CS is being defined as per the address value lies in below range - Flash A1 address range: 0x ~ FA1_SIZE - Flash A2 address range: FA1_SIZE ~ (FA1_SIZE + FA2_SIZE) - Flash B1 address range: (FA1_SIZE + FA2_SIZE) ~ (FA1_SIZE + FA2_SIZE + FB1_SIZE) - Flash B2 address range: FA1_SIZE + FA2_SIZE + FB1_SIZE) ~ (FA1_SIZE + FA2_SIZE + FB1_SIZE + FB2_SIZE) and FAx_SIZE is determined from register FLSHxxCR0[FLASHSZ] Thus, for QuadSPI controller we can actually go away with the fla
RE: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI controller
Hi Frieder, > -Original Message- > From: Frieder Schrempf [mailto:frieder.schre...@exceet.de] > Sent: Thursday, September 6, 2018 1:56 PM > To: Yogesh Narayan Gaur ; Boris Brezillon > > Cc: linux-...@lists.infradead.org; marek.va...@gmail.com; linux- > s...@vger.kernel.org; devicet...@vger.kernel.org; r...@kernel.org; > mark.rutl...@arm.com; shawn...@kernel.org; linux-arm- > ker...@lists.infradead.org; computersforpe...@gmail.com; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI controller > > >> Hi Yogesh, > >> > >> On Fri, 31 Aug 2018 16:00:00 +0530 > >> Yogesh Gaur wrote: > >> > >>> - Add a driver for NXP FlexSPI host controller > >>> > >> > >> Yep, I had a quick look at the code and they really look similar. Why > >> not extending the existing driver instead of creating a new one from > >> scratch? > >> > > > > FlexSPI is different controller not related to the QuadSPI controller > > in its working behavior Both these controller are having > > * Different registers name, registers address, registers functionality > > etc, section 30.5.2[1] > > * Different programming sequence for initialization of the controller, > > section 30.8.1[1] > > * Different programming sequence for performing Read and Write > > operation using IP, section 30.7.9[1] and AHB mode > > * Different programming sequence for checking controller current state > > like busy or not > > * New mechanism to program for the connected slave device i.e. flash > > access mode and flash memory map 30.7.4[1] and 30.7.5[1] > > * New entries added for FlexSPI controller for LUT_XX mode for various > > commands, section 30.7.8[1] > > > > There are few similarities between these two like LUT programming > > logic is same i.e. LUT needs to be programmed in same sequence of 'INSTR > PAD OPCODE', but again LUT register address and LUT command mode values > are different. > > > > Creating common driver for both FlexSPI and QuadSPI controller, would > involve creation of one more layer between driver/spi/spi-xxx and the actual > controller driver, this layer would going to have less functionality like > doing LUT > creation programming and then would re-direct calls to the respective > controller > driver functionality to perform desired programming sequence. > > > >>> > >>> (1) The FlexSPI controller is driven by the LUT(Look-up Table) > >>> registers. > >>> The LUT registers are a look-up-table for sequences of instructions. > >>> A valid sequence consists of four LUT registers. > >>> Maximum 32 LUT sequences can be programmed simultaneously. > >>> > >> > >> Do we really want to have this level of details in the commit message? > >> I mean, this is something you can document in the driver, but not here. > >> > >> BTW, you might want to have a look at [1]. I think we can get rid of > >> the ->size field you're adding if this driver implements the dirmap hooks. > >> > > > > I need size information for the connected device to program the controller > register FLSHXXCRO as Flash Chip select is determined by flash access address > and Flash size setting in register FLSHXXCR0[FLSHz], section 30.7.9[1]. > > It's the same situation we had with the QSPI driver before. We decided to > **not** pass information about flash size directly to the controller for now. > That's why we currently don't support mapping the flash device in the > implementation of the QSPI driver. > > I think we should not start this discussion all over again for the FlexSPI > driver, but > stick to what we decided for QSPI. > There is difference between FlexSPI and QuadSPI controller functionality in detecting the current CS. As per table-10.32[1] for QuadSPI controller, access to flash is being assigned as per the address values provided i.e. it would be CS0 if address is between TOP_ADDR_MEMXX and QSPI_AMBA_BASE and CS1 if access is in between TOP_ADDR_MEMA2 and TOP_ADDR_MEMA1. But for case of FlexSPI controller, section 30.7.5[2], CS is being defined as per the address value lies in below range - Flash A1 address range: 0x ~ FA1_SIZE - Flash A2 address range: FA1_SIZE ~ (FA1_SIZE + FA2_SIZE) - Flash B1 address range: (FA1_SIZE + FA2_SIZE) ~ (FA1_SIZE + FA2_SIZE + FB1_SIZE) - Flash B2 address range: FA1_SIZE + FA2_SIZE + FB1_SIZE) ~ (FA1_SIZE + FA2_SIZE + FB1_SIZE + FB2_SIZE) and FAx_SIZE is determined from register FLSHxxCR0[FLASHSZ] Thus, for QuadSPI controller we can actually go away with the fla
RE: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Tuesday, September 4, 2018 8:29 PM > To: Yogesh Narayan Gaur > Cc: linux-...@lists.infradead.org; marek.va...@gmail.com; linux- > s...@vger.kernel.org; devicet...@vger.kernel.org; r...@kernel.org; > mark.rutl...@arm.com; shawn...@kernel.org; linux-arm- > ker...@lists.infradead.org; computersforpe...@gmail.com; > frieder.schre...@exceet.de; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI controller > > Hi Yogesh, > > On Fri, 31 Aug 2018 16:00:00 +0530 > Yogesh Gaur wrote: > > > - Add a driver for NXP FlexSPI host controller > > > > (0) What is the FlexSPI controller? > > FlexSPI is a flexsible SPI host controller which supports two SPI > > channels and up to 4 external devices. > > Each channel supports Single/Dual/Quad/Octal mode data transfer > > (1/2/4/8 bidirectional data lines) i.e. FlexSPI acts as an interface > > to external devices, maximum 4, each with up to 8 bidirectional data > > lines. > > > > It uses new SPI memory interface of the SPI framework to issue flash > > memory operations to up to four connected flash chips (2 buses with > > 2 CS each). > > Chipselect for each flash can be selected as per address assigned in > > controller specific registers. > > > > FlexSPI controller is similar to the existing Freescale/NXP QuadSPI > > controller with advanced features. > > Yep, I had a quick look at the code and they really look similar. Why not > extending the existing driver instead of creating a new one from scratch? > FlexSPI is different controller not related to the QuadSPI controller in its working behavior Both these controller are having * Different registers name, registers address, registers functionality etc, section 30.5.2[1] * Different programming sequence for initialization of the controller, section 30.8.1[1] * Different programming sequence for performing Read and Write operation using IP, section 30.7.9[1] and AHB mode * Different programming sequence for checking controller current state like busy or not * New mechanism to program for the connected slave device i.e. flash access mode and flash memory map 30.7.4[1] and 30.7.5[1] * New entries added for FlexSPI controller for LUT_XX mode for various commands, section 30.7.8[1] There are few similarities between these two like LUT programming logic is same i.e. LUT needs to be programmed in same sequence of 'INSTR PAD OPCODE', but again LUT register address and LUT command mode values are different. Creating common driver for both FlexSPI and QuadSPI controller, would involve creation of one more layer between driver/spi/spi-xxx and the actual controller driver, this layer would going to have less functionality like doing LUT creation programming and then would re-direct calls to the respective controller driver functionality to perform desired programming sequence. > > > > (1) The FlexSPI controller is driven by the LUT(Look-up Table) > > registers. > > The LUT registers are a look-up-table for sequences of instructions. > > A valid sequence consists of four LUT registers. > > Maximum 32 LUT sequences can be programmed simultaneously. > > > > (2) The definition of the LUT register shows below: > > --- > > | INSTR1 | PAD1 | OPRND1 | INSTR0 | PAD0 | OPRND0 | > > --- > > > > There are several types of INSTRx, such as: > > CMD : the SPI NOR command. > > ADDR: the address for the SPI NOR command. > > DUMMY : the dummy cycles needed by the SPI NOR command. > > > > > > There are several types of PADx, such as: > > PAD1: use a singe I/O line. > > PAD2: use two I/O lines. > > PAD4: use quad I/O lines. > > PAD8: use octal I/O lines. > > > > > > (3) LUTs are being created at run-time based on the commands passed > > from the spi-mem framework. Thus, using single LUT index. > > > > (4) Software triggered Flash read/write access by IP Bus. > > > > (5) Memory mapped read access by AHB Bus. > > Do we really want to have this level of details in the commit message? > I mean, this is something you can document in the driver, but not here. > > BTW, you might want to have a look at [1]. I think we can get rid of the > ->size > field you're adding if this driver implements the dirmap hooks. > I need size information for the connected device to program the controller register FLSHXXCRO as Flash Chip select is determined by flash
RE: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Tuesday, September 4, 2018 8:29 PM > To: Yogesh Narayan Gaur > Cc: linux-...@lists.infradead.org; marek.va...@gmail.com; linux- > s...@vger.kernel.org; devicet...@vger.kernel.org; r...@kernel.org; > mark.rutl...@arm.com; shawn...@kernel.org; linux-arm- > ker...@lists.infradead.org; computersforpe...@gmail.com; > frieder.schre...@exceet.de; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 3/7] spi: spi-mem: Add a driver for NXP FlexSPI controller > > Hi Yogesh, > > On Fri, 31 Aug 2018 16:00:00 +0530 > Yogesh Gaur wrote: > > > - Add a driver for NXP FlexSPI host controller > > > > (0) What is the FlexSPI controller? > > FlexSPI is a flexsible SPI host controller which supports two SPI > > channels and up to 4 external devices. > > Each channel supports Single/Dual/Quad/Octal mode data transfer > > (1/2/4/8 bidirectional data lines) i.e. FlexSPI acts as an interface > > to external devices, maximum 4, each with up to 8 bidirectional data > > lines. > > > > It uses new SPI memory interface of the SPI framework to issue flash > > memory operations to up to four connected flash chips (2 buses with > > 2 CS each). > > Chipselect for each flash can be selected as per address assigned in > > controller specific registers. > > > > FlexSPI controller is similar to the existing Freescale/NXP QuadSPI > > controller with advanced features. > > Yep, I had a quick look at the code and they really look similar. Why not > extending the existing driver instead of creating a new one from scratch? > FlexSPI is different controller not related to the QuadSPI controller in its working behavior Both these controller are having * Different registers name, registers address, registers functionality etc, section 30.5.2[1] * Different programming sequence for initialization of the controller, section 30.8.1[1] * Different programming sequence for performing Read and Write operation using IP, section 30.7.9[1] and AHB mode * Different programming sequence for checking controller current state like busy or not * New mechanism to program for the connected slave device i.e. flash access mode and flash memory map 30.7.4[1] and 30.7.5[1] * New entries added for FlexSPI controller for LUT_XX mode for various commands, section 30.7.8[1] There are few similarities between these two like LUT programming logic is same i.e. LUT needs to be programmed in same sequence of 'INSTR PAD OPCODE', but again LUT register address and LUT command mode values are different. Creating common driver for both FlexSPI and QuadSPI controller, would involve creation of one more layer between driver/spi/spi-xxx and the actual controller driver, this layer would going to have less functionality like doing LUT creation programming and then would re-direct calls to the respective controller driver functionality to perform desired programming sequence. > > > > (1) The FlexSPI controller is driven by the LUT(Look-up Table) > > registers. > > The LUT registers are a look-up-table for sequences of instructions. > > A valid sequence consists of four LUT registers. > > Maximum 32 LUT sequences can be programmed simultaneously. > > > > (2) The definition of the LUT register shows below: > > --- > > | INSTR1 | PAD1 | OPRND1 | INSTR0 | PAD0 | OPRND0 | > > --- > > > > There are several types of INSTRx, such as: > > CMD : the SPI NOR command. > > ADDR: the address for the SPI NOR command. > > DUMMY : the dummy cycles needed by the SPI NOR command. > > > > > > There are several types of PADx, such as: > > PAD1: use a singe I/O line. > > PAD2: use two I/O lines. > > PAD4: use quad I/O lines. > > PAD8: use octal I/O lines. > > > > > > (3) LUTs are being created at run-time based on the commands passed > > from the spi-mem framework. Thus, using single LUT index. > > > > (4) Software triggered Flash read/write access by IP Bus. > > > > (5) Memory mapped read access by AHB Bus. > > Do we really want to have this level of details in the commit message? > I mean, this is something you can document in the driver, but not here. > > BTW, you might want to have a look at [1]. I think we can get rid of the > ->size > field you're adding if this driver implements the dirmap hooks. > I need size information for the connected device to program the controller register FLSHXXCRO as Flash Chip select is determined by flash
RE: [PATCH 4/7] dt-bindings: spi: add binding file for NXP FlexSPI driver
Hi Prabhakar, > -Original Message- > From: Prabhakar Kushwaha > Sent: Monday, September 3, 2018 3:24 PM > To: Yogesh Narayan Gaur ; linux- > m...@lists.infradead.org; boris.brezil...@bootlin.com; marek.va...@gmail.com; > linux-...@vger.kernel.org; devicet...@vger.kernel.org > Cc: r...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org; linux- > arm-ker...@lists.infradead.org; computersforpe...@gmail.com; > frieder.schre...@exceet.de; linux-kernel@vger.kernel.org; Yogesh Narayan > Gaur > Subject: RE: [PATCH 4/7] dt-bindings: spi: add binding file for NXP FlexSPI > driver > > Dear Yogesh, > > > -Original Message- > > From: linux-kernel-ow...@vger.kernel.org > ow...@vger.kernel.org> On Behalf Of Yogesh Gaur > > Sent: Friday, August 31, 2018 4:00 PM > > To: linux-...@lists.infradead.org; boris.brezil...@bootlin.com; > > marek.va...@gmail.com; linux-...@vger.kernel.org; > > devicet...@vger.kernel.org > > Cc: r...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org; linux- > > arm-ker...@lists.infradead.org; computersforpe...@gmail.com; > > frieder.schre...@exceet.de; linux-kernel@vger.kernel.org; Yogesh > > Narayan Gaur > > Subject: [PATCH 4/7] dt-bindings: spi: add binding file for NXP > > FlexSPI driver > > > > Add binding file for NXP FlexSPI driver. > > > > Signed-off-by: Yogesh Gaur > > --- > > .../devicetree/bindings/spi/spi-nxp-fspi.txt | 42 > > ++ > > 1 file changed, 42 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/spi/spi-nxp- > > fspi.txt > > > > diff --git a/Documentation/devicetree/bindings/spi/spi-nxp-fspi.txt > > b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.txt > > new file mode 100644 > > index 000..9f07116 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.txt > > @@ -0,0 +1,42 @@ > > +* NXP Flex Serial Peripheral Interface (FSPI) > > + > > +Required properties: > > + - compatible : Should be "nxp,lx2160a-fspi" > > + - reg :First contains the register location and length, > > + Second contains the memory mapping address and > > +length > > Why are we overriding reg property. Is there any special requirement. > > Can we use "reg" and "ranges" property in device tree. > Here "reg" represents controller registers and "ranges" represent the memory > address of flash. 'reg' property is being mentioned in the file './Documentation/devicetree/bindings/spi/spi-bus.txt' No mention has been given for the 'ranges' property. Shawn, Please suggest, did we add different property node 'reg' for the configuration register address space and 'ranges' for memory mapped address space. -- Regards Yogesh Gaur > > --pk >
RE: [PATCH 4/7] dt-bindings: spi: add binding file for NXP FlexSPI driver
Hi Prabhakar, > -Original Message- > From: Prabhakar Kushwaha > Sent: Monday, September 3, 2018 3:24 PM > To: Yogesh Narayan Gaur ; linux- > m...@lists.infradead.org; boris.brezil...@bootlin.com; marek.va...@gmail.com; > linux-...@vger.kernel.org; devicet...@vger.kernel.org > Cc: r...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org; linux- > arm-ker...@lists.infradead.org; computersforpe...@gmail.com; > frieder.schre...@exceet.de; linux-kernel@vger.kernel.org; Yogesh Narayan > Gaur > Subject: RE: [PATCH 4/7] dt-bindings: spi: add binding file for NXP FlexSPI > driver > > Dear Yogesh, > > > -Original Message- > > From: linux-kernel-ow...@vger.kernel.org > ow...@vger.kernel.org> On Behalf Of Yogesh Gaur > > Sent: Friday, August 31, 2018 4:00 PM > > To: linux-...@lists.infradead.org; boris.brezil...@bootlin.com; > > marek.va...@gmail.com; linux-...@vger.kernel.org; > > devicet...@vger.kernel.org > > Cc: r...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org; linux- > > arm-ker...@lists.infradead.org; computersforpe...@gmail.com; > > frieder.schre...@exceet.de; linux-kernel@vger.kernel.org; Yogesh > > Narayan Gaur > > Subject: [PATCH 4/7] dt-bindings: spi: add binding file for NXP > > FlexSPI driver > > > > Add binding file for NXP FlexSPI driver. > > > > Signed-off-by: Yogesh Gaur > > --- > > .../devicetree/bindings/spi/spi-nxp-fspi.txt | 42 > > ++ > > 1 file changed, 42 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/spi/spi-nxp- > > fspi.txt > > > > diff --git a/Documentation/devicetree/bindings/spi/spi-nxp-fspi.txt > > b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.txt > > new file mode 100644 > > index 000..9f07116 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.txt > > @@ -0,0 +1,42 @@ > > +* NXP Flex Serial Peripheral Interface (FSPI) > > + > > +Required properties: > > + - compatible : Should be "nxp,lx2160a-fspi" > > + - reg :First contains the register location and length, > > + Second contains the memory mapping address and > > +length > > Why are we overriding reg property. Is there any special requirement. > > Can we use "reg" and "ranges" property in device tree. > Here "reg" represents controller registers and "ranges" represent the memory > address of flash. 'reg' property is being mentioned in the file './Documentation/devicetree/bindings/spi/spi-bus.txt' No mention has been given for the 'ranges' property. Shawn, Please suggest, did we add different property node 'reg' for the configuration register address space and 'ranges' for memory mapped address space. -- Regards Yogesh Gaur > > --pk >
RE: [PATCH 1/7] spi: add slave device size in spi_device struct
Hi Lothar, > -Original Message- > From: Lothar Waßmann [mailto:l...@karo-electronics.de] > Sent: Friday, August 31, 2018 5:28 PM > To: Yogesh Narayan Gaur > Cc: linux-...@lists.infradead.org; boris.brezil...@bootlin.com; > marek.va...@gmail.com; linux-...@vger.kernel.org; > devicet...@vger.kernel.org; mark.rutl...@arm.com; r...@kernel.org; linux- > ker...@vger.kernel.org; frieder.schre...@exceet.de; > computersforpe...@gmail.com; shawn...@kernel.org; linux-arm- > ker...@lists.infradead.org > Subject: Re: [PATCH 1/7] spi: add slave device size in spi_device struct > > Hi, > > On Fri, 31 Aug 2018 15:59:58 +0530 Yogesh Gaur wrote: > > Add 'size' data variable in spi_device struct. > > This is to save the size of the connected slave device. > > > > After slave device scan, spi_nor_scan, size being assigned to this > > from MTD layer. > > > > SFDP read is being requested before completion of spi_nor_scan() > > routine, thus populate device size before making read request to the > > SPI controller. > > > > Signed-off-by: Yogesh Gaur > > --- > > drivers/mtd/devices/m25p80.c | 6 ++ > > drivers/mtd/spi-nor/spi-nor.c | 2 ++ > > include/linux/spi/spi.h | 2 ++ > > 3 files changed, 10 insertions(+) > > > > diff --git a/drivers/mtd/devices/m25p80.c > > b/drivers/mtd/devices/m25p80.c index fe260cc..6c7ad86 100644 > > --- a/drivers/mtd/devices/m25p80.c > > +++ b/drivers/mtd/devices/m25p80.c > > @@ -124,6 +124,10 @@ static ssize_t m25p80_read(struct spi_nor *nor, > loff_t from, size_t len, > > /* convert the dummy cycles to the number of bytes */ > > op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8; > > > > + /* for case of SFDP header read commands, populate spi device size */ > > + if (flash->spimem->spi->size == 0) > > + flash->spimem->spi->size = nor->mtd.size; > > + > If the 'size' is an spimem specific variable it should be added to the spi_mem > struct rather than the spi_device struct. Sure, would move 'size' from struct spi_device to the struct spi_mem. -- Regards Yogesh Gaur. > > Lothar Waßmann
RE: [PATCH 1/7] spi: add slave device size in spi_device struct
Hi Lothar, > -Original Message- > From: Lothar Waßmann [mailto:l...@karo-electronics.de] > Sent: Friday, August 31, 2018 5:28 PM > To: Yogesh Narayan Gaur > Cc: linux-...@lists.infradead.org; boris.brezil...@bootlin.com; > marek.va...@gmail.com; linux-...@vger.kernel.org; > devicet...@vger.kernel.org; mark.rutl...@arm.com; r...@kernel.org; linux- > ker...@vger.kernel.org; frieder.schre...@exceet.de; > computersforpe...@gmail.com; shawn...@kernel.org; linux-arm- > ker...@lists.infradead.org > Subject: Re: [PATCH 1/7] spi: add slave device size in spi_device struct > > Hi, > > On Fri, 31 Aug 2018 15:59:58 +0530 Yogesh Gaur wrote: > > Add 'size' data variable in spi_device struct. > > This is to save the size of the connected slave device. > > > > After slave device scan, spi_nor_scan, size being assigned to this > > from MTD layer. > > > > SFDP read is being requested before completion of spi_nor_scan() > > routine, thus populate device size before making read request to the > > SPI controller. > > > > Signed-off-by: Yogesh Gaur > > --- > > drivers/mtd/devices/m25p80.c | 6 ++ > > drivers/mtd/spi-nor/spi-nor.c | 2 ++ > > include/linux/spi/spi.h | 2 ++ > > 3 files changed, 10 insertions(+) > > > > diff --git a/drivers/mtd/devices/m25p80.c > > b/drivers/mtd/devices/m25p80.c index fe260cc..6c7ad86 100644 > > --- a/drivers/mtd/devices/m25p80.c > > +++ b/drivers/mtd/devices/m25p80.c > > @@ -124,6 +124,10 @@ static ssize_t m25p80_read(struct spi_nor *nor, > loff_t from, size_t len, > > /* convert the dummy cycles to the number of bytes */ > > op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8; > > > > + /* for case of SFDP header read commands, populate spi device size */ > > + if (flash->spimem->spi->size == 0) > > + flash->spimem->spi->size = nor->mtd.size; > > + > If the 'size' is an spimem specific variable it should be added to the spi_mem > struct rather than the spi_device struct. Sure, would move 'size' from struct spi_device to the struct spi_mem. -- Regards Yogesh Gaur. > > Lothar Waßmann
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Tuesday, June 19, 2018 12:59 PM > To: Yogesh Narayan Gaur ; > marek.va...@gmail.com; Frieder Schrempf ; > broo...@kernel.org > Cc: Fabio Estevam ; David Wolfe > ; dw...@infradead.org; rich...@nod.at; Prabhakar > Kushwaha ; Han Xu ; linux- > ker...@vger.kernel.org; linux-...@vger.kernel.org; linux- > m...@lists.infradead.org; miquel.ray...@bootlin.com; > computersforpe...@gmail.com > Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI > controller > > Hi Yogesh, > > Could you please use a mailer that is quoting things correctly. I have a hard > time > differentiating your replies from mine. Sorry for this, have changed my mailer settings. > > On Tue, 19 Jun 2018 07:10:37 + > Yogesh Narayan Gaur wrote: > > > Let us take below layout of memory address space map. > > QuadSPI Controller can access range from 0x2000_ - 0x2FFF_ i.e. 256 > MB address space reserved and it is having 4 slave devices connected. > > These slave devices[of size 64MB, 64MB, 32MB and 64MB ] are connected > > at below address 0x2000_, 0x2400_, 0x2A00_, 0x2C00_ > > i.e. there is gap of 32MB from 0x2800_ to 0x29FF_. > > Okay, I'm fine with pre-reserving 32MB per chip select. > > > > > As per my understanding of the controller, flash XX top address, register > > should > have below values: > > QUADSPI_SFA1AD - 0x0 > > QUADSPI_SFA2AD - 0x400_ > > QUADSPI_SFB1AD - 0xA00_ > > QUADSPI_SFB2AD - 0xC00_ > > And Register QUADSPI_SFAR should point to the range for the flash in which > operation is happening. My mistake values of these register would be for said case are: QUADSPI_SFA1AD - 0x400_ QUADSPI_SFA2AD - 0x800_ QUADSPI_SFB1AD - 0xC00_ QUADSPI_SFB2AD - 0x1000_ i.e. as per controller each register is having the Top address for serial flash connected at A1/A2/B1/B2 respectively. > > Wait, I thought it was supposed to be an absolute address, not one relative to > the 0x2000 offset. > > > > > Please check Table10-32, page 1657, in [1] for more details on flash address > assignment. > > Yes, I still don't see where it says that having one of the range with a zero > size is > forbidden, or anything mentioning a required alignment. > > > > > But say if I assign address to register QUADSPI_SFA2AD as "0 + 2 * - > >ahb_buf_size" then this address value is not correct as per the value range > explained in above mentioned table. > > Why? If the SFA1AD is set to zero, that should not, right? What this table says that for TOP_ADDR_MEMA1 defines the top address for flash connected at A1 and any address space between TOP_ADDR_MEMA1 and QSPI_AMBA_BASE will be routed to Serial Flash A1. In my example case TOP_ADDR_MEMA1 is 0x400_ If assign value to SFAR register is "0 + 2 * ->ahb_buf_size", then this would lie in access range of Serial Flash A1 and access happens to A1 flash whereas we want access to A2 flash. For access of serial flash A2, any address space access between TOP_ADDR_MEMA2 and TOP_ADDR_MEMA1 would be routed to serial flash A2. Thus to access A2 flash, SFAR would be in range from 0x400_ and 0x800_ -- Regards Yogesh Gaur
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Tuesday, June 19, 2018 12:59 PM > To: Yogesh Narayan Gaur ; > marek.va...@gmail.com; Frieder Schrempf ; > broo...@kernel.org > Cc: Fabio Estevam ; David Wolfe > ; dw...@infradead.org; rich...@nod.at; Prabhakar > Kushwaha ; Han Xu ; linux- > ker...@vger.kernel.org; linux-...@vger.kernel.org; linux- > m...@lists.infradead.org; miquel.ray...@bootlin.com; > computersforpe...@gmail.com > Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI > controller > > Hi Yogesh, > > Could you please use a mailer that is quoting things correctly. I have a hard > time > differentiating your replies from mine. Sorry for this, have changed my mailer settings. > > On Tue, 19 Jun 2018 07:10:37 + > Yogesh Narayan Gaur wrote: > > > Let us take below layout of memory address space map. > > QuadSPI Controller can access range from 0x2000_ - 0x2FFF_ i.e. 256 > MB address space reserved and it is having 4 slave devices connected. > > These slave devices[of size 64MB, 64MB, 32MB and 64MB ] are connected > > at below address 0x2000_, 0x2400_, 0x2A00_, 0x2C00_ > > i.e. there is gap of 32MB from 0x2800_ to 0x29FF_. > > Okay, I'm fine with pre-reserving 32MB per chip select. > > > > > As per my understanding of the controller, flash XX top address, register > > should > have below values: > > QUADSPI_SFA1AD - 0x0 > > QUADSPI_SFA2AD - 0x400_ > > QUADSPI_SFB1AD - 0xA00_ > > QUADSPI_SFB2AD - 0xC00_ > > And Register QUADSPI_SFAR should point to the range for the flash in which > operation is happening. My mistake values of these register would be for said case are: QUADSPI_SFA1AD - 0x400_ QUADSPI_SFA2AD - 0x800_ QUADSPI_SFB1AD - 0xC00_ QUADSPI_SFB2AD - 0x1000_ i.e. as per controller each register is having the Top address for serial flash connected at A1/A2/B1/B2 respectively. > > Wait, I thought it was supposed to be an absolute address, not one relative to > the 0x2000 offset. > > > > > Please check Table10-32, page 1657, in [1] for more details on flash address > assignment. > > Yes, I still don't see where it says that having one of the range with a zero > size is > forbidden, or anything mentioning a required alignment. > > > > > But say if I assign address to register QUADSPI_SFA2AD as "0 + 2 * - > >ahb_buf_size" then this address value is not correct as per the value range > explained in above mentioned table. > > Why? If the SFA1AD is set to zero, that should not, right? What this table says that for TOP_ADDR_MEMA1 defines the top address for flash connected at A1 and any address space between TOP_ADDR_MEMA1 and QSPI_AMBA_BASE will be routed to Serial Flash A1. In my example case TOP_ADDR_MEMA1 is 0x400_ If assign value to SFAR register is "0 + 2 * ->ahb_buf_size", then this would lie in access range of Serial Flash A1 and access happens to A1 flash whereas we want access to A2 flash. For access of serial flash A2, any address space access between TOP_ADDR_MEMA2 and TOP_ADDR_MEMA1 would be routed to serial flash A2. Thus to access A2 flash, SFAR would be in range from 0x400_ and 0x800_ -- Regards Yogesh Gaur
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Tuesday, June 19, 2018 12:46 AM To: Yogesh Narayan Gaur Cc: Fabio Estevam ; David Wolfe ; dw...@infradead.org; rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; computersforpe...@gmail.com Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Yogesh, On Mon, 18 Jun 2018 13:32:27 + Yogesh Narayan Gaur wrote: > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Friday, June 15, 2018 7:26 PM > To: Yogesh Narayan Gaur ; Fabio Estevam > ; David Wolfe ; > dw...@infradead.org > Cc: rich...@nod.at; Prabhakar Kushwaha ; > Han Xu ; linux-kernel@vger.kernel.org; > linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf > ; broo...@kernel.org; > linux-...@lists.infradead.org; miquel.ray...@bootlin.com; > computersforpe...@gmail.com > Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP > QuadSPI controller > > On Fri, 15 Jun 2018 13:42:12 + > Yogesh Narayan Gaur wrote: > > > Hi Boris, > > > > I am still debugging the issue. > > With some analysis, able to check that proper values are not being written > > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > > > In current code, value of map_addr are being assigned to these register. > > map_addr = q->memmap_phy + > > 2 * q->devtype_data->ahb_buf_size; > > > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. > > No, because we're only using 2 * ->ahb_buf_size in the direct mapping for > each device, and we're modifying the mapping dynamically based on the > selected device. Maybe we got the logic wrong though. > > Yes, for register QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD, we need to > save starting actual address from where this flash is getting started. > Thus, if my first flash size is 64MB, then register QUADSPI_SFA2AD > would have value of q->memmap_phy + 0x400 i.e. (QUADSPI_SFA1AD + sizeof > First Flash) If second flash is of size 32MB, then register QUADSPI_SFB1AD > would have value of value of QUADSPI_SFA2AD + sizeof second flash. Again, no, that's not what I'm trying to do, and the fact that it worked fine with CS0 makes me think you don't need to map the whole device to get it to work, just 2 * ->ahb_buf_size per device. > > > For my case flash size is 0x400 and with this hard coded value I am > > able to perform Write and Erase operation. > > One more change, I have to do is adding the flash_size when writing the > > base_address in SFAR register for case when "mem->spi->chip_select == 1" > > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); > > I don't want to expose the full device in the direct mapping yet (that's part > of the direct-mapping API I posted here [1]). What this version of the driver > does is, map only 2 time the ahb_size so that we can bypass the internal > cache of the QSPI engine. > > To perform any operation on second flash, we need to provide it's base > address should be saved in SFAR register for this particular operation. That's what we tried to do, we tried to make all CS start at 0 when they are used and declare unused CS at having a size of 0. So, say you're trying to access CS1, you should have the following ranges: CS0: 0 -> 0 (size = 0) CS1: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) CS2: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) CS3: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) now, if you're trying to access CS3: CS0: 0 -> 0 (size = 0) CS1: 0 -> 0 (size = 0) CS2: 0 -> 0 (size = 0) CS3: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) maybe this approach does not work, but that's not clearly stated as 'not supported' in the datasheet. > Exposing only 2 time of ahb_size is design decision but value in SFAR > register should be correct. > > > > > Thus, there should be mechanism or the entry in structure where we can have > > the information of the size of the connected slave device. > > Because that's exactly the kind of thing I'd like to avoid. What if the > device is bigger than the reserved memory region? What if the sum of all > devices does not fit in there? Here I tried to support all cases by j
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Tuesday, June 19, 2018 12:46 AM To: Yogesh Narayan Gaur Cc: Fabio Estevam ; David Wolfe ; dw...@infradead.org; rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; computersforpe...@gmail.com Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Yogesh, On Mon, 18 Jun 2018 13:32:27 + Yogesh Narayan Gaur wrote: > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Friday, June 15, 2018 7:26 PM > To: Yogesh Narayan Gaur ; Fabio Estevam > ; David Wolfe ; > dw...@infradead.org > Cc: rich...@nod.at; Prabhakar Kushwaha ; > Han Xu ; linux-kernel@vger.kernel.org; > linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf > ; broo...@kernel.org; > linux-...@lists.infradead.org; miquel.ray...@bootlin.com; > computersforpe...@gmail.com > Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP > QuadSPI controller > > On Fri, 15 Jun 2018 13:42:12 + > Yogesh Narayan Gaur wrote: > > > Hi Boris, > > > > I am still debugging the issue. > > With some analysis, able to check that proper values are not being written > > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > > > In current code, value of map_addr are being assigned to these register. > > map_addr = q->memmap_phy + > > 2 * q->devtype_data->ahb_buf_size; > > > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. > > No, because we're only using 2 * ->ahb_buf_size in the direct mapping for > each device, and we're modifying the mapping dynamically based on the > selected device. Maybe we got the logic wrong though. > > Yes, for register QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD, we need to > save starting actual address from where this flash is getting started. > Thus, if my first flash size is 64MB, then register QUADSPI_SFA2AD > would have value of q->memmap_phy + 0x400 i.e. (QUADSPI_SFA1AD + sizeof > First Flash) If second flash is of size 32MB, then register QUADSPI_SFB1AD > would have value of value of QUADSPI_SFA2AD + sizeof second flash. Again, no, that's not what I'm trying to do, and the fact that it worked fine with CS0 makes me think you don't need to map the whole device to get it to work, just 2 * ->ahb_buf_size per device. > > > For my case flash size is 0x400 and with this hard coded value I am > > able to perform Write and Erase operation. > > One more change, I have to do is adding the flash_size when writing the > > base_address in SFAR register for case when "mem->spi->chip_select == 1" > > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); > > I don't want to expose the full device in the direct mapping yet (that's part > of the direct-mapping API I posted here [1]). What this version of the driver > does is, map only 2 time the ahb_size so that we can bypass the internal > cache of the QSPI engine. > > To perform any operation on second flash, we need to provide it's base > address should be saved in SFAR register for this particular operation. That's what we tried to do, we tried to make all CS start at 0 when they are used and declare unused CS at having a size of 0. So, say you're trying to access CS1, you should have the following ranges: CS0: 0 -> 0 (size = 0) CS1: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) CS2: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) CS3: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) now, if you're trying to access CS3: CS0: 0 -> 0 (size = 0) CS1: 0 -> 0 (size = 0) CS2: 0 -> 0 (size = 0) CS3: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) maybe this approach does not work, but that's not clearly stated as 'not supported' in the datasheet. > Exposing only 2 time of ahb_size is design decision but value in SFAR > register should be correct. > > > > > Thus, there should be mechanism or the entry in structure where we can have > > the information of the size of the connected slave device. > > Because that's exactly the kind of thing I'd like to avoid. What if the > device is bigger than the reserved memory region? What if the sum of all > devices does not fit in there? Here I tried to support all cases by j
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
-Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Friday, June 15, 2018 7:26 PM To: Yogesh Narayan Gaur ; Fabio Estevam ; David Wolfe ; dw...@infradead.org Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; computersforpe...@gmail.com Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Fri, 15 Jun 2018 13:42:12 + Yogesh Narayan Gaur wrote: > Hi Boris, > > I am still debugging the issue. > With some analysis, able to check that proper values are not being written > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > In current code, value of map_addr are being assigned to these register. > map_addr = q->memmap_phy + > 2 * q->devtype_data->ahb_buf_size; > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. No, because we're only using 2 * ->ahb_buf_size in the direct mapping for each device, and we're modifying the mapping dynamically based on the selected device. Maybe we got the logic wrong though. Yes, for register QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD, we need to save starting actual address from where this flash is getting started. Thus, if my first flash size is 64MB, then register QUADSPI_SFA2AD would have value of q->memmap_phy + 0x400 i.e. (QUADSPI_SFA1AD + sizeof First Flash) If second flash is of size 32MB, then register QUADSPI_SFB1AD would have value of value of QUADSPI_SFA2AD + sizeof second flash. > For my case flash size is 0x400 and with this hard coded value I am able > to perform Write and Erase operation. > One more change, I have to do is adding the flash_size when writing the > base_address in SFAR register for case when "mem->spi->chip_select == 1" > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); I don't want to expose the full device in the direct mapping yet (that's part of the direct-mapping API I posted here [1]). What this version of the driver does is, map only 2 time the ahb_size so that we can bypass the internal cache of the QSPI engine. To perform any operation on second flash, we need to provide it's base address should be saved in SFAR register for this particular operation. Exposing only 2 time of ahb_size is design decision but value in SFAR register should be correct. > > Thus, there should be mechanism or the entry in structure where we can have > the information of the size of the connected slave device. Because that's exactly the kind of thing I'd like to avoid. What if the device is bigger than the reserved memory region? What if the sum of all devices does not fit in there? Here I tried to support all cases by just mapping the portion of memory we need. So IMO, there should be mechanism to have value of start address of each slave device. This might can be done from DTS entry of each slave device connected to the controller.
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
-Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Friday, June 15, 2018 7:26 PM To: Yogesh Narayan Gaur ; Fabio Estevam ; David Wolfe ; dw...@infradead.org Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; computersforpe...@gmail.com Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Fri, 15 Jun 2018 13:42:12 + Yogesh Narayan Gaur wrote: > Hi Boris, > > I am still debugging the issue. > With some analysis, able to check that proper values are not being written > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > In current code, value of map_addr are being assigned to these register. > map_addr = q->memmap_phy + > 2 * q->devtype_data->ahb_buf_size; > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. No, because we're only using 2 * ->ahb_buf_size in the direct mapping for each device, and we're modifying the mapping dynamically based on the selected device. Maybe we got the logic wrong though. Yes, for register QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD, we need to save starting actual address from where this flash is getting started. Thus, if my first flash size is 64MB, then register QUADSPI_SFA2AD would have value of q->memmap_phy + 0x400 i.e. (QUADSPI_SFA1AD + sizeof First Flash) If second flash is of size 32MB, then register QUADSPI_SFB1AD would have value of value of QUADSPI_SFA2AD + sizeof second flash. > For my case flash size is 0x400 and with this hard coded value I am able > to perform Write and Erase operation. > One more change, I have to do is adding the flash_size when writing the > base_address in SFAR register for case when "mem->spi->chip_select == 1" > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); I don't want to expose the full device in the direct mapping yet (that's part of the direct-mapping API I posted here [1]). What this version of the driver does is, map only 2 time the ahb_size so that we can bypass the internal cache of the QSPI engine. To perform any operation on second flash, we need to provide it's base address should be saved in SFAR register for this particular operation. Exposing only 2 time of ahb_size is design decision but value in SFAR register should be correct. > > Thus, there should be mechanism or the entry in structure where we can have > the information of the size of the connected slave device. Because that's exactly the kind of thing I'd like to avoid. What if the device is bigger than the reserved memory region? What if the sum of all devices does not fit in there? Here I tried to support all cases by just mapping the portion of memory we need. So IMO, there should be mechanism to have value of start address of each slave device. This might can be done from DTS entry of each slave device connected to the controller.
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, I am still debugging the issue. With some analysis, able to check that proper values are not being written for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. In current code, value of map_addr are being assigned to these register. map_addr = q->memmap_phy + 2 * q->devtype_data->ahb_buf_size; qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); But instead of "q->devtype_data->ahb_buf_size" it should be flash size. For my case flash size is 0x400 and with this hard coded value I am able to perform Write and Erase operation. One more change, I have to do is adding the flash_size when writing the base_address in SFAR register for case when "mem->spi->chip_select == 1" qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); Thus, there should be mechanism or the entry in structure where we can have the information of the size of the connected slave device. With both of above hardcoded changes, I am able to perform Write and Erase operation on my second flash device but still facing issue in Read operation, debugging in progress for that. -- Regards Yogesh Gaur -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Friday, June 15, 2018 6:20 PM To: Yogesh Narayan Gaur Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; Fabio Estevam ; David Wolfe ; computersforpe...@gmail.com; dw...@infradead.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Tue, 12 Jun 2018 08:51:25 + Yogesh Narayan Gaur wrote: > > I am working on lsxxx platform. With further debugging, I found that my erase > operation for second flash device is not working properly. > Need to have debugging for this in Frieder Patch. Did you find the problem? Could it be a wrong "reg = <>" definition in your DT (Frieder changed the CS numbering scheme in the new driver)?
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, I am still debugging the issue. With some analysis, able to check that proper values are not being written for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. In current code, value of map_addr are being assigned to these register. map_addr = q->memmap_phy + 2 * q->devtype_data->ahb_buf_size; qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); But instead of "q->devtype_data->ahb_buf_size" it should be flash size. For my case flash size is 0x400 and with this hard coded value I am able to perform Write and Erase operation. One more change, I have to do is adding the flash_size when writing the base_address in SFAR register for case when "mem->spi->chip_select == 1" qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); Thus, there should be mechanism or the entry in structure where we can have the information of the size of the connected slave device. With both of above hardcoded changes, I am able to perform Write and Erase operation on my second flash device but still facing issue in Read operation, debugging in progress for that. -- Regards Yogesh Gaur -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Friday, June 15, 2018 6:20 PM To: Yogesh Narayan Gaur Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; Fabio Estevam ; David Wolfe ; computersforpe...@gmail.com; dw...@infradead.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Tue, 12 Jun 2018 08:51:25 + Yogesh Narayan Gaur wrote: > > I am working on lsxxx platform. With further debugging, I found that my erase > operation for second flash device is not working properly. > Need to have debugging for this in Frieder Patch. Did you find the problem? Could it be a wrong "reg = <>" definition in your DT (Frieder changed the CS numbering scheme in the new driver)?
RE: [PATCH] mtd: spi-nor: honour max_data_size for spi-nor writes
Hi Boris,
-Original Message-
From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
Sent: Monday, June 11, 2018 3:19 PM
To: Yogesh Narayan Gaur
Cc: linux-...@lists.infradead.org; boris.brezil...@free-electrons.com;
frieder.schre...@exceet.de; computersforpe...@gmail.com; David Wolfe
; Han Xu ; feste...@gmail.com;
marek.va...@gmail.com; Prabhakar Kushwaha ;
linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; NeilBrown
Subject: Re: [PATCH] mtd: spi-nor: honour max_data_size for spi-nor writes
Hi Yogesh,
Unrelated note: no need to send your patches to both
boris.brezi...@free-electrons.com and boris.brezi...@bootlin.com, just use the
latter.
Ok, Sure.
On Mon, 11 Jun 2018 14:48:14 +0530
Yogesh Gaur wrote:
> Honour max_data_size for spi-nor writes
^ This is no longer relevant.
> In new spi-mem framework, data size to be written is being calculated
> using spi_mem_adjust_op_size().
> This can return value less than nor->page_size.
>
> Add check value of data size return from API spi_mem_adjust_op_size()
> with the actual requested data size and write, max, only supported
> data size.
This part is not clear.
Also, I'd prefer to have this patch split in 2:
1/ one patch removing the check in spi_nor_write() 2/ and the second patch
removing the while() loop in m25p80_write()
How about the following commit messages for those 2 patches:
1:
"
mtd: spi-nor: Support controllers with limited TX FIFO size
Some SPI controllers can't write nor->page_size bytes in a single step because
their TX FIFO is too small.
Allow nor->write() to return a size that is smaller than the requested write
size to gracefully handle this case.
"
2:
"
mtd: devices: m25p80: Make sure WRITE_EN is issued before each write
Some SPI controllers can't write nor->page_size bytes in a single step because
their TX FIFO is too small, but when that happens we should make sure a
WRITE_EN command is issued before each write access.
The core is already taking care of that, so all we have to do here is return
the actual number of bytes that were written during the
spi_mem_exec_op() operation.
"
Both patches send [1][2]
--
Regards
Yogesh Gaur.
>
> Signed-off-by: NeilBrown
> Signed-off-by: Yogesh Gaur
> ---
> drivers/mtd/devices/m25p80.c | 23 ---
> drivers/mtd/spi-nor/spi-nor.c | 7 ---
> 2 files changed, 8 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/mtd/devices/m25p80.c
> b/drivers/mtd/devices/m25p80.c index e84563d..60224fe 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -72,7 +72,6 @@ static ssize_t m25p80_write(struct spi_nor *nor, loff_t to,
> size_t len,
> SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
> SPI_MEM_OP_DUMMY(0, 1),
> SPI_MEM_OP_DATA_OUT(len, buf, 1));
> - size_t remaining = len;
> int ret;
>
> /* get transfer protocols. */
> @@ -84,22 +83,16 @@ static ssize_t m25p80_write(struct spi_nor *nor, loff_t
> to, size_t len,
> if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
> op.addr.nbytes = 0;
>
> - while (remaining) {
> - op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
> - ret = spi_mem_adjust_op_size(flash->spimem, );
> - if (ret)
> - return ret;
> -
> - ret = spi_mem_exec_op(flash->spimem, );
> - if (ret)
> - return ret;
> + ret = spi_mem_adjust_op_size(flash->spimem, );
> + if (ret)
> + return ret;
> + op.data.nbytes = len < op.data.nbytes ? len : op.data.nbytes;
>
> - op.addr.val += op.data.nbytes;
> - remaining -= op.data.nbytes;
> - op.data.buf.out += op.data.nbytes;
> - }
> + ret = spi_mem_exec_op(flash->spimem, );
> + if (ret)
> + return ret;
>
> - return len;
> + return op.data.nbytes;
> }
>
> /*
> diff --git a/drivers/mtd/spi-nor/spi-nor.c
> b/drivers/mtd/spi-nor/spi-nor.c index 5bfa36e..3e63543 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1431,13 +1431,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t
> to, size_t len,
> goto write_err;
> *retlen += written;
> i += written;
> - if (written != page_remain) {
> - dev_err(nor->dev,
> - "While writing %zu bytes written %zd bytes\n",
> - page_remain, written);
> - ret = -EIO;
> - goto write_err;
> - }
> }
>
> write_err:
[1] https://patchwork.ozlabs.org/patch/928677/
[2] https://patchwork.ozlabs.org/patch/928678/
RE: [PATCH] mtd: spi-nor: honour max_data_size for spi-nor writes
Hi Boris,
-Original Message-
From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
Sent: Monday, June 11, 2018 3:19 PM
To: Yogesh Narayan Gaur
Cc: linux-...@lists.infradead.org; boris.brezil...@free-electrons.com;
frieder.schre...@exceet.de; computersforpe...@gmail.com; David Wolfe
; Han Xu ; feste...@gmail.com;
marek.va...@gmail.com; Prabhakar Kushwaha ;
linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; NeilBrown
Subject: Re: [PATCH] mtd: spi-nor: honour max_data_size for spi-nor writes
Hi Yogesh,
Unrelated note: no need to send your patches to both
boris.brezi...@free-electrons.com and boris.brezi...@bootlin.com, just use the
latter.
Ok, Sure.
On Mon, 11 Jun 2018 14:48:14 +0530
Yogesh Gaur wrote:
> Honour max_data_size for spi-nor writes
^ This is no longer relevant.
> In new spi-mem framework, data size to be written is being calculated
> using spi_mem_adjust_op_size().
> This can return value less than nor->page_size.
>
> Add check value of data size return from API spi_mem_adjust_op_size()
> with the actual requested data size and write, max, only supported
> data size.
This part is not clear.
Also, I'd prefer to have this patch split in 2:
1/ one patch removing the check in spi_nor_write() 2/ and the second patch
removing the while() loop in m25p80_write()
How about the following commit messages for those 2 patches:
1:
"
mtd: spi-nor: Support controllers with limited TX FIFO size
Some SPI controllers can't write nor->page_size bytes in a single step because
their TX FIFO is too small.
Allow nor->write() to return a size that is smaller than the requested write
size to gracefully handle this case.
"
2:
"
mtd: devices: m25p80: Make sure WRITE_EN is issued before each write
Some SPI controllers can't write nor->page_size bytes in a single step because
their TX FIFO is too small, but when that happens we should make sure a
WRITE_EN command is issued before each write access.
The core is already taking care of that, so all we have to do here is return
the actual number of bytes that were written during the
spi_mem_exec_op() operation.
"
Both patches send [1][2]
--
Regards
Yogesh Gaur.
>
> Signed-off-by: NeilBrown
> Signed-off-by: Yogesh Gaur
> ---
> drivers/mtd/devices/m25p80.c | 23 ---
> drivers/mtd/spi-nor/spi-nor.c | 7 ---
> 2 files changed, 8 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/mtd/devices/m25p80.c
> b/drivers/mtd/devices/m25p80.c index e84563d..60224fe 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -72,7 +72,6 @@ static ssize_t m25p80_write(struct spi_nor *nor, loff_t to,
> size_t len,
> SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
> SPI_MEM_OP_DUMMY(0, 1),
> SPI_MEM_OP_DATA_OUT(len, buf, 1));
> - size_t remaining = len;
> int ret;
>
> /* get transfer protocols. */
> @@ -84,22 +83,16 @@ static ssize_t m25p80_write(struct spi_nor *nor, loff_t
> to, size_t len,
> if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
> op.addr.nbytes = 0;
>
> - while (remaining) {
> - op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
> - ret = spi_mem_adjust_op_size(flash->spimem, );
> - if (ret)
> - return ret;
> -
> - ret = spi_mem_exec_op(flash->spimem, );
> - if (ret)
> - return ret;
> + ret = spi_mem_adjust_op_size(flash->spimem, );
> + if (ret)
> + return ret;
> + op.data.nbytes = len < op.data.nbytes ? len : op.data.nbytes;
>
> - op.addr.val += op.data.nbytes;
> - remaining -= op.data.nbytes;
> - op.data.buf.out += op.data.nbytes;
> - }
> + ret = spi_mem_exec_op(flash->spimem, );
> + if (ret)
> + return ret;
>
> - return len;
> + return op.data.nbytes;
> }
>
> /*
> diff --git a/drivers/mtd/spi-nor/spi-nor.c
> b/drivers/mtd/spi-nor/spi-nor.c index 5bfa36e..3e63543 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1431,13 +1431,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t
> to, size_t len,
> goto write_err;
> *retlen += written;
> i += written;
> - if (written != page_remain) {
> - dev_err(nor->dev,
> - "While writing %zu bytes written %zd bytes\n",
> - page_remain, written);
> - ret = -EIO;
> - goto write_err;
> - }
> }
>
> write_err:
[1] https://patchwork.ozlabs.org/patch/928677/
[2] https://patchwork.ozlabs.org/patch/928678/
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Tuesday, June 12, 2018 12:43 PM To: Yogesh Narayan Gaur Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; Fabio Estevam ; David Wolfe ; computersforpe...@gmail.com; dw...@infradead.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Tue, 12 Jun 2018 06:42:42 + Yogesh Narayan Gaur wrote: > I have tried JFFS2 mounting with smaller partition size but still getting > failure. > For partition size equal or less than 1MB, getting errors as > [ 25.044930] jffs2: Too few erase blocks (4) > Thus, need to have size more than 1MB. > > For 2MB partition size getting error message from jffs2_scan_eraseblock(). > root@ls1012ardb:~# cat /proc/mtd > dev:size erasesize name > mtd0: 0400 0004 "20c.quadspi-0" > mtd1: 0050 0004 "rcw" > mtd2: 00a0 0004 "test" > mtd3: 0020 0004 "rootfs" > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > flash_eraseall has been replaced by `flash_erase 0 0`; please > use it > Erasing 256 Kibyte @ 1c -- 100 % complete > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > [ 26.380989] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x: 0x0dd0 instead > [ 26.390509] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x004c: 0x7366 instead > [ 26.39] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x0050: 0x736c instead That's weird. Can you tell me on which platform you're testing? lsxxx or vf610? Can you dump the NOR after the erase to make sure the memory is actually erased (filled with 0xff)? I am working on lsxxx platform. With further debugging, I found that my erase operation for second flash device is not working properly. Need to have debugging for this in Frieder Patch. When I have created multiple partition for First flash device, then JFFS2 mounting and booting of Linux kernel from rootfstype=jffs2 is successful. root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0050 0004 "rcw" mtd1: 00a0 0004 "test" mtd2: 02e0 0004 "rootfs" mtd3: 0400 0004 "20c.quadspi-1" In above list, for MTD2 partition, able to perform JFFS2 mounting. Below is logs of erase for both flashes: root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0400 0004 "20c.quadspi-0" mtd1: 0400 0004 "20c.quadspi-1" root@ls1012ardb:~# mtd_debug erase /dev/mtd0 0x100 0x200 Erased 33554432 bytes from address 0x0100 in flash root@ls1012ardb:~# root@ls1012ardb:~# mtd_debug read /dev/mtd0 0x100 0xa0 rp Copied 10485760 bytes from address 0x0100 in flash to rp root@ls1012ardb:~# hexdump rp 000 * 0a0 root@ls1012ardb:~# root@ls1012ardb:~# mtd_debug erase /dev/mtd1 0x100 0x200 [ 25.023027] random: crng init done Erased 33554432 bytes from address 0x0100 in flash root@ls1012ardb:~# mtd_debug read /dev/mtd1 0x100 0xa0 rp Copied 10485760 bytes from address 0x0100 in flash to rp root@ls1012ardb:~# root@ls1012ardb:~# hexdump rp 000 1985 2003 000c b0b1 e41e 010 * 004 1985 2003 000c b0b1 e41e 0040010 -- Yogesh Gaur
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Tuesday, June 12, 2018 12:43 PM To: Yogesh Narayan Gaur Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; Fabio Estevam ; David Wolfe ; computersforpe...@gmail.com; dw...@infradead.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Tue, 12 Jun 2018 06:42:42 + Yogesh Narayan Gaur wrote: > I have tried JFFS2 mounting with smaller partition size but still getting > failure. > For partition size equal or less than 1MB, getting errors as > [ 25.044930] jffs2: Too few erase blocks (4) > Thus, need to have size more than 1MB. > > For 2MB partition size getting error message from jffs2_scan_eraseblock(). > root@ls1012ardb:~# cat /proc/mtd > dev:size erasesize name > mtd0: 0400 0004 "20c.quadspi-0" > mtd1: 0050 0004 "rcw" > mtd2: 00a0 0004 "test" > mtd3: 0020 0004 "rootfs" > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > flash_eraseall has been replaced by `flash_erase 0 0`; please > use it > Erasing 256 Kibyte @ 1c -- 100 % complete > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > [ 26.380989] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x: 0x0dd0 instead > [ 26.390509] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x004c: 0x7366 instead > [ 26.39] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x0050: 0x736c instead That's weird. Can you tell me on which platform you're testing? lsxxx or vf610? Can you dump the NOR after the erase to make sure the memory is actually erased (filled with 0xff)? I am working on lsxxx platform. With further debugging, I found that my erase operation for second flash device is not working properly. Need to have debugging for this in Frieder Patch. When I have created multiple partition for First flash device, then JFFS2 mounting and booting of Linux kernel from rootfstype=jffs2 is successful. root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0050 0004 "rcw" mtd1: 00a0 0004 "test" mtd2: 02e0 0004 "rootfs" mtd3: 0400 0004 "20c.quadspi-1" In above list, for MTD2 partition, able to perform JFFS2 mounting. Below is logs of erase for both flashes: root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0400 0004 "20c.quadspi-0" mtd1: 0400 0004 "20c.quadspi-1" root@ls1012ardb:~# mtd_debug erase /dev/mtd0 0x100 0x200 Erased 33554432 bytes from address 0x0100 in flash root@ls1012ardb:~# root@ls1012ardb:~# mtd_debug read /dev/mtd0 0x100 0xa0 rp Copied 10485760 bytes from address 0x0100 in flash to rp root@ls1012ardb:~# hexdump rp 000 * 0a0 root@ls1012ardb:~# root@ls1012ardb:~# mtd_debug erase /dev/mtd1 0x100 0x200 [ 25.023027] random: crng init done Erased 33554432 bytes from address 0x0100 in flash root@ls1012ardb:~# mtd_debug read /dev/mtd1 0x100 0xa0 rp Copied 10485760 bytes from address 0x0100 in flash to rp root@ls1012ardb:~# root@ls1012ardb:~# hexdump rp 000 1985 2003 000c b0b1 e41e 010 * 004 1985 2003 000c b0b1 e41e 0040010 -- Yogesh Gaur
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: linux-mtd [mailto:linux-mtd-boun...@lists.infradead.org] On Behalf Of Yogesh Narayan Gaur Sent: Monday, June 11, 2018 3:51 PM To: Boris Brezillon Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; Fabio Estevam ; David Wolfe ; computersforpe...@gmail.com; dw...@infradead.org Subject: RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Monday, June 11, 2018 3:46 PM To: Yogesh Narayan Gaur Cc: marek.va...@gmail.com; Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Mon, 11 Jun 2018 09:38:14 + Yogesh Narayan Gaur wrote: > > > Observation 3: > > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > > commands should work fine on NOR flash. > > > But with this driver change my mount command is not working. > > > > > > In my target there are 2 flash slave devices connected, and I have given > > > argument to create MTD partition like > > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > > Below is output for /proc/mtd commands > > > root@ls1012ardb:~# cat /proc/mtd > > > dev:size erasesize name > > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > > mtd1: 0050 0004 "rcw" --> > > > Second 64 MB flash device, 3 MTD partition are created for it. > > > mtd2: 00a0 0004 "test" > > > mtd3: 02e0 0004 "rootfs" When I do mtd1 + mtd2 + mtd3, I end up with 0x3d0 instead of 0x400. Is that normal? Do you reserve a bit of space at the end or is it that rcw is not starting at 0? I have given partition size n bootargs as mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) 5 + 10 + 46 ==> 61M i.e. 0x3d0. I have just reserve the bit at the end, we can modify these settings also. > > > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > > flash_eraseall has been replaced by `flash_erase 0 0`; > > > please use it > > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > > init done > > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > > > This command didn't finish successfully and there are lot of messages > > > coming on console mentioning failure in jffs2_scan_eraseblock() > > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c: 0x2886 instead >> Did you try to create a smaller partition? Maybe we have a problem when >> accessing addresses higher than X with the new driver (X to be determined). > Would try and update you. I have tried JFFS2 mounting with smaller partition size but still getting failure. For partition size equal or less than 1MB, getting errors as [ 25.044930] jffs2: Too few erase blocks (4) Thus, need to have size more than 1MB. For 2MB partition size getting error message from jffs2_scan_eraseblock(). root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0400 0004 "20c.quadspi-0" mtd1: 0050 0004 "rcw" mtd2: 00a0 0004 "test" mtd3: 0020 0004 "rootfs" root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 flash_eraseall has been replaced by `flash_erase 0 0`; please use it Erasing 256 Kibyte @ 1c -- 100 % complete root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ [ 26.380989] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x: 0x0dd0 instead [ 26.390509] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x004c: 0x7366 instead [ 26.39] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x0050: 0x736c instead -- Regards Yogesh Gaur > > > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 > > > not found at 0x013c0004: 0x7a3
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: linux-mtd [mailto:linux-mtd-boun...@lists.infradead.org] On Behalf Of Yogesh Narayan Gaur Sent: Monday, June 11, 2018 3:51 PM To: Boris Brezillon Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; Fabio Estevam ; David Wolfe ; computersforpe...@gmail.com; dw...@infradead.org Subject: RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Monday, June 11, 2018 3:46 PM To: Yogesh Narayan Gaur Cc: marek.va...@gmail.com; Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Mon, 11 Jun 2018 09:38:14 + Yogesh Narayan Gaur wrote: > > > Observation 3: > > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > > commands should work fine on NOR flash. > > > But with this driver change my mount command is not working. > > > > > > In my target there are 2 flash slave devices connected, and I have given > > > argument to create MTD partition like > > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > > Below is output for /proc/mtd commands > > > root@ls1012ardb:~# cat /proc/mtd > > > dev:size erasesize name > > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > > mtd1: 0050 0004 "rcw" --> > > > Second 64 MB flash device, 3 MTD partition are created for it. > > > mtd2: 00a0 0004 "test" > > > mtd3: 02e0 0004 "rootfs" When I do mtd1 + mtd2 + mtd3, I end up with 0x3d0 instead of 0x400. Is that normal? Do you reserve a bit of space at the end or is it that rcw is not starting at 0? I have given partition size n bootargs as mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) 5 + 10 + 46 ==> 61M i.e. 0x3d0. I have just reserve the bit at the end, we can modify these settings also. > > > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > > flash_eraseall has been replaced by `flash_erase 0 0`; > > > please use it > > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > > init done > > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > > > This command didn't finish successfully and there are lot of messages > > > coming on console mentioning failure in jffs2_scan_eraseblock() > > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c: 0x2886 instead >> Did you try to create a smaller partition? Maybe we have a problem when >> accessing addresses higher than X with the new driver (X to be determined). > Would try and update you. I have tried JFFS2 mounting with smaller partition size but still getting failure. For partition size equal or less than 1MB, getting errors as [ 25.044930] jffs2: Too few erase blocks (4) Thus, need to have size more than 1MB. For 2MB partition size getting error message from jffs2_scan_eraseblock(). root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0400 0004 "20c.quadspi-0" mtd1: 0050 0004 "rcw" mtd2: 00a0 0004 "test" mtd3: 0020 0004 "rootfs" root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 flash_eraseall has been replaced by `flash_erase 0 0`; please use it Erasing 256 Kibyte @ 1c -- 100 % complete root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ [ 26.380989] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x: 0x0dd0 instead [ 26.390509] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x004c: 0x7366 instead [ 26.39] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x0050: 0x736c instead -- Regards Yogesh Gaur > > > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 > > > not found at 0x013c0004: 0x7a3
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Monday, June 11, 2018 3:46 PM To: Yogesh Narayan Gaur Cc: marek.va...@gmail.com; Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Mon, 11 Jun 2018 09:38:14 + Yogesh Narayan Gaur wrote: > > > Observation 3: > > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > > commands should work fine on NOR flash. > > > But with this driver change my mount command is not working. > > > > > > In my target there are 2 flash slave devices connected, and I have given > > > argument to create MTD partition like > > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > > Below is output for /proc/mtd commands > > > root@ls1012ardb:~# cat /proc/mtd > > > dev:size erasesize name > > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > > mtd1: 0050 0004 "rcw" --> > > > Second 64 MB flash device, 3 MTD partition are created for it. > > > mtd2: 00a0 0004 "test" > > > mtd3: 02e0 0004 "rootfs" When I do mtd1 + mtd2 + mtd3, I end up with 0x3d0 instead of 0x400. Is that normal? Do you reserve a bit of space at the end or is it that rcw is not starting at 0? I have given partition size n bootargs as mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) 5 + 10 + 46 ==> 61M i.e. 0x3d0. I have just reserve the bit at the end, we can modify these settings also. > > > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > > flash_eraseall has been replaced by `flash_erase 0 0`; > > > please use it > > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > > init done > > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > > > This command didn't finish successfully and there are lot of messages > > > coming on console mentioning failure in jffs2_scan_eraseblock() > > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c: 0x2886 instead Did you try to create a smaller partition? Maybe we have a problem when accessing addresses higher than X with the new driver (X to be determined). Would try and update you. -- Regards Yogesh Gaur > > > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 > > > not found at 0x013c0004: 0x7a3b instead > > > [ 187.137641] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c0008: 0xb10f instead > > >
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Monday, June 11, 2018 3:46 PM To: Yogesh Narayan Gaur Cc: marek.va...@gmail.com; Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Mon, 11 Jun 2018 09:38:14 + Yogesh Narayan Gaur wrote: > > > Observation 3: > > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > > commands should work fine on NOR flash. > > > But with this driver change my mount command is not working. > > > > > > In my target there are 2 flash slave devices connected, and I have given > > > argument to create MTD partition like > > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > > Below is output for /proc/mtd commands > > > root@ls1012ardb:~# cat /proc/mtd > > > dev:size erasesize name > > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > > mtd1: 0050 0004 "rcw" --> > > > Second 64 MB flash device, 3 MTD partition are created for it. > > > mtd2: 00a0 0004 "test" > > > mtd3: 02e0 0004 "rootfs" When I do mtd1 + mtd2 + mtd3, I end up with 0x3d0 instead of 0x400. Is that normal? Do you reserve a bit of space at the end or is it that rcw is not starting at 0? I have given partition size n bootargs as mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) 5 + 10 + 46 ==> 61M i.e. 0x3d0. I have just reserve the bit at the end, we can modify these settings also. > > > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > > flash_eraseall has been replaced by `flash_erase 0 0`; > > > please use it > > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > > init done > > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > > > This command didn't finish successfully and there are lot of messages > > > coming on console mentioning failure in jffs2_scan_eraseblock() > > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c: 0x2886 instead Did you try to create a smaller partition? Maybe we have a problem when accessing addresses higher than X with the new driver (X to be determined). Would try and update you. -- Regards Yogesh Gaur > > > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 > > > not found at 0x013c0004: 0x7a3b instead > > > [ 187.137641] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c0008: 0xb10f instead > > >
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Monday, June 11, 2018 1:16 PM To: Yogesh Narayan Gaur ; marek.va...@gmail.com Cc: Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Yogesh, On Mon, 11 Jun 2018 06:31:00 + Yogesh Narayan Gaur wrote: > > > > > Observation 2: > > I have observed data sanity issue after performing read/write > > operations using MTD interface. Explained below > > > > root:~# mtd_debug erase /dev/mtd0 0x100 0x4 > > Erased 262144 bytes from address 0x0100 in flash > > --> Erase at address 0x100 of erase size 0x4 > > root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp > > Copied 256 bytes from address 0x in flash to rp > > --> Read 0x100 bytes from flash from address 0x0 in file rp > > root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp > > Copied 256 bytes from rp to address 0x0100 in flash > > --> Write 0x100 bytes to flash address 0x100 from file rp > > root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp > > Copied 256 bytes from address 0x0100 in flash to wp > > --> Read 0x100 bytes from flash from address 0x100 in file wp > > root:~# diff rp wp > > --> compare both rp and wp files, if they > > are different output comes on console stating file are different > > Files rp and wp differ > > root:~# hexdump wp > > 000 aa55 aa55 8010 541c 4000 0040 > > 010 000a > > 020 0030 11a0 00a0 2580 > > 030 0040 005b > > 040 > > * > > 100 > > root:~# hexdump rp > > 000 aa55 aa55 8010 541c 4000 0040 > > 010 000a > > 020 0030 11a0 00a0 2580 > > 030 0040 005b > > 040 2403 > > 050 > > * > > 070 0011 09e7 4411 9555 0050 > > 080 f9bc afa1 0404 31e0 > > 090 0400 31e0 2010 08dc 31eb > > 0a0 2880 0050 1300 31eb 4e20 8010 80ff > > 0b0 beef dead beef dead beef dead > > 0c0 beef dead beef dead beef dead beef dead > > * > > 100 > > root:~# > > > > In hexdump output of the file which being read from address 0x100,wp, > > it can be observed that only first 64 bytes (0x40) are written on the flash. > > > > Observation 3: > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > commands should work fine on NOR flash. > > But with this driver change my mount command is not working. > > > > In my target there are 2 flash slave devices connected, and I have given > > argument to create MTD partition like > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > Below is output for /proc/mtd commands > > root@ls1012ardb:~# cat /proc/mtd > > dev:size erasesize name > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > mtd1: 0050 0004 "rcw" --> Second > > 64 MB flash device, 3 MTD partition are created for it. > > mtd2: 00a0 0004 "test" > > mtd3: 02e0 0004 "rootfs" > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > flash_eraseall has been replaced by `flash_erase 0 0`; please > > use it > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > init done > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > This command didn't finish successfully and there are lot of messages > > coming on console mentioning failure in jffs2_scan_eraseblock() > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > > found at 0x013c: 0x2886
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Monday, June 11, 2018 1:16 PM To: Yogesh Narayan Gaur ; marek.va...@gmail.com Cc: Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Yogesh, On Mon, 11 Jun 2018 06:31:00 + Yogesh Narayan Gaur wrote: > > > > > Observation 2: > > I have observed data sanity issue after performing read/write > > operations using MTD interface. Explained below > > > > root:~# mtd_debug erase /dev/mtd0 0x100 0x4 > > Erased 262144 bytes from address 0x0100 in flash > > --> Erase at address 0x100 of erase size 0x4 > > root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp > > Copied 256 bytes from address 0x in flash to rp > > --> Read 0x100 bytes from flash from address 0x0 in file rp > > root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp > > Copied 256 bytes from rp to address 0x0100 in flash > > --> Write 0x100 bytes to flash address 0x100 from file rp > > root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp > > Copied 256 bytes from address 0x0100 in flash to wp > > --> Read 0x100 bytes from flash from address 0x100 in file wp > > root:~# diff rp wp > > --> compare both rp and wp files, if they > > are different output comes on console stating file are different > > Files rp and wp differ > > root:~# hexdump wp > > 000 aa55 aa55 8010 541c 4000 0040 > > 010 000a > > 020 0030 11a0 00a0 2580 > > 030 0040 005b > > 040 > > * > > 100 > > root:~# hexdump rp > > 000 aa55 aa55 8010 541c 4000 0040 > > 010 000a > > 020 0030 11a0 00a0 2580 > > 030 0040 005b > > 040 2403 > > 050 > > * > > 070 0011 09e7 4411 9555 0050 > > 080 f9bc afa1 0404 31e0 > > 090 0400 31e0 2010 08dc 31eb > > 0a0 2880 0050 1300 31eb 4e20 8010 80ff > > 0b0 beef dead beef dead beef dead > > 0c0 beef dead beef dead beef dead beef dead > > * > > 100 > > root:~# > > > > In hexdump output of the file which being read from address 0x100,wp, > > it can be observed that only first 64 bytes (0x40) are written on the flash. > > > > Observation 3: > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > commands should work fine on NOR flash. > > But with this driver change my mount command is not working. > > > > In my target there are 2 flash slave devices connected, and I have given > > argument to create MTD partition like > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > Below is output for /proc/mtd commands > > root@ls1012ardb:~# cat /proc/mtd > > dev:size erasesize name > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > mtd1: 0050 0004 "rcw" --> Second > > 64 MB flash device, 3 MTD partition are created for it. > > mtd2: 00a0 0004 "test" > > mtd3: 02e0 0004 "rootfs" > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > flash_eraseall has been replaced by `flash_erase 0 0`; please > > use it > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > init done > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > This command didn't finish successfully and there are lot of messages > > coming on console mentioning failure in jffs2_scan_eraseblock() > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > > found at 0x013c: 0x2886
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Friday, June 8, 2018 6:22 PM To: Yogesh Narayan Gaur Cc: Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Yogesh, On Fri, 8 Jun 2018 11:54:12 + Yogesh Narayan Gaur wrote: > Hi Frieder, > > I have tried to validate your patch on fsl,ls2080a target having 2 Spansion > NOR flash, S25FS512S, as slave device. > Below are my observations: > > Observation 1: > In Linux boot logs after driver probing is successful, getting below log > messages > [1.435986] m25p80 spi0.0: found s25fl512s, expected m25p80 > [1.441564] m25p80 spi0.0: s25fl512s (65536 Kbytes) > [1.446972] m25p80 spi0.1: found s25fl512s, expected m25p80 > [1.452548] m25p80 spi0.1: s25fl512s (65536 Kbytes) > > IMHO, we need to correct message as 'found s25fl512s, expected m25p80' as > final underlying connected flash device is s25fl512s. Not sure what you mean here. What would you like us to fix exactly? > > Observation 2: > I have observed data sanity issue after performing read/write > operations using MTD interface. Explained below > > root:~# mtd_debug erase /dev/mtd0 0x100 0x4 > Erased 262144 bytes from address 0x0100 in flash --> > Erase at address 0x100 of erase size 0x4 > root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp > Copied 256 bytes from address 0x in flash to rp --> > Read 0x100 bytes from flash from address 0x0 in file rp > root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp > Copied 256 bytes from rp to address 0x0100 in flash --> > Write 0x100 bytes to flash address 0x100 from file rp > root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp > Copied 256 bytes from address 0x0100 in flash to wp --> > Read 0x100 bytes from flash from address 0x100 in file wp > root:~# diff rp wp >--> compare both rp and wp files, if they are > different output comes on console stating file are different > Files rp and wp differ > root:~# hexdump wp > 000 aa55 aa55 8010 541c 4000 0040 > 010 000a > 020 0030 11a0 00a0 2580 > 030 0040 005b > 040 > * > 100 > root:~# hexdump rp > 000 aa55 aa55 8010 541c 4000 0040 > 010 000a > 020 0030 11a0 00a0 2580 > 030 0040 005b > 040 2403 > 050 > * > 070 0011 09e7 4411 9555 0050 > 080 f9bc afa1 0404 31e0 > 090 0400 31e0 2010 08dc 31eb > 0a0 2880 0050 1300 31eb 4e20 8010 80ff > 0b0 beef dead beef dead beef dead > 0c0 beef dead beef dead beef dead beef dead > * > 100 > root:~# > > In hexdump output of the file which being read from address 0x100,wp, it > can be observed that only first 64 bytes (0x40) are written on the flash. > > Observation 3: > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > commands should work fine on NOR flash. > But with this driver change my mount command is not working. > > In my target there are 2 flash slave devices connected, and I have given > argument to create MTD partition like > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > Below is output for /proc/mtd commands > root@ls1012ardb:~# cat /proc/mtd > dev:size erasesize name > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > mtd1: 0050 0004 "rcw" --> Second 64 > MB flash device, 3 MTD partition are created for it. > mtd2: 00a0 0004 "test" > mtd3: 02e0 0004 "rootfs" > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > flash_eraseall has been replaced by `flash_erase 0 0`; please > use it > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng init > done > Eras
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Friday, June 8, 2018 6:22 PM To: Yogesh Narayan Gaur Cc: Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Yogesh, On Fri, 8 Jun 2018 11:54:12 + Yogesh Narayan Gaur wrote: > Hi Frieder, > > I have tried to validate your patch on fsl,ls2080a target having 2 Spansion > NOR flash, S25FS512S, as slave device. > Below are my observations: > > Observation 1: > In Linux boot logs after driver probing is successful, getting below log > messages > [1.435986] m25p80 spi0.0: found s25fl512s, expected m25p80 > [1.441564] m25p80 spi0.0: s25fl512s (65536 Kbytes) > [1.446972] m25p80 spi0.1: found s25fl512s, expected m25p80 > [1.452548] m25p80 spi0.1: s25fl512s (65536 Kbytes) > > IMHO, we need to correct message as 'found s25fl512s, expected m25p80' as > final underlying connected flash device is s25fl512s. Not sure what you mean here. What would you like us to fix exactly? > > Observation 2: > I have observed data sanity issue after performing read/write > operations using MTD interface. Explained below > > root:~# mtd_debug erase /dev/mtd0 0x100 0x4 > Erased 262144 bytes from address 0x0100 in flash --> > Erase at address 0x100 of erase size 0x4 > root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp > Copied 256 bytes from address 0x in flash to rp --> > Read 0x100 bytes from flash from address 0x0 in file rp > root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp > Copied 256 bytes from rp to address 0x0100 in flash --> > Write 0x100 bytes to flash address 0x100 from file rp > root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp > Copied 256 bytes from address 0x0100 in flash to wp --> > Read 0x100 bytes from flash from address 0x100 in file wp > root:~# diff rp wp >--> compare both rp and wp files, if they are > different output comes on console stating file are different > Files rp and wp differ > root:~# hexdump wp > 000 aa55 aa55 8010 541c 4000 0040 > 010 000a > 020 0030 11a0 00a0 2580 > 030 0040 005b > 040 > * > 100 > root:~# hexdump rp > 000 aa55 aa55 8010 541c 4000 0040 > 010 000a > 020 0030 11a0 00a0 2580 > 030 0040 005b > 040 2403 > 050 > * > 070 0011 09e7 4411 9555 0050 > 080 f9bc afa1 0404 31e0 > 090 0400 31e0 2010 08dc 31eb > 0a0 2880 0050 1300 31eb 4e20 8010 80ff > 0b0 beef dead beef dead beef dead > 0c0 beef dead beef dead beef dead beef dead > * > 100 > root:~# > > In hexdump output of the file which being read from address 0x100,wp, it > can be observed that only first 64 bytes (0x40) are written on the flash. > > Observation 3: > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > commands should work fine on NOR flash. > But with this driver change my mount command is not working. > > In my target there are 2 flash slave devices connected, and I have given > argument to create MTD partition like > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > Below is output for /proc/mtd commands > root@ls1012ardb:~# cat /proc/mtd > dev:size erasesize name > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > mtd1: 0050 0004 "rcw" --> Second 64 > MB flash device, 3 MTD partition are created for it. > mtd2: 00a0 0004 "test" > mtd3: 02e0 0004 "rootfs" > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > flash_eraseall has been replaced by `flash_erase 0 0`; please > use it > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng init > done > Eras
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
.@bootlin.com; linux-...@vger.kernel.org Cc: dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Yogesh Narayan Gaur ; Han Xu ; Frieder Schrempf ; linux-kernel@vger.kernel.org Subject: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller This driver is derived from the SPI NOR driver at mtd/spi-nor/fsl-quadspi.c. It uses the new SPI memory interface of the SPI framework to issue flash memory operations to up to four connected flash chips (2 buses with 2 CS each). The controller does not support generic SPI messages. Signed-off-by: Frieder Schrempf --- drivers/spi/Kconfig| 11 + drivers/spi/Makefile | 1 + drivers/spi/spi-fsl-qspi.c | 929 3 files changed, 941 insertions(+) diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index e62ac32..6de0df5 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -251,6 +251,17 @@ config SPI_FSL_LPSPI help This enables Freescale i.MX LPSPI controllers in master mode. +config SPI_FSL_QSPI + tristate "Freescale QSPI controller" + depends on ARCH_MXC || SOC_LS1021A || ARCH_LAYERSCAPE || COMPILE_TEST + depends on HAS_IOMEM + help + This enables support for the Quad SPI controller in master mode. + Up to four flash chips can be connected on two buses with two + chipselects each. + This controller does not support generic SPI messages. It only + supports the high-level SPI memory interface. + config SPI_GPIO tristate "GPIO-based bitbanging SPI Master" depends on GPIOLIB || COMPILE_TEST diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index cb1f437..a8f7fda 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -43,6 +43,7 @@ obj-$(CONFIG_SPI_FSL_DSPI)+= spi-fsl-dspi.o obj-$(CONFIG_SPI_FSL_LIB) += spi-fsl-lib.o obj-$(CONFIG_SPI_FSL_ESPI) += spi-fsl-espi.o obj-$(CONFIG_SPI_FSL_LPSPI)+= spi-fsl-lpspi.o +obj-$(CONFIG_SPI_FSL_QSPI) += spi-fsl-qspi.o obj-$(CONFIG_SPI_FSL_SPI) += spi-fsl-spi.o obj-$(CONFIG_SPI_GPIO) += spi-gpio.o obj-$(CONFIG_SPI_IMG_SPFI) += spi-img-spfi.o diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c new file mode 100644 index 000..c16d070 --- /dev/null +++ b/drivers/spi/spi-fsl-qspi.c @@ -0,0 +1,929 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/* + * Freescale QuadSPI driver. + * + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * Copyright (C) 2018 Bootlin + * Copyright (C) 2018 Exceet Electronics GmbH + * + * Transition to SPI MEM interface: + * Author: + * Boris Brezillion + * Frieder Schrempf + * + * Based on the original fsl-quadspi.c spi-nor driver: + * Author: Freescale Semiconductor, Inc. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +/* + * The driver only uses one single LUT entry, that is updated on + * each call of exec_op(). Index 0 is preset at boot with a basic + * read operation, so let's use the last entry (15). + */ +#defineSEQID_LUT 15 + +/* Registers used by the driver */ +#define QUADSPI_MCR0x00 +#define QUADSPI_MCR_RESERVED_MASK (0xF << 16) +#define QUADSPI_MCR_MDIS_MASK BIT(14) +#define QUADSPI_MCR_CLR_TXF_MASK BIT(11) +#define QUADSPI_MCR_CLR_RXF_MASK BIT(10) +#define QUADSPI_MCR_DDR_EN_MASKBIT(7) +#define QUADSPI_MCR_END_CFG_MASK (0x3 << 2) +#define QUADSPI_MCR_SWRSTHD_MASK BIT(1) +#define QUADSPI_MCR_SWRSTSD_MASK BIT(0) + +#define QUADSPI_IPCR 0x08 +#define QUADSPI_IPCR_SEQID_SHIFT 24 + +#define QUADSPI_BUF3CR 0x1c +#define QUADSPI_BUF3CR_ALLMST_MASK BIT(31) +#define QUADSPI_BUF3CR_ADATSZ_SHIFT8 +#define QUADSPI_BUF3CR_ADATSZ_MASK (0xFF << QUADSPI_BUF3CR_ADATSZ_SHIFT) + +#define QUADSPI_BFGENCR0x20 +#define QUADSPI_BFGENCR_SEQID_SHIFT12 + +#define QUADSPI_BUF0IND0x30 +#define QUADSPI_BUF1IND0x34 +#define QUADSPI_BUF2IND0x38 +#define QUADSPI_SFAR 0x100 + +#define QUADSPI_SMPR 0x108 +#define QUADSPI_SMPR_DDRSMP_MASK (7 << 16) +#define QUADSPI_SMPR_FSDLY_MASKBIT(6) +#define QUADSPI_SMPR_FSPHS_MASKBIT(5) +#define QUADSPI_SMPR_HSENA_MASKBIT(0) + +#define QUADSPI_RBCT 0x110 +#define QUADSPI_RBCT_WMRK_MASK 0x1F +#define QUADSPI_RBCT_RXBRD_USEIPS BIT(8) +
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
.@bootlin.com; linux-...@vger.kernel.org Cc: dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Yogesh Narayan Gaur ; Han Xu ; Frieder Schrempf ; linux-kernel@vger.kernel.org Subject: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller This driver is derived from the SPI NOR driver at mtd/spi-nor/fsl-quadspi.c. It uses the new SPI memory interface of the SPI framework to issue flash memory operations to up to four connected flash chips (2 buses with 2 CS each). The controller does not support generic SPI messages. Signed-off-by: Frieder Schrempf --- drivers/spi/Kconfig| 11 + drivers/spi/Makefile | 1 + drivers/spi/spi-fsl-qspi.c | 929 3 files changed, 941 insertions(+) diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index e62ac32..6de0df5 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -251,6 +251,17 @@ config SPI_FSL_LPSPI help This enables Freescale i.MX LPSPI controllers in master mode. +config SPI_FSL_QSPI + tristate "Freescale QSPI controller" + depends on ARCH_MXC || SOC_LS1021A || ARCH_LAYERSCAPE || COMPILE_TEST + depends on HAS_IOMEM + help + This enables support for the Quad SPI controller in master mode. + Up to four flash chips can be connected on two buses with two + chipselects each. + This controller does not support generic SPI messages. It only + supports the high-level SPI memory interface. + config SPI_GPIO tristate "GPIO-based bitbanging SPI Master" depends on GPIOLIB || COMPILE_TEST diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index cb1f437..a8f7fda 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -43,6 +43,7 @@ obj-$(CONFIG_SPI_FSL_DSPI)+= spi-fsl-dspi.o obj-$(CONFIG_SPI_FSL_LIB) += spi-fsl-lib.o obj-$(CONFIG_SPI_FSL_ESPI) += spi-fsl-espi.o obj-$(CONFIG_SPI_FSL_LPSPI)+= spi-fsl-lpspi.o +obj-$(CONFIG_SPI_FSL_QSPI) += spi-fsl-qspi.o obj-$(CONFIG_SPI_FSL_SPI) += spi-fsl-spi.o obj-$(CONFIG_SPI_GPIO) += spi-gpio.o obj-$(CONFIG_SPI_IMG_SPFI) += spi-img-spfi.o diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c new file mode 100644 index 000..c16d070 --- /dev/null +++ b/drivers/spi/spi-fsl-qspi.c @@ -0,0 +1,929 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/* + * Freescale QuadSPI driver. + * + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * Copyright (C) 2018 Bootlin + * Copyright (C) 2018 Exceet Electronics GmbH + * + * Transition to SPI MEM interface: + * Author: + * Boris Brezillion + * Frieder Schrempf + * + * Based on the original fsl-quadspi.c spi-nor driver: + * Author: Freescale Semiconductor, Inc. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +/* + * The driver only uses one single LUT entry, that is updated on + * each call of exec_op(). Index 0 is preset at boot with a basic + * read operation, so let's use the last entry (15). + */ +#defineSEQID_LUT 15 + +/* Registers used by the driver */ +#define QUADSPI_MCR0x00 +#define QUADSPI_MCR_RESERVED_MASK (0xF << 16) +#define QUADSPI_MCR_MDIS_MASK BIT(14) +#define QUADSPI_MCR_CLR_TXF_MASK BIT(11) +#define QUADSPI_MCR_CLR_RXF_MASK BIT(10) +#define QUADSPI_MCR_DDR_EN_MASKBIT(7) +#define QUADSPI_MCR_END_CFG_MASK (0x3 << 2) +#define QUADSPI_MCR_SWRSTHD_MASK BIT(1) +#define QUADSPI_MCR_SWRSTSD_MASK BIT(0) + +#define QUADSPI_IPCR 0x08 +#define QUADSPI_IPCR_SEQID_SHIFT 24 + +#define QUADSPI_BUF3CR 0x1c +#define QUADSPI_BUF3CR_ALLMST_MASK BIT(31) +#define QUADSPI_BUF3CR_ADATSZ_SHIFT8 +#define QUADSPI_BUF3CR_ADATSZ_MASK (0xFF << QUADSPI_BUF3CR_ADATSZ_SHIFT) + +#define QUADSPI_BFGENCR0x20 +#define QUADSPI_BFGENCR_SEQID_SHIFT12 + +#define QUADSPI_BUF0IND0x30 +#define QUADSPI_BUF1IND0x34 +#define QUADSPI_BUF2IND0x38 +#define QUADSPI_SFAR 0x100 + +#define QUADSPI_SMPR 0x108 +#define QUADSPI_SMPR_DDRSMP_MASK (7 << 16) +#define QUADSPI_SMPR_FSDLY_MASKBIT(6) +#define QUADSPI_SMPR_FSPHS_MASKBIT(5) +#define QUADSPI_SMPR_HSENA_MASKBIT(0) + +#define QUADSPI_RBCT 0x110 +#define QUADSPI_RBCT_WMRK_MASK 0x1F +#define QUADSPI_RBCT_RXBRD_USEIPS BIT(8) +
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Frieder, Thanks for migrating the fsl-quadspi.c driver on the new SPI framework. This patch is using dynamic LUT approach to create the LUT at run time instead of fixed static LUT as being used in current driver present at mtd/spi-nor/fsl-quadspi.c. I have pushed the changes for dynamic LUT on mtd/spi-nor/fsl-quadspi.c and v10 has been in review stage. Request you to please add 'signed-off' mentioned in those patches in this patch, patchwork link is https://patchwork.ozlabs.org/patch/896534/ Thanks Yogesh Gaur -Original Message- From: Frieder Schrempf [mailto:frieder.schre...@exceet.de] Sent: Wednesday, May 30, 2018 6:45 PM To: linux-...@lists.infradead.org; boris.brezil...@bootlin.com; linux-...@vger.kernel.org Cc: dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Yogesh Narayan Gaur ; Han Xu ; Frieder Schrempf ; linux-kernel@vger.kernel.org Subject: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller This driver is derived from the SPI NOR driver at mtd/spi-nor/fsl-quadspi.c. It uses the new SPI memory interface of the SPI framework to issue flash memory operations to up to four connected flash chips (2 buses with 2 CS each). The controller does not support generic SPI messages. Signed-off-by: Frieder Schrempf --- drivers/spi/Kconfig| 11 + drivers/spi/Makefile | 1 + drivers/spi/spi-fsl-qspi.c | 929 3 files changed, 941 insertions(+) diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index e62ac32..6de0df5 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -251,6 +251,17 @@ config SPI_FSL_LPSPI help This enables Freescale i.MX LPSPI controllers in master mode. +config SPI_FSL_QSPI + tristate "Freescale QSPI controller" + depends on ARCH_MXC || SOC_LS1021A || ARCH_LAYERSCAPE || COMPILE_TEST + depends on HAS_IOMEM + help + This enables support for the Quad SPI controller in master mode. + Up to four flash chips can be connected on two buses with two + chipselects each. + This controller does not support generic SPI messages. It only + supports the high-level SPI memory interface. + config SPI_GPIO tristate "GPIO-based bitbanging SPI Master" depends on GPIOLIB || COMPILE_TEST diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index cb1f437..a8f7fda 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -43,6 +43,7 @@ obj-$(CONFIG_SPI_FSL_DSPI)+= spi-fsl-dspi.o obj-$(CONFIG_SPI_FSL_LIB) += spi-fsl-lib.o obj-$(CONFIG_SPI_FSL_ESPI) += spi-fsl-espi.o obj-$(CONFIG_SPI_FSL_LPSPI)+= spi-fsl-lpspi.o +obj-$(CONFIG_SPI_FSL_QSPI) += spi-fsl-qspi.o obj-$(CONFIG_SPI_FSL_SPI) += spi-fsl-spi.o obj-$(CONFIG_SPI_GPIO) += spi-gpio.o obj-$(CONFIG_SPI_IMG_SPFI) += spi-img-spfi.o diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c new file mode 100644 index 000..c16d070 --- /dev/null +++ b/drivers/spi/spi-fsl-qspi.c @@ -0,0 +1,929 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/* + * Freescale QuadSPI driver. + * + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * Copyright (C) 2018 Bootlin + * Copyright (C) 2018 Exceet Electronics GmbH + * + * Transition to SPI MEM interface: + * Author: + * Boris Brezillion + * Frieder Schrempf + * + * Based on the original fsl-quadspi.c spi-nor driver: + * Author: Freescale Semiconductor, Inc. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +/* + * The driver only uses one single LUT entry, that is updated on + * each call of exec_op(). Index 0 is preset at boot with a basic + * read operation, so let's use the last entry (15). + */ +#defineSEQID_LUT 15 + +/* Registers used by the driver */ +#define QUADSPI_MCR0x00 +#define QUADSPI_MCR_RESERVED_MASK (0xF << 16) +#define QUADSPI_MCR_MDIS_MASK BIT(14) +#define QUADSPI_MCR_CLR_TXF_MASK BIT(11) +#define QUADSPI_MCR_CLR_RXF_MASK BIT(10) +#define QUADSPI_MCR_DDR_EN_MASKBIT(7) +#define QUADSPI_MCR_END_CFG_MASK (0x3 << 2) +#define QUADSPI_MCR_SWRSTHD_MASK BIT(1) +#define QUADSPI_MCR_SWRSTSD_MASK BIT(0) + +#define QUADSPI_IPCR 0x08 +#define QUADSPI_IPCR_SEQID_SHIFT 24 + +#define QUADSPI_BUF3CR 0x1c +#define QUADSPI_BUF3CR_ALLMST_MASK BIT(31) +#define QUADSPI_BUF3CR_ADATSZ_SHIFT8 +#define QUADSPI_BUF3CR_ADATSZ_MASK (0xFF << QUADSPI_BUF3CR_AD
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Frieder, Thanks for migrating the fsl-quadspi.c driver on the new SPI framework. This patch is using dynamic LUT approach to create the LUT at run time instead of fixed static LUT as being used in current driver present at mtd/spi-nor/fsl-quadspi.c. I have pushed the changes for dynamic LUT on mtd/spi-nor/fsl-quadspi.c and v10 has been in review stage. Request you to please add 'signed-off' mentioned in those patches in this patch, patchwork link is https://patchwork.ozlabs.org/patch/896534/ Thanks Yogesh Gaur -Original Message- From: Frieder Schrempf [mailto:frieder.schre...@exceet.de] Sent: Wednesday, May 30, 2018 6:45 PM To: linux-...@lists.infradead.org; boris.brezil...@bootlin.com; linux-...@vger.kernel.org Cc: dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Yogesh Narayan Gaur ; Han Xu ; Frieder Schrempf ; linux-kernel@vger.kernel.org Subject: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller This driver is derived from the SPI NOR driver at mtd/spi-nor/fsl-quadspi.c. It uses the new SPI memory interface of the SPI framework to issue flash memory operations to up to four connected flash chips (2 buses with 2 CS each). The controller does not support generic SPI messages. Signed-off-by: Frieder Schrempf --- drivers/spi/Kconfig| 11 + drivers/spi/Makefile | 1 + drivers/spi/spi-fsl-qspi.c | 929 3 files changed, 941 insertions(+) diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index e62ac32..6de0df5 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -251,6 +251,17 @@ config SPI_FSL_LPSPI help This enables Freescale i.MX LPSPI controllers in master mode. +config SPI_FSL_QSPI + tristate "Freescale QSPI controller" + depends on ARCH_MXC || SOC_LS1021A || ARCH_LAYERSCAPE || COMPILE_TEST + depends on HAS_IOMEM + help + This enables support for the Quad SPI controller in master mode. + Up to four flash chips can be connected on two buses with two + chipselects each. + This controller does not support generic SPI messages. It only + supports the high-level SPI memory interface. + config SPI_GPIO tristate "GPIO-based bitbanging SPI Master" depends on GPIOLIB || COMPILE_TEST diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index cb1f437..a8f7fda 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -43,6 +43,7 @@ obj-$(CONFIG_SPI_FSL_DSPI)+= spi-fsl-dspi.o obj-$(CONFIG_SPI_FSL_LIB) += spi-fsl-lib.o obj-$(CONFIG_SPI_FSL_ESPI) += spi-fsl-espi.o obj-$(CONFIG_SPI_FSL_LPSPI)+= spi-fsl-lpspi.o +obj-$(CONFIG_SPI_FSL_QSPI) += spi-fsl-qspi.o obj-$(CONFIG_SPI_FSL_SPI) += spi-fsl-spi.o obj-$(CONFIG_SPI_GPIO) += spi-gpio.o obj-$(CONFIG_SPI_IMG_SPFI) += spi-img-spfi.o diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c new file mode 100644 index 000..c16d070 --- /dev/null +++ b/drivers/spi/spi-fsl-qspi.c @@ -0,0 +1,929 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/* + * Freescale QuadSPI driver. + * + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * Copyright (C) 2018 Bootlin + * Copyright (C) 2018 Exceet Electronics GmbH + * + * Transition to SPI MEM interface: + * Author: + * Boris Brezillion + * Frieder Schrempf + * + * Based on the original fsl-quadspi.c spi-nor driver: + * Author: Freescale Semiconductor, Inc. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +/* + * The driver only uses one single LUT entry, that is updated on + * each call of exec_op(). Index 0 is preset at boot with a basic + * read operation, so let's use the last entry (15). + */ +#defineSEQID_LUT 15 + +/* Registers used by the driver */ +#define QUADSPI_MCR0x00 +#define QUADSPI_MCR_RESERVED_MASK (0xF << 16) +#define QUADSPI_MCR_MDIS_MASK BIT(14) +#define QUADSPI_MCR_CLR_TXF_MASK BIT(11) +#define QUADSPI_MCR_CLR_RXF_MASK BIT(10) +#define QUADSPI_MCR_DDR_EN_MASKBIT(7) +#define QUADSPI_MCR_END_CFG_MASK (0x3 << 2) +#define QUADSPI_MCR_SWRSTHD_MASK BIT(1) +#define QUADSPI_MCR_SWRSTSD_MASK BIT(0) + +#define QUADSPI_IPCR 0x08 +#define QUADSPI_IPCR_SEQID_SHIFT 24 + +#define QUADSPI_BUF3CR 0x1c +#define QUADSPI_BUF3CR_ALLMST_MASK BIT(31) +#define QUADSPI_BUF3CR_ADATSZ_SHIFT8 +#define QUADSPI_BUF3CR_ADATSZ_MASK (0xFF << QUADSPI_BUF3CR_AD
Re: Re: [EDT] oom_killer: find bulkiest task based on pss value
EP-2DAD0AFA905A4ACB804C4F82A001242F --- Original Message --- Sender : yalin wang Date : May 08, 2015 13:17 (GMT+05:30) Title : Re: [EDT] oom_killer: find bulkiest task based on pss value 2015-05-08 13:29 GMT+08:00 Yogesh Narayan Gaur : >> >> EP-2DAD0AFA905A4ACB804C4F82A001242F >> Hi Andrew, >> >> Presently in oom_kill.c we calculate badness score of the victim task as per >> the present RSS counter value of the task. >> RSS counter value for any task is usually '[Private (Dirty/Clean)] + [Shared >> (Dirty/Clean)]' of the task. >> We have encountered a situation where values for Private fields are less but >> value for Shared fields are more and hence make total RSS counter value >> large. Later on oom situation killing task with highest RSS value but as >> Private field values are not large hence memory gain after killing this >> process is not as per the expectation. >> >> For e.g. take below use-case scenario, in which 3 process are running in >> system. >> All these process done mmap for file exist in present directory and then >> copying data from this file to local allocated pointers in while(1) loop >> with some sleep. Out of 3 process, 2 process has mmaped file with MAP_SHARED >> setting and one has mapped file with MAP_PRIVATE setting. >> I have all 3 processes in background and checks RSS/PSS value from user >> space utility (utility over cat /proc/pid/smaps) >> Before OOM, below is the consumed memory status for these 3 process (all >> processes run with oom_score_adj = 0) >> >> Comm : 1prg, Pid : 213 (values in kB) >> Rss Shared Private Pss >> Process : 375764194596181168 278460 >> >> Comm : 3prg, Pid : 217 (values in kB) >> RssShared Private Pss >> Process : 305760 32 305728305738 >> >> Comm : 2prg, Pid : 218 (values in kB) >> Rss Shared Private Pss >> Process : 389980 194596 195384292676 >> >> >> Thus as per present code design, first it would select process [2prg : 218] >> as bulkiest process as its RSS value is highest to kill. But if we kill this >> process then only ~195MB would be free as compare to expected ~389MB. >> Thus identifying the task based on RSS value is not accurate design and >> killing that identified process didn’t release expected memory back to >> system. >> >> We need to calculate victim task based on PSS instead of RSS as PSS value >> calculates as >> PSS value = [Private (Dirty/Clean)] + [Shared (Dirty/Clean) / no. of shared >> task] >> For above use-case scenario also, it can be checked that process [3prg : >> 217] is having largest PSS value and by killing this process we can gain >> maximum memory (~305MB) as compare to killing process identified based on >> RSS value. >> >> -- >> Regards, >> Yogesh Gaur. > >Great, > > in fact, i also encounter this scenario, > I use USS (page map counter == 1) pages > to decide which process should be killed, > seems have the same result as you use PSS, > but PSS is better , it also consider shared pages, > in case some process have large shared pages mapping > but little Private page mapping > > BRs, > Yalin I have made patch which identifies bulkiest task on basis of PSS value. Please check below patch. This patch is correcting the way victim task gets identified in oom condition. == From 1c3d7f552f696bdbc0126c8e23beabedbd80e423 Mon Sep 17 00:00:00 2001 From: Yogesh Gaur Date: Thu, 7 May 2015 01:52:13 +0530 Subject: [PATCH] oom: find victim task based on pss This patch is identifying bulkiest task to kill by OOM on the basis of PSS value instead of present RSS values. There can be scenario where task with highest RSS counter is consuming lot of shared memory and killing that task didn't release expected amount of memory to system. PSS value = [Private (Dirty/Clean)] + [Shared (Dirty/Clean) / no. of shared task] RSS value = [Private (Dirty/Clean)] + [Shared (Dirty/Clean)] Thus, using PSS value instead of RSS value as PSS value closely matches with actual memory usage by the task. This patch is using smaps_pte_range() interface defined in CONFIG_PROC_PAGE_MONITOR. For case when CONFIG_PROC_PAGE_MONITOR disabled, this simply returns RSS value count. Signed-off-by: Yogesh Gaur Signed-off-by: Amit Arora Reviewed-b
Re: Re: [EDT] oom_killer: find bulkiest task based on pss value
EP-2DAD0AFA905A4ACB804C4F82A001242F
--- Original Message ---
Sender : yalin wangyalin.wang2...@gmail.com
Date : May 08, 2015 13:17 (GMT+05:30)
Title : Re: [EDT] oom_killer: find bulkiest task based on pss value
2015-05-08 13:29 GMT+08:00 Yogesh Narayan Gaur :
EP-2DAD0AFA905A4ACB804C4F82A001242F
Hi Andrew,
Presently in oom_kill.c we calculate badness score of the victim task as per
the present RSS counter value of the task.
RSS counter value for any task is usually '[Private (Dirty/Clean)] + [Shared
(Dirty/Clean)]' of the task.
We have encountered a situation where values for Private fields are less but
value for Shared fields are more and hence make total RSS counter value
large. Later on oom situation killing task with highest RSS value but as
Private field values are not large hence memory gain after killing this
process is not as per the expectation.
For e.g. take below use-case scenario, in which 3 process are running in
system.
All these process done mmap for file exist in present directory and then
copying data from this file to local allocated pointers in while(1) loop
with some sleep. Out of 3 process, 2 process has mmaped file with MAP_SHARED
setting and one has mapped file with MAP_PRIVATE setting.
I have all 3 processes in background and checks RSS/PSS value from user
space utility (utility over cat /proc/pid/smaps)
Before OOM, below is the consumed memory status for these 3 process (all
processes run with oom_score_adj = 0)
Comm : 1prg, Pid : 213 (values in kB)
Rss Shared Private Pss
Process : 375764194596181168 278460
Comm : 3prg, Pid : 217 (values in kB)
RssShared Private Pss
Process : 305760 32 305728305738
Comm : 2prg, Pid : 218 (values in kB)
Rss Shared Private Pss
Process : 389980 194596 195384292676
Thus as per present code design, first it would select process [2prg : 218]
as bulkiest process as its RSS value is highest to kill. But if we kill this
process then only ~195MB would be free as compare to expected ~389MB.
Thus identifying the task based on RSS value is not accurate design and
killing that identified process didn’t release expected memory back to
system.
We need to calculate victim task based on PSS instead of RSS as PSS value
calculates as
PSS value = [Private (Dirty/Clean)] + [Shared (Dirty/Clean) / no. of shared
task]
For above use-case scenario also, it can be checked that process [3prg :
217] is having largest PSS value and by killing this process we can gain
maximum memory (~305MB) as compare to killing process identified based on
RSS value.
--
Regards,
Yogesh Gaur.
Great,
in fact, i also encounter this scenario,
I use USS (page map counter == 1) pages
to decide which process should be killed,
seems have the same result as you use PSS,
but PSS is better , it also consider shared pages,
in case some process have large shared pages mapping
but little Private page mapping
BRs,
Yalin
I have made patch which identifies bulkiest task on basis of PSS value. Please
check below patch.
This patch is correcting the way victim task gets identified in oom condition.
==
From 1c3d7f552f696bdbc0126c8e23beabedbd80e423 Mon Sep 17 00:00:00 2001
From: Yogesh Gaur yn.g...@samsung.com
Date: Thu, 7 May 2015 01:52:13 +0530
Subject: [PATCH] oom: find victim task based on pss
This patch is identifying bulkiest task to kill by OOM on the basis of PSS value
instead of present RSS values.
There can be scenario where task with highest RSS counter is consuming lot of
shared
memory and killing that task didn't release expected amount of memory to system.
PSS value = [Private (Dirty/Clean)] + [Shared (Dirty/Clean) / no. of shared
task]
RSS value = [Private (Dirty/Clean)] + [Shared (Dirty/Clean)]
Thus, using PSS value instead of RSS value as PSS value closely matches with
actual
memory usage by the task.
This patch is using smaps_pte_range() interface defined in
CONFIG_PROC_PAGE_MONITOR.
For case when CONFIG_PROC_PAGE_MONITOR disabled, this simply returns RSS value
count.
Signed-off-by: Yogesh Gaur yn.g...@samsung.com
Signed-off-by: Amit Arora amit.ar...@samsung.com
Reviewed-by: Ajeet Yadav ajee...@samsung.com
---
fs/proc/task_mmu.c | 47 +++
include/linux/mm.h |9 +
mm/oom_kill.c |9 +++--
3 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 956b75d..dd962ff 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -964,6 +964,53 @@ struct pagemapread {
bool
[EDT] oom_killer: find bulkiest task based on pss value
EP-2DAD0AFA905A4ACB804C4F82A001242F
Hi Andrew,
Presently in oom_kill.c we calculate badness score of the victim task as per
the present RSS counter value of the task.
RSS counter value for any task is usually '[Private (Dirty/Clean)] + [Shared
(Dirty/Clean)]' of the task.
We have encountered a situation where values for Private fields are less but
value for Shared fields are more and hence make total RSS counter value large.
Later on oom situation killing task with highest RSS value but as Private field
values are not large hence memory gain after killing this process is not as per
the expectation.
For e.g. take below use-case scenario, in which 3 process are running in
system.
All these process done mmap for file exist in present directory and then
copying data from this file to local allocated pointers in while(1) loop with
some sleep. Out of 3 process, 2 process has mmaped file with MAP_SHARED setting
and one has mapped file with MAP_PRIVATE setting.
I have all 3 processes in background and checks RSS/PSS value from user space
utility (utility over cat /proc/pid/smaps)
Before OOM, below is the consumed memory status for these 3 process (all
processes run with oom_score_adj = 0)
Comm : 1prg, Pid : 213 (values in kB)
Rss Shared Private Pss
Process : 375764194596181168 278460
Comm : 3prg, Pid : 217 (values in kB)
RssShared Private Pss
Process : 305760 32 305728305738
Comm : 2prg, Pid : 218 (values in kB)
Rss Shared Private Pss
Process : 389980 194596 195384292676
Thus as per present code design, first it would select process [2prg : 218] as
bulkiest process as its RSS value is highest to kill. But if we kill this
process then only ~195MB would be free as compare to expected ~389MB.
Thus identifying the task based on RSS value is not accurate design and killing
that identified process didn’t release expected memory back to system.
We need to calculate victim task based on PSS instead of RSS as PSS value
calculates as
PSS value = [Private (Dirty/Clean)] + [Shared (Dirty/Clean) / no. of shared
task]
For above use-case scenario also, it can be checked that process [3prg : 217]
is having largest PSS value and by killing this process we can gain maximum
memory (~305MB) as compare to killing process identified based on RSS value.
--
Regards,
Yogesh Gaur.N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü¨}©ž²Æ
zÚ:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~††Ûiÿûàz¹ ®w¥¢¸?™¨èÚ&¢)ߢf”ù^jÇ«y§m…á@A«a¶Úÿ
0¶ìh®å’i
[EDT] oom_killer: find bulkiest task based on pss value
EP-2DAD0AFA905A4ACB804C4F82A001242F
Hi Andrew,
Presently in oom_kill.c we calculate badness score of the victim task as per
the present RSS counter value of the task.
RSS counter value for any task is usually '[Private (Dirty/Clean)] + [Shared
(Dirty/Clean)]' of the task.
We have encountered a situation where values for Private fields are less but
value for Shared fields are more and hence make total RSS counter value large.
Later on oom situation killing task with highest RSS value but as Private field
values are not large hence memory gain after killing this process is not as per
the expectation.
For e.g. take below use-case scenario, in which 3 process are running in
system.
All these process done mmap for file exist in present directory and then
copying data from this file to local allocated pointers in while(1) loop with
some sleep. Out of 3 process, 2 process has mmaped file with MAP_SHARED setting
and one has mapped file with MAP_PRIVATE setting.
I have all 3 processes in background and checks RSS/PSS value from user space
utility (utility over cat /proc/pid/smaps)
Before OOM, below is the consumed memory status for these 3 process (all
processes run with oom_score_adj = 0)
Comm : 1prg, Pid : 213 (values in kB)
Rss Shared Private Pss
Process : 375764194596181168 278460
Comm : 3prg, Pid : 217 (values in kB)
RssShared Private Pss
Process : 305760 32 305728305738
Comm : 2prg, Pid : 218 (values in kB)
Rss Shared Private Pss
Process : 389980 194596 195384292676
Thus as per present code design, first it would select process [2prg : 218] as
bulkiest process as its RSS value is highest to kill. But if we kill this
process then only ~195MB would be free as compare to expected ~389MB.
Thus identifying the task based on RSS value is not accurate design and killing
that identified process didn’t release expected memory back to system.
We need to calculate victim task based on PSS instead of RSS as PSS value
calculates as
PSS value = [Private (Dirty/Clean)] + [Shared (Dirty/Clean) / no. of shared
task]
For above use-case scenario also, it can be checked that process [3prg : 217]
is having largest PSS value and by killing this process we can gain maximum
memory (~305MB) as compare to killing process identified based on RSS value.
--
Regards,
Yogesh Gaur.N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü¨}©ž²Æ
zÚj:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~††Ûiÿûàz¹ ®w¥¢¸?™¨èÚ¢)ߢf”ù^jÇ«y§m…á@A«a¶Úÿ
0¶ìh®å’i
