diff mbox

[2/3] mtd: davinci-nand: add dts property for NAND_NO_SUBPAGE_WRITE option

Message ID 1395335184-4745-3-git-send-email-ivan.khoronzhuk@ti.com
State New
Headers show

Commit Message

Ivan Khoronzhuk March 20, 2014, 5:06 p.m. UTC
From: Murali Karicheri <m-karicheri2@ti.com>

After testing NAND flash with ubifs for k2hk-emv board were committed
that flash doesn't support subpage writing, so we can fix it by
adding a property to disable subpage write.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 Documentation/devicetree/bindings/mtd/davinci-nand.txt | 2 ++
 drivers/mtd/nand/davinci_nand.c                        | 3 +++
 2 files changed, 5 insertions(+)

Comments

Santosh Shilimkar March 20, 2014, 5:12 p.m. UTC | #1
Boris,

On Thursday 20 March 2014 01:06 PM, Ivan Khoronzhuk wrote:
> From: Murali Karicheri <m-karicheri2@ti.com>
> 
> After testing NAND flash with ubifs for k2hk-emv board were committed
> that flash doesn't support subpage writing, so we can fix it by
> adding a property to disable subpage write.
> 
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
Can you please pick this up for 3.15 fixes ? Ofcourse assuming DT folks
are ok with the patch.

I can then take patch 1/3 and 3/3 in my 3.15 fixes queue.

>  Documentation/devicetree/bindings/mtd/davinci-nand.txt | 2 ++
>  drivers/mtd/nand/davinci_nand.c                        | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
> index cfb18ab..50af930 100644
> --- a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
> @@ -53,6 +53,8 @@ Recommended properties :
>  				identifier is saved in OOB area. If not present
>  				false.
>  
> +- ti,davinci-nosubpage-write:	disable subpage write for the device
> +
>  Deprecated properties:
>  
>  - ti,davinci-ecc-mode:		operation mode of the NAND ecc mode. ECC mode
> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
> index 4615d79..3ba058d 100644
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
> @@ -581,6 +581,9 @@ static struct davinci_nand_pdata
>  		    of_property_read_bool(pdev->dev.of_node,
>  			"ti,davinci-nand-use-bbt"))
>  			pdata->bbt_options = NAND_BBT_USE_FLASH;
> +		if (of_property_read_bool(pdev->dev.of_node,
> +			"ti,davinci-no-subpage-write"))
> +			pdata->options |= NAND_NO_SUBPAGE_WRITE;
>  	}
>  
>  	return dev_get_platdata(&pdev->dev);
>
Brian Norris March 20, 2014, 5:29 p.m. UTC | #2
On Thu, Mar 20, 2014 at 01:12:35PM -0400, Santosh Shilimkar wrote:
> Boris,

Who's Boris? And why should Boris be taking this patch? It's an MTD
patch.

> On Thursday 20 March 2014 01:06 PM, Ivan Khoronzhuk wrote:
> > From: Murali Karicheri <m-karicheri2@ti.com>
> > 
> > After testing NAND flash with ubifs for k2hk-emv board were committed
> > that flash doesn't support subpage writing, so we can fix it by
> > adding a property to disable subpage write.

What flash? We try to autodetect NAND as much as possible. Perhaps we
should be adding infrastructure support instead of hacking it with a DT
property.

> > Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> > ---
> Can you please pick this up for 3.15 fixes ? Ofcourse assuming DT folks
> are ok with the patch.
> 
> I can then take patch 1/3 and 3/3 in my 3.15 fixes queue.
> 
> >  Documentation/devicetree/bindings/mtd/davinci-nand.txt | 2 ++
> >  drivers/mtd/nand/davinci_nand.c                        | 3 +++
> >  2 files changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
> > index cfb18ab..50af930 100644
> > --- a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
> > @@ -53,6 +53,8 @@ Recommended properties :
> >  				identifier is saved in OOB area. If not present
> >  				false.
> >  
> > +- ti,davinci-nosubpage-write:	disable subpage write for the device

If we really need the DT property, I'd prefer a generic
"nand-numer-of-partial-programs" or something like that, and if it's
equal to 1, then the flash doesn't support partial page programming.

But I'd prefer not to encode that in DT if possible.

> >  Deprecated properties:
> >  
> >  - ti,davinci-ecc-mode:		operation mode of the NAND ecc mode. ECC mode
> > diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
> > index 4615d79..3ba058d 100644
> > --- a/drivers/mtd/nand/davinci_nand.c
> > +++ b/drivers/mtd/nand/davinci_nand.c
> > @@ -581,6 +581,9 @@ static struct davinci_nand_pdata
> >  		    of_property_read_bool(pdev->dev.of_node,
> >  			"ti,davinci-nand-use-bbt"))
> >  			pdata->bbt_options = NAND_BBT_USE_FLASH;
> > +		if (of_property_read_bool(pdev->dev.of_node,
> > +			"ti,davinci-no-subpage-write"))
> > +			pdata->options |= NAND_NO_SUBPAGE_WRITE;
> >  	}
> >  
> >  	return dev_get_platdata(&pdev->dev);
> > 
> 

