| Message ID | 1452831723-22081-1-git-send-email-judge.packham@gmail.com |
|---|---|
| State | RFC |
| Headers | show |
On Fri, 2016-01-15 at 17:22 +1300, Chris Packham wrote: > Claim the MPP pins for the NAND flash controller only when it's actually > being used. This allows the pins to be shared with the SPI interface > which already supports an equivalent on-access MPP reconfiguration. > > Reviewed-by: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz> > Signed-off-by: Chris Packham <judge.packham@gmail.com> > --- > I haven't wrapped this with a configuration option because I think it > should be safe to enable by default. It will either re-apply the same > MPP configuration that has already been done in the board init or put > the MPP pins into the correct mode to access NAND. > > I've only got access to one kirkwood based board with NAND flash so I'd > appreciate some feedback from someone with access to a few different > boards. > > From the datasheets I have access to it looks like there is only one > possible MPP configuration for NF_IO[0-7] so that is what I've > implemented. I'm not aware of anything using this driver that needs a > different MPP config. > > drivers/mtd/nand/kirkwood_nand.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/mtd/nand/kirkwood_nand.c > b/drivers/mtd/nand/kirkwood_nand.c > index 4fc34d6..ff81212 100644 > --- a/drivers/mtd/nand/kirkwood_nand.c > +++ b/drivers/mtd/nand/kirkwood_nand.c > @@ -9,6 +9,7 @@ > #include <common.h> > #include <asm/io.h> > #include <asm/arch/soc.h> > +#include <asm/arch/mpp.h> > #include <nand.h> > > /* NAND Flash Soc registers */ > @@ -22,6 +23,8 @@ struct kwnandf_registers { > static struct kwnandf_registers *nf_reg = > (struct kwnandf_registers *)KW_NANDF_BASE; > > +static u32 nand_mpp_backup[9] = { 0 }; There's a non-static nand_mpp_backup[] in drivers/spi/kirkwood_spi.c, with a different length. And the length assumed by kirkwood_mpp_conf seems to be different still (and larger than the spi array). > + > /* > * hardware specific access to control-lines/bits > */ > @@ -49,6 +52,22 @@ static void kw_nand_hwcontrol(struct mtd_info *mtd, int > cmd, > void kw_nand_select_chip(struct mtd_info *mtd, int chip) > { > u32 data; > + u32 nand_config[] = { > + MPP0_NF_IO2, > + MPP1_NF_IO3, > + MPP2_NF_IO4, > + MPP3_NF_IO5, > + MPP4_NF_IO6, > + MPP5_NF_IO7, > + MPP18_NF_IO0, > + MPP19_NF_IO1, > + 0 > + }; If you make this static const the compiler won't need to create the table at runtime. -Scott
Hi Scott, On Sat, Jan 23, 2016 at 9:11 AM, Scott Wood <oss@buserror.net> wrote: > On Fri, 2016-01-15 at 17:22 +1300, Chris Packham wrote: >> Claim the MPP pins for the NAND flash controller only when it's actually >> being used. This allows the pins to be shared with the SPI interface >> which already supports an equivalent on-access MPP reconfiguration. >> >> Reviewed-by: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz> >> Signed-off-by: Chris Packham <judge.packham@gmail.com> >> --- >> I haven't wrapped this with a configuration option because I think it >> should be safe to enable by default. It will either re-apply the same >> MPP configuration that has already been done in the board init or put >> the MPP pins into the correct mode to access NAND. >> >> I've only got access to one kirkwood based board with NAND flash so I'd >> appreciate some feedback from someone with access to a few different >> boards. >> >> From the datasheets I have access to it looks like there is only one >> possible MPP configuration for NF_IO[0-7] so that is what I've >> implemented. I'm not aware of anything using this driver that needs a >> different MPP config. >> >> drivers/mtd/nand/kirkwood_nand.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/drivers/mtd/nand/kirkwood_nand.c >> b/drivers/mtd/nand/kirkwood_nand.c >> index 4fc34d6..ff81212 100644 >> --- a/drivers/mtd/nand/kirkwood_nand.c >> +++ b/drivers/mtd/nand/kirkwood_nand.c >> @@ -9,6 +9,7 @@ >> #include <common.h> >> #include <asm/io.h> >> #include <asm/arch/soc.h> >> +#include <asm/arch/mpp.h> >> #include <nand.h> >> >> /* NAND Flash Soc registers */ >> @@ -22,6 +23,8 @@ struct kwnandf_registers { >> static struct kwnandf_registers *nf_reg = >> (struct kwnandf_registers *)KW_NANDF_BASE; >> >> +static u32 nand_mpp_backup[9] = { 0 }; > > There's a non-static nand_mpp_backup[] in drivers/spi/kirkwood_spi.c, with a > different length. And the length assumed by kirkwood_mpp_conf seems to be > different still (and larger than the spi array). I assume you mean "u32 spi_mpp_backup[4];". The length of the backup reflects the pins that are being changed. In kirkwood_spi it's configuring SPI_CK, SPI_MSIO, SPI_MOSI. So spi_mpp_config[4] reflects these 3 pin settings plus an empty end marker, spi_mpp_backup[4] is symmetrical although possibly only 3 are necessary. In the nand case there are 8 NF_IO pins, hence the array sizes are 8+1. kirkwood_mpp_conf doesn't enforce any particular size for mpp_save it is required to be at least as big as mpp_list. > >> + >> /* >> * hardware specific access to control-lines/bits >> */ >> @@ -49,6 +52,22 @@ static void kw_nand_hwcontrol(struct mtd_info *mtd, int >> cmd, >> void kw_nand_select_chip(struct mtd_info *mtd, int chip) >> { >> u32 data; >> + u32 nand_config[] = { >> + MPP0_NF_IO2, >> + MPP1_NF_IO3, >> + MPP2_NF_IO4, >> + MPP3_NF_IO5, >> + MPP4_NF_IO6, >> + MPP5_NF_IO7, >> + MPP18_NF_IO0, >> + MPP19_NF_IO1, >> + 0 >> + }; > > If you make this static const the compiler won't need to create the table at > runtime. Agreed. Will do in v2. > > -Scott >
diff --git a/drivers/mtd/nand/kirkwood_nand.c b/drivers/mtd/nand/kirkwood_nand.c index 4fc34d6..ff81212 100644 --- a/drivers/mtd/nand/kirkwood_nand.c +++ b/drivers/mtd/nand/kirkwood_nand.c @@ -9,6 +9,7 @@ #include <common.h> #include <asm/io.h> #include <asm/arch/soc.h> +#include <asm/arch/mpp.h> #include <nand.h> /* NAND Flash Soc registers */ @@ -22,6 +23,8 @@ struct kwnandf_registers { static struct kwnandf_registers *nf_reg = (struct kwnandf_registers *)KW_NANDF_BASE; +static u32 nand_mpp_backup[9] = { 0 }; + /* * hardware specific access to control-lines/bits */ @@ -49,6 +52,22 @@ static void kw_nand_hwcontrol(struct mtd_info *mtd, int cmd, void kw_nand_select_chip(struct mtd_info *mtd, int chip) { u32 data; + u32 nand_config[] = { + MPP0_NF_IO2, + MPP1_NF_IO3, + MPP2_NF_IO4, + MPP3_NF_IO5, + MPP4_NF_IO6, + MPP5_NF_IO7, + MPP18_NF_IO0, + MPP19_NF_IO1, + 0 + }; + + if (chip >= 0) + kirkwood_mpp_conf(nand_config, nand_mpp_backup); + else + kirkwood_mpp_conf(nand_mpp_backup, NULL); data = readl(&nf_reg->ctrl); data |= NAND_ACTCEBOOT_BIT;