This is the mail archive of the
ecos-patches@sourceware.org
mailing list for the eCos project.
[Issue 1000910] New port: Ethernet over SPI driver for ENC424J600
- From: bugzilla-daemon at ecoscentric dot com
- To: ecos-patches at ecos dot sourceware dot org
- Date: Mon, 17 Oct 2011 03:47:50 +0100
- Subject: [Issue 1000910] New port: Ethernet over SPI driver for ENC424J600
- Auto-submitted: auto-generated
- References: <bug-1000910-104@http.bugzilla.ecoscentric.com/>
Please do not reply to this email. Use the web interface provided at:
https://bugzilla.ecoscentric.com/show_bug.cgi?id=1000910
Jonathan Larmour <jifl@ecoscentric.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|MODIFIED |NEEDINFO
--- Comment #12 from Jonathan Larmour <jifl@ecoscentric.com> 2011-10-17 03:47:45 BST ---
I think this patch is looking very good now, including the further improvements
you've made. Some (hopefully final) comments:
> enc424j600_spi_init() does not loop infinitely anymore if there is no Ethernet
> chip on SPI. Instead it tries finite number of times and CYG_FAILs (or returns
> false) if there is no chip. The same mechanism is later implemented for
> checking if the enc424j600's clock is ready. Number of retries and time between
> retries are #defined in the source. Should they go to cdl?
That's probably overkill. You can if you are feeling keen.
>> - A trivial thing that would be nice: CYGNUM_DEVS_ETH_ENC424J600_FLOWC has
>> values which could be friendlier when e.g. presented in the configuration tool.
>> Any chance they could actually be just None and Auto? If you wanted, some
>> #ifdef magic could still translate them if you preferred.
>
> There is a possibility for third option, besides no-flow-control and
> on-chip-flow-control, that is in-driver-flow-control. I did not implement it,
> but I think that it might be implemented in future. So, I decided to keep
> CYGNUM_DEVS_ETH_ENC424J600_FLOWC unchanged.
What I was suggesting wouldn't change that. It would just mean that in that
case the legal_values now would be { "None" "On-chip" } and in future you could
add "In-driver" to that.
I'm just noticing that that option should probably be CYGSEM_...
If you want to keep the rest of the code the same as it is now, you could add
this in a header:
#if (CYGSEM_DEVS_ETH_ENC424J600_FLOWC == On-chip)
# define CYGSEM_DEVS_ETH_ENC424J600_FLOWC_ONCHIP_AUTO_FC
#endif
I see the stm3210e changes include a change to increase RAM use by 16k to allow
for network-enabled redboot. That's quite a large change for something that
most people don't need. I think a different MLT file would be better for that.
You could either create a new HAL startup type, or you could just add a new
option, and change CYGHWR_MEMORY_LAYOUT to be something like:
calculated { (CYG_HAL_STARTUP == "RAM" ) ?
(CYGMEM_HAL_CORTEXM_STM32_STM3210E_RESERVE_EXTRA_BASE_RAM ?
"cortexm_stm3210e_eval_extrabaseram : "cortexm_stm3210e_eval_ram") :
(CYG_HAL_STARTUP == "SRAM" ) ?
"cortexm_stm3210e_eval_sram" :
(CYG_HAL_STARTUP == "ROM" ) ?
"cortexm_stm3210e_eval_rom" :
(CYG_HAL_STARTUP == "ROMINT" ) ?
"cortexm_stm3210e_eval_romint" :
(CYG_HAL_STARTUP == "JTAG" ) ?
"cortexm_stm3210e_eval_jtag" :
"undefined" }
Although I'm suggesting
CYGMEM_HAL_CORTEXM_STM32_STM3210E_RESERVE_EXTRA_BASE_RAM for the option name,
you may well be able to choose something with a better name.
Let me know what you think.
Jifl
--
Configure issuemail: https://bugzilla.ecoscentric.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the issue.