Brian
--
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
Santosh Shilimkar March 20, 2014, 5:37 p.m. UTC | #3
On Thursday 20 March 2014 01:29 PM, Brian Norris wrote:
> On Thu, Mar 20, 2014 at 01:12:35PM -0400, Santosh Shilimkar wrote:
>> Boris,
>
> Who's Boris? And why should Boris be taking this patch? It's an MTD
> patch.
> 
I got your name completely wrong. Sorry....

>> On Thursday 20 March 2014 01:06 PM, Ivan Khoronzhuk wrote:
>>> From: Murali Karicheri <m-karicheri2@ti.com>
>>>
>>> After testing NAND flash with ubifs for k2hk-emv board were committed
>>> that flash doesn't support subpage writing, so we can fix it by
>>> adding a property to disable subpage write.
> 
> What flash? We try to autodetect NAND as much as possible. Perhaps we
> should be adding infrastructure support instead of hacking it with a DT
> property.
>
We can't auto detect it and thats why the DT approach was taken. We
will double check that.

>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> ---
>> Can you please pick this up for 3.15 fixes ? Ofcourse assuming DT folks
>> are ok with the patch.
>>
>> I can then take patch 1/3 and 3/3 in my 3.15 fixes queue.
>>
>>>  Documentation/devicetree/bindings/mtd/davinci-nand.txt | 2 ++
>>>  drivers/mtd/nand/davinci_nand.c                        | 3 +++
>>>  2 files changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>> index cfb18ab..50af930 100644
>>> --- a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>> @@ -53,6 +53,8 @@ Recommended properties :
>>>  				identifier is saved in OOB area. If not present
>>>  				false.
>>>  
>>> +- ti,davinci-nosubpage-write:	disable subpage write for the device
> 
> If we really need the DT property, I'd prefer a generic
> "nand-numer-of-partial-programs" or something like that, and if it's
> equal to 1, then the flash doesn't support partial page programming.
>
OK. It can be updated as you suggested.
 
Thanks for quick response and sorry for the rename

Regards,
Santosh


--
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
Warner Losh March 20, 2014, 5:44 p.m. UTC | #4
On Mar 20, 2014, at 11:37 AM, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:

> 
> 
> On Thursday 20 March 2014 01:29 PM, Brian Norris wrote:
>> On Thu, Mar 20, 2014 at 01:12:35PM -0400, Santosh Shilimkar wrote:
>>> Boris,
>> 
>> Who's Boris? And why should Boris be taking this patch? It's an MTD
>> patch.
>> 
> I got your name completely wrong. Sorry....
> 
>>> On Thursday 20 March 2014 01:06 PM, Ivan Khoronzhuk wrote:
>>>> From: Murali Karicheri <m-karicheri2@ti.com>
>>>> 
>>>> After testing NAND flash with ubifs for k2hk-emv board were committed
>>>> that flash doesn't support subpage writing, so we can fix it by
>>>> adding a property to disable subpage write.
>> 
>> What flash? We try to autodetect NAND as much as possible. Perhaps we
>> should be adding infrastructure support instead of hacking it with a DT
>> property.
>> 
> We can't auto detect it and thats why the DT approach was taken. We
> will double check that.

I though sub page writing was one of the fields in the onfi and/or jedec(toggle) meta data structures. Have you looked there?

Warner

--
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
Santosh Shilimkar March 20, 2014, 6:02 p.m. UTC | #5
On Thursday 20 March 2014 01:44 PM, Warner Losh wrote:
> 
> On Mar 20, 2014, at 11:37 AM, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> 
>>
>>
>> On Thursday 20 March 2014 01:29 PM, Brian Norris wrote:
>>> On Thu, Mar 20, 2014 at 01:12:35PM -0400, Santosh Shilimkar wrote:
>>>> Boris,
>>>
>>> Who's Boris? And why should Boris be taking this patch? It's an MTD
>>> patch.
>>>
>> I got your name completely wrong. Sorry....
>>
>>>> On Thursday 20 March 2014 01:06 PM, Ivan Khoronzhuk wrote:
>>>>> From: Murali Karicheri <m-karicheri2@ti.com>
>>>>>
>>>>> After testing NAND flash with ubifs for k2hk-emv board were committed
>>>>> that flash doesn't support subpage writing, so we can fix it by
>>>>> adding a property to disable subpage write.
>>>
>>> What flash? We try to autodetect NAND as much as possible. Perhaps we
>>> should be adding infrastructure support instead of hacking it with a DT
>>> property.
>>>
>> We can't auto detect it and thats why the DT approach was taken. We
>> will double check that.
> 
> I though sub page writing was one of the fields in the onfi and/or jedec(toggle) meta data structures. Have you looked there?
> 
Am not sure if I follow you. The limitation is from the TI NAND controller(AEMIF) and
not the NAND memory. 

