[U-Boot] [RFC PATCH v1] kirkwood_nand: claim MPP pins on the fly

[Chris Packham]

Hi Scott,

On Sat, Jan 23, 2016 at 9:11 AM, Scott Wood <oss at 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 at alliedtelesis.co.nz>
>> Signed-off-by: Chris Packham <judge.packham at 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
>