| Message ID | 20231213091346.956789-4-haibo.chen@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] spi: spi-nxp-fspi: enable runtime pm for fspi | expand |
> From: Haibo Chen <haibo.chen@nxp.com> > > fspi define four mode for sample clock source selection. > > Here is the list of modes: > mode 0: Dummy Read strobe generated by FlexSPI Controller and loopback internally > mode 1: Dummy Read strobe generated by FlexSPI Controller and loopback from DQS pad > mode 2: Reserved > mode 3: Flash provided Read strobe and input from DQS pad > > In default, fspi use mode 0 after reset. > For 8-8-8-DTR mode, need to use mode 3, otherwise 8-8-8-DTR read always > get incorrect data. I'd say this is board dependant, right? If you now hardcode 8d8d8d to always use mode 3. I'm not sure how a board which doesn't have the DQS connected to the flash can change this to another mode again. Looks like we'd need a (DT) property which tells you if there is actually a DQS line connected to the flash. Btw you don't check buswidth, so you'll enable that mode for any DTR mode. -michael
> -----Original Message----- > From: Michael Walle <mwalle@kernel.org> > Sent: 2023年12月14日 1:21 > To: Bough Chen <haibo.chen@nxp.com> > Cc: broonie@kernel.org; Han Xu <han.xu@nxp.com>; dl-linux-imx > <linux-imx@nxp.com>; linux-spi@vger.kernel.org; yogeshgaur.83@gmail.com; > Michael Walle <mwalle@kernel.org> > Subject: Re: [PATCH 4/5] spi: spi-nxp-fspi: add function to select sample clock > source for flash reading > > > From: Haibo Chen <haibo.chen@nxp.com> > > > > fspi define four mode for sample clock source selection. > > > > Here is the list of modes: > > mode 0: Dummy Read strobe generated by FlexSPI Controller and loopback > > internally mode 1: Dummy Read strobe generated by FlexSPI Controller > > and loopback from DQS pad mode 2: Reserved mode 3: Flash provided Read > > strobe and input from DQS pad > > > > In default, fspi use mode 0 after reset. > > For 8-8-8-DTR mode, need to use mode 3, otherwise 8-8-8-DTR read > > always get incorrect data. > > I'd say this is board dependant, right? If you now hardcode 8d8d8d to always use > mode 3. I'm not sure how a board which doesn't have the DQS connected to the > flash can change this to another mode again. Looks like we'd need a (DT) > property which tells you if there is actually a DQS line connected to the flash. Currently we distinguish through SoC chip, not board. Like patch5. If SoC contain the DQS, but the board do not connect it to flash device, then this is a real issue. But I think if user use one octal flash device which support dtr mode, they should connect this DQS pad if want to work in DTR mode. If forget to connect the DQS pad, they can limit the tx/rx buswidth to 4 or 1 in dts. Anyway, add a DT property seems better. DQS is a must requirement for octal dtr mode, if detect no DQS, we can also disable the DTR mode support. Will add in next version. > > Btw you don't check buswidth, so you'll enable that mode for any DTR mode. Seems current spi-nor code only support one DTR mode, that is 8d-8d-8d. Best Regards Haibo Chen > > -michael
Hi, >> > From: Haibo Chen <haibo.chen@nxp.com> >> > >> > fspi define four mode for sample clock source selection. >> > >> > Here is the list of modes: >> > mode 0: Dummy Read strobe generated by FlexSPI Controller and loopback >> > internally mode 1: Dummy Read strobe generated by FlexSPI Controller >> > and loopback from DQS pad mode 2: Reserved mode 3: Flash provided Read >> > strobe and input from DQS pad >> > >> > In default, fspi use mode 0 after reset. >> > For 8-8-8-DTR mode, need to use mode 3, otherwise 8-8-8-DTR read >> > always get incorrect data. >> >> I'd say this is board dependant, right? If you now hardcode 8d8d8d to >> always use >> mode 3. I'm not sure how a board which doesn't have the DQS connected >> to the >> flash can change this to another mode again. Looks like we'd need a >> (DT) >> property which tells you if there is actually a DQS line connected to >> the flash. > > Currently we distinguish through SoC chip, not board. Like patch5. > If SoC contain the DQS, but the board do not connect it to flash > device, then this is a real issue. See below, there are alternatives to the DQS pin. But it really is frequency dependent. I'd bet you can use mode 0 with a slow frequency in 8d8d8d mode. I.e. the LS1028ARM will always refer you to mode 3 if you want to archive "the highest frequency". > But I think if user use one octal flash device which support dtr mode, > they should > connect this DQS pad if want to work in DTR mode. > If forget to connect the DQS pad, they can limit the tx/rx buswidth to > 4 or 1 in dts. > > Anyway, add a DT property seems better. > > DQS is a must requirement for octal dtr mode, if detect no DQS, we can > also disable the DTR mode support. I don't think this is true in general. I haven't checked with the FSPI controller. But DQS is used for higher frequencies which isn't necessarily related to DTR. Also, there are other methods, like manual calibration of the delay lines on the I/Os. There was once a patchset to add that calibration for a TI (?) controller, but it was never merged. I've seen the fspi also supports some DLL configuration and some kind of data learning feature. As far as I understand that, these are all alternatives. I.e. (1) don't use high frequencies (2) use DQS (driven by the flash) (3) manually/automatically set the DLL > Will add in next version. > >> Btw you don't check buswidth, so you'll enable that mode for any DTR >> mode. > > Seems current spi-nor code only support one DTR mode, that is 8d-8d-8d. Yes. But that doesn't mean the spi controller should assume that. If you don't want to support any other mode, you should probably check that in your .supports_op() callback. -michael
diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c index 2562d524149e..0330454b76c6 100644 --- a/drivers/spi/spi-nxp-fspi.c +++ b/drivers/spi/spi-nxp-fspi.c @@ -395,6 +395,7 @@ struct nxp_fspi { struct pm_qos_request pm_qos_req; int selected; #define FSPI_INITILIZED (1 << 0) +#define FSPI_RXCLKSRC_3 (1 << 1) int flags; }; @@ -928,6 +929,50 @@ static int nxp_fspi_do_op(struct nxp_fspi *f, const struct spi_mem_op *op) return err; } +/* + * Sample Clock source selection for Flash Reading + * Four modes defined by fspi: + * mode 0: Dummy Read strobe generated by FlexSPI Controller + * and loopback internally + * mode 1: Dummy Read strobe generated by FlexSPI Controller + * and loopback from DQS pad + * mode 2: Reserved + * mode 3: Flash provided Read strobe and input from DQS pad + * + * fspi default use mode 0 after reset + */ +static void nxp_fspi_select_rx_sample_clk_source(struct nxp_fspi *f, + const struct spi_mem_op *op) +{ + u32 reg; + + /* + * For 8-8-8-DTR mode, need to use mode 3 (Flash provided Read + * strobe and input from DQS pad), otherwise read operaton may + * meet issue. + * This mode require flash device connect the DQS pad on board. + * For other modes, still use mode 0, keep align with before. + * spi_nor_suspend will disable 8-8-8-DTR mode, also need to + * change the mode back to mode 0. + */ + if (!(f->flags & FSPI_RXCLKSRC_3) && + op->cmd.dtr && op->addr.dtr && + op->dummy.dtr && op->data.dtr) { + reg = fspi_readl(f, f->iobase + FSPI_MCR0); + reg |= FSPI_MCR0_RXCLKSRC(3); + fspi_writel(f, reg, f->iobase + FSPI_MCR0); + f->flags |= FSPI_RXCLKSRC_3; + } else if ((f->flags & FSPI_RXCLKSRC_3) && + !op->cmd.dtr && !op->addr.dtr && + !op->dummy.dtr && !op->data.dtr) { + reg = fspi_readl(f, f->iobase + FSPI_MCR0); + reg &= ~FSPI_MCR0_RXCLKSRC(3); /* select mode 0 */ + fspi_writel(f, reg, f->iobase + FSPI_MCR0); + f->flags &= ~FSPI_RXCLKSRC_3; + } + +} + static int nxp_fspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op) { struct nxp_fspi *f = spi_controller_get_devdata(mem->spi->controller); @@ -948,6 +993,8 @@ static int nxp_fspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op) nxp_fspi_select_mem(f, mem->spi); + nxp_fspi_select_rx_sample_clk_source(f, op); + nxp_fspi_prepare_lut(f, op); /* * If we have large chunks of data, we read them through the AHB bus by