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

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

John Dallaway <john@dallaway.org.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |MODIFIED
                 CC|                            |ecos-patches@ecos.sourcewar
                   |                            |e.org
          Component|Ethernet                    |Patches and contributions
         AssignedTo|backlog@bugs.ecos.sourcewar |unassigned@bugs.ecos.source
                   |e.org                       |ware.org

--- Comment #10 from John Dallaway <john@dallaway.org.uk> 2011-02-17 09:48:51 GMT ---
Changing status to MODIFIED

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

Ilija Stanislevik <ilijas@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1033|0                           |1
        is obsolete|                            |

--- Comment #11 from Ilija Stanislevik <ilijas@siva.com.mk> 2011-02-17 10:43:46 GMT ---
Created an attachment (id=1132)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1132)
Patch to ecos.db, ChangeLog and STM3210eval HAL

STM3210e_eval HAL:

STM3210e_eval HAL defined resources used for SPI Ethernet driver are now
parented in SPI Ethernet driver.

User enters port and pin to which interrupt from SPI Ethernet controller is
connected. Default values are intentionally made unacceptable, so the user is
given a chance to think and enter data which match his hardware situation.

Interrupt vector is automatically calculated based on pin entered.

User can select to which of the STM32's SPI buses is connected the Ethernet
chip. Only the buses which are enabled in SPI driver are available for
selection. This option requires at least one SPI bus to be enabled. If none is
enabled, the "requires" mechanism automatically enables bus 1. I find this safe
enough because if there is only this SPI device in the system, it is highly
likely that it is connected to bus 1. If someone adds more SPI devices, or a
device to another SPI bus, he/she will be aware of the possibilities and will
enable and select correct buses. Using the same logic, the chip select number
defaults to 0 (indicating the first cs from the list defined in SPI driver),
which I made the lowest legal_value. I wasn't able to establish a hard upper
limit, so I put one which I deem fair enough.

SPI bus pin toggle rate is required to be 50MHz, which accommodates for
enc424j600's SPI 14MHZ rate.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

Ilija Stanislevik <ilijas@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1131|0                           |1
        is obsolete|                            |

--- Comment #13 from Ilija Stanislevik <ilijas@siva.com.mk> 2011-10-27 15:22:27 BST ---
Created an attachment (id=1420)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1420)
Updated enc424j600 Ethernet driver

(In reply to comment #12)

> >> - 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. I've made changes as per your suggestion, except for OnChip instead of
On-chip. I didn't find a way to persuade the preprocessor not to take the
hyphen for a minus sign.

I've also changed the legal_values of some other components into, as I hope,
better readable ones.

I've changed the way the interrupt priority is specified. Now it can be
specified from HAL. If not, the user can specify it directly in driver, just as
it is the case with interrupt vector.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

Ilija Stanislevik <ilijas@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1132|0                           |1
        is obsolete|                            |

--- Comment #14 from Ilija Stanislevik <ilijas@siva.com.mk> 2011-10-27 15:41:08 BST ---
Created an attachment (id=1421)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1421)
Patch to stm3210e-eval HAL - initialization of enc424j600

(In reply to comment #12)

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

I agree. I've introduced new option
CYGMEM_HAL_CORTEXM_STM32_STM3210E_EXTRA_BASE_RAM and corresponding ldi files.
Also, there is a provision to specify the quantity of reserved RAM.

In addition, I've modified the HAL to produce non-conflicting values for
enc424j600 related options. I believe this is better than having conflicting
values at the beginning.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

--- Comment #15 from Ilija Stanislevik <ilijas@siva.com.mk> 2011-10-27 15:42:49 BST ---
Created an attachment (id=1422)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1422)
Patch to ecos.db

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

--- Comment #16 from Jonathan Larmour <jifl@ecoscentric.com> 2012-04-02 21:21:22 BST ---
Hi Ilija,

Let's try and get this committed. I've gone through the current version of the
patch right from the beginning again.

Driver comments:

- In the CDL you don't want to use
!is_active(CYGINT_IO_ETH_INT_SUPPORT_REQUIRED), is it will always be active if
the package is included which it always would be. Instead you want something
like:

