[U-Boot-Users] [PATCH 1/7] 83xx: nand support for MPC837XRDB boards

[U-Boot-Users] [PATCH 1/7] 83xx: nand support for MPC837XRDB boards

[Anton Vorontsov]

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 include/configs/MPC837XERDB.h |   24 ++++++++++++++++++++++--
 1 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/include/configs/MPC837XERDB.h b/include/configs/MPC837XERDB.h
index 2da4f29..7c19d1e 100644
--- a/include/configs/MPC837XERDB.h
+++ b/include/configs/MPC837XERDB.h
@@ -251,9 +251,29 @@
 #define CFG_FLASH_ERASE_TOUT	60000	/* Flash Erase Timeout (ms) */
 #define CFG_FLASH_WRITE_TOUT	500	/* Flash Write Timeout (ms) */
 
-#define CFG_VSC7385_BASE	0xF0000000
+/*
+ * NAND Flash on the Local Bus
+ */
+#define CFG_NAND_BASE		0xE0600000	/* 0xE0600000 */
+#define CFG_BR1_PRELIM		(CFG_NAND_BASE | \
+				 (2 << BR_DECC_SHIFT) |	/* Use HW ECC */ \
+				 BR_PS_8 |		/* Port Size = 8 bit */ \
+				 BR_MS_FCM |		/* MSEL = FCM */ \
+				 BR_V)			/* valid */
+#define CFG_OR1_PRELIM		(0xFFFF8000 |		/* length 32K */ \
+				 OR_FCM_CSCT | \
+				 OR_FCM_CST | \
+				 OR_FCM_CHT | \
+				 OR_FCM_SCY_1 | \
+				 OR_FCM_TRLX | \
+				 OR_FCM_EHTR)
+#define CFG_LBLAWBAR1_PRELIM	CFG_NAND_BASE
+#define CFG_LBLAWAR1_PRELIM	0x8000000E	/* 32KB  */
 
-/* VSC7385 Gigabit Switch support */
+/*
+ * VSC7385 Gigabit Switch support
+ */
+#define CFG_VSC7385_BASE	0xF0000000
 #define CONFIG_VSC7385_ENET
 #define CFG_BR2_PRELIM		0xf0000801		/* Base address */
 #define CFG_OR2_PRELIM		0xfffe09ff		/* 128K bytes*/
-- 
1.5.2.2

[U-Boot-Users] [PATCH 1/7] 83xx: nand support for MPC837XRDB boards

[Scott Wood]

On Fri, Mar 14, 2008 at 11:19:42PM +0300, Anton Vorontsov wrote:
> -#define CFG_VSC7385_BASE	0xF0000000
> +/*
> + * NAND Flash on the Local Bus
> + */
> +#define CFG_NAND_BASE		0xE0600000	/* 0xE0600000 */
> +#define CFG_BR1_PRELIM		(CFG_NAND_BASE | \
> +				 (2 << BR_DECC_SHIFT) |	/* Use HW ECC */ \
> +				 BR_PS_8 |		/* Port Size = 8 bit */ \
> +				 BR_MS_FCM |		/* MSEL = FCM */ \
> +				 BR_V)			/* valid */
> +#define CFG_OR1_PRELIM		(0xFFFF8000 |		/* length 32K */ \
> +				 OR_FCM_CSCT | \
> +				 OR_FCM_CST | \
> +				 OR_FCM_CHT | \
> +				 OR_FCM_SCY_1 | \
> +				 OR_FCM_TRLX | \
> +				 OR_FCM_EHTR)

Again, are you sure about this SCY value?  Note that the Linux driver resets
it due to existing 8313 u-boots out there that have the wrong value (I'd
send a patch to fix it if I know the correct value), so remove that to see
what actually happens with SCY_1.

> -/* VSC7385 Gigabit Switch support */
> +/*
> + * VSC7385 Gigabit Switch support
> + */
> +#define CFG_VSC7385_BASE	0xF0000000

