| Message ID | 87si81m15m.fsf@belgarion.home (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 3 August 2015 at 01:44, Robert Jarzmik <robert.jarzmik@free.fr> wrote: > Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes: > >> Just tested linux-next (hence *without* the patchset) and I see >> the same "Wait time out". In other words, pxa3xx-nand is broken >> on PXA :/ >> >> Interestingly, the culprit doesn't seem to be in pxa3xx-nand itself. >> Reverting the recent commits on pxa3xx-nand doesn't help. >> >> ce914e6 mtd: nand: pxa3xx: fix build on ARM64 >> afca11e mtd: nand: pxa3xx: Switch FIFO draining to jiffies-based timeout >> e5860c1 mtd: pxa3xx_nand: cleanup wait_for_completion handling >> 7c2f717 mtd: pxa3xx_nand: initialiaze pxa3xx_flash_ids to 0 >> ed446cc Merge MTD updates into -next >> e423c90 mtd: pxa3xx_nand: fix driver when num_cs is 0 >> 2454225 mtd: pxa3xx-nand: handle PIO in threaded interrupt >> 8dad038 mtd: nand: pxa3xx: Fix PIO FIFO draining >> b7e46062 mtd: pxa3xx_nand: make the driver work on big-endian systems >> 5b3e507 mtd: nand: pxa3xx: Use ECC strength and step size devicetree binding >> eee0166 mtd: nand: pxa3xx: Clean pxa_ecc_init() error handling >> 17754ad mtd: nand: pxa3xx: Make of_device_id array const >> e634ce5 mtd: nand: pxa3xx: Print actual ECC strength in error message >> >> Yet v3.18 succeeds to pass a few NAND blocks on nandtest. >> >> Robert: any ideas? > Actually yes, I worked on that this weekend. > Would you try the following patch [1] to see if it works for you ? > > The issue I see is that : > - there is a bug in the clk driver for pxa3xx I introduced (CKEN_AB) > - and shutting down the GCU clock prevents the NAND from working (I can't > explain that one yet) > IIRC, on Armada 370/XP SoCs we have two clocks feeding the NAND controller: one for the NAND logic (not really sure?), and one for the NAND ECC engine. Maybe the GCU feeds the ECC? > Cheers. > > -- > Robert > > [1] Clocks patch Ah, good. I was almost sure it was the clock (after trying a bunch of other stuff it was the only big change left). I'll try this and will let you know. > ---8>--- > diff --git a/drivers/clk/pxa/clk-pxa3xx.c b/drivers/clk/pxa/clk-pxa3xx.c > index c677b9ab5367..a47a0c40f937 100644 > --- a/drivers/clk/pxa/clk-pxa3xx.c > +++ b/drivers/clk/pxa/clk-pxa3xx.c > @@ -126,7 +126,7 @@ PARENTS(pxa3xx_ac97_bus) = { "ring_osc_60mhz", "ac97" }; > PARENTS(pxa3xx_sbus) = { "ring_osc_60mhz", "system_bus" }; > PARENTS(pxa3xx_smemcbus) = { "ring_osc_60mhz", "smemc" }; > > -#define CKEN_AB(bit) ((CKEN_ ## bit > 31) ? &CKENA : &CKENB) > +#define CKEN_AB(bit) ((CKEN_ ## bit > 31) ? &CKENB : &CKENA) > #define PXA3XX_CKEN(dev_id, con_id, parents, mult_lp, div_lp, mult_hp, \ > div_hp, bit, is_lp, flags) \ > PXA_CKEN(dev_id, con_id, bit, parents, mult_lp, div_lp, \ > @@ -136,6 +136,10 @@ PARENTS(pxa3xx_smemcbus) = { "ring_osc_60mhz", "smemc" }; > mult_hp, div_hp, delay) \ > PXA3XX_CKEN(dev_id, con_id, pxa3xx_pbus_parents, mult_lp, \ > div_lp, mult_hp, div_hp, bit, pxa3xx_is_ring_osc_forced, 0) > +#define PXA3XX_PBUS_CKENF(dev_id, con_id, bit, mult_lp, div_lp, \ > + mult_hp, div_hp, delay, flag) \ > + PXA3XX_CKEN(dev_id, con_id, pxa3xx_pbus_parents, mult_lp, \ > + div_lp, mult_hp, div_hp, bit, pxa3xx_is_ring_osc_forced, flag) > #define PXA3XX_CKEN_1RATE(dev_id, con_id, bit, parents) \ > PXA_CKEN_1RATE(dev_id, con_id, bit, parents, \ > CKEN_AB(bit), (CKEN_ ## bit % 32), 0) > @@ -173,13 +177,13 @@ static struct desc_clk_cken pxa3xx_clocks[] __initdata = { > > static struct desc_clk_cken pxa300_310_clocks[] __initdata = { > > - PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0), > - PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0), > + PXA3XX_PBUS_CKENF("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0, CLK_IGNORE_UNUSED), > + PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED), > PXA3XX_CKEN_1RATE("pxa3xx-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents), > }; > > static struct desc_clk_cken pxa320_clocks[] __initdata = { > - PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 6, 0), > + PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED), > PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA320_GCU, 1, 1, 1, 1, 0), > PXA3XX_CKEN_1RATE("pxa3xx-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents), > }; > @@ -187,7 +191,7 @@ static struct desc_clk_cken pxa320_clocks[] __initdata = { > static struct desc_clk_cken pxa93x_clocks[] __initdata = { > > PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0), > - PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0), > + PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED), > PXA3XX_CKEN_1RATE("pxa93x-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents), > }; >
Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes: >> >> The issue I see is that : >> - there is a bug in the clk driver for pxa3xx I introduced (CKEN_AB) >> - and shutting down the GCU clock prevents the NAND from working (I can't >> explain that one yet) Ah actually it's even trickier, but it has nothing to do with the GCU clock, that was a wrong interpretation of the test sequence. What actually happens is that on the platform I have, the NAND is sharing the DFI bus with the Static Memory Controller. Now let's see what happens on boot-up, knowing that my pxa3xx-nand is a module, not builtin : - the kernel boots - the core clock initializes - the ethernet card initializes (it is on the static memory controller) - the kernel finishes the boot sequence => the NAND clock is unused => as such, the core clock framework disables it And here is the catch : on the next ethernet access, the system bus will be stalled forever. The reason behind is that because the bootloader left the "NDCR_ND_ARB_EN" bit enabled, the DFI bus sees the ethernet register access, and asks for arbitration; as the NAND clock is down, the arbitration never happens, and the pxa3xx is stalled forever. The very same thing happens when you load and unload pxa3xx-nand with a platform where it was loaded with "enable-arbiter" platform-data, and if that platform has any driver mapped in the SMC address space (0x0 .. 0x14000000). If you have an opportunity to load/unload a pxa3xx-nand, I'd be glad to have someone verify this theory. The first fix comming to my mind would be to : - disable the NDCR_ND_ARB_EN in the pxa3xx core bring up - keep enablement in pxa3xx-nand - ensure it is disabled on the probe error path or remove of pxa3xx-nand Cheers. -- Robert PS: That also means that the fix I posted for CKENA/CKENB inversion should fix the issues you see.
diff --git a/drivers/clk/pxa/clk-pxa3xx.c b/drivers/clk/pxa/clk-pxa3xx.c index c677b9ab5367..a47a0c40f937 100644 --- a/drivers/clk/pxa/clk-pxa3xx.c +++ b/drivers/clk/pxa/clk-pxa3xx.c @@ -126,7 +126,7 @@ PARENTS(pxa3xx_ac97_bus) = { "ring_osc_60mhz", "ac97" }; PARENTS(pxa3xx_sbus) = { "ring_osc_60mhz", "system_bus" }; PARENTS(pxa3xx_smemcbus) = { "ring_osc_60mhz", "smemc" }; -#define CKEN_AB(bit) ((CKEN_ ## bit > 31) ? &CKENA : &CKENB) +#define CKEN_AB(bit) ((CKEN_ ## bit > 31) ? &CKENB : &CKENA) #define PXA3XX_CKEN(dev_id, con_id, parents, mult_lp, div_lp, mult_hp, \ div_hp, bit, is_lp, flags) \ PXA_CKEN(dev_id, con_id, bit, parents, mult_lp, div_lp, \ @@ -136,6 +136,10 @@ PARENTS(pxa3xx_smemcbus) = { "ring_osc_60mhz", "smemc" }; mult_hp, div_hp, delay) \ PXA3XX_CKEN(dev_id, con_id, pxa3xx_pbus_parents, mult_lp, \ div_lp, mult_hp, div_hp, bit, pxa3xx_is_ring_osc_forced, 0) +#define PXA3XX_PBUS_CKENF(dev_id, con_id, bit, mult_lp, div_lp, \ + mult_hp, div_hp, delay, flag) \ + PXA3XX_CKEN(dev_id, con_id, pxa3xx_pbus_parents, mult_lp, \ + div_lp, mult_hp, div_hp, bit, pxa3xx_is_ring_osc_forced, flag) #define PXA3XX_CKEN_1RATE(dev_id, con_id, bit, parents) \ PXA_CKEN_1RATE(dev_id, con_id, bit, parents, \ CKEN_AB(bit), (CKEN_ ## bit % 32), 0) @@ -173,13 +177,13 @@ static struct desc_clk_cken pxa3xx_clocks[] __initdata = { static struct desc_clk_cken pxa300_310_clocks[] __initdata = { - PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0), - PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0), + PXA3XX_PBUS_CKENF("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0, CLK_IGNORE_UNUSED), + PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED), PXA3XX_CKEN_1RATE("pxa3xx-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents), }; static struct desc_clk_cken pxa320_clocks[] __initdata = { - PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 6, 0), + PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED), PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA320_GCU, 1, 1, 1, 1, 0), PXA3XX_CKEN_1RATE("pxa3xx-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents), }; @@ -187,7 +191,7 @@ static struct desc_clk_cken pxa320_clocks[] __initdata = { static struct desc_clk_cken pxa93x_clocks[] __initdata = { PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0), - PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0), + PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED), PXA3XX_CKEN_1RATE("pxa93x-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents), };