requires { CYGINT_IO_ETH_INT_SUPPORT_REQUIRED implies
(CYGNUM_DEVS_ETH_ENC424J600_INTERRUPT_VECTOR != "\"\"") }

(and the same for _PRIORITY)

- I think I'd prefer the option
CYGNUM_DEVS_ETH_ENC424J600_ACCEPTABLE_PACKET_SIZE to be
CYGNUM_DEVS_ETH_ENC424J600_MAX_RX_SIZE since that's what it is really.

- In CYGNUM_DEVS_ETH_ENC424J600_TXBUF_SIZE, I think the lowest transmit buffer
size could be lower than 1518. lwIP for example allows you to limit the packet
size, so it would be possible for a user to guarantee that larger packets
weren't sent by the stack. But I'd leave the default as it is.

- Purely for consistency, please change
CYGNUM_ETH_ENC424J600_USER_INTERRUPT_VECTOR ->
CYGNUM_DEVS_ETH_ENC424J600_USER_INTERRUPT_VECTOR

- "treshold" should be "threshold"

- For CYGNUM_DEVS_ETH_ENC424J600_INTERRUPT_PRIORITY and
CYGNUM_DEVS_ETH_ENC424J600_INTERRUPT_VECTOR, you don't need to have a separate
"user" option. You just need to have the default_value of
CYGNUM_DEVS_ETH_ENC424J600_INTERRUPT_PRIORITY to be: 

is_active(CYGNUM_HAL_SPIETH_INTERRUPT_PRIORITY) ?
CYGNUM_HAL_SPIETH_INTERRUPT_PRIORITY : "\"\""

That way it also allows users to override the HAL if they've hooked it up
differently.

- On committing I will probably just tidy up a few of the CDL descriptions for
English clarity, please don't be offended!


STM3210e changes comments:

- It may need to be updated due to recent commits in the stm3210e HAL.

- You can remove these from CYGNUM_ETH_SPIETH_HAL_INTERRUPT_VECTOR:
            requires { CYGHWR_HAL_SPIETH_INTERRUPT_PIN != "" } 
            requires { CYGHWR_HAL_CORTEXM_STM32_SPIETH_INTERRUPT_PORT != "" } 

- Think of how the CDL will look in the graphical config tool -   "STM32 HAL
resources assigned to SPI Ethernet controller" is a very long string to
display. It would be better just as "Optional ENC424J600 SPI Ethernet"

- In CYGHWR_HAL_CORTEXM_STM32_SPIETH_SPI_BUS, rather than setting the legal
values from the enabled buses, it should be the other way round - require the
buses to be enabled based on the setting. In other words:
  legal_values { 1 2 3 }
  requires { (CYGHWR_HAL_CORTEXM_STM32_SPIETH_SPI_BUS == 1) implies
CYGHWR_DEVS_SPI_CORTEXM_STM32_BUS1 }
  requires { (CYGHWR_HAL_CORTEXM_STM32_SPIETH_SPI_BUS == 2) implies
CYGHWR_DEVS_SPI_CORTEXM_STM32_BUS2 }
  requires { (CYGHWR_HAL_CORTEXM_STM32_SPIETH_SPI_BUS == 3) implies
CYGHWR_DEVS_SPI_CORTEXM_STM32_BUS3 }

- CYGHWR_HAL_SPIETH_INTERRUPT_PIN can have just "legal_values 0 to 15"

- I don't think you should be putting anything in stm3210e_eval_misc here. That
should be kept for HAL stuff, and this is a very optional peripheral add-on. It
should be a separate file. But even then, what you do doesn't seem quite right.
Any port specific initialisation should be performed via the
enc424j600_spi_init() function. The eth driver should use a HAL provided hook.
It can get this by including <cyg/hal/hal_io.h> but in practice it will usually
be defined in the platform's plf_io.h. For example for stm3210e's plf_io.h:

struct cyg_netdevtab_entry;
__externC cyg_devs_cortexm_stm3210e_enc424j600_init( struct cyg_netdevtab_entry
* );
#define CYG_DEVS_ETH_ENC424J600_PLF_INIT( _tab_ ) \
  cyg_devs_cortexm_stm3210e_enc424j600_init( _tab_ )