What does this have to do with NAND?

-Scott

[U-Boot-Users] [PATCH 1/7] 83xx: nand support for MPC837XRDB boards

[Anton Vorontsov]

On Mon, Mar 17, 2008 at 06:02:29PM -0500, Scott Wood wrote:
> On Fri, Mar 14, 2008 at 11:19:42PM +0300, Anton Vorontsov wrote:
> > -#define CFG_VSC7385_BASE	0xF0000000
> > +/*
> > + * NAND Flash on the Local Bus
> > + */
> > +#define CFG_NAND_BASE		0xE0600000	/* 0xE0600000 */
> > +#define CFG_BR1_PRELIM		(CFG_NAND_BASE | \
> > +				 (2 << BR_DECC_SHIFT) |	/* Use HW ECC */ \
> > +				 BR_PS_8 |		/* Port Size = 8 bit */ \
> > +				 BR_MS_FCM |		/* MSEL = FCM */ \
> > +				 BR_V)			/* valid */
> > +#define CFG_OR1_PRELIM		(0xFFFF8000 |		/* length 32K */ \
> > +				 OR_FCM_CSCT | \
> > +				 OR_FCM_CST | \
> > +				 OR_FCM_CHT | \
> > +				 OR_FCM_SCY_1 | \
> > +				 OR_FCM_TRLX | \
> > +				 OR_FCM_EHTR)
> 
> Again, are you sure about this SCY value?

Yes. The same value is used by the MDS boards, with the same nand
chip, I guess.

> Note that the Linux driver resets
> it due to existing 8313 u-boots out there that have the wrong value (I'd
> send a patch to fix it if I know the correct value),

Thanks for the info.

> so remove that to see
> what actually happens with SCY_1.

It works. With ndelay writing works too. So far I didn't have
time to debug further, sorry.

> > -/* VSC7385 Gigabit Switch support */
> > +/*
> > + * VSC7385 Gigabit Switch support
> > + */
> > +#define CFG_VSC7385_BASE	0xF0000000
> 
> What does this have to do with NAND?

Nothing. I'm expanding VSC7385 comment to better separate it from
the NAND code. Most lengthy parts of the RDB config are using

/*
 *
 */

as a logical separator. So did I.

-- 
Anton Vorontsov
email: cboumailru at gmail.com
irc://irc.freenode.net/bd2

[U-Boot-Users] [PATCH 1/7] 83xx: nand support for MPC837XRDB boards

[Kim Phillips]

On Tue, 18 Mar 2008 17:10:55 +0300
Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> 
> > > -/* VSC7385 Gigabit Switch support */
> > > +/*
> > > + * VSC7385 Gigabit Switch support
> > > + */
> > > +#define CFG_VSC7385_BASE	0xF0000000
> > 
> > What does this have to do with NAND?
> 
> Nothing. I'm expanding VSC7385 comment to better separate it from
> the NAND code. Most lengthy parts of the RDB config are using
> 
> /*
>  *
>  */
> 
> as a logical separator. So did I.

that's fine, it just makes patch review easier and less susceptible to
comments like this if you keep your patches on-subject.

Also, this patch doesn't apply cleanly to the top of
u-boot-mpc83xx.git, because of the VSC7385 patches I applied last week.
Can you rebase your patches to the top of the mpc83xx git tree?  I just
updated it to Wolfgang's top-of-tree.

Kim

[U-Boot-Users] [PATCH 1/7] 83xx: nand support for MPC837XRDB boards

[Scott Wood]

