* [PATCH v1] mtd: spi nor: modify the boot and flash type of FMC
@ 2017-01-06 9:12 linshunquan 00354166
2017-01-06 13:44 ` Cyrille Pitchen
0 siblings, 1 reply; 3+ messages in thread
From: linshunquan 00354166 @ 2017-01-06 9:12 UTC (permalink / raw)
To: dwmw2, computersforpeace, boris.brezillon, marek.vasut, richard,
cyrille.pitchen, robh+dt, mark.rutland
Cc: xuejiancheng, linux-mtd, linux-kernel, devicetree, howell.yang,
jalen.hsu, suwenping, raojun, kevin.lixu, lvkuanliang,
linshunquan 00354166
(1) The HiSilicon Flash Memory Controller(FMC) is a multi-functions
device which supports SPI Nor flash controller, SPI nand Flash
controller and parallel nand flash controller. So when we are prepare
to operation SPI Nor, we should make sure the flash type is SPI Nor.
(2) Make sure the boot type is Normal Type before initialize the SPI
Nor controller.
Signed-off-by: linshunquan 00354166 <linshunquan1@hisilicon.com>
---
drivers/mtd/spi-nor/hisi-sfc.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/mtd/spi-nor/hisi-sfc.c b/drivers/mtd/spi-nor/hisi-sfc.c
index 20378b0..7855024 100644
--- a/drivers/mtd/spi-nor/hisi-sfc.c
+++ b/drivers/mtd/spi-nor/hisi-sfc.c
@@ -32,6 +32,8 @@
#define FMC_CFG_OP_MODE_MASK BIT_MASK(0)
#define FMC_CFG_OP_MODE_BOOT 0
#define FMC_CFG_OP_MODE_NORMAL 1
+#define FMC_CFG_OP_MODE_SEL(mode) ((mode) & 0x1)
+#define FMC_CFG_FLASH_SEL_SPI_NOR (0x0 << 1)
#define FMC_CFG_FLASH_SEL(type) (((type) & 0x3) << 1)
#define FMC_CFG_FLASH_SEL_MASK 0x6
#define FMC_ECC_TYPE(type) (((type) & 0x7) << 5)
@@ -141,10 +143,36 @@ static int get_if_type(enum read_mode flash_read)
return if_type;
}
+static void spi_nor_switch_spi_type(struct hifmc_host *host)
+{
+ unsigned int reg;
+
+ reg = readl(host->regbase + FMC_CFG);
+ if ((reg & FMC_CFG_FLASH_SEL_MASK)
+ == FMC_CFG_FLASH_SEL_SPI_NOR)
+ return;
+
+ /* if the flash type isn't spi nor, change it */
+ reg &= ~FMC_CFG_FLASH_SEL_MASK;
+ reg |= FMC_CFG_FLASH_SEL(0);
+ writel(reg, host->regbase + FMC_CFG);
+}
+
static void hisi_spi_nor_init(struct hifmc_host *host)
{
u32 reg;
+ /* switch the flash type to spi nor */
+ spi_nor_switch_spi_type(host);
+
+ /* set the boot mode to normal */
+ reg = readl(host->regbase + FMC_CFG);
+ if ((reg & FMC_CFG_OP_MODE_MASK) == FMC_CFG_OP_MODE_BOOT) {
+ reg |= FMC_CFG_OP_MODE_SEL(FMC_CFG_OP_MODE_NORMAL);
+ writel(reg, host->regbase + FMC_CFG);
+ }
+
+ /* set timming */
reg = TIMING_CFG_TCSH(CS_HOLD_TIME)
| TIMING_CFG_TCSS(CS_SETUP_TIME)
| TIMING_CFG_TSHSL(CS_DESELECT_TIME);
@@ -167,6 +195,8 @@ static int hisi_spi_nor_prep(struct spi_nor *nor, enum spi_nor_ops ops)
if (ret)
goto out;
+ spi_nor_switch_spi_type(host);
+
return 0;
out:
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v1] mtd: spi nor: modify the boot and flash type of FMC 2017-01-06 9:12 [PATCH v1] mtd: spi nor: modify the boot and flash type of FMC linshunquan 00354166 @ 2017-01-06 13:44 ` Cyrille Pitchen 2017-01-07 7:33 ` linshunquan (A) 0 siblings, 1 reply; 3+ messages in thread From: Cyrille Pitchen @ 2017-01-06 13:44 UTC (permalink / raw) To: linshunquan 00354166, dwmw2, computersforpeace, boris.brezillon, marek.vasut, richard, robh+dt, mark.rutland Cc: suwenping, devicetree, howell.yang, jalen.hsu, linux-kernel, raojun, kevin.lixu, linux-mtd, lvkuanliang, xuejiancheng Hi, Le 06/01/2017 à 10:12, linshunquan 00354166 a écrit : > (1) The HiSilicon Flash Memory Controller(FMC) is a multi-functions > device which supports SPI Nor flash controller, SPI nand Flash > controller and parallel nand flash controller. So when we are prepare > to operation SPI Nor, we should make sure the flash type is SPI Nor. > > (2) Make sure the boot type is Normal Type before initialize the SPI > Nor controller. > > Signed-off-by: linshunquan 00354166 <linshunquan1@hisilicon.com> > --- > drivers/mtd/spi-nor/hisi-sfc.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/mtd/spi-nor/hisi-sfc.c b/drivers/mtd/spi-nor/hisi-sfc.c > index 20378b0..7855024 100644 > --- a/drivers/mtd/spi-nor/hisi-sfc.c > +++ b/drivers/mtd/spi-nor/hisi-sfc.c > @@ -32,6 +32,8 @@ > #define FMC_CFG_OP_MODE_MASK BIT_MASK(0) > #define FMC_CFG_OP_MODE_BOOT 0 > #define FMC_CFG_OP_MODE_NORMAL 1 > +#define FMC_CFG_OP_MODE_SEL(mode) ((mode) & 0x1) > +#define FMC_CFG_FLASH_SEL_SPI_NOR (0x0 << 1) > #define FMC_CFG_FLASH_SEL(type) (((type) & 0x3) << 1) > #define FMC_CFG_FLASH_SEL_MASK 0x6 > #define FMC_ECC_TYPE(type) (((type) & 0x7) << 5) > @@ -141,10 +143,36 @@ static int get_if_type(enum read_mode flash_read) > return if_type; > } > > +static void spi_nor_switch_spi_type(struct hifmc_host *host) > +{ > + unsigned int reg; > + > + reg = readl(host->regbase + FMC_CFG); > + if ((reg & FMC_CFG_FLASH_SEL_MASK) > + == FMC_CFG_FLASH_SEL_SPI_NOR) > + return; > + > + /* if the flash type isn't spi nor, change it */ > + reg &= ~FMC_CFG_FLASH_SEL_MASK; > + reg |= FMC_CFG_FLASH_SEL(0); > + writel(reg, host->regbase + FMC_CFG); > +} > + This is not consistent: we have to check the macro definitions to understand that FMC_CFG_FLASH_SPI_NOR == FMC_CFG_FLASH_SEL(0) In such a function, you should use the very same macro for both the test and the update of reg; either FMC_CFG_FLASH_SEL_SPI_NOR or FMC_CFG_FLASH_SEL(0). Please don't mix the use of those macros. > static void hisi_spi_nor_init(struct hifmc_host *host) > { > u32 reg; > > + /* switch the flash type to spi nor */ > + spi_nor_switch_spi_type(host); > + > + /* set the boot mode to normal */ > + reg = readl(host->regbase + FMC_CFG); > + if ((reg & FMC_CFG_OP_MODE_MASK) == FMC_CFG_OP_MODE_BOOT) { > + reg |= FMC_CFG_OP_MODE_SEL(FMC_CFG_OP_MODE_NORMAL); This is not consistent: you test FMC_CFG_OP_MODE_BOOT, hence without FMC_CFG_OP_MODE_SEL() but you set FMC_CFG_OP_MODE_SEL(FMC_CFG_OP_MODE_NORMAL), with FMC_CFG_OP_MODE_SEL(). Of course, looking at the macro definitions, it works as is but once again we have to check the macro definitions to understand why sometime you use FMC_CFG_OP_MODE_SEL() whereas other times you don't. > + writel(reg, host->regbase + FMC_CFG); > + } spi_nor_switch_spi_type() already updates the FMC_CFG register in the very same manner: read, test, modify, write. Hence you should write a more generic function and update both bitfields at once. static void hisi_spi_nor_update_reg(struct hifmc_host *host, unsigned int reg_offset, unsigned int value, unsigned int mask) { unsigned int reg; reg = readl(host->regbase + reg_offset); if (((reg ^ value) & mask) == 0) return; reg = (reg & ~mask) | (value & mask); writel(reg, host->regbase + reg_offset); } ... unsigned int value, mask; /* * switch the flash type to spi nor and set the boot mode to * normal. */ value = FMC_CFG_OP_MODE_NORMAL | FMC_CFG_FLASH_SEL_SPI_NOR; mask = FMC_CFG_OP_MODE_MASK | FMC_CFG_FLASH_SEL_MASK; hisi_spi_nor_update_reg(host, FMC_CFG, value, mask); > + > + /* set timming */ > reg = TIMING_CFG_TCSH(CS_HOLD_TIME) > | TIMING_CFG_TCSS(CS_SETUP_TIME) > | TIMING_CFG_TSHSL(CS_DESELECT_TIME); > @@ -167,6 +195,8 @@ static int hisi_spi_nor_prep(struct spi_nor *nor, enum spi_nor_ops ops) > if (ret) > goto out; > > + spi_nor_switch_spi_type(host); > + > return 0; > > out: > Best regards, Cyrille ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1] mtd: spi nor: modify the boot and flash type of FMC 2017-01-06 13:44 ` Cyrille Pitchen @ 2017-01-07 7:33 ` linshunquan (A) 0 siblings, 0 replies; 3+ messages in thread From: linshunquan (A) @ 2017-01-07 7:33 UTC (permalink / raw) To: Cyrille Pitchen, dwmw2, computersforpeace, boris.brezillon, marek.vasut, richard, robh+dt, mark.rutland Cc: suwenping, devicetree, howell.yang, jalen.hsu, linux-kernel, raojun, kevin.lixu, linux-mtd, lvkuanliang, xuejiancheng Hi Cyrille, Thanks for your relay, I will update this patch soon. On 2017/1/6 21:44, Cyrille Pitchen wrote: > Hi, > > Le 06/01/2017 à 10:12, linshunquan 00354166 a écrit : >> (1) The HiSilicon Flash Memory Controller(FMC) is a multi-functions >> device which supports SPI Nor flash controller, SPI nand Flash >> controller and parallel nand flash controller. So when we are prepare >> to operation SPI Nor, we should make sure the flash type is SPI Nor. >> >> (2) Make sure the boot type is Normal Type before initialize the SPI >> Nor controller. >> >> Signed-off-by: linshunquan 00354166 <linshunquan1@hisilicon.com> >> --- >> drivers/mtd/spi-nor/hisi-sfc.c | 30 ++++++++++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/hisi-sfc.c b/drivers/mtd/spi-nor/hisi-sfc.c >> index 20378b0..7855024 100644 >> --- a/drivers/mtd/spi-nor/hisi-sfc.c >> +++ b/drivers/mtd/spi-nor/hisi-sfc.c >> @@ -32,6 +32,8 @@ >> #define FMC_CFG_OP_MODE_MASK BIT_MASK(0) >> #define FMC_CFG_OP_MODE_BOOT 0 >> #define FMC_CFG_OP_MODE_NORMAL 1 >> +#define FMC_CFG_OP_MODE_SEL(mode) ((mode) & 0x1) >> +#define FMC_CFG_FLASH_SEL_SPI_NOR (0x0 << 1) >> #define FMC_CFG_FLASH_SEL(type) (((type) & 0x3) << 1) >> #define FMC_CFG_FLASH_SEL_MASK 0x6 >> #define FMC_ECC_TYPE(type) (((type) & 0x7) << 5) >> @@ -141,10 +143,36 @@ static int get_if_type(enum read_mode flash_read) >> return if_type; >> } >> >> +static void spi_nor_switch_spi_type(struct hifmc_host *host) >> +{ >> + unsigned int reg; >> + >> + reg = readl(host->regbase + FMC_CFG); >> + if ((reg & FMC_CFG_FLASH_SEL_MASK) >> + == FMC_CFG_FLASH_SEL_SPI_NOR) >> + return; >> + >> + /* if the flash type isn't spi nor, change it */ >> + reg &= ~FMC_CFG_FLASH_SEL_MASK; >> + reg |= FMC_CFG_FLASH_SEL(0); >> + writel(reg, host->regbase + FMC_CFG); >> +} >> + > > This is not consistent: we have to check the macro definitions to > understand that FMC_CFG_FLASH_SPI_NOR == FMC_CFG_FLASH_SEL(0) > > In such a function, you should use the very same macro for both the test > and the update of reg; either FMC_CFG_FLASH_SEL_SPI_NOR or > FMC_CFG_FLASH_SEL(0). Please don't mix the use of those macros. > >> static void hisi_spi_nor_init(struct hifmc_host *host) >> { >> u32 reg; >> >> + /* switch the flash type to spi nor */ >> + spi_nor_switch_spi_type(host); >> + >> + /* set the boot mode to normal */ >> + reg = readl(host->regbase + FMC_CFG); >> + if ((reg & FMC_CFG_OP_MODE_MASK) == FMC_CFG_OP_MODE_BOOT) { >> + reg |= FMC_CFG_OP_MODE_SEL(FMC_CFG_OP_MODE_NORMAL); > > This is not consistent: you test FMC_CFG_OP_MODE_BOOT, hence without > FMC_CFG_OP_MODE_SEL() but you set > FMC_CFG_OP_MODE_SEL(FMC_CFG_OP_MODE_NORMAL), with FMC_CFG_OP_MODE_SEL(). > > Of course, looking at the macro definitions, it works as is but once again > we have to check the macro definitions to understand why sometime you use > FMC_CFG_OP_MODE_SEL() whereas other times you don't. > >> + writel(reg, host->regbase + FMC_CFG); >> + } > > spi_nor_switch_spi_type() already updates the FMC_CFG register in the very > same manner: read, test, modify, write. Hence you should write a more > generic function and update both bitfields at once. > > static void hisi_spi_nor_update_reg(struct hifmc_host *host, > unsigned int reg_offset, > unsigned int value, > unsigned int mask) > { > unsigned int reg; > > reg = readl(host->regbase + reg_offset); > if (((reg ^ value) & mask) == 0) > return; > > reg = (reg & ~mask) | (value & mask); > writel(reg, host->regbase + reg_offset); > } > > ... > > unsigned int value, mask; > > /* > * switch the flash type to spi nor and set the boot mode to > * normal. > */ > value = FMC_CFG_OP_MODE_NORMAL | FMC_CFG_FLASH_SEL_SPI_NOR; > mask = FMC_CFG_OP_MODE_MASK | FMC_CFG_FLASH_SEL_MASK; > hisi_spi_nor_update_reg(host, FMC_CFG, value, mask); > >> + >> + /* set timming */ >> reg = TIMING_CFG_TCSH(CS_HOLD_TIME) >> | TIMING_CFG_TCSS(CS_SETUP_TIME) >> | TIMING_CFG_TSHSL(CS_DESELECT_TIME); >> @@ -167,6 +195,8 @@ static int hisi_spi_nor_prep(struct spi_nor *nor, enum spi_nor_ops ops) >> if (ret) >> goto out; >> >> + spi_nor_switch_spi_type(host); >> + >> return 0; >> >> out: >> > > Best regards, > > Cyrille > . > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-01-07 7:34 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-06 9:12 [PATCH v1] mtd: spi nor: modify the boot and flash type of FMC linshunquan 00354166 2017-01-06 13:44 ` Cyrille Pitchen 2017-01-07 7:33 ` linshunquan (A)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).