Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
On 30.04.2015 16:31, Antoine Tenart wrote: On Thu, Apr 16, 2015 at 01:59:32PM -0300, Ezequiel Garcia wrote: On 04/16/2015 10:41 AM, Sebastian Hesselbarth wrote: All we need is a function to convert sdr_timings to sane driver timings. And we really need to split this patch into tiny pieces otherwise it is not reviewable - or at least I need a full overview about the driver first. I think that's a bit of a different issue. This patch seems to be doing two things: it removes the in-driver flash detection *and* reworks timing setup. How about we split this in two or even three patches? Along these lines: 1) introduce timing helpers, 2) rework timing setup, 3) remove in-driver flash detection. Not sure how feasible it is. That's quite difficult, as you cannot have 1) without having the changes made in 2). Flash detection and timing reworks are linked and I'm not sure we can have this split into 2 or 3 patches without having a state where the driver does not work. Antoine, functionally you are right. But splitting the patch into the three pieces above will heavily reduce the diff-per-patch significantly. For example, if you introduce 1) without using it, we can only look at the helper. Then in 2) you actually use that helper and 3) will clean the mess up. BTW, I am fine with running the flash on mach-berlin boards with some default timing for now until we worked out how mtd will deal with protocol and array timings. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
On Thu, Apr 16, 2015 at 01:59:32PM -0300, Ezequiel Garcia wrote: > On 04/16/2015 10:41 AM, Sebastian Hesselbarth wrote: > > > All we need is a function to convert sdr_timings to sane driver > > timings. And we really need to split this patch into tiny pieces > > otherwise it is not reviewable - or at least I need a full overview > > about the driver first. > > > > I think that's a bit of a different issue. This patch seems to be doing > two things: it removes the in-driver flash detection *and* reworks > timing setup. > > How about we split this in two or even three patches? Along these lines: > 1) introduce timing helpers, 2) rework timing setup, 3) remove in-driver > flash detection. Not sure how feasible it is. That's quite difficult, as you cannot have 1) without having the changes made in 2). Flash detection and timing reworks are linked and I'm not sure we can have this split into 2 or 3 patches without having a state where the driver does not work. Antoine -- Antoine Ténart, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
On Thu, Apr 16, 2015 at 01:59:32PM -0300, Ezequiel Garcia wrote: On 04/16/2015 10:41 AM, Sebastian Hesselbarth wrote: All we need is a function to convert sdr_timings to sane driver timings. And we really need to split this patch into tiny pieces otherwise it is not reviewable - or at least I need a full overview about the driver first. I think that's a bit of a different issue. This patch seems to be doing two things: it removes the in-driver flash detection *and* reworks timing setup. How about we split this in two or even three patches? Along these lines: 1) introduce timing helpers, 2) rework timing setup, 3) remove in-driver flash detection. Not sure how feasible it is. That's quite difficult, as you cannot have 1) without having the changes made in 2). Flash detection and timing reworks are linked and I'm not sure we can have this split into 2 or 3 patches without having a state where the driver does not work. Antoine -- Antoine Ténart, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
On 30.04.2015 16:31, Antoine Tenart wrote: On Thu, Apr 16, 2015 at 01:59:32PM -0300, Ezequiel Garcia wrote: On 04/16/2015 10:41 AM, Sebastian Hesselbarth wrote: All we need is a function to convert sdr_timings to sane driver timings. And we really need to split this patch into tiny pieces otherwise it is not reviewable - or at least I need a full overview about the driver first. I think that's a bit of a different issue. This patch seems to be doing two things: it removes the in-driver flash detection *and* reworks timing setup. How about we split this in two or even three patches? Along these lines: 1) introduce timing helpers, 2) rework timing setup, 3) remove in-driver flash detection. Not sure how feasible it is. That's quite difficult, as you cannot have 1) without having the changes made in 2). Flash detection and timing reworks are linked and I'm not sure we can have this split into 2 or 3 patches without having a state where the driver does not work. Antoine, functionally you are right. But splitting the patch into the three pieces above will heavily reduce the diff-per-patch significantly. For example, if you introduce 1) without using it, we can only look at the helper. Then in 2) you actually use that helper and 3) will clean the mess up. BTW, I am fine with running the flash on mach-berlin boards with some default timing for now until we worked out how mtd will deal with protocol and array timings. Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
Ezequiel Garcia writes: >> Also, as soon as Robert moves pxa3xx boards fully to DT, we'll loose >> the pdata timings option above. *sigh* >> > Well, such move would include proper timing DT properties for non-ONFI > devices. I will move several boards to DT, including several pxa3x boards, but not _all_ the boards. For reference, I said in [1] this : > Actually, this deserves another discussion alltogether. My plan was not to > convert all pxa board files to dt support, but all internal SoC IPs drivers + > mach/plat support. > > Or put another way at the end : > - there will be at least one pxa25x board which is fully DT converted > - there will be at least one pxa27x board which is fully DT converted > - there will be at least one pxa3xx board which is fully DT converted > > - there will be at least one pxa25x board which is not DT converted > - there will be at least one pxa27x board which is not DT converted > - there will be at least one pxa3xx board which is not DT converted > > I want to keep the support for both legacy platform_data and DT for pxa > architecture. The idea I had was that only fully DT converted machines will > benefit from multiplatform support. And as you said Ezequiel, the problem will arise regardless of being DT or not, it's a lack of conformance of the zylonite board's nand amongst others. So the timings will have to survive somewhere, whichever place is seen fit. Cheers. -- Robert [1] http://lists.openwall.net/linux-kernel/2015/03/24/101 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
Ezequiel Garcia ezequiel.gar...@free-electrons.com writes: Also, as soon as Robert moves pxa3xx boards fully to DT, we'll loose the pdata timings option above. *sigh* Well, such move would include proper timing DT properties for non-ONFI devices. I will move several boards to DT, including several pxa3x boards, but not _all_ the boards. For reference, I said in [1] this : Actually, this deserves another discussion alltogether. My plan was not to convert all pxa board files to dt support, but all internal SoC IPs drivers + mach/plat support. Or put another way at the end : - there will be at least one pxa25x board which is fully DT converted - there will be at least one pxa27x board which is fully DT converted - there will be at least one pxa3xx board which is fully DT converted - there will be at least one pxa25x board which is not DT converted - there will be at least one pxa27x board which is not DT converted - there will be at least one pxa3xx board which is not DT converted I want to keep the support for both legacy platform_data and DT for pxa architecture. The idea I had was that only fully DT converted machines will benefit from multiplatform support. And as you said Ezequiel, the problem will arise regardless of being DT or not, it's a lack of conformance of the zylonite board's nand amongst others. So the timings will have to survive somewhere, whichever place is seen fit. Cheers. -- Robert [1] http://lists.openwall.net/linux-kernel/2015/03/24/101 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
On 04/16/2015 10:41 AM, Sebastian Hesselbarth wrote: > On 04/16/2015 03:10 PM, Ezequiel Garcia wrote: >> On 04/15/2015 04:11 PM, Sebastian Hesselbarth wrote: >>> On 15.04.2015 19:24, Antoine Tenart wrote: Rework the pxa3xx_nand driver to allow using functions exported by the nand framework to detect the flash and to configure the timings. Because this driver supports some non-ONFI devices, we also keep the custom timing setup of this driver so these devices won't break. Signed-off-by: Antoine Tenart --- >>> [...] >>> How about we get rid of the driver specific timings completely >>> and pick up the best onfi timing match instead? The nand_ids table >>> allows for a default_onfi_timing parameter even if onfi itself is >>> not supported. >>> >>> For generic flash, i.e. no specific entry in the nand_ids table, >>> we either choose onfi mode 0 (most conservative) or an even slower >>> one. >>> >> >> I think Robert mentioned [1] that using "ONFI default timings" on >> non-ONFI devices didn't work for him. >> >> [1] https://lkml.org/lkml/2015/3/8/124 > > Ok, I see. But there is still the option to pass board specific > timings with driver's platform_data. We could use > > (a) pdata timings if passed > (b) onfi timings if available > (c) equivalent onfi timings if set > (d) conservative equivalent onfi timings otherwise > Right, using platform_data sounds like a nice compromise solution. I'm willing to accept this series with the current timing rework; and leave the timing setup in-driver replacement for followup patches. > All we need is a function to convert sdr_timings to sane driver > timings. And we really need to split this patch into tiny pieces > otherwise it is not reviewable - or at least I need a full overview > about the driver first. > I think that's a bit of a different issue. This patch seems to be doing two things: it removes the in-driver flash detection *and* reworks timing setup. How about we split this in two or even three patches? Along these lines: 1) introduce timing helpers, 2) rework timing setup, 3) remove in-driver flash detection. Not sure how feasible it is. > Also, as soon as Robert moves pxa3xx boards fully to DT, we'll loose > the pdata timings option above. *sigh* > Well, such move would include proper timing DT properties for non-ONFI devices. -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
On 04/16/2015 03:10 PM, Ezequiel Garcia wrote: On 04/15/2015 04:11 PM, Sebastian Hesselbarth wrote: On 15.04.2015 19:24, Antoine Tenart wrote: Rework the pxa3xx_nand driver to allow using functions exported by the nand framework to detect the flash and to configure the timings. Because this driver supports some non-ONFI devices, we also keep the custom timing setup of this driver so these devices won't break. Signed-off-by: Antoine Tenart --- [...] How about we get rid of the driver specific timings completely and pick up the best onfi timing match instead? The nand_ids table allows for a default_onfi_timing parameter even if onfi itself is not supported. For generic flash, i.e. no specific entry in the nand_ids table, we either choose onfi mode 0 (most conservative) or an even slower one. I think Robert mentioned [1] that using "ONFI default timings" on non-ONFI devices didn't work for him. [1] https://lkml.org/lkml/2015/3/8/124 Ok, I see. But there is still the option to pass board specific timings with driver's platform_data. We could use (a) pdata timings if passed (b) onfi timings if available (c) equivalent onfi timings if set (d) conservative equivalent onfi timings otherwise All we need is a function to convert sdr_timings to sane driver timings. And we really need to split this patch into tiny pieces otherwise it is not reviewable - or at least I need a full overview about the driver first. Also, as soon as Robert moves pxa3xx boards fully to DT, we'll loose the pdata timings option above. *sigh* Thoughts? Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
On 04/15/2015 04:11 PM, Sebastian Hesselbarth wrote: > On 15.04.2015 19:24, Antoine Tenart wrote: >> Rework the pxa3xx_nand driver to allow using functions exported by the >> nand framework to detect the flash and to configure the timings. >> >> Because this driver supports some non-ONFI devices, we also keep the >> custom timing setup of this driver so these devices won't break. >> >> Signed-off-by: Antoine Tenart >> --- > [...] > > Antoine, > > there are some issues with this patch. > >> diff --git a/drivers/mtd/nand/pxa3xx_nand.c >> b/drivers/mtd/nand/pxa3xx_nand.c >> index dc0edbc406bb..438770c56bd3 100644 >> --- a/drivers/mtd/nand/pxa3xx_nand.c >> +++ b/drivers/mtd/nand/pxa3xx_nand.c >> @@ -251,15 +251,14 @@ static struct pxa3xx_nand_timing timing[] = { >> }; >> >> static struct pxa3xx_nand_flash builtin_flash_types[] = { >> -{ "DEFAULT FLASH", 0, 0, 2048, 8, 8,0, [0] }, >> -{ "64MiB 16-bit", 0x46ec, 32, 512, 16, 16, 4096, [1] }, >> -{ "256MiB 8-bit", 0xdaec, 64, 2048, 8, 8, 2048, [1] }, >> -{ "4GiB 8-bit",0xd7ec, 128, 4096, 8, 8, 8192, [1] }, >> -{ "128MiB 8-bit", 0xa12c, 64, 2048, 8, 8, 1024, [2] }, >> -{ "128MiB 16-bit", 0xb12c, 64, 2048, 16, 16, 1024, [2] }, >> -{ "512MiB 8-bit", 0xdc2c, 64, 2048, 8, 8, 4096, [2] }, >> -{ "512MiB 16-bit", 0xcc2c, 64, 2048, 16, 16, 4096, [2] }, >> -{ "256MiB 16-bit", 0xba20, 64, 2048, 16, 16, 2048, [3] }, >> +{ 0x46ec, 16, 16, [1] }, >> +{ 0xdaec, 8, 8, [1] }, >> +{ 0xd7ec, 8, 8, [1] }, >> +{ 0xa12c, 8, 8, [2] }, >> +{ 0xb12c, 16, 16, [2] }, >> +{ 0xdc2c, 8, 8, [2] }, >> +{ 0xcc2c, 16, 16, [2] }, >> +{ 0xba20, 16, 16, [3] }, > > How about we get rid of the driver specific timings completely > and pick up the best onfi timing match instead? The nand_ids table > allows for a default_onfi_timing parameter even if onfi itself is > not supported. > > For generic flash, i.e. no specific entry in the nand_ids table, > we either choose onfi mode 0 (most conservative) or an even slower > one. > I think Robert mentioned [1] that using "ONFI default timings" on non-ONFI devices didn't work for him. [1] https://lkml.org/lkml/2015/3/8/124 -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
On 04/16/2015 10:41 AM, Sebastian Hesselbarth wrote: On 04/16/2015 03:10 PM, Ezequiel Garcia wrote: On 04/15/2015 04:11 PM, Sebastian Hesselbarth wrote: On 15.04.2015 19:24, Antoine Tenart wrote: Rework the pxa3xx_nand driver to allow using functions exported by the nand framework to detect the flash and to configure the timings. Because this driver supports some non-ONFI devices, we also keep the custom timing setup of this driver so these devices won't break. Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com --- [...] How about we get rid of the driver specific timings completely and pick up the best onfi timing match instead? The nand_ids table allows for a default_onfi_timing parameter even if onfi itself is not supported. For generic flash, i.e. no specific entry in the nand_ids table, we either choose onfi mode 0 (most conservative) or an even slower one. I think Robert mentioned [1] that using ONFI default timings on non-ONFI devices didn't work for him. [1] https://lkml.org/lkml/2015/3/8/124 Ok, I see. But there is still the option to pass board specific timings with driver's platform_data. We could use (a) pdata timings if passed (b) onfi timings if available (c) equivalent onfi timings if set (d) conservative equivalent onfi timings otherwise Right, using platform_data sounds like a nice compromise solution. I'm willing to accept this series with the current timing rework; and leave the timing setup in-driver replacement for followup patches. All we need is a function to convert sdr_timings to sane driver timings. And we really need to split this patch into tiny pieces otherwise it is not reviewable - or at least I need a full overview about the driver first. I think that's a bit of a different issue. This patch seems to be doing two things: it removes the in-driver flash detection *and* reworks timing setup. How about we split this in two or even three patches? Along these lines: 1) introduce timing helpers, 2) rework timing setup, 3) remove in-driver flash detection. Not sure how feasible it is. Also, as soon as Robert moves pxa3xx boards fully to DT, we'll loose the pdata timings option above. *sigh* Well, such move would include proper timing DT properties for non-ONFI devices. -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
On 04/15/2015 04:11 PM, Sebastian Hesselbarth wrote: On 15.04.2015 19:24, Antoine Tenart wrote: Rework the pxa3xx_nand driver to allow using functions exported by the nand framework to detect the flash and to configure the timings. Because this driver supports some non-ONFI devices, we also keep the custom timing setup of this driver so these devices won't break. Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com --- [...] Antoine, there are some issues with this patch. diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index dc0edbc406bb..438770c56bd3 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -251,15 +251,14 @@ static struct pxa3xx_nand_timing timing[] = { }; static struct pxa3xx_nand_flash builtin_flash_types[] = { -{ DEFAULT FLASH, 0, 0, 2048, 8, 8,0, timing[0] }, -{ 64MiB 16-bit, 0x46ec, 32, 512, 16, 16, 4096, timing[1] }, -{ 256MiB 8-bit, 0xdaec, 64, 2048, 8, 8, 2048, timing[1] }, -{ 4GiB 8-bit,0xd7ec, 128, 4096, 8, 8, 8192, timing[1] }, -{ 128MiB 8-bit, 0xa12c, 64, 2048, 8, 8, 1024, timing[2] }, -{ 128MiB 16-bit, 0xb12c, 64, 2048, 16, 16, 1024, timing[2] }, -{ 512MiB 8-bit, 0xdc2c, 64, 2048, 8, 8, 4096, timing[2] }, -{ 512MiB 16-bit, 0xcc2c, 64, 2048, 16, 16, 4096, timing[2] }, -{ 256MiB 16-bit, 0xba20, 64, 2048, 16, 16, 2048, timing[3] }, +{ 0x46ec, 16, 16, timing[1] }, +{ 0xdaec, 8, 8, timing[1] }, +{ 0xd7ec, 8, 8, timing[1] }, +{ 0xa12c, 8, 8, timing[2] }, +{ 0xb12c, 16, 16, timing[2] }, +{ 0xdc2c, 8, 8, timing[2] }, +{ 0xcc2c, 16, 16, timing[2] }, +{ 0xba20, 16, 16, timing[3] }, How about we get rid of the driver specific timings completely and pick up the best onfi timing match instead? The nand_ids table allows for a default_onfi_timing parameter even if onfi itself is not supported. For generic flash, i.e. no specific entry in the nand_ids table, we either choose onfi mode 0 (most conservative) or an even slower one. I think Robert mentioned [1] that using ONFI default timings on non-ONFI devices didn't work for him. [1] https://lkml.org/lkml/2015/3/8/124 -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
On 04/16/2015 03:10 PM, Ezequiel Garcia wrote: On 04/15/2015 04:11 PM, Sebastian Hesselbarth wrote: On 15.04.2015 19:24, Antoine Tenart wrote: Rework the pxa3xx_nand driver to allow using functions exported by the nand framework to detect the flash and to configure the timings. Because this driver supports some non-ONFI devices, we also keep the custom timing setup of this driver so these devices won't break. Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com --- [...] How about we get rid of the driver specific timings completely and pick up the best onfi timing match instead? The nand_ids table allows for a default_onfi_timing parameter even if onfi itself is not supported. For generic flash, i.e. no specific entry in the nand_ids table, we either choose onfi mode 0 (most conservative) or an even slower one. I think Robert mentioned [1] that using ONFI default timings on non-ONFI devices didn't work for him. [1] https://lkml.org/lkml/2015/3/8/124 Ok, I see. But there is still the option to pass board specific timings with driver's platform_data. We could use (a) pdata timings if passed (b) onfi timings if available (c) equivalent onfi timings if set (d) conservative equivalent onfi timings otherwise All we need is a function to convert sdr_timings to sane driver timings. And we really need to split this patch into tiny pieces otherwise it is not reviewable - or at least I need a full overview about the driver first. Also, as soon as Robert moves pxa3xx boards fully to DT, we'll loose the pdata timings option above. *sigh* Thoughts? Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
On 15.04.2015 19:24, Antoine Tenart wrote: Rework the pxa3xx_nand driver to allow using functions exported by the nand framework to detect the flash and to configure the timings. Because this driver supports some non-ONFI devices, we also keep the custom timing setup of this driver so these devices won't break. Signed-off-by: Antoine Tenart --- [...] Antoine, there are some issues with this patch. diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index dc0edbc406bb..438770c56bd3 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -251,15 +251,14 @@ static struct pxa3xx_nand_timing timing[] = { }; static struct pxa3xx_nand_flash builtin_flash_types[] = { -{ "DEFAULT FLASH", 0, 0, 2048, 8, 8,0, [0] }, -{ "64MiB 16-bit", 0x46ec, 32, 512, 16, 16, 4096, [1] }, -{ "256MiB 8-bit", 0xdaec, 64, 2048, 8, 8, 2048, [1] }, -{ "4GiB 8-bit",0xd7ec, 128, 4096, 8, 8, 8192, [1] }, -{ "128MiB 8-bit", 0xa12c, 64, 2048, 8, 8, 1024, [2] }, -{ "128MiB 16-bit", 0xb12c, 64, 2048, 16, 16, 1024, [2] }, -{ "512MiB 8-bit", 0xdc2c, 64, 2048, 8, 8, 4096, [2] }, -{ "512MiB 16-bit", 0xcc2c, 64, 2048, 16, 16, 4096, [2] }, -{ "256MiB 16-bit", 0xba20, 64, 2048, 16, 16, 2048, [3] }, + { 0x46ec, 16, 16, [1] }, + { 0xdaec, 8, 8, [1] }, + { 0xd7ec, 8, 8, [1] }, + { 0xa12c, 8, 8, [2] }, + { 0xb12c, 16, 16, [2] }, + { 0xdc2c, 8, 8, [2] }, + { 0xcc2c, 16, 16, [2] }, + { 0xba20, 16, 16, [3] }, How about we get rid of the driver specific timings completely and pick up the best onfi timing match instead? The nand_ids table allows for a default_onfi_timing parameter even if onfi itself is not supported. For generic flash, i.e. no specific entry in the nand_ids table, we either choose onfi mode 0 (most conservative) or an even slower one. [...] +static void pxa3xx_nand_set_sdr_timing(struct pxa3xx_nand_host *host, + const struct nand_sdr_timings *t) +{ + struct pxa3xx_nand_info *info = host->info_data; + unsigned long nand_clk = clk_get_rate(info->clk); + uint32_t ndtr0, ndtr1; + + u32 tCH_min = DIV_ROUND_UP(t->tCH_min, 1000); + u32 tCS_min = DIV_ROUND_UP(t->tCS_min, 1000); + u32 tWH_min = DIV_ROUND_UP(t->tWH_min, 1000); + u32 tWP_min = DIV_ROUND_UP(t->tWP_min, 1000); While tWH_min is the minimum hold time of WE_n and tWP_min the minimum pulse width, the sum of the two rounded values may well exceed the minimum WE_n cycle width tWC_min. How about we do below instead? u32 tWH_min = DIV_ROUND_UP(t->tWH_min, 1000); u32 tWP_min = DIV_ROUND_UP(t->tWC_min - tWH_min, 1000); (note the missing t-> in front of tWH_min) + u32 tREH_min = DIV_ROUND_UP(t->tREH_min, 1000); + u32 tRP_min = DIV_ROUND_UP(t->tRP_min, 1000); Same applies to tRH and tRP. + u32 tRST_max = DIV_ROUND_UP_ULL(t->tRST_max, 1000); + u32 tWHR_min = DIV_ROUND_UP(t->tWHR_min, 1000); + u32 tAR_min = DIV_ROUND_UP(t->tAR_min, 1000); + + ndtr0 = NDTR0_tCH(ns2cycle(tCH_min, nand_clk)) | + NDTR0_tCS(ns2cycle(tCS_min, nand_clk)) | + NDTR0_tWH(ns2cycle(tWH_min, nand_clk)) | + NDTR0_tWP(ns2cycle(tWP_min, nand_clk)) | + NDTR0_tRH(ns2cycle(tREH_min, nand_clk)) | + NDTR0_tRP(ns2cycle(tRP_min, nand_clk)); + + ndtr1 = NDTR1_tR(ns2cycle(tRST_max, nand_clk)) | Well, tRST (max reset duration) is not tR (max page read time) but much higher. I see that the onfi timings do not directly encode tR but have extra timings for tPROG, tBERS, tR, and tCCS. I guess we'll have to amend sdr_timings for non-onfi devices then. Also, if you check the armada370 functional spec, there is several meanings for tR register. Currently, pxa3xx nfc driver does not use WAIT_MODE which is supported for NFCv2 (non-pxa3xx SoCs). That will allow the controller to use R/Bn signal instead. Anyways, timing conversion is already tricky enough, let's start with 1:1 conversion and add additional modes later. + NDTR1_tWHR(ns2cycle(tWHR_min, nand_clk)) | + NDTR1_tAR(ns2cycle(tAR_min, nand_clk)); + + info->ndtr0cs0 = ndtr0; + info->ndtr1cs0 = ndtr1; + nand_writel(info, NDTR0CS0, ndtr0); + nand_writel(info, NDTR1CS0, ndtr1); +} + +static int pxa3xx_nand_init_timings(struct pxa3xx_nand_host *host) +{ + const struct nand_sdr_timings *timings; + struct nand_chip *chip = >chip; + struct pxa3xx_nand_info *info = host->info_data; + const struct pxa3xx_nand_flash *f = NULL; + int mode, id, ntypes, i; + + mode = onfi_get_async_timing_mode(chip); + if (mode == ONFI_TIMING_MODE_UNKNOWN) { + ntypes = ARRAY_SIZE(builtin_flash_types); + + chip->cmdfunc(host->mtd, NAND_CMD_READID, 0x00, -1); + + id = chip->read_byte(host->mtd); + id |=
[PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
Rework the pxa3xx_nand driver to allow using functions exported by the nand framework to detect the flash and to configure the timings. Because this driver supports some non-ONFI devices, we also keep the custom timing setup of this driver so these devices won't break. Signed-off-by: Antoine Tenart --- drivers/mtd/nand/pxa3xx_nand.c| 220 ++ include/linux/platform_data/mtd-nand-pxa3xx.h | 11 +- 2 files changed, 123 insertions(+), 108 deletions(-) diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index dc0edbc406bb..438770c56bd3 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -251,15 +251,14 @@ static struct pxa3xx_nand_timing timing[] = { }; static struct pxa3xx_nand_flash builtin_flash_types[] = { -{ "DEFAULT FLASH", 0, 0, 2048, 8, 8,0, [0] }, -{ "64MiB 16-bit", 0x46ec, 32, 512, 16, 16, 4096, [1] }, -{ "256MiB 8-bit", 0xdaec, 64, 2048, 8, 8, 2048, [1] }, -{ "4GiB 8-bit",0xd7ec, 128, 4096, 8, 8, 8192, [1] }, -{ "128MiB 8-bit", 0xa12c, 64, 2048, 8, 8, 1024, [2] }, -{ "128MiB 16-bit", 0xb12c, 64, 2048, 16, 16, 1024, [2] }, -{ "512MiB 8-bit", 0xdc2c, 64, 2048, 8, 8, 4096, [2] }, -{ "512MiB 16-bit", 0xcc2c, 64, 2048, 16, 16, 4096, [2] }, -{ "256MiB 16-bit", 0xba20, 64, 2048, 16, 16, 2048, [3] }, + { 0x46ec, 16, 16, [1] }, + { 0xdaec, 8, 8, [1] }, + { 0xd7ec, 8, 8, [1] }, + { 0xa12c, 8, 8, [2] }, + { 0xb12c, 16, 16, [2] }, + { 0xdc2c, 8, 8, [2] }, + { 0xcc2c, 16, 16, [2] }, + { 0xba20, 16, 16, [3] }, }; static u8 bbt_pattern[] = {'M', 'V', 'B', 'b', 't', '0' }; @@ -320,9 +319,6 @@ static struct nand_ecclayout ecc_layout_4KB_bch8bit = { .oobfree = { } }; -/* Define a default flash type setting serve as flash detecting only */ -#define DEFAULT_FLASH_TYPE (_flash_types[0]) - #define NDTR0_tCH(c) (min((c), 7) << 19) #define NDTR0_tCS(c) (min((c), 7) << 16) #define NDTR0_tWH(c) (min((c), 7) << 11) @@ -384,6 +380,92 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host, nand_writel(info, NDTR1CS0, ndtr1); } +static void pxa3xx_nand_set_sdr_timing(struct pxa3xx_nand_host *host, + const struct nand_sdr_timings *t) +{ + struct pxa3xx_nand_info *info = host->info_data; + unsigned long nand_clk = clk_get_rate(info->clk); + uint32_t ndtr0, ndtr1; + + u32 tCH_min = DIV_ROUND_UP(t->tCH_min, 1000); + u32 tCS_min = DIV_ROUND_UP(t->tCS_min, 1000); + u32 tWH_min = DIV_ROUND_UP(t->tWH_min, 1000); + u32 tWP_min = DIV_ROUND_UP(t->tWP_min, 1000); + u32 tREH_min = DIV_ROUND_UP(t->tREH_min, 1000); + u32 tRP_min = DIV_ROUND_UP(t->tRP_min, 1000); + u32 tRST_max = DIV_ROUND_UP_ULL(t->tRST_max, 1000); + u32 tWHR_min = DIV_ROUND_UP(t->tWHR_min, 1000); + u32 tAR_min = DIV_ROUND_UP(t->tAR_min, 1000); + + ndtr0 = NDTR0_tCH(ns2cycle(tCH_min, nand_clk)) | + NDTR0_tCS(ns2cycle(tCS_min, nand_clk)) | + NDTR0_tWH(ns2cycle(tWH_min, nand_clk)) | + NDTR0_tWP(ns2cycle(tWP_min, nand_clk)) | + NDTR0_tRH(ns2cycle(tREH_min, nand_clk)) | + NDTR0_tRP(ns2cycle(tRP_min, nand_clk)); + + ndtr1 = NDTR1_tR(ns2cycle(tRST_max, nand_clk)) | + NDTR1_tWHR(ns2cycle(tWHR_min, nand_clk)) | + NDTR1_tAR(ns2cycle(tAR_min, nand_clk)); + + info->ndtr0cs0 = ndtr0; + info->ndtr1cs0 = ndtr1; + nand_writel(info, NDTR0CS0, ndtr0); + nand_writel(info, NDTR1CS0, ndtr1); +} + +static int pxa3xx_nand_init_timings(struct pxa3xx_nand_host *host) +{ + const struct nand_sdr_timings *timings; + struct nand_chip *chip = >chip; + struct pxa3xx_nand_info *info = host->info_data; + const struct pxa3xx_nand_flash *f = NULL; + int mode, id, ntypes, i; + + mode = onfi_get_async_timing_mode(chip); + if (mode == ONFI_TIMING_MODE_UNKNOWN) { + ntypes = ARRAY_SIZE(builtin_flash_types); + + chip->cmdfunc(host->mtd, NAND_CMD_READID, 0x00, -1); + + id = chip->read_byte(host->mtd); + id |= chip->read_byte(host->mtd) << 0x8; + + for (i = 0; i < ntypes; i++) { + f = _flash_types[i]; + + if (f->chip_id == id) + break; + } + + if (i == ntypes) { + dev_err(>pdev->dev, "Error: timings not found\n"); + return -EINVAL; + } + + pxa3xx_nand_set_timing(host, f->timing); + + if (f->flash_width == 16) { + info->reg_ndcr |= NDCR_DWIDTH_M; + chip->options |= NAND_BUSWIDTH_16; + } + + info->reg_ndcr |= (f->dfc_width == 16) ? NDCR_DWIDTH_C : 0; + } else { +
[PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
Rework the pxa3xx_nand driver to allow using functions exported by the nand framework to detect the flash and to configure the timings. Because this driver supports some non-ONFI devices, we also keep the custom timing setup of this driver so these devices won't break. Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com --- drivers/mtd/nand/pxa3xx_nand.c| 220 ++ include/linux/platform_data/mtd-nand-pxa3xx.h | 11 +- 2 files changed, 123 insertions(+), 108 deletions(-) diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index dc0edbc406bb..438770c56bd3 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -251,15 +251,14 @@ static struct pxa3xx_nand_timing timing[] = { }; static struct pxa3xx_nand_flash builtin_flash_types[] = { -{ DEFAULT FLASH, 0, 0, 2048, 8, 8,0, timing[0] }, -{ 64MiB 16-bit, 0x46ec, 32, 512, 16, 16, 4096, timing[1] }, -{ 256MiB 8-bit, 0xdaec, 64, 2048, 8, 8, 2048, timing[1] }, -{ 4GiB 8-bit,0xd7ec, 128, 4096, 8, 8, 8192, timing[1] }, -{ 128MiB 8-bit, 0xa12c, 64, 2048, 8, 8, 1024, timing[2] }, -{ 128MiB 16-bit, 0xb12c, 64, 2048, 16, 16, 1024, timing[2] }, -{ 512MiB 8-bit, 0xdc2c, 64, 2048, 8, 8, 4096, timing[2] }, -{ 512MiB 16-bit, 0xcc2c, 64, 2048, 16, 16, 4096, timing[2] }, -{ 256MiB 16-bit, 0xba20, 64, 2048, 16, 16, 2048, timing[3] }, + { 0x46ec, 16, 16, timing[1] }, + { 0xdaec, 8, 8, timing[1] }, + { 0xd7ec, 8, 8, timing[1] }, + { 0xa12c, 8, 8, timing[2] }, + { 0xb12c, 16, 16, timing[2] }, + { 0xdc2c, 8, 8, timing[2] }, + { 0xcc2c, 16, 16, timing[2] }, + { 0xba20, 16, 16, timing[3] }, }; static u8 bbt_pattern[] = {'M', 'V', 'B', 'b', 't', '0' }; @@ -320,9 +319,6 @@ static struct nand_ecclayout ecc_layout_4KB_bch8bit = { .oobfree = { } }; -/* Define a default flash type setting serve as flash detecting only */ -#define DEFAULT_FLASH_TYPE (builtin_flash_types[0]) - #define NDTR0_tCH(c) (min((c), 7) 19) #define NDTR0_tCS(c) (min((c), 7) 16) #define NDTR0_tWH(c) (min((c), 7) 11) @@ -384,6 +380,92 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host, nand_writel(info, NDTR1CS0, ndtr1); } +static void pxa3xx_nand_set_sdr_timing(struct pxa3xx_nand_host *host, + const struct nand_sdr_timings *t) +{ + struct pxa3xx_nand_info *info = host-info_data; + unsigned long nand_clk = clk_get_rate(info-clk); + uint32_t ndtr0, ndtr1; + + u32 tCH_min = DIV_ROUND_UP(t-tCH_min, 1000); + u32 tCS_min = DIV_ROUND_UP(t-tCS_min, 1000); + u32 tWH_min = DIV_ROUND_UP(t-tWH_min, 1000); + u32 tWP_min = DIV_ROUND_UP(t-tWP_min, 1000); + u32 tREH_min = DIV_ROUND_UP(t-tREH_min, 1000); + u32 tRP_min = DIV_ROUND_UP(t-tRP_min, 1000); + u32 tRST_max = DIV_ROUND_UP_ULL(t-tRST_max, 1000); + u32 tWHR_min = DIV_ROUND_UP(t-tWHR_min, 1000); + u32 tAR_min = DIV_ROUND_UP(t-tAR_min, 1000); + + ndtr0 = NDTR0_tCH(ns2cycle(tCH_min, nand_clk)) | + NDTR0_tCS(ns2cycle(tCS_min, nand_clk)) | + NDTR0_tWH(ns2cycle(tWH_min, nand_clk)) | + NDTR0_tWP(ns2cycle(tWP_min, nand_clk)) | + NDTR0_tRH(ns2cycle(tREH_min, nand_clk)) | + NDTR0_tRP(ns2cycle(tRP_min, nand_clk)); + + ndtr1 = NDTR1_tR(ns2cycle(tRST_max, nand_clk)) | + NDTR1_tWHR(ns2cycle(tWHR_min, nand_clk)) | + NDTR1_tAR(ns2cycle(tAR_min, nand_clk)); + + info-ndtr0cs0 = ndtr0; + info-ndtr1cs0 = ndtr1; + nand_writel(info, NDTR0CS0, ndtr0); + nand_writel(info, NDTR1CS0, ndtr1); +} + +static int pxa3xx_nand_init_timings(struct pxa3xx_nand_host *host) +{ + const struct nand_sdr_timings *timings; + struct nand_chip *chip = host-chip; + struct pxa3xx_nand_info *info = host-info_data; + const struct pxa3xx_nand_flash *f = NULL; + int mode, id, ntypes, i; + + mode = onfi_get_async_timing_mode(chip); + if (mode == ONFI_TIMING_MODE_UNKNOWN) { + ntypes = ARRAY_SIZE(builtin_flash_types); + + chip-cmdfunc(host-mtd, NAND_CMD_READID, 0x00, -1); + + id = chip-read_byte(host-mtd); + id |= chip-read_byte(host-mtd) 0x8; + + for (i = 0; i ntypes; i++) { + f = builtin_flash_types[i]; + + if (f-chip_id == id) + break; + } + + if (i == ntypes) { + dev_err(info-pdev-dev, Error: timings not found\n); + return -EINVAL; + } + + pxa3xx_nand_set_timing(host, f-timing); + + if (f-flash_width == 16) { + info-reg_ndcr |= NDCR_DWIDTH_M; + chip-options |= NAND_BUSWIDTH_16; +
Re: [PATCH v4 04/10] mtd: pxa3xx_nand: rework flash detection and timing setup
On 15.04.2015 19:24, Antoine Tenart wrote: Rework the pxa3xx_nand driver to allow using functions exported by the nand framework to detect the flash and to configure the timings. Because this driver supports some non-ONFI devices, we also keep the custom timing setup of this driver so these devices won't break. Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com --- [...] Antoine, there are some issues with this patch. diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index dc0edbc406bb..438770c56bd3 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -251,15 +251,14 @@ static struct pxa3xx_nand_timing timing[] = { }; static struct pxa3xx_nand_flash builtin_flash_types[] = { -{ DEFAULT FLASH, 0, 0, 2048, 8, 8,0, timing[0] }, -{ 64MiB 16-bit, 0x46ec, 32, 512, 16, 16, 4096, timing[1] }, -{ 256MiB 8-bit, 0xdaec, 64, 2048, 8, 8, 2048, timing[1] }, -{ 4GiB 8-bit,0xd7ec, 128, 4096, 8, 8, 8192, timing[1] }, -{ 128MiB 8-bit, 0xa12c, 64, 2048, 8, 8, 1024, timing[2] }, -{ 128MiB 16-bit, 0xb12c, 64, 2048, 16, 16, 1024, timing[2] }, -{ 512MiB 8-bit, 0xdc2c, 64, 2048, 8, 8, 4096, timing[2] }, -{ 512MiB 16-bit, 0xcc2c, 64, 2048, 16, 16, 4096, timing[2] }, -{ 256MiB 16-bit, 0xba20, 64, 2048, 16, 16, 2048, timing[3] }, + { 0x46ec, 16, 16, timing[1] }, + { 0xdaec, 8, 8, timing[1] }, + { 0xd7ec, 8, 8, timing[1] }, + { 0xa12c, 8, 8, timing[2] }, + { 0xb12c, 16, 16, timing[2] }, + { 0xdc2c, 8, 8, timing[2] }, + { 0xcc2c, 16, 16, timing[2] }, + { 0xba20, 16, 16, timing[3] }, How about we get rid of the driver specific timings completely and pick up the best onfi timing match instead? The nand_ids table allows for a default_onfi_timing parameter even if onfi itself is not supported. For generic flash, i.e. no specific entry in the nand_ids table, we either choose onfi mode 0 (most conservative) or an even slower one. [...] +static void pxa3xx_nand_set_sdr_timing(struct pxa3xx_nand_host *host, + const struct nand_sdr_timings *t) +{ + struct pxa3xx_nand_info *info = host-info_data; + unsigned long nand_clk = clk_get_rate(info-clk); + uint32_t ndtr0, ndtr1; + + u32 tCH_min = DIV_ROUND_UP(t-tCH_min, 1000); + u32 tCS_min = DIV_ROUND_UP(t-tCS_min, 1000); + u32 tWH_min = DIV_ROUND_UP(t-tWH_min, 1000); + u32 tWP_min = DIV_ROUND_UP(t-tWP_min, 1000); While tWH_min is the minimum hold time of WE_n and tWP_min the minimum pulse width, the sum of the two rounded values may well exceed the minimum WE_n cycle width tWC_min. How about we do below instead? u32 tWH_min = DIV_ROUND_UP(t-tWH_min, 1000); u32 tWP_min = DIV_ROUND_UP(t-tWC_min - tWH_min, 1000); (note the missing t- in front of tWH_min) + u32 tREH_min = DIV_ROUND_UP(t-tREH_min, 1000); + u32 tRP_min = DIV_ROUND_UP(t-tRP_min, 1000); Same applies to tRH and tRP. + u32 tRST_max = DIV_ROUND_UP_ULL(t-tRST_max, 1000); + u32 tWHR_min = DIV_ROUND_UP(t-tWHR_min, 1000); + u32 tAR_min = DIV_ROUND_UP(t-tAR_min, 1000); + + ndtr0 = NDTR0_tCH(ns2cycle(tCH_min, nand_clk)) | + NDTR0_tCS(ns2cycle(tCS_min, nand_clk)) | + NDTR0_tWH(ns2cycle(tWH_min, nand_clk)) | + NDTR0_tWP(ns2cycle(tWP_min, nand_clk)) | + NDTR0_tRH(ns2cycle(tREH_min, nand_clk)) | + NDTR0_tRP(ns2cycle(tRP_min, nand_clk)); + + ndtr1 = NDTR1_tR(ns2cycle(tRST_max, nand_clk)) | Well, tRST (max reset duration) is not tR (max page read time) but much higher. I see that the onfi timings do not directly encode tR but have extra timings for tPROG, tBERS, tR, and tCCS. I guess we'll have to amend sdr_timings for non-onfi devices then. Also, if you check the armada370 functional spec, there is several meanings for tR register. Currently, pxa3xx nfc driver does not use WAIT_MODE which is supported for NFCv2 (non-pxa3xx SoCs). That will allow the controller to use R/Bn signal instead. Anyways, timing conversion is already tricky enough, let's start with 1:1 conversion and add additional modes later. + NDTR1_tWHR(ns2cycle(tWHR_min, nand_clk)) | + NDTR1_tAR(ns2cycle(tAR_min, nand_clk)); + + info-ndtr0cs0 = ndtr0; + info-ndtr1cs0 = ndtr1; + nand_writel(info, NDTR0CS0, ndtr0); + nand_writel(info, NDTR1CS0, ndtr1); +} + +static int pxa3xx_nand_init_timings(struct pxa3xx_nand_host *host) +{ + const struct nand_sdr_timings *timings; + struct nand_chip *chip = host-chip; + struct pxa3xx_nand_info *info = host-info_data; + const struct pxa3xx_nand_flash *f = NULL; + int mode, id, ntypes, i; + + mode = onfi_get_async_timing_mode(chip); + if (mode == ONFI_TIMING_MODE_UNKNOWN) { + ntypes = ARRAY_SIZE(builtin_flash_types); + + chip-cmdfunc(host-mtd,