[PATCH] OMAP2 NAND: Fix __raw_readsl() length argument

[PATCH] OMAP2 NAND: Fix __raw_readsl() length argument

[Juha Kuikka]

__raw_readsl() takes data length in longs so divide byte length by
four instead of two.

Signed-off-by: Juha Kuikka <juha.kuikka@gmail.com>
---
 drivers/mtd/nand/omap2.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
 mode change 100644 => 100755 drivers/mtd/nand/omap2.c

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
old mode 100644
new mode 100755
index 2ede116..2673ae4
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -196,7 +196,7 @@ static void omap_read_buf16(struct mtd_info *mtd,
u_char *buf, int len)
 {
        struct nand_chip *nand = mtd->priv;

-       __raw_readsl(nand->IO_ADDR_R, buf, len / 2);
+       __raw_readsl(nand->IO_ADDR_R, buf, len / 4);
 }

 /*
--
1.6.0.1

Re: [PATCH] OMAP2 NAND: Fix __raw_readsl() length argument

[David Brownell]

On Wednesday 22 October 2008, Juha Kuikka wrote:
> -       __raw_readsl(nand->IO_ADDR_R, buf, len / 2);
> +       __raw_readsl(nand->IO_ADDR_R, buf, len / 4);
>  }

Shouldn't that have been __raw_readsw() though?

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP2 NAND: Fix __raw_readsl() length argument

[Juha Kuikka]

On Wed, Oct 22, 2008 at 2:46 PM, David Brownell <david-b@pacbell.net> wrote:
> On Wednesday 22 October 2008, Juha Kuikka wrote:
>> -       __raw_readsl(nand->IO_ADDR_R, buf, len / 2);
>> +       __raw_readsl(nand->IO_ADDR_R, buf, len / 4);
>>  }
>
> Shouldn't that have been __raw_readsw() though?
>
Hmh, good point.

>From the original code it looks like that was the intention but
readsl() works just as well.

I tested this on OMAP2430 and it works ok.

I don't see any mentions in the TRM about the width of the
GPMC_NAND_DATA registers but apparently the NAND engine happily
accepts 32bit accesses on bus.


 - Juha


-- 
Madness takes it's toll. Please have exact change.

Re: [PATCH] OMAP2 NAND: Fix __raw_readsl() length argument

[David Brownell]

On Wednesday 22 October 2008, Juha Kuikka wrote:
> >> -       __raw_readsl(nand->IO_ADDR_R, buf, len / 2);
> >> +       __raw_readsl(nand->IO_ADDR_R, buf, len / 4);
> >>  }
> >
> > Shouldn't that have been __raw_readsw() though?
>
> Hmh, good point.

Yeah, but the bug was from a patch from me ... sigh.

 
> From the original code it looks like that was the intention but
> readsl() works just as well.

Really?  Both upper and lower 16-bit units have the right data?


> I tested this on OMAP2430 and it works ok.
> 
> I don't see any mentions in the TRM about the width of the
> GPMC_NAND_DATA registers but apparently the NAND engine happily
> accepts 32bit accesses on bus.

Maybe this has to do with the FIFO behavior.  It would certainly
make sense to allow reads of any size from the FIFO.  If it were
raw reads on the data bus, then I'd expect that both 8 and 16 bit
widths would work.  (Assuming NAND chips weren't in parallel...)

If the FIFO is active, and specified to support arbitrary width
accesses (that don't match the data bus), then by all means use
the __raw_readsl() call to maximize bandwidth use.

- Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP2 NAND: Fix __raw_readsl() length argument

[Juha Kuikka]

On Wed, Oct 22, 2008 at 3:02 PM, David Brownell <david-b@pacbell.net> wrote:
> On Wednesday 22 October 2008, Juha Kuikka wrote:
>> >> -       __raw_readsl(nand->IO_ADDR_R, buf, len / 2);
>> >> +       __raw_readsl(nand->IO_ADDR_R, buf, len / 4);
>> >>  }
>> >
>> > Shouldn't that have been __raw_readsw() though?
>>
>> Hmh, good point.
>
> Yeah, but the bug was from a patch from me ... sigh.
>
>
>> From the original code it looks like that was the intention but
>> readsl() works just as well.
>
> Really?  Both upper and lower 16-bit units have the right data?

Yes, all data comes in normally. I'm running JFFS2 on top of this.

>> I tested this on OMAP2430 and it works ok.
>>
>> I don't see any mentions in the TRM about the width of the
>> GPMC_NAND_DATA registers but apparently the NAND engine happily
>> accepts 32bit accesses on bus.
>
> Maybe this has to do with the FIFO behavior.  It would certainly
> make sense to allow reads of any size from the FIFO.  If it were
> raw reads on the data bus, then I'd expect that both 8 and 16 bit
> widths would work.  (Assuming NAND chips weren't in parallel...)

Right, for the FIFO in prefetch/writeposting block the TRM explicitly
states that it does accept 8/16/32 bit accesses but I don't see this
driver using the prefetch engine.

No parallel chips here, just a regular 16 bit wide LP NAND.

> If the FIFO is active, and specified to support arbitrary width
> accesses (that don't match the data bus), then by all means use
> the __raw_readsl() call to maximize bandwidth use.

Sure. But interestingly enough even without explicitly using the FIFO
it seems to accept longer bus accesses.

Just to be on the safe side, maybe the patch should indeed be revised
to use __raw_readsw(..., len/2) ?

 - Juha
-- 
Madness takes it's toll. Please have exact change.

Re: [PATCH] OMAP2 NAND: Fix __raw_readsl() length argument

[Juha Kuikka]

On Wed, Oct 22, 2008 at 3:23 PM, Juha Kuikka <juha.kuikka@gmail.com> wrote:
> On Wed, Oct 22, 2008 at 3:02 PM, David Brownell <david-b@pacbell.net> wrote:
>> On Wednesday 22 October 2008, Juha Kuikka wrote:
>>> >> -       __raw_readsl(nand->IO_ADDR_R, buf, len / 2);
>>> >> +       __raw_readsl(nand->IO_ADDR_R, buf, len / 4);
>>> >>  }
>>> >
>>> > Shouldn't that have been __raw_readsw() though?
>>>
>>> Hmh, good point.
>>
>> Yeah, but the bug was from a patch from me ... sigh.
>>
>>
>>> From the original code it looks like that was the intention but
>>> readsl() works just as well.
>>
>> Really?  Both upper and lower 16-bit units have the right data?
>
> Yes, all data comes in normally. I'm running JFFS2 on top of this.
>
>>> I tested this on OMAP2430 and it works ok.
>>>
>>> I don't see any mentions in the TRM about the width of the
>>> GPMC_NAND_DATA registers but apparently the NAND engine happily
>>> accepts 32bit accesses on bus.
>>
>> Maybe this has to do with the FIFO behavior.  It would certainly
>> make sense to allow reads of any size from the FIFO.  If it were
>> raw reads on the data bus, then I'd expect that both 8 and 16 bit
>> widths would work.  (Assuming NAND chips weren't in parallel...)
>
> Right, for the FIFO in prefetch/writeposting block the TRM explicitly
> states that it does accept 8/16/32 bit accesses but I don't see this
> driver using the prefetch engine.
>
> No parallel chips here, just a regular 16 bit wide LP NAND.
>
>> If the FIFO is active, and specified to support arbitrary width
>> accesses (that don't match the data bus), then by all means use
>> the __raw_readsl() call to maximize bandwidth use.
>
> Sure. But interestingly enough even without explicitly using the FIFO
> it seems to accept longer bus accesses.
>
> Just to be on the safe side, maybe the patch should indeed be revised
> to use __raw_readsw(..., len/2) ?
>
How does this look?

Subject: [PATCH] OMAP2 NAND: Change __raw_readsl() to __raw_readsw()


Signed-off-by: Juha Kuikka <juha.kuikka@gmail.com>
---
 drivers/mtd/nand/omap2.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 2ede116..55ba746 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -196,7 +196,7 @@ static void omap_read_buf16(struct mtd_info *mtd,
u_char *buf, int len)
 {
        struct nand_chip *nand = mtd->priv;

-       __raw_readsl(nand->IO_ADDR_R, buf, len / 2);
+       __raw_readsw(nand->IO_ADDR_R, buf, len / 2);
 }

 /*
--
1.6.0.1

Re: [PATCH] OMAP2 NAND: Fix __raw_readsl() length argument

[David Brownell]

On Thursday 23 October 2008, Juha Kuikka wrote:
> How does this look?
> 
> Subject: [PATCH] OMAP2 NAND: Change __raw_readsl() to __raw_readsw()

Looks fine.  Although I'm still wondering what happened to that
patch teaching it about the prefetch engine ... it'd be nice to
maximize throughput.

(And it's too bad we can't confirm it's safe that raw_readsl is
actually safe without the FIFO and prefetch being enabled...)

- Dave

Bug: enabling USB-TLL fclk

[Pandita, Vikram]

Paul

 I was debugging why EHCI is not coming up and getting stuck in infinite loop for TLL not enabling.

I found that there is issue with clock framework in enabling "usbtll_fck" clock.

On enabling tll-fck clock, the framework returns error for the node:

static struct clk dpll5_ck = {
        .name           = "dpll5_ck",
        .parent         = &sys_ck,
        .prcm_mod       = PLL_MOD,
        .dpll_data      = &dpll5_dd,
        .flags          = CLOCK_IN_OMAP3430ES2 | RATE_PROPAGATES,
        .enable         = &omap3_noncore_dpll_enable,
        .disable        = &omap3_noncore_dpll_disable,
        .round_rate     = &omap2_dpll_round_rate,
        .set_rate       = &omap3_noncore_dpll_set_rate,
        .clkdm          = { .name = "dpll5_clkdm" },
        .recalc         = &omap3_dpll_recalc,
};


If I set the FCLK for USB-TLL directly as per following patch, EHCI works fine.
Please check what is wrong with the above clock node.

---
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 419e70a..96b9df5 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -236,10 +236,9 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
 #endif
 
 	/* Configure TLL for 60Mhz clk for ULPI */