where "_tab_" is the cyg_netdevtab_entry passed to enc424j600_spi_init().

This would also get rid of the very yucky duplication of netdev_lookup().

Comments for both: The ChangeLog format still isn't quite right I'm afraid. It
doesn't mention the files affected. It's probably not worth worrying about this
for the eth driver since it's a new package (but please do it for new changes
in that package), but you should follow the format for the existing ChangeLogs.

Sorry that there's rather more comments than I'd have expected at this point,
but I've been through it in more detail this time than before.

Jifl

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

--- Comment #17 from Ilija Stanislevik <ilijas@siva.com.mk> 2012-04-05 07:21:40 BST ---
(In reply to comment #16)
Hi Jonathan,

> - In the CDL you don't want to use
> !is_active(CYGINT_IO_ETH_INT_SUPPORT_REQUIRED), is it will always be active if
> the package is included which it always would be.

I think that RedBoot is a case where package CYGPKG_DEVS_ETH_ENC424J600 is
included and CYGINT_IO_ETH_INT_SUPPORT_REQUIRED is not active.

> Instead you want something like:
>
> requires { CYGINT_IO_ETH_INT_SUPPORT_REQUIRED implies
> (CYGNUM_DEVS_ETH_ENC424J600_INTERRUPT_VECTOR != "\"\"") }
>
> (and the same for _PRIORITY)
>

Anyway, this above is better.

> - I think I'd prefer the option
> CYGNUM_DEVS_ETH_ENC424J600_ACCEPTABLE_PACKET_SIZE to be
> CYGNUM_DEVS_ETH_ENC424J600_MAX_RX_SIZE since that's what it is really.

This parameter is upper limit for both receive and transmit packet size. The
description is wrong. I will change the description. According to what I've
been able to find out, the lower limit for an Ethernet packet is 60 bytes, so I
will set the legal_values accordingly.

>
> - In CYGNUM_DEVS_ETH_ENC424J600_TXBUF_SIZE, I think the lowest transmit buffer
> size could be lower than 1518. lwIP for example allows you to limit the packet
> size, so it would be possible for a user to guarantee that larger packets
> weren't sent by the stack. But I'd leave the default as it is.
>

The chip will truncate both Rx and Tx packets to
CYGNUM_DEVS_ETH_ENC424J600_ACCEPTABLE_PACKET_SIZE bytes.
CYGNUM_DEVS_ETH_ENC424J600_TXBUF_SIZE serves to reserve space for transmit
buffer in on-chip RAM. I will set its lowest legal value to reflect
CYGNUM_DEVS_ETH_ENC424J600_ACCEPTABLE_PACKET_SIZE.

> - Purely for consistency, please change
> CYGNUM_ETH_ENC424J600_USER_INTERRUPT_VECTOR ->
> CYGNUM_DEVS_ETH_ENC424J600_USER_INTERRUPT_VECTOR
>
> - "treshold" should be "threshold"
>

Of course.

> - For CYGNUM_DEVS_ETH_ENC424J600_INTERRUPT_PRIORITY and
> CYGNUM_DEVS_ETH_ENC424J600_INTERRUPT_VECTOR, you don't need to have a separate
> "user" option. You just need to have the default_value of
> CYGNUM_DEVS_ETH_ENC424J600_INTERRUPT_PRIORITY to be: 
>
> is_active(CYGNUM_HAL_SPIETH_INTERRUPT_PRIORITY) ?
> CYGNUM_HAL_SPIETH_INTERRUPT_PRIORITY : "\"\""
>
> That way it also allows users to override the HAL if they've hooked it up
> differently.
>

I agree.

> - On committing I will probably just tidy up a few of the CDL descriptions for
> English clarity, please don't be offended!
>

I'll do my best.

>
> STM3210e changes comments:
>
> - It may need to be updated due to recent commits in the stm3210e HAL.
>
> - You can remove these from CYGNUM_ETH_SPIETH_HAL_INTERRUPT_VECTOR:
>             requires { CYGHWR_HAL_SPIETH_INTERRUPT_PIN != "" } 
>             requires { CYGHWR_HAL_CORTEXM_STM32_SPIETH_INTERRUPT_PORT != "" } 
>

OK

> - Think of how the CDL will look in the graphical config tool -   "STM32 HAL
> resources assigned to SPI Ethernet controller" is a very long string to
> display. It would be better just as "Optional ENC424J600 SPI Ethernet"
>

Your suggestion isn't optimal because this component is parented in
CYGPKG_DEVS_ETH_ENC424J600, so in configtool it appears there. The title should
depict the component as part of HAL. You are right about the length, it should
be shorter. I'll try "STM32 HAL resources" and some better description.

> - In CYGHWR_HAL_CORTEXM_STM32_SPIETH_SPI_BUS, rather than setting the legal
> values from the enabled buses, it should be the other way round - require the
> buses to be enabled based on the setting. In other words:
>   legal_values { 1 2 3 }
>   requires { (CYGHWR_HAL_CORTEXM_STM32_SPIETH_SPI_BUS == 1) implies
> CYGHWR_DEVS_SPI_CORTEXM_STM32_BUS1 }
>   requires { (CYGHWR_HAL_CORTEXM_STM32_SPIETH_SPI_BUS == 2) implies
> CYGHWR_DEVS_SPI_CORTEXM_STM32_BUS2 }
>   requires { (CYGHWR_HAL_CORTEXM_STM32_SPIETH_SPI_BUS == 3) implies
> CYGHWR_DEVS_SPI_CORTEXM_STM32_BUS3 }
>

Yes, this is much better.

> - CYGHWR_HAL_SPIETH_INTERRUPT_PIN can have just "legal_values 0 to 15"
>

I agree.

> - I don't think you should be putting anything in stm3210e_eval_misc here. That
> should be kept for HAL stuff, and this is a very optional peripheral add-on. It
> should be a separate file. But even then, what you do doesn't seem quite right.
> Any port specific initialization should be performed via the
> enc424j600_spi_init() function. The eth driver should use a HAL provided hook.
> It can get this by including <cyg/hal/hal_io.h> but in practice it will usually
> be defined in the platform's plf_io.h. For example for stm3210e's plf_io.h:
>
> struct cyg_netdevtab_entry;
> __externC cyg_devs_cortexm_stm3210e_enc424j600_init( struct cyg_netdevtab_entry
> * );
> #define CYG_DEVS_ETH_ENC424J600_PLF_INIT( _tab_ ) \
>   cyg_devs_cortexm_stm3210e_enc424j600_init( _tab_ )
>
> where "_tab_" is the cyg_netdevtab_entry passed to enc424j600_spi_init().
>
> This would also get rid of the very yucky duplication of netdev_lookup().

I see now. Looks quite elegant.

I will update with recent changes to stm3210e then I will submit the changes.


Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

--- Comment #18 from Jonathan Larmour <jifl@ecoscentric.com> 2012-04-05 16:51:40 BST ---
(In reply to comment #17)
> (In reply to comment #16)
> Hi Jonathan,
> 
> > - In the CDL you don't want to use
> > !is_active(CYGINT_IO_ETH_INT_SUPPORT_REQUIRED), is it will always be active if
> > the package is included which it always would be.
> 
> I think that RedBoot is a case where package CYGPKG_DEVS_ETH_ENC424J600 is
> included and CYGINT_IO_ETH_INT_SUPPORT_REQUIRED is not active.

I think it is active. Look at a redboot build's ecos.ecc file. You will see for
other options it clearly says "This option is not active". For
CYGINT_IO_ETH_INT_SUPPORT_REQUIRED it does not say that. Remember that active
is different from enabled.

> > Instead you want something like:
> >
> > requires { CYGINT_IO_ETH_INT_SUPPORT_REQUIRED implies
> > (CYGNUM_DEVS_ETH_ENC424J600_INTERRUPT_VECTOR != "\"\"") }
> >
> > (and the same for _PRIORITY)
> 
> Anyway, this above is better.

Then problem solved :-).

[snip other stuff which is fine, no need for me to acknowledge]

> > - On committing I will probably just tidy up a few of the CDL descriptions for
> > English clarity, please don't be offended!
> 
> I'll do my best.

As I said, please don't be offended!

> > - Think of how the CDL will look in the graphical config tool -   "STM32 HAL
> > resources assigned to SPI Ethernet controller" is a very long string to
> > display. It would be better just as "Optional ENC424J600 SPI Ethernet"
> >
> 
> Your suggestion isn't optimal because this component is parented in
> CYGPKG_DEVS_ETH_ENC424J600, so in configtool it appears there.

True, good point.

> The title should
> depict the component as part of HAL. You are right about the length, it should
> be shorter. I'll try "STM32 HAL resources" and some better description.

Thanks.

[snip] 
> I will update with recent changes to stm3210e then I will submit the changes.

Thank you for bearing with me! The result will be better quality which will no
doubt be appreciated by many :-).

Jifl

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

Ilija Stanislevik <ilijas@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1420|0                           |1
        is obsolete|                            |

--- Comment #19 from Ilija Stanislevik <ilijas@siva.com.mk> 2012-04-20 09:00:23 BST ---
Created an attachment (id=1705)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1705)
ENC424J600 Ethernet driver

