[Issue 1000910] New port: Ethernet over SPI driver for ENC424J600

[Issue 1000910] New port: Ethernet over SPI driver for ENC424J600

[bugzilla-daemon]

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.

[Issue 1000910] New port: Ethernet over SPI driver for ENC424J600

[bugzilla-daemon]

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
----------------------------------------------------------------------------
         AssignedTo|unassigned@bugs.ecos.source |jifl@ecoscentric.com
                   |ware.org                    |

-- 
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.