-	ehci_clocks->usbtll_fck_clk = clk_get(&dev->dev, USBHOST_TLL_FCLK);
-	if (IS_ERR(ehci_clocks->usbtll_fck_clk))
-		return PTR_ERR(ehci_clocks->usbtll_fck_clk);
-	clk_enable(ehci_clocks->usbtll_fck_clk);
+
+	/* Force enable FCLK for USB-TLL */
+	omap_writel(1<<2, 0x48004A08); /* CM_FCLKEN3_CORE */
 
 	ehci_clocks->usbtll_ick_clk = clk_get(&dev->dev, USBHOST_TLL_ICKL);
 	if (IS_ERR(ehci_clocks->usbtll_ick_clk))

Re: Bug: enabling USB-TLL fclk

[Steve Sakoman]

On Fri, Oct 24, 2008 at 4:19 PM, Pandita, Vikram <vikram.pandita@ti.com> wrote:
>
> Paul
>
>  I was debugging why EHCI is not coming up and getting stuck in infinite loop for TLL not enabling.

EHCI is still broken on the current top of tree -- same infinite loop.

Have anyone figured out what is going wrong here?

Steve


> I found that there is issue with clock framework in enabling "usbtll_fck" clock.
>
> On enabling tll-fck clock, the framework returns error for the node:
>
> static struct clk dpll5_ck = {
>        .name           = "dpll5_ck",
>        .parent         = &sys_ck,
>        .prcm_mod       = PLL_MOD,
>        .dpll_data      = &dpll5_dd,
>        .flags          = CLOCK_IN_OMAP3430ES2 | RATE_PROPAGATES,
>        .enable         = &omap3_noncore_dpll_enable,
>        .disable        = &omap3_noncore_dpll_disable,
>        .round_rate     = &omap2_dpll_round_rate,
>        .set_rate       = &omap3_noncore_dpll_set_rate,
>        .clkdm          = { .name = "dpll5_clkdm" },
>        .recalc         = &omap3_dpll_recalc,
> };
>
>
> If I set the FCLK for USB-TLL directly as per following patch, EHCI works fine.
> Please check what is wrong with the above clock node.
>
> ---
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index 419e70a..96b9df5 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -236,10 +236,9 @@ static int omap_start_ehc(struct platform_device *dev, struct usb_hcd *hcd)
>  #endif
>
>        /* Configure TLL for 60Mhz clk for ULPI */
> -       ehci_clocks->usbtll_fck_clk = clk_get(&dev->dev, USBHOST_TLL_FCLK);
> -       if (IS_ERR(ehci_clocks->usbtll_fck_clk))
> -               return PTR_ERR(ehci_clocks->usbtll_fck_clk);
> -       clk_enable(ehci_clocks->usbtll_fck_clk);
> +
> +       /* Force enable FCLK for USB-TLL */
> +       omap_writel(1<<2, 0x48004A08); /* CM_FCLKEN3_CORE */
>
>        ehci_clocks->usbtll_ick_clk = clk_get(&dev->dev, USBHOST_TLL_ICKL);
>        if (IS_ERR(ehci_clocks->usbtll_ick_clk))
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

RE: [PATCH] OMAP2 NAND: Fix __raw_readsl() length argument

[Singh, Vimal]

> -       __raw_readsl(nand->IO_ADDR_R, buf, len / 2);
> +       __raw_readsw(nand->IO_ADDR_R, buf, len / 2);

I would prefer len >> 1 rather len / 2.

---
vimal