Changes according to comments 16 and 18

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

Ilija Stanislevik <ilijas@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1421|0                           |1
        is obsolete|                            |

--- Comment #20 from Ilija Stanislevik <ilijas@siva.com.mk> 2012-04-20 09:07:29 BST ---
Created an attachment (id=1706)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1706)
Initialization of enc424j600 for stm3210e-eval HAL

Compliance with new STM32 HAL, comments 16 and 18.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

--- Comment #21 from Jonathan Larmour <jifl@ecoscentric.com> 2012-05-04 17:35:48 BST ---
I have made a bunch of cosmetic updates and applied the patch, thanks! I will
attach the updated version of the patch, along with the updates themselves. A
lot of them were just whitespace changes, most especially in the CDL file. It
now conforms to our existing practice much better, and in future you may want
to watch out for the sort of things I changed if you submit patches in future.
There are a few other textual changes too, and some fixes of changing things
like cdl_components which should really be cdl_options. But nothing of great
importance.

Note, despite the earlier discussion, I have in the end changed the description
of CYGHWR_HAL_CORTEXM_STM32_SPIETH_CONTROLLER because I realised that in the
configuration tool, if the CYGPKG_DEVS_ETH_ENC424J600 package is not loaded,
the package will be displayed at the top level, albeit inactive. With a title
of just "STM3210E HAL resources", this might confuse some people.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1706|0                           |1
        is obsolete|                            |

