On Mon, Aug 29, 2011 at 5:53 PM, Mike Frysinger <[email protected]> wrote:
> On Monday, August 29, 2011 17:22:16 Jie Zhang wrote:
>> On Mon, Aug 29, 2011 at 4:13 PM, Mike Frysinger <[email protected]> wrote:
>> > On Monday, August 29, 2011 15:54:35 Jie Zhang wrote:
>> >> On Mon, Aug 29, 2011 at 3:22 PM, Mike Frysinger <[email protected]>
> wrote:
>> >> > On Monday, August 29, 2011 10:38:17 Jie Zhang wrote:
>> >> >> I have committed this patch to add BF527 SDP board bus support. I did
>> >> >> the test by detectflash, but did not try flashmem.
>> >> >
>> >> > this already works with the bf52x bus. simply pass HWAIT=PG0 when
>> >> > initializing it. i tested that with detecting, reading, and writing
>> >> > flash when i implemented it.
>> >>
>> >> I was wondering what's the use of HWAIT. Now I see. Thank you!
>> >>
>> >> But the problem is it's not easy for user to know they can use this
>> >> parameter for BF527 SDP board. As I said before, we should allow user
>> >> to add board names to initbus explicitly. Otherwise, people will keep
>> >> asking if board FOO is supported or not.
>> >
>> > the trouble is that PG0 isnt specific to the BF527 SDP board. if you
>> > read the HRM, PG0 is the default HWAIT signal for all BF52x procs. and
>> > iirc, you can change this in the OTP. i dont have a BF52x HRM handy atm
>> > to refresh my memory on the specifics.
>>
>> I don't know what HWAIT stands for. I cannot think it's related to
>> flash enable signal from the name. If you have named it as
>> sig_flash_enable and sig_flash_enable_level, I might have read the
>> related code carefully. I thought it was some kind of hack from the
>> first look. I think it will be better to make a patch to rename it to
>> something more meaningful.
>
> HWAIT is the "hardware wait" signal. the Blackfin bootrom drives this to
> signal so that the device it is loading from knows when to send data and when
> to hold. think of it as CTS/RTS on a UART (after all, that's exactly how the
> HWAIT signal gets used in UART boot mode).
>
> when it comes to parallel flashes, the bootrom drives PG0 as its HWAIT signal.
> the SDP board is tricky in that it is trying to minimize interference from its
> own board so that it can expose as much of its lower pins as possible to the
> USB side so that people can maximize the prototyping capabilities. since
> parallel flash sucks up quite a lot of the async bank, it uses the PG0 as a
> part of its trick: if PG0 is not being driven, then none of the async pins
> route to the flash. instead, they route to the connector so people can use
> them for something else. so when the processor is booting, the bootrom drives
> PG0, and the flash is readable. when the processor is done, it stops driving
> PG0, and the flash is no longer accessible.
>
Now I know what HWAIT signal is for. Thank you for explanation.
>From your explanation, the name HWAIT has nothing to do with flash. It
seems to me that it's just a coincidence BF527 SDP board uses PG0 as
flash_enable signal and PG0 happens to be a HWAIT signal for UART.
I still think we should rename hwait and hwait_enable_level to
something more meaningful for flash.
>> > i dont mind adding an alias for it, but it shouldnt be a dedicated board
>> > like this.
>> >
>> >> Is there a way to put bf527_sdp into bf537_stamp.c like setting a
>> >> default value to hwait and hwait_level?
>> >
>> > we might have to restructure a little to make it simpler ...
>>
>> After some thinking, I come up with this patch. The code can be
>> extended for several more boards like bf527_sdp, but will not look
>> pretty for more than that. What do you think?
>
> hmm, how about we extend bfin_bus_new() to accept a default array structure ?
> and that func can search the array for matching bus/key tuples ? that way we
> can easily do this for all procs.
>
> blackfin.h:
> struct bfin_bus_defaults {
> const char *bus_name;
> int key;
> const char *signal;
> };
> bf537_stamp.c:
> static const bf537_stamp_defaults[] = {
> { "bf527_sdp", URJ_BUS_PARAM_KEY_HWAIT, "PG0", },
> };
>
> thinking about it a bit more, we could probably do this globally for all
> buses, but for now let's implement it in the Blackfin bus core and see how it
> goes. i know how much Chad loves you implementing my grand ideas out of scope
> of your original needs ;).
>
Here is the patch. It has a FIXME in it. It will not hurt now. It will
be fixed if we do it globally because then we can parse defaults
before user input. OK?
Regards,
Jie
* src/bus/blackfin.c (bfin_bus_new): Add new argument for default
parameter values. Update all callers.
* src/bus/blackfin.h (bfin_bus_new): Update declaration.
(bfin_bus_default_t): Typedef.
* src/bus/bf537_stamp.c (bf537_stamp_defaults[]): New.
(bf537_stamp_bus_new): Pass bf537_stamp_defaults to bfin_bus_new.
(bf527_sdp): _BFIN_BUS_DECLARE.
* src/bus/Makefile.am (libbus_la_SOURCES): Remove bf527_sdp.c.
* src/bus/bf527_sdp.c: Remove.
* src/global/params.c (urj_param_init_list): Add missing {, }.
Index: src/bus/bf548_ezkit.c
===================================================================
--- src/bus/bf548_ezkit.c (revision 1992)
+++ src/bus/bf548_ezkit.c (working copy)
@@ -71,7 +71,7 @@ bf548_ezkit_bus_new (urj_chain_t *chain,
params->data_cnt = 16;
params->select_flash = bf548_ezkit_select_flash;
params->unselect_flash = bf548_ezkit_unselect_flash;
- failed |= bfin_bus_new (bus, cmd_params);
+ failed |= bfin_bus_new (bus, cmd_params, NULL);
failed |= urj_bus_generic_attach_sig (part, &DCS0, "CS0_B");
failed |= urj_bus_generic_attach_sig (part, &NCE, "PJ1");
Index: src/bus/bf518f_ezbrd.c
===================================================================
--- src/bus/bf518f_ezbrd.c (revision 1992)
+++ src/bus/bf518f_ezbrd.c (working copy)
@@ -48,7 +48,7 @@ bf518f_ezbrd_bus_new (urj_chain_t *chain
params->addr_cnt = 19;
params->data_cnt = 16;
params->sdram = 1;
- failed |= bfin_bus_new (bus, cmd_params);
+ failed |= bfin_bus_new (bus, cmd_params, NULL);
if (failed)
{
Index: src/bus/bf533_stamp.c
===================================================================
--- src/bus/bf533_stamp.c (revision 1992)
+++ src/bus/bf533_stamp.c (working copy)
@@ -70,7 +70,7 @@ bf533_stamp_bus_new (urj_chain_t *chain,
params->select_flash = bf533_stamp_select_flash;
params->unselect_flash = bf533_stamp_unselect_flash;
params->sdram = 1;
- failed |= bfin_bus_new (bus, cmd_params);
+ failed |= bfin_bus_new (bus, cmd_params, NULL);
failed |= urj_bus_generic_attach_sig (part, &PF[0], "PF0");
failed |= urj_bus_generic_attach_sig (part, &PF[1], "PF1");
Index: src/bus/bf537_stamp.c
===================================================================
--- src/bus/bf537_stamp.c (revision 1992)
+++ src/bus/bf537_stamp.c (working copy)
@@ -29,6 +29,12 @@ typedef struct
bfin_bus_params_t params; /* needs to be first */
} bus_params_t;
+static const bfin_bus_default_t bf537_stamp_defaults[] = {
+ /* BF527 SDP board uses PG0 as ~FLASH_EN. */
+ {"bf527_sdp", "hwait=PG0"},
+ {NULL},
+};
+
static urj_bus_t *
bf537_stamp_bus_new (urj_chain_t *chain, const urj_bus_driver_t *driver,
const urj_param_t *cmd_params[])
@@ -48,7 +54,7 @@ bf537_stamp_bus_new (urj_chain_t *chain,
params->addr_cnt = 19;
params->data_cnt = 16;
params->sdram = 1;
- failed |= bfin_bus_new (bus, cmd_params);
+ failed |= bfin_bus_new (bus, cmd_params, bf537_stamp_defaults);
if (failed)
{
@@ -62,6 +68,7 @@ bf537_stamp_bus_new (urj_chain_t *chain,
BFIN_BUS_DECLARE(bf537_stamp, "BF537 Stamp board");
_BFIN_BUS_DECLARE(bf537_ezkit, bf537_stamp, "BF537 EZ-Kit board");
_BFIN_BUS_DECLARE(bf527_ezkit, bf537_stamp, "BF527 EZ-Kit board");
+_BFIN_BUS_DECLARE(bf527_sdp, bf537_stamp, "BF527 SDP board");
_BFIN_BUS_DECLARE(bf538f_ezkit, bf537_stamp, "BF538F EZ-Kit board");
_BFIN_BUS_DECLARE(bf526_ezkit, bf537_stamp, "BF526 EZ-Kit board");
_BFIN_BUS_DECLARE(bf533_ezkit, bf537_stamp, "BF533 EZ-Kit board");
Index: src/bus/blackfin.c
===================================================================
--- src/bus/blackfin.c (revision 1992)
+++ src/bus/blackfin.c (working copy)
@@ -39,13 +39,30 @@ bfin_bus_attach_sigs (urj_part_t *part,
}
int
-bfin_bus_new (urj_bus_t *bus, const urj_param_t *cmd_params[])
+bfin_bus_new (urj_bus_t *bus, const urj_param_t *cmd_params[],
+ const bfin_bus_default_t *defaults)
{
bfin_bus_params_t *params = bus->params;
urj_part_t *part = bus->part;
int ret = 0;
size_t i;
+ /* FIXME The default parameter value can't be overridden by user. */
+ if (defaults != NULL)
+ for (i = 0; defaults[i].bus_name != NULL; ++i)
+ {
+ if (strcmp (defaults[i].bus_name, bus->driver->name))
+ continue;
+
+ ret = urj_param_push (&urj_bus_param_list, &cmd_params,
+ defaults[i].param);
+ if (ret != URJ_STATUS_OK)
+ {
+ urj_param_clear (&cmd_params);
+ return ret;
+ }
+ }
+
for (i = 0; cmd_params[i]; ++i)
switch (cmd_params[i]->key)
{
Index: src/bus/blackfin.h
===================================================================
--- src/bus/blackfin.h (revision 1992)
+++ src/bus/blackfin.h (working copy)
@@ -25,6 +25,11 @@
#include "generic_bus.h"
typedef struct {
+ const char *bus_name;
+ const char *param;
+} bfin_bus_default_t;
+
+typedef struct {
uint32_t async_base, async_size;
int ams_cnt, data_cnt, addr_cnt, abe_cnt;
@@ -41,7 +46,8 @@ typedef struct {
void (*unselect_flash) (urj_bus_t *bus);
} bfin_bus_params_t;
-int bfin_bus_new (urj_bus_t *bus, const urj_param_t *cmd_params[]);
+int bfin_bus_new (urj_bus_t *bus, const urj_param_t *cmd_params[],
+ const bfin_bus_default_t *defaults);
int bfin_bus_area (urj_bus_t *bus, uint32_t adr, urj_bus_area_t *area);
Index: src/bus/Makefile.am
===================================================================
--- src/bus/Makefile.am (revision 1992)
+++ src/bus/Makefile.am (working copy)
@@ -56,7 +56,6 @@ libbus_la_SOURCES += \
blackfin.c \
blackfin.h \
bf518f_ezbrd.c \
- bf527_sdp.c \
bf533_stamp.c \
bf537_stamp.c \
bf548_ezkit.c \
Index: src/bus/bf561_ezkit.c
===================================================================
--- src/bus/bf561_ezkit.c (revision 1992)
+++ src/bus/bf561_ezkit.c (working copy)
@@ -49,7 +49,7 @@ bf561_ezkit_bus_new (urj_chain_t *chain,
params->data_cnt = 32;
params->sdram = 1;
params->sms_cnt = 4;
- failed |= bfin_bus_new (bus, cmd_params);
+ failed |= bfin_bus_new (bus, cmd_params, NULL);
if (failed)
{
Index: src/bus/bf527_sdp.c
===================================================================
--- src/bus/bf527_sdp.c (revision 1992)
+++ src/bus/bf527_sdp.c (working copy)
@@ -1,87 +0,0 @@
-/*
- * $Id$
- *
- * Analog Devices ADSP-BF527 SDP board bus driver
- * Copyright (C) 2011 Analog Devices, Inc.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
- * 02111-1307, USA.
- *
- * Written by Jie Zhang <[email protected]>, 2011.
- */
-
-#include "blackfin.h"
-
-typedef struct
-{
- bfin_bus_params_t params; /* needs to be first */
- urj_part_signal_t *pg0; /* ~FLASH_EN */
-} bus_params_t;
-
-#define PG0 ((bus_params_t *) bus->params)->pg0
-
-static void
-bf527_sdp_select_flash (urj_bus_t *bus, uint32_t addr)
-{
- urj_part_t *part = bus->part;
-
- urj_part_set_signal (part, PG0, 1, 0);
-}
-
-static void
-bf527_sdp_unselect_flash (urj_bus_t *bus)
-{
- urj_part_t *part = bus->part;
-
- urj_part_set_signal (part, PG0, 1, 1);
-}
-
-static urj_bus_t *
-bf527_sdp_bus_new (urj_chain_t *chain, const urj_bus_driver_t *driver,
- const urj_param_t *cmd_params[])
-{
- urj_bus_t *bus;
- urj_part_t *part;
- bfin_bus_params_t *params;
- int failed = 0;
-
- bus = urj_bus_generic_new (chain, driver, sizeof (bus_params_t));
- if (bus == NULL)
- return NULL;
- part = bus->part;
-
- params = bus->params;
- params->async_size = 4 * 1024 * 1024;
- params->ams_cnt = 4;
- params->abe_cnt = 2;
- params->addr_cnt = 19;
- params->data_cnt = 16;
- params->sdram = 1;
- params->select_flash = bf527_sdp_select_flash;
- params->unselect_flash = bf527_sdp_unselect_flash;
- failed |= bfin_bus_new (bus, cmd_params);
-
- failed |= urj_bus_generic_attach_sig (part, &PG0, "PG0");
-
- if (failed)
- {
- urj_bus_generic_free (bus);
- return NULL;
- }
-
- return bus;
-}
-
-BFIN_BUS_DECLARE(bf527_sdp, "BF527 SDP board");
Index: src/global/params.c
===================================================================
--- src/global/params.c (revision 1992)
+++ src/global/params.c (working copy)
@@ -334,12 +334,14 @@ urj_param_init_list (const urj_param_t *
return ret;
for (i = 0; params[i] != NULL; ++i)
+ {
ret = urj_param_push (param_list, bp, params[i]);
if (ret != URJ_STATUS_OK)
{
urj_param_clear (bp);
return ret;
}
+ }
return URJ_STATUS_OK;
}
------------------------------------------------------------------------------
Special Offer -- Download ArcSight Logger for FREE!
Finally, a world-class log management solution at an even better
price-free! And you'll get a free "Love Thy Logs" t-shirt when you
download Logger. Secure your free ArcSight Logger TODAY!
http://p.sf.net/sfu/arcsisghtdev2dev
_______________________________________________
UrJTAG-development mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/urjtag-development