On Wed, Mar 19, 2008 at 08:16:45PM -0500, Kim Phillips wrote:
> On Tue, 18 Mar 2008 17:10:55 +0300
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> 
> > 
> > > > -/* VSC7385 Gigabit Switch support */
> > > > +/*
> > > > + * VSC7385 Gigabit Switch support
> > > > + */
> > > > +#define CFG_VSC7385_BASE	0xF0000000
> > > 
> > > What does this have to do with NAND?
> > 
> > Nothing. I'm expanding VSC7385 comment to better separate it from
> > the NAND code. Most lengthy parts of the RDB config are using
> > 
> > /*
> >  *
> >  */
> > 
> > as a logical separator. So did I.
> 
> that's fine, it just makes patch review easier and less susceptible to
> comments like this if you keep your patches on-subject.

To be fair, the main reason for the comment was that I missed this line:
-#define CFG_VSC7385_BASE	0xF0000000

way at the top of the diff, and thought the patch was introducing the
#define rather than just reformatting the comment.

-Scott

[U-Boot-Users] [PATCH 1/7] 83xx: nand support for MPC837XRDB boards

[Anton Vorontsov]

On Thu, Mar 20, 2008 at 11:26:45AM -0500, Scott Wood wrote:
> On Wed, Mar 19, 2008 at 08:16:45PM -0500, Kim Phillips wrote:
> > On Tue, 18 Mar 2008 17:10:55 +0300
> > Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> > 
> > > 
> > > > > -/* VSC7385 Gigabit Switch support */
> > > > > +/*
> > > > > + * VSC7385 Gigabit Switch support
> > > > > + */
> > > > > +#define CFG_VSC7385_BASE	0xF0000000
> > > > 
> > > > What does this have to do with NAND?
> > > 
> > > Nothing. I'm expanding VSC7385 comment to better separate it from
> > > the NAND code. Most lengthy parts of the RDB config are using
> > > 
> > > /*
> > >  *
> > >  */
> > > 
> > > as a logical separator. So did I.
> > 
> > that's fine, it just makes patch review easier and less susceptible to
> > comments like this if you keep your patches on-subject.
> 
> To be fair, the main reason for the comment was that I missed this line:
> -#define CFG_VSC7385_BASE	0xF0000000
> 
> way at the top of the diff, and thought the patch was introducing the
> #define rather than just reformatting the comment.

Nothing was introduced, but moved around. Anyway, I'll just drop it
on the next resend.

-- 
Anton Vorontsov
email: cboumailru at gmail.com
irc://irc.freenode.net/bd2

[U-Boot-Users] [PATCH 1/7] 83xx: nand support for MPC837XRDB boards

[Jerry Van Baren]

Anton Vorontsov wrote:
> On Thu, Mar 20, 2008 at 11:26:45AM -0500, Scott Wood wrote:
>> On Wed, Mar 19, 2008 at 08:16:45PM -0500, Kim Phillips wrote:
>>> On Tue, 18 Mar 2008 17:10:55 +0300
>>> Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
>>>
>>>>>> -/* VSC7385 Gigabit Switch support */
>>>>>> +/*
>>>>>> + * VSC7385 Gigabit Switch support
>>>>>> + */
>>>>>> +#define CFG_VSC7385_BASE	0xF0000000
>>>>> What does this have to do with NAND?
>>>> Nothing. I'm expanding VSC7385 comment to better separate it from
>>>> the NAND code. Most lengthy parts of the RDB config are using
>>>>
>>>> /*
>>>>  *
>>>>  */
>>>>
>>>> as a logical separator. So did I.
>>> that's fine, it just makes patch review easier and less susceptible to
>>> comments like this if you keep your patches on-subject.
>> To be fair, the main reason for the comment was that I missed this line:
>> -#define CFG_VSC7385_BASE	0xF0000000
>>
>> way at the top of the diff, and thought the patch was introducing the
>> #define rather than just reformatting the comment.
> 
> Nothing was introduced, but moved around. Anyway, I'll just drop it
> on the next resend.

Hi Anton,

IMHO the move and reformat was a Good Thing[tm].  We (myself included) 
simply didn't understand what happened and it looked odd.

I would encourage you to keep the move/reformat and leave the world a 
little cleaner and brighter.

Thanks,
gvb