| Message ID | 1395335184-4745-3-git-send-email-ivan.khoronzhuk@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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); >
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
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
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
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
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
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
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
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
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", }, {}, };
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
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);