diff mbox

[PATCHv2,1/1] mtd: gpmi: make blockmark swapping optional

Message ID 1395399017-19005-1-git-send-email-LW@KARO-electronics.de
State Accepted, archived
Headers show

Commit Message

Lothar Waßmann March 21, 2014, 10:50 a.m. UTC
With a flash-based BBT there is no reason to move the Factory Bad
Block Marker from the data area buffer (to where it is mapped by the
GPMI NAND controller) to the OOB buffer. Thus, make this feature
configurable via DT. This is required for the Ka-Ro electronics
platforms.

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 Documentation/devicetree/bindings/mtd/gpmi-nand.txt |    3 +++
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c              |   10 +++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

Comments

Huang Shijie March 24, 2014, 9:59 a.m. UTC | #1
On Fri, Mar 21, 2014 at 11:50:17AM +0100, Lothar Waßmann wrote:
> With a flash-based BBT there is no reason to move the Factory Bad
> Block Marker from the data area buffer (to where it is mapped by the
> GPMI NAND controller) to the OOB buffer. Thus, make this feature
> configurable via DT. This is required for the Ka-Ro electronics
> platforms.
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
>  Documentation/devicetree/bindings/mtd/gpmi-nand.txt |    3 +++
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c              |   10 +++++++---
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
> index 458d596..f28949a 100644
> --- a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
> @@ -25,6 +25,9 @@ Optional properties:
>                         discoverable or this property is not enabled,
>                         the software may chooses an implementation-defined
>                         ECC scheme.
> +  - fsl,no-blockmark-swap: Don't swap the bad block marker from the OOB
> +                       area with the byte in the data area but rely on the
> +                       BBT for identifying bad blocks.

Please add the following:
       "NOTE: This property is not supported by the imx23 and imx28"
