From: bugzilla-daemon@ecoscentric.com
To: ecos-patches@ecos.sourceware.org
Subject: [Issue 1000910] New port: Ethernet over SPI driver for ENC424J600
Date: Mon, 17 Oct 2011 02:48:00 -0000 [thread overview]
Message-ID: <20111017024750.F14942F78006@mail.ecoscentric.com> (raw)
In-Reply-To: <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.
next parent reply other threads:[~2011-10-17 2:48 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <bug-1000910-104@http.bugzilla.ecoscentric.com/>
2011-10-17 2:48 ` bugzilla-daemon [this message]
2011-10-17 22:35 ` bugzilla-daemon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111017024750.F14942F78006@mail.ecoscentric.com \
--to=bugzilla-daemon@ecoscentric.com \
--cc=ecos-patches@ecos.sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).