--- Comment #23 from Jonathan Larmour <jifl@ecoscentric.com> 2012-05-04 17:37:13 BST ---
Created an attachment (id=1733)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1733)
Jifl updated stm3210e patch

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1705|0                           |1
        is obsolete|                            |

--- Comment #22 from Jonathan Larmour <jifl@ecoscentric.com> 2012-05-04 17:36:46 BST ---
Created an attachment (id=1732)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1732)
Jifl updated enc424j600 patch

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

--- Comment #24 from Jonathan Larmour <jifl@ecoscentric.com> 2012-05-04 17:37:47 BST ---
Created an attachment (id=1734)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1734)
Changes just against Ilija's previous patch

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

--- Comment #25 from Jonathan Larmour <jifl@ecoscentric.com> 2012-05-04 17:38:26 BST ---
Created an attachment (id=1735)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1735)
Changes just against Ilija's previous enc424j600 patch

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

--- Comment #26 from Jonathan Larmour <jifl@ecoscentric.com> 2012-05-04 17:38:56 BST ---
Created an attachment (id=1736)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1736)
Changes just against Ilija's previous enc424j600 patch ignoring whitespace

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

--- Comment #27 from Jonathan Larmour <jifl@ecoscentric.com> 2012-05-04 17:40:35 BST ---
Ilija, please confirm you are happy with what has been committed.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

--- Comment #28 from Ilija Stanislevik <ilijas@siva.com.mk> 2012-05-07 17:43:14 BST ---
(In reply to comment #27)

Hi Jonathan, I have tested the committed against my applications. It works
fine. Thank you for the guidances and for the patience.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

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

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000910

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |RESOLVED
         Resolution|                            |CURRENTRELEASE

--- Comment #29 from Jonathan Larmour <jifl@ecoscentric.com> 2012-05-10 16:48:06 BST ---
Great, thanks.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.