>  
>  The device tree may optionally contain sub-nodes describing partitions of the
>  address space. See partition.txt for more detail.
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index bb77f75..98562eb 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -1632,9 +1632,6 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
>  	struct bch_geometry *bch_geo = &this->bch_geometry;
>  	int ret;
>  
> -	/* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
> -	this->swap_block_mark = !GPMI_IS_MX23(this);
> -
>  	/* Set up the medium geometry */
>  	ret = gpmi_set_geometry(this);
>  	if (ret)
> @@ -1701,6 +1698,13 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
>  	if (of_get_nand_on_flash_bbt(this->dev->of_node))
>  		chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
>  
> +	/* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
> +	if (!of_property_read_bool(this->dev->of_node, "fsl,no-blockmark-swap"))
> +		this->swap_block_mark = !GPMI_IS_MX23(this);

the code should like this:
----------------------------------------------
	this->swap_block_mark = !GPMI_IS_MX23(this);

	if (!GPMI_IS_IMX23(this) && !GPMI_IS_MX28(this) &&
		of_property_read_bool(this->dev->of_node, "fsl,no-blockmark-swap"))
		this->swap_block_mark = false;
----------------------------------------------
          
> +
> +	dev_dbg(this->dev, "Blockmark swapping %sabled\n",
> +		this->swap_block_mark ? "en" : "dis");
> +
>  	/*
>  	 * Allocate a temporary DMA buffer for reading ID in the
>  	 * nand_scan_ident().

There are some bugs in the gpmi driver which is caused by the assumption that
we always enable the swapping except the imx23. 

So, there are some places should be changed with this patch:
   [1] the subpage hook,
        Please also change the gpmi_ecc_read_subpage() too.

   [2] the OOB hook,
   	Please also change the gpmi_ecc_read_oob(). 


   [3] the markbad hook,
   	Please also change the gpmi_block_markbad() . 

thanks
Huang Shijie
        

	



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lothar Waßmann March 26, 2014, 8:51 a.m. UTC | #2
Hi,

Huang Shijie wrote:
> On Fri, Mar 21, 2014 at 11:50:17AM +0100, Lothar Waßmann wrote:
> > With a flash-based BBT there is no reason to move the Factory Bad
> > Block Marker from the data area buffer (to where it is mapped by the
> > GPMI NAND controller) to the OOB buffer. Thus, make this feature
> > configurable via DT. This is required for the Ka-Ro electronics
> > platforms.
> > 
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > ---
> >  Documentation/devicetree/bindings/mtd/gpmi-nand.txt |    3 +++
> >  drivers/mtd/nand/gpmi-nand/gpmi-nand.c              |   10 +++++++---
> >  2 files changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
> > index 458d596..f28949a 100644
> > --- a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
> > @@ -25,6 +25,9 @@ Optional properties:
> >                         discoverable or this property is not enabled,
> >                         the software may chooses an implementation-defined
> >                         ECC scheme.
> > +  - fsl,no-blockmark-swap: Don't swap the bad block marker from the OOB
> > +                       area with the byte in the data area but rely on the
> > +                       BBT for identifying bad blocks.
> 
> Please add the following:
>        "NOTE: This property is not supported by the imx23 and imx28"
>
I don't see why this should not be supported on i.MX28 (i.MX23 doesn't
do byteswapping anyway, so this wouldn't change anything for i.MX23).
The partitions used by Linux need not necessarily be accessible for the
Boot ROM code (and vice versa).

> >  The device tree may optionally contain sub-nodes describing partitions of the
> >  address space. See partition.txt for more detail.
> > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > index bb77f75..98562eb 100644
> > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > @@ -1632,9 +1632,6 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
> >  	struct bch_geometry *bch_geo = &this->bch_geometry;
> >  	int ret;
> >  
> > -	/* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
> > -	this->swap_block_mark = !GPMI_IS_MX23(this);
> > -
> >  	/* Set up the medium geometry */
> >  	ret = gpmi_set_geometry(this);
> >  	if (ret)
> > @@ -1701,6 +1698,13 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
> >  	if (of_get_nand_on_flash_bbt(this->dev->of_node))
> >  		chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
> >  
> > +	/* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
> > +	if (!of_property_read_bool(this->dev->of_node, "fsl,no-blockmark-swap"))
> > +		this->swap_block_mark = !GPMI_IS_MX23(this);
> 
> the code should like this:
> ----------------------------------------------
> 	this->swap_block_mark = !GPMI_IS_MX23(this);
> 
> 	if (!GPMI_IS_IMX23(this) && !GPMI_IS_MX28(this) &&
>
the !GPMI_IS_MX23() is redundant here as i.MX23 will always have
swap_block_mark == false.
I would rather add a dependency on nand-on-flash-bbt as a
prerequisite for not checking the BB marks in each block.

> > +
> > +	dev_dbg(this->dev, "Blockmark swapping %sabled\n",
> > +		this->swap_block_mark ? "en" : "dis");
> > +
> >  	/*
> >  	 * Allocate a temporary DMA buffer for reading ID in the
> >  	 * nand_scan_ident().
> 
> There are some bugs in the gpmi driver which is caused by the assumption that
> we always enable the swapping except the imx23. 
> 
> So, there are some places should be changed with this patch:
>    [1] the subpage hook,
>         Please also change the gpmi_ecc_read_subpage() too.
> 
>    [2] the OOB hook,
>    	Please also change the gpmi_ecc_read_oob(). 
> 
>    [3] the markbad hook,
>    	Please also change the gpmi_block_markbad() . 
> 
OK, I'll check that.


Lothar Waßmann
Huang Shijie March 26, 2014, 10:41 a.m. UTC | #3
于 2014年03月26日 16:51, Lothar Waßmann 写道:
> I don't see why this should not be supported on i.MX28 (i.MX23 doesn't
> do byteswapping anyway, so this wouldn't change anything for i.MX23).
> The partitions used by Linux need not necessarily be accessible for the
> Boot ROM code (and vice versa).
But the first partition used to store the u-boot is accessible for the ROM.

Please see "Figure 12-13" in the 12.12.1.12:
   "In order to preserve the BI (bad block information), flash updater 
or gang programmer
applications need to swap Bad Block Information (BI) data to byte 0 of 
metadata area for
every page before programming NAND Flash. ROM when loading firmware, 
copies back
the value at metadata[0] to BI offset in page data. The following figure 
shows how the
factory bad block marker is preserved."

So please the imx28 should _NOT_ support this feature.

thanks
Huang Shijie



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lothar Waßmann March 26, 2014, 11:55 a.m. UTC | #4
Hi,

Huang Shijie wrote:
> 于 2014年03月26日 16:51, Lothar Waßmann 写道:
> > I don't see why this should not be supported on i.MX28 (i.MX23 doesn't
> > do byteswapping anyway, so this wouldn't change anything for i.MX23).
> > The partitions used by Linux need not necessarily be accessible for the
> > Boot ROM code (and vice versa).
> But the first partition used to store the u-boot is accessible for the ROM.
> 
Whether this partition is available to Linux depends on the partition table
passed in the DT.

> Please see "Figure 12-13" in the 12.12.1.12:
>    "In order to preserve the BI (bad block information), flash updater 
> or gang programmer
> applications need to swap Bad Block Information (BI) data to byte 0 of 
> metadata area for
> every page before programming NAND Flash. ROM when loading firmware, 
> copies back
> the value at metadata[0] to BI offset in page data. The following figure 
> shows how the
> factory bad block marker is preserved."
> 
The inspection of the BB markers is only a fallback for the case that
there is no DBBT. From the same chapter that you quoted above:
| ROM uses DBBT to skip any bad block that falls within firmware data
| on NAND Flash device.
| If the address of DBBT Search Area in FCB is 0, ROM will rely on
| factory marked bad block markers to find out if a block is good or bad.

Thus, even the boot ROM of i.MX28 can well live without blockmark
swapping.


Lothar Waßmann
Huang Shijie March 27, 2014, 9:59 a.m. UTC | #5
On Wed, Mar 26, 2014 at 12:55:02PM +0100, Lothar Waßmann wrote:
> Hi,
> 
> Huang Shijie wrote:
> > 于 2014年03月26日 16:51, Lothar Waßmann 写道:
> > > I don't see why this should not be supported on i.MX28 (i.MX23 doesn't
> > > do byteswapping anyway, so this wouldn't change anything for i.MX23).
> > > The partitions used by Linux need not necessarily be accessible for the
> > > Boot ROM code (and vice versa).
> > But the first partition used to store the u-boot is accessible for the ROM.
> > 
> Whether this partition is available to Linux depends on the partition table
> passed in the DT.
yes, i agree.

But it is strange if we do not export this partition to Linux.

> 
> > Please see "Figure 12-13" in the 12.12.1.12:
> >    "In order to preserve the BI (bad block information), flash updater 
> > or gang programmer
> > applications need to swap Bad Block Information (BI) data to byte 0 of 
> > metadata area for
> > every page before programming NAND Flash. ROM when loading firmware, 
> > copies back
> > the value at metadata[0] to BI offset in page data. The following figure 
> > shows how the
> > factory bad block marker is preserved."
> > 
> The inspection of the BB markers is only a fallback for the case that
> there is no DBBT. From the same chapter that you quoted above:
> | ROM uses DBBT to skip any bad block that falls within firmware data
> | on NAND Flash device.
> | If the address of DBBT Search Area in FCB is 0, ROM will rely on
> | factory marked bad block markers to find out if a block is good or bad.
> 
> Thus, even the boot ROM of i.MX28 can well live without blockmark
> swapping.

Assume that there is a NAND block "A",  and the A consist of 256 pages.
the uboot is burned to the "A", can occupy 6 pages:

  -----------------------------------------------------------------------------
 | page 0 |  page 1 | page 2 | page 3 | page 4 | page 5 | ... | ... | page 255 |
  -----------------------------------------------------------------------------
 
  \-------------------------------------- ------------------------------------/
                                         V  
                                        "A"					 

The DBBT is used to track if "A" is bad or not.
Assume we know that "A" is a good block, ROM then need to read out the uboot.
When the ROM needs to read out the 6 pages one by one. And each time the ROM read
the page, it should do the swapping for this page.

In this case, the ROM will do the swapping six times.

Please read the sector again, you will see the "every page" in it:
--------------------------------------------------------------------
   "In order to preserve the BI (bad block information), flash updater 
or gang programmer applications need to swap Bad Block Information (BI) data to byte 0 of 
metadata area for every page before programming NAND Flash. ROM when loading firmware, 
copies back
--------------------------------------------------------------------

thanks
Huang Shijie

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lothar Waßmann March 27, 2014, 12:21 p.m. UTC | #6
Hi,

Huang Shijie wrote:
> > > Please see "Figure 12-13" in the 12.12.1.12:
> > >    "In order to preserve the BI (bad block information), flash updater 
> > > or gang programmer
> > > applications need to swap Bad Block Information (BI) data to byte 0 of 
> > > metadata area for
> > > every page before programming NAND Flash. ROM when loading firmware, 
> > > copies back
> > > the value at metadata[0] to BI offset in page data. The following figure 
> > > shows how the
> > > factory bad block marker is preserved."
> > > 
> > The inspection of the BB markers is only a fallback for the case that
> > there is no DBBT. From the same chapter that you quoted above:
> > | ROM uses DBBT to skip any bad block that falls within firmware data
> > | on NAND Flash device.
> > | If the address of DBBT Search Area in FCB is 0, ROM will rely on
> > | factory marked bad block markers to find out if a block is good or bad.
> > 
> > Thus, even the boot ROM of i.MX28 can well live without blockmark
> > swapping.
> 
> Assume that there is a NAND block "A",  and the A consist of 256 pages.
> the uboot is burned to the "A", can occupy 6 pages:
> 
>   -----------------------------------------------------------------------------
>  | page 0 |  page 1 | page 2 | page 3 | page 4 | page 5 | ... | ... | page 255 |
>   -----------------------------------------------------------------------------
>  
>   \-------------------------------------- ------------------------------------/
>                                          V  
>                                         "A"					 
> 
> The DBBT is used to track if "A" is bad or not.
> Assume we know that "A" is a good block, ROM then need to read out the uboot.
> When the ROM needs to read out the 6 pages one by one. And each time the ROM read
> the page, it should do the swapping for this page.
> 
> In this case, the ROM will do the swapping six times.
> 
> Please read the sector again, you will see the "every page" in it:
> --------------------------------------------------------------------
>    "In order to preserve the BI (bad block information), flash updater 
> or gang programmer applications need to swap Bad Block Information (BI) data to byte 0 of 
> metadata area for every page before programming NAND Flash. ROM when loading firmware, 
> copies back
> --------------------------------------------------------------------
>
I can assure you that the >100.000 i.MX28 based modules, that we sold
up to now boot from NAND just fine without any block mark swapping in
the U-Boot pages.


Lothar Waßmann
Huang Shijie March 28, 2014, 2:09 a.m. UTC | #7
于 2014年03月27日 20:21, Lothar Waßmann 写道:
> I can assure you that the>100.000 i.MX28 based modules, that we sold
> up to now boot from NAND just fine without any block mark swapping in
> the U-Boot pages.
>
Our driver should follow the Reference manual, not the product.

thanks
Huang Shijie

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lothar Waßmann March 28, 2014, 8:16 a.m. UTC | #8
Hi,

Huang Shijie wrote:
> 于 2014年03月27日 20:21, Lothar Waßmann 写道:
> > I can assure you that the>100.000 i.MX28 based modules, that we sold
> > up to now boot from NAND just fine without any block mark swapping in
> > the U-Boot pages.
> >
> Our driver should follow the Reference manual, not the product.
> 
Still there is no need for the Linux NAND driver to be able to read or
write partitions in a format that the Boot ROM can understand. Thus it
is perfectly legal to allow disregarding the BB marks and solely rely on
a flash based BBT.


Lothar Waßmann
Huang Shijie March 28, 2014, 8:39 a.m. UTC | #9
于 2014年03月28日 16:16, Lothar Waßmann 写道:
> Still there is no need for the Linux NAND driver to be able to read or
> write partitions in a format that the Boot ROM can understand. Thus it
If you do not use the NAND boot, there really no need to do so.

Since you need the NAND boot, we should enable the swapping for imx28.
> is perfectly legal to allow disregarding the BB marks and solely rely on
> a flash based BBT.
>
The BB mark is in the page 0 of a NAND block. But the swapping can occur 
in _each_ page of
a NAND block, _NOT_ only the page 0.

I think you are confusing at these two things.

thanks
Huang Shijie









--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sascha Hauer March 28, 2014, 9 a.m. UTC | #10
On Fri, Mar 28, 2014 at 04:39:00PM +0800, Huang Shijie wrote:
> 于 2014年03月28日 16:16, Lothar Waßmann 写道:
> >Still there is no need for the Linux NAND driver to be able to read or
> >write partitions in a format that the Boot ROM can understand. Thus it
> If you do not use the NAND boot, there really no need to do so.
> 
> Since you need the NAND boot, we should enable the swapping for imx28.
> >is perfectly legal to allow disregarding the BB marks and solely rely on
> >a flash based BBT.
> >
> The BB mark is in the page 0 of a NAND block. But the swapping can
> occur in _each_ page of
> a NAND block, _NOT_ only the page 0.
> 
> I think you are confusing at these two things.

I think you are confusing two things. If Lothar:

- disables swapping of BB marks in the FCB the ROM won't swap bytes.
- writes data without swapping
- Uses a DBBT for telling the ROM code where bad blocks are
- uses a Flash BBT for mtd

Then everything should work just fine.

Sascha
Lothar Waßmann March 28, 2014, 9:01 a.m. UTC | #11
Hi,

Huang Shijie wrote:
> 于 2014年03月28日 16:16, Lothar Waßmann 写道:
> > Still there is no need for the Linux NAND driver to be able to read or
> > write partitions in a format that the Boot ROM can understand. Thus it
> If you do not use the NAND boot, there really no need to do so.
>
There is no need for the ROM code to access any other partition than
the bootloader itself. Thus Linux can perfectly well be booted from
NAND without any byte swapping.

> Since you need the NAND boot, we should enable the swapping for imx28.
>
Why? There is no need for Linux to do any swapping when not accessing
the bootloader partition.

> > is perfectly legal to allow disregarding the BB marks and solely rely on
> > a flash based BBT.
> >
> The BB mark is in the page 0 of a NAND block. But the swapping can occur 
> in _each_ page of
> a NAND block, _NOT_ only the page 0.
> 
> I think you are confusing at these two things.
> 
No, I'm not confusing anything. The swapping in any other page than
page 0 is completely useless. Thus, when there is no need to recognize
the BB markers, there in no need to do any swapping too.


Lothar Waßmann
Huang Shijie March 28, 2014, 9:26 a.m. UTC | #12
于 2014年03月28日 17:00, Sascha Hauer 写道:
> - disables swapping of BB marks in the FCB the ROM won't swap bytes.
We can not disable the swapping in the FCB in the imx28.
The DISBBM bit in the FCB does _NOT_ exit in the imx28's FCB.


thanks
Huang Shijie

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lothar Waßmann March 28, 2014, 9:31 a.m. UTC | #13
Hi,

Huang Shijie wrote:
> 于 2014年03月28日 17:00, Sascha Hauer 写道:
> > - disables swapping of BB marks in the FCB the ROM won't swap bytes.
> We can not disable the swapping in the FCB in the imx28.
> The DISBBM bit in the FCB does _NOT_ exit in the imx28's FCB.
> 
The Linux kernel does not have any business with the FCB, so it is
completely irrelevant for Linux whether byte swapping can be disabled
in the FCB or not.


Lothar Waßmann
Huang Shijie March 28, 2014, 9:33 a.m. UTC | #14
于 2014年03月28日 17:01, Lothar Waßmann 写道:
> There is no need for the ROM code to access any other partition than
> the bootloader itself. Thus Linux can perfectly well be booted from
Assume the ROM only access the partition which contains the bootloader, and
the ROM does the swapping for the pages(but we do not do the swapping to 
these pages),

what will happen?



thanks
Huang Shijie

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Huang Shijie March 28, 2014, 9:38 a.m. UTC | #15
于 2014年03月28日 17:33, Huang Shijie 写道:
> 于 2014年03月28日 17:01, Lothar Waßmann 写道:
>> There is no need for the ROM code to access any other partition than
>> the bootloader itself. Thus Linux can perfectly well be booted from
> Assume the ROM only access the partition which contains the 
> bootloader, and
> the ROM does the swapping for the pages(but we do not do the swapping 
> to these pages),
I mean the kernel do not do the swapping when we burn the uboot to the NAND.

thanks
Huang Shijie
>
> what will happen?
>
>


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Huang Shijie March 28, 2014, 10:09 a.m. UTC | #16
于 2014年03月28日 17:31, Lothar Waßmann 写道:
> Hi,
>
> Huang Shijie wrote:
>> 于 2014年03月28日 17:00, Sascha Hauer 写道:
>>> - disables swapping of BB marks in the FCB the ROM won't swap bytes.
>> We can not disable the swapping in the FCB in the imx28.
>> The DISBBM bit in the FCB does _NOT_ exit in the imx28's FCB.
>>
> The Linux kernel does not have any business with the FCB, so it is
> completely irrelevant for Linux whether byte swapping can be disabled
> in the FCB or not.
>
But the gpmi is relevant to the ROM.
the gpmi driver should keep consistency with the ROM.

I do not object you add the swapping optional the gpmi driver(since it's 
ok for imx50/imx6q/imx6sx),
i just object that you enable this feature for imx28.



thanks
Huang Shijie

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lothar Waßmann March 28, 2014, 10:13 a.m. UTC | #17
Hi,

Huang Shijie wrote:
> 于 2014年03月28日 17:33, Huang Shijie 写道:
> > 于 2014年03月28日 17:01, Lothar Waßmann 写道:
> >> There is no need for the ROM code to access any other partition than
> >> the bootloader itself. Thus Linux can perfectly well be booted from
> > Assume the ROM only access the partition which contains the 
> > bootloader, and
> > the ROM does the swapping for the pages(but we do not do the swapping 
> > to these pages),
> I mean the kernel do not do the swapping when we burn the uboot to the NAND.
> 
The kernel need not be able to write the bootloader to NAND. 
If you want a kernel that can do it, you just need to boot it with one
line in the DTB removed.


Lothar Waßmann
Lothar Waßmann March 28, 2014, 10:18 a.m. UTC | #18
Hi,

Huang Shijie wrote:
> 于 2014年03月28日 17:31, Lothar Waßmann 写道:
> > Hi,
> >
> > Huang Shijie wrote:
> >> 于 2014年03月28日 17:00, Sascha Hauer 写道:
> >>> - disables swapping of BB marks in the FCB the ROM won't swap bytes.
> >> We can not disable the swapping in the FCB in the imx28.
> >> The DISBBM bit in the FCB does _NOT_ exit in the imx28's FCB.
> >>
> > The Linux kernel does not have any business with the FCB, so it is
> > completely irrelevant for Linux whether byte swapping can be disabled
> > in the FCB or not.
> >
> But the gpmi is relevant to the ROM.
> the gpmi driver should keep consistency with the ROM.
> 
> I do not object you add the swapping optional the gpmi driver(since it's 
> ok for imx50/imx6q/imx6sx),
> i just object that you enable this feature for imx28.
> 
We are talking about a Linux Kernel Driver here, not about a flashtool
for i.MX28.
It is a valid usecase for Linux not being able to write the bootloader,
so there is no reason to disallow this feature in the driver code.
Since it is a DT option it can be trivially switched on and off without
having to change anything in the kernel.


Lothar Waßmann
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
index 458d596..f28949a 100644
--- a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
@@ -25,6 +25,9 @@  Optional properties:
                        discoverable or this property is not enabled,
                        the software may chooses an implementation-defined
                        ECC scheme.
+  - fsl,no-blockmark-swap: Don't swap the bad block marker from the OOB
+                       area with the byte in the data area but rely on the
+                       BBT for identifying bad blocks.
 
 The device tree may optionally contain sub-nodes describing partitions of the
 address space. See partition.txt for more detail.
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index bb77f75..98562eb 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1632,9 +1632,6 @@  static int gpmi_init_last(struct gpmi_nand_data *this)
 	struct bch_geometry *bch_geo = &this->bch_geometry;
 	int ret;
 
-	/* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
-	this->swap_block_mark = !GPMI_IS_MX23(this);
-
 	/* Set up the medium geometry */
 	ret = gpmi_set_geometry(this);
 	if (ret)
@@ -1701,6 +1698,13 @@  static int gpmi_nand_init(struct gpmi_nand_data *this)
 	if (of_get_nand_on_flash_bbt(this->dev->of_node))
 		chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
 
+	/* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
+	if (!of_property_read_bool(this->dev->of_node, "fsl,no-blockmark-swap"))
+		this->swap_block_mark = !GPMI_IS_MX23(this);
+
+	dev_dbg(this->dev, "Blockmark swapping %sabled\n",
+		this->swap_block_mark ? "en" : "dis");
+
 	/*
 	 * Allocate a temporary DMA buffer for reading ID in the
 	 * nand_scan_ident().