regards,
Santosh
Warner Losh March 20, 2014, 6:09 p.m. UTC | #6
On Mar 20, 2014, at 12:02 PM, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:

> On Thursday 20 March 2014 01:44 PM, Warner Losh wrote:
>> 
>> On Mar 20, 2014, at 11:37 AM, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>> 
>>> 
>>> 
>>> On Thursday 20 March 2014 01:29 PM, Brian Norris wrote:
>>>> On Thu, Mar 20, 2014 at 01:12:35PM -0400, Santosh Shilimkar wrote:
>>>>> Boris,
>>>> 
>>>> Who's Boris? And why should Boris be taking this patch? It's an MTD
>>>> patch.
>>>> 
>>> I got your name completely wrong. Sorry....
>>> 
>>>>> On Thursday 20 March 2014 01:06 PM, Ivan Khoronzhuk wrote:
>>>>>> From: Murali Karicheri <m-karicheri2@ti.com>
>>>>>> 
>>>>>> After testing NAND flash with ubifs for k2hk-emv board were committed
>>>>>> that flash doesn't support subpage writing, so we can fix it by
>>>>>> adding a property to disable subpage write.
>>>> 
>>>> What flash? We try to autodetect NAND as much as possible. Perhaps we
>>>> should be adding infrastructure support instead of hacking it with a DT
>>>> property.
>>>> 
>>> We can't auto detect it and thats why the DT approach was taken. We
>>> will double check that.
>> 
>> I though sub page writing was one of the fields in the onfi and/or jedec(toggle) meta data structures. Have you looked there?
>> 
> Am not sure if I follow you. The limitation is from the TI NAND controller(AEMIF) and
> not the NAND memory. 

Then never mind. I thought it was the other side of things.

Warner
Brian Norris March 20, 2014, 6:42 p.m. UTC | #7
On Thu, Mar 20, 2014 at 01:37:42PM -0400, Santosh Shilimkar wrote:
> On Thursday 20 March 2014 01:29 PM, Brian Norris wrote:
> > On Thu, Mar 20, 2014 at 01:12:35PM -0400, Santosh Shilimkar wrote:
> >> Boris,
> >
> > Who's Boris? And why should Boris be taking this patch? It's an MTD
> > patch.
> > 
> I got your name completely wrong. Sorry....

I see. Maybe you were just dropping letters from "BrianNorris" :)

Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Brian Norris March 20, 2014, 6:54 p.m. UTC | #8
On Thu, Mar 20, 2014 at 02:02:39PM -0400, Santosh Shilimkar wrote:
> On Thursday 20 March 2014 01:44 PM, Warner Losh wrote:
> > I though sub page writing was one of the fields in the onfi and/or jedec(toggle) meta data structures. Have you looked there?
> > 
> Am not sure if I follow you. The limitation is from the TI NAND controller(AEMIF) and
> not the NAND memory. 

That doesn't match the patch description, which says "that flash doesn't
support subpage writing". Flash != flash controller.

Which one is it? If it's a controller limitation, I think we should be
able to pull this from a "compatible" property, no?

Brian
--
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
Santosh Shilimkar March 20, 2014, 7:11 p.m. UTC | #9
On Thursday 20 March 2014 02:54 PM, Brian Norris wrote:
> On Thu, Mar 20, 2014 at 02:02:39PM -0400, Santosh Shilimkar wrote:
>> On Thursday 20 March 2014 01:44 PM, Warner Losh wrote:
>>> I though sub page writing was one of the fields in the onfi and/or jedec(toggle) meta data structures. Have you looked there?
>>>
>> Am not sure if I follow you. The limitation is from the TI NAND controller(AEMIF) and
>> not the NAND memory. 
> 
> That doesn't match the patch description, which says "that flash doesn't
> support subpage writing". Flash != flash controller.
>
Patch description is indeed doesn't reflect the actual issue.

> Which one is it? If it's a controller limitation, I think we should be
> able to pull this from a "compatible" property, no?
> 
Just to be accurate, the limitation(bug) is on the controller found on Keystone
SOCs. AEMIF controller is also used on DaVinci SOCs which don't seems to have
any issue. So even for compatible, you need to add keystone specific one.
Hence thought dt property is better option.

regards,
Santosh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Ivan Khoronzhuk March 20, 2014, 7:26 p.m. UTC | #10
On 03/20/2014 09:11 PM, Santosh Shilimkar wrote:
> On Thursday 20 March 2014 02:54 PM, Brian Norris wrote:
>> On Thu, Mar 20, 2014 at 02:02:39PM -0400, Santosh Shilimkar wrote:
>>> On Thursday 20 March 2014 01:44 PM, Warner Losh wrote:
>>>> I though sub page writing was one of the fields in the onfi and/or jedec(toggle) meta data structures. Have you looked there?
>>>>
>>> Am not sure if I follow you. The limitation is from the TI NAND controller(AEMIF) and
>>> not the NAND memory.
>> That doesn't match the patch description, which says "that flash doesn't
>> support subpage writing". Flash != flash controller.
>>
> Patch description is indeed doesn't reflect the actual issue.
>
>> Which one is it? If it's a controller limitation, I think we should be
>> able to pull this from a "compatible" property, no?
>>
> Just to be accurate, the limitation(bug) is on the controller found on Keystone
> SOCs. AEMIF controller is also used on DaVinci SOCs which don't seems to have
> any issue. So even for compatible, you need to add keystone specific one.
> Hence thought dt property is better option.
>
> regards,
> Santosh
>

I will use compatible approach.
We have keystone compatible in k2hk-evm.dts,
so I need to add it only in the davinci-nand driver.
I will add the following:

if (of_device_is_compatible(np, "ti,keystone-nand")) {
             pdata->options |= NAND_NO_SUBPAGE_WRITE;
  }

...

static const struct of_device_id davinci_nand_of_match[] = {
     {.compatible = "ti,davinci-nand", },
     {.compatible = "ti,keystone-nand", },
     {},
};
Santosh Shilimkar March 20, 2014, 7:28 p.m. UTC | #11
On Thursday 20 March 2014 03:26 PM, Ivan Khoronzhuk wrote:
> 
> On 03/20/2014 09:11 PM, Santosh Shilimkar wrote:
>> On Thursday 20 March 2014 02:54 PM, Brian Norris wrote:
>>> On Thu, Mar 20, 2014 at 02:02:39PM -0400, Santosh Shilimkar wrote:
>>>> On Thursday 20 March 2014 01:44 PM, Warner Losh wrote:
>>>>> I though sub page writing was one of the fields in the onfi and/or jedec(toggle) meta data structures. Have you looked there?
>>>>>
>>>> Am not sure if I follow you. The limitation is from the TI NAND controller(AEMIF) and
>>>> not the NAND memory.
>>> That doesn't match the patch description, which says "that flash doesn't
>>> support subpage writing". Flash != flash controller.
>>>
>> Patch description is indeed doesn't reflect the actual issue.
>>
>>> Which one is it? If it's a controller limitation, I think we should be
>>> able to pull this from a "compatible" property, no?
>>>
>> Just to be accurate, the limitation(bug) is on the controller found on Keystone
>> SOCs. AEMIF controller is also used on DaVinci SOCs which don't seems to have
>> any issue. So even for compatible, you need to add keystone specific one.
>> Hence thought dt property is better option.
>>

> 
> I will use compatible approach.
> We have keystone compatible in k2hk-evm.dts,
> so I need to add it only in the davinci-nand driver.
> I will add the following:
> 
> if (of_device_is_compatible(np, "ti,keystone-nand")) {
>             pdata->options |= NAND_NO_SUBPAGE_WRITE;
>  }
> 
> ...
> 
> static const struct of_device_id davinci_nand_of_match[] = {
>     {.compatible = "ti,davinci-nand", },
>     {.compatible = "ti,keystone-nand", },
>     {},
> };
> 
Looks good. I think you should go ahead and respin the patch
with above.

Regards,
Santosh
--
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
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
index cfb18ab..50af930 100644
--- a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
@@ -53,6 +53,8 @@  Recommended properties :
 				identifier is saved in OOB area. If not present
 				false.
 
+- ti,davinci-nosubpage-write:	disable subpage write for the device
+
 Deprecated properties:
 
 - ti,davinci-ecc-mode:		operation mode of the NAND ecc mode. ECC mode
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 4615d79..3ba058d 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -581,6 +581,9 @@  static struct davinci_nand_pdata
 		    of_property_read_bool(pdev->dev.of_node,
 			"ti,davinci-nand-use-bbt"))
 			pdata->bbt_options = NAND_BBT_USE_FLASH;
+		if (of_property_read_bool(pdev->dev.of_node,
+			"ti,davinci-no-subpage-write"))
+			pdata->options |= NAND_NO_SUBPAGE_WRITE;
 	}
 
 	return dev_get_platdata(&pdev->dev);