[Bug 1001219] New: Ethernet driver for STM32 connectivity line with port on MMstm32f107 board.

[bugzilla-daemon]

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

           Summary: Ethernet driver for STM32 connectivity line with port
                    on MMstm32f107 board.
           Product: eCos
           Version: CVS
          Platform: Other (please specify)
        OS/Version: ARM
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: low
         Component: Patches and contributions
        AssignedTo: [hidden email]
        ReportedBy: [hidden email]
                CC: [hidden email]
             Class: Advice Request


Created an attachment (id=1218)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1218)
Ethernet driver for STM32 connectivity line with port on MMstm32f107 board

Hello all,

I would like to contribute Ethernet driver for STM32 connectivity line but due
to fact that the some support from HAL package is also required. I provide port
on MMstm32f107 board.

Beside of extending stm32/var include files to provide description of Ethernet
module and adding support of new model of clock distribution in STM32
connectivity line I would like to point functionality which cause me a lot of
problem to start Ethernet driver i.e.
stm32_misc.c in function hal_variant_init() is enabled clocks for all APB
peripherals. In case of Ethernet driver it should be done :
1) after PLL configuration (switching it on this stage cause hang up of MDIO
bus)
2) only used peripherals (  switching on all APB module cause hang up of RMII
bus)

My suggestion is to move it to board dependent area i.e. hal_platform_init()
and switch only those modules which are really added to eCos image.

Best regards,
jerzy

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

[bugzilla-daemon]

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

Ilija Kocho <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #1 from Ilija Kocho <[hidden email]> 2011-05-04 08:41:11 BST ---
(In reply to comment #0)
> Created an attachment (id=1218)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1218) [details]
> Ethernet driver for STM32 connectivity line with port on MMstm32f107 board
>

Hi Jerzy

Thanks for contribution. Just a hint, i found it's better to submit diff in
plain text format (not compressed) since Bugzilla will recognize it as a patch
(or you can additionally mark it "patch"). Bugzilla can present diff in user
friendly side-by side format.

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.

[bugzilla-daemon]

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

Jerzy Dyrda <[hidden email]> changed:

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

--- Comment #2 from Jerzy Dyrda <[hidden email]> 2011-05-12 10:17:56 BST ---
Created an attachment (id=1230)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1230)
Ethernet driver for STM32 connectivity line with port on MMstm32f107 board -
not gzipped

Plain text - not compressed.

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

[bugzilla-daemon]

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

--- Comment #3 from Ilija Kocho <[hidden email]> 2011-10-08 15:02:21 BST ---
Hi Jerzy

Thank you for your contribution. Here are my first remarks:

1. Some unresolved conflicts remain upon Propox board selection in Configtool
and it may mislead the user that something is broken. It seems that Configtool
can't resolve the CYGHWR_HAL_CORTEXM_STM32_CLOCK_PLL_SOURCE on it's own so
let's make it little-bit easier:
  -  Is it possible to re-arrange legal_values expression in order to make it
easier for Configtool?
  - Or it is better to calculate default_value conditionally
(CYGINT_HAL_CORTEXM_STM32_CL==0 ?)

2. CYGPKG_IO_ETH_DRIVERS is normally not included in the target. Not everybody
would use Ethernet. Please remove it.

3. FYI, the PHY driver DP8348 has been resolved in meantime by Bug 1001235.
Please synchronize your code with it.

Now some general hints: It usually takes a number of iterations before code is
fit for commit and it would be easier for me (hopefully also for you) if you
break the patch in several diffs (divide and conquer). Ideally - a diff for
every affected package, in this case: /eth driver/, /variant/, /platform/ and
eventually /phy driver/. As exception ecos.db is better not sent as diff,
instead put your ecos.db entries in a plain file and name it ecos_db.txt .

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

[bugzilla-daemon]

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

--- Comment #4 from Ilija Kocho <[hidden email]> 2011-10-08 15:27:25 BST ---
(In reply to comment #3)

> Hi Jerzy
>
> Thank you for your contribution. Here are my first remarks:
>
> 1. Some unresolved conflicts remain upon Propox board selection in Configtool
> and it may mislead the user that something is broken. It seems that Configtool
> can't resolve the CYGHWR_HAL_CORTEXM_STM32_CLOCK_PLL_SOURCE on it's own so
> let's make it little-bit easier:
>   -  Is it possible to re-arrange legal_values expression in order to make it
> easier for Configtool?
>   - Or it is better to calculate default_value conditionally
> (CYGINT_HAL_CORTEXM_STM32_CL==0 ?)
>
> 2. CYGPKG_IO_ETH_DRIVERS is normally not included in the target. Not everybody
> would use Ethernet. Please remove it.
>
> 3. FYI, the PHY driver DP8348 has been resolved in meantime by Bug 1001235.
> Please synchronize your code with it.

4. Just one more thing: Won't it be better if Propox on-board chip member
appears (CYGHWR_HAL_CORTEXM_STM32) once this board is selected?

>
> Now some general hints: It usually takes a number of iterations before code is
> fit for commit and it would be easier for me (hopefully also for you) if you
> break the patch in several diffs (divide and conquer). Ideally - a diff for
> every affected package, in this case: /eth driver/, /variant/, /platform/ and
> eventually /phy driver/. As exception ecos.db is better not sent as diff,
> instead put your ecos.db entries in a plain file and name it ecos_db.txt .

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

[bugzilla-daemon]

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

--- Comment #5 from Jerzy Dyrda <[hidden email]> 2011-10-10 21:50:42 BST ---
Created an attachment (id=1394)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1394)
Ethernet driver for STM32 CL

Ethernet driver for STM32 Connectivity Line

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

[bugzilla-daemon]

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

Jerzy Dyrda <[hidden email]> changed:

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

--- Comment #6 from Jerzy Dyrda <[hidden email]> 2011-10-10 21:54:08 BST ---
Created an attachment (id=1395)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1395)
Platform for Propox MMstm32f107 module

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

[bugzilla-daemon]

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

--- Comment #7 from Jerzy Dyrda <[hidden email]> 2011-10-10 21:55:57 BST ---
Created an attachment (id=1396)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1396)
Variant modification needed by STM32 CL

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

[bugzilla-daemon]

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

--- Comment #8 from Jerzy Dyrda <[hidden email]> 2011-10-10 22:12:55 BST ---
Created an attachment (id=1397)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1397)
ecos.db for Ethernet driver for STM32 connectivity line with port on
MMstm32f107 board

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

[bugzilla-daemon]

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

--- Comment #9 from Jerzy Dyrda <[hidden email]> 2011-10-10 22:49:41 BST ---
Hello Ilija,

(In reply to comment #3)

> 1. Some unresolved conflicts remain upon Propox board selection in Configtool
> and it may mislead the user that something is broken. It seems that Configtool
> can't resolve the CYGHWR_HAL_CORTEXM_STM32_CLOCK_PLL_SOURCE on it's own so
> let's make it little-bit easier:
>   -  Is it possible to re-arrange legal_values expression in order to make it
> easier for Configtool?
>   - Or it is better to calculate default_value conditionally
> (CYGINT_HAL_CORTEXM_STM32_CL==0 ?)
Done according to second hint.

> 2. CYGPKG_IO_ETH_DRIVERS is normally not included in the target. Not everybody
> would use Ethernet. Please remove it.
OK.

> 3. FYI, the PHY driver DP8348 has been resolved in meantime by Bug 1001235.
> Please synchronize your code with it.
Done.

> Now some general hints: It usually takes a number of iterations before code is
> fit for commit and it would be easier for me (hopefully also for you) if you
> break the patch in several diffs (divide and conquer). Ideally - a diff for
> every affected package, in this case: /eth driver/, /variant/, /platform/ and
> eventually /phy driver/. As exception ecos.db is better not sent as diff,
> instead put your ecos.db entries in a plain file and name it ecos_db.txt .
OK. I split patch into three parts but please consider that initially I would
like to contribute only eth driver. However STM32 Connectivity Line introduces
so many modification that even platform package is required. Summarizing eth
driver can't be introduced without rest of stuff.

Best regards,
jerzy

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

[bugzilla-daemon]

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

--- Comment #10 from Ilija Kocho <[hidden email]> 2011-10-11 20:53:18 BST ---
(In reply to comment #8)
> Created an attachment (id=1397)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1397) [details]
> ecos.db for Ethernet driver for STM32 connectivity line with port on
> MMstm32f107 board

Some lines are missing spaces and configtool can't read the repository.

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

[bugzilla-daemon]

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

--- Comment #11 from Ilija Kocho <[hidden email]> 2011-10-11 21:39:04 BST ---
(In reply to comment #7)
> Created an attachment (id=1396)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1396) [details]
> Variant modification needed by STM32 CL

http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/include/var_io.h_sec7

Ethernet definitions are usually stored with Ethernet driver, either with C
source or in a header file (I prefer the second). If you move it there than
that would imply that names do not contain "_HAL_".


http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/src/hal_diag.c_sec1

What is the reason for renaming of hal_stm32_serial_init() into
hal_stm32_serial_init_channel()


http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/src/stm32_misc.c_sec2

Changes like this may look attractive but (as well as previous one) may (will)
break other user's applications. Conditional compilation may help but in this
case I suggest to keep old code, it's simple and provides working conditions to
all peripherals.

http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/src/stm32_misc.c_sec3

Is this necessary?

http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/src/stm32_misc.c_sec4

I'm referring to following:
-    hal_stm32_sysclk /= 2;
+    hal_stm32_sysclk = CYGARC_HAL_CORTEXM_STM32_INTERNAL_CLOCK / 2;

If it works don't fix it :)

Now some minor remarks:
Comments like
#endif // CYGINT_HAL_CORTEXM_STM32_CL>0
on some places are not consistent with their respective #ifs.
And: (this is my personal only)
#if CYGINT_HAL_CORTEXM_STM32_CL != 0
or
#if CYGINT_HAL_CORTEXM_STM32_CL
is more common than >0.

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

[bugzilla-daemon]

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

--- Comment #12 from Ilija Kocho <[hidden email]> 2011-10-11 22:08:06 BST ---
(In reply to comment #6)
> Created an attachment (id=1395)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1395) [details]
> Platform for Propox MMstm32f107 module

First to remind you to my remark in comment #4

At present the user will see F103ZE which is unlikely to be the target device.
Platform may require the "STM variany in use" (CYGHWR_HAL_CORTEXM_STM32) to be
a specific device, or a set of eligible devices. If you opt for second case
just make sure that configtool resolves with the one found on Propox board.

Why JTAG startup? AFAIK Connectivity line desn't feature FSMC (or am I out of
date?). I guess SRAM startup should work (but test it first).

Regarding ROM startup there's one issue: RAM region is required in order to
build RedBoot. True, there's not much use of RedBoot on device with 64KiB RAM
but somebody would like to try. Solution is simple rename SRAM region (nor
section) into RAM.

Last but not least, please spend some time on Copyright banners (all files that
need it). They should for instance show correct year, author/contributor, etc.

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.

[bugzilla-daemon]

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

--- Comment #13 from Jerzy Dyrda <[hidden email]> 2011-10-11 22:44:36 BST ---

(In reply to comment #11)
> (In reply to comment #7)
> > Created an attachment (id=1396)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1396) [details]
[details]
> > Variant modification needed by STM32 CL
>
> http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/include/var_io.h_sec7
>
> Ethernet definitions are usually stored with Ethernet driver, either with C
> source or in a header file (I prefer the second). If you move it there than
> that would imply that names do not contain "_HAL_".
I would like to follow convention e.g. ADC registers with pin description are
stored in this header. I thought that any new peripheral description will be
stored in var_io.h

>
> http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/src/hal_diag.c_sec1
>
> What is the reason for renaming of hal_stm32_serial_init() into
> hal_stm32_serial_init_channel()
I'm not sure now :) but as I suppose it doesn't work.
BTW.
Definition of function is static void hal_stm32_serial_init(void) but function
is called with argument
&stm32_ser_channels[CYGNUM_HAL_VIRTUAL_VECTOR_CONSOLE_CHANNEL]
IMHO in case of non virtual vector is initialized other serial port than I
expect

>
> http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/src/stm32_misc.c_sec2
>
> Changes like this may look attractive but (as well as previous one) may (will)
> break other user's applications. Conditional compilation may help but in this
> case I suggest to keep old code, it's simple and provides working conditions
> to all peripherals.
I agree I have just added it because in case of non virtual vector isn't such
function.

> http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/src/stm32_misc.c_sec3
>
> Is this necessary?
Yes RCC (clock generation) module for CL has another PLL stage which is
required to achieve proper clock for eth module.    

BTW
I have further bad news : in STM32F2xx family RCC module is different than
STM32f105/7 family.

>
> http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/src/stm32_misc.c_sec4
>
> I'm referring to following:
> -    hal_stm32_sysclk /= 2;
> +    hal_stm32_sysclk = CYGARC_HAL_CORTEXM_STM32_INTERNAL_CLOCK / 2;
>
> If it works don't fix it :)
It was made assumption that internal clock is the same as input clock but it's
wrong for CL controller.

> Now some minor remarks:
> Comments like
> #endif // CYGINT_HAL_CORTEXM_STM32_CL>0
> on some places are not consistent with their respective #ifs.
> And: (this is my personal only)
> #if CYGINT_HAL_CORTEXM_STM32_CL != 0
> or
> #if CYGINT_HAL_CORTEXM_STM32_CL
> is more common than >0.
OK I check it.

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

[bugzilla-daemon]

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

--- Comment #14 from Jerzy Dyrda <[hidden email]> 2011-10-12 12:24:35 BST ---
Hello Ilija,

(In reply to comment #12)
> (In reply to comment #6)
> > Created an attachment (id=1395)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1395) [details]
[details]
> > Platform for Propox MMstm32f107 module
>
> First to remind you to my remark in comment #4
>
> At present the user will see F103ZE which is unlikely to be the target device.
Please consider that such situation will always happen if further target will
be introduced.

> Platform may require the "STM variany in use" (CYGHWR_HAL_CORTEXM_STM32) to be
> a specific device, or a set of eligible devices. If you opt for second case
> just make sure that configtool resolves with the one found on Propox board.
IMHO Either platform should provide proper STM32 variant or this option should
be remove - nobody is interested in this option.

Please review this option considering targets existing STM32F100/1/2/3 further
like my STM32F105/7 and possible STM32F2xx, STM32F4xx and give me "order" how I
should introduce STM32F105/7 family.


> Why JTAG startup? AFAIK Connectivity line desn't feature FSMC (or am I out of
> date?). I guess SRAM startup should work (but test it first).
You are absolute right. JTAG startup doesn't have sens and I remove it.

> Regarding ROM startup there's one issue: RAM region is required in order to
> build RedBoot. True, there's not much use of RedBoot on device with 64KiB RAM
> but somebody would like to try. Solution is simple rename SRAM region (nor
> section) into RAM.
I follow other cortex-m target like lm3s where is:
- only one startup ROM
- only SRAM section

BTW. I've built RedBoot successful with only SRAM section.

If it's possible I would like to shrink amount of startups only to ROM.

>
> Last but not least, please spend some time on Copyright banners
> (all files that need it).
> They should for instance show correct year, author/contributor, etc.
Of course I correct it.

Best regards,
jerzy

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

[bugzilla-daemon]

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

--- Comment #15 from Ilija Kocho <[hidden email]> 2011-10-12 21:33:58 BST ---
(In reply to comment #13)
> (In reply to comment #11)
> > (In reply to comment #7)
> > > Created an attachment (id=1396)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1396) [details]
[details] [details]

> > > Variant modification needed by STM32 CL
> >
> > http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/include/var_io.h_sec7
> >
> > Ethernet definitions are usually stored with Ethernet driver, either with C
> > source or in a header file (I prefer the second). If you move it there than
> > that would imply that names do not contain "_HAL_".
> I would like to follow convention e.g. ADC registers with pin description are
> stored in this header. I thought that any new peripheral description will be
> stored in var_io.h
>

Obviously there's more than one convention. However I think that driver is
better place for Ethernet #defines than HAL. I can imagine somebody using ADC
without driver so it would be convenient having defines handy in HAL. On the
other hand using Ethernet without driver is unlikely. Also, if for some reason,
in future new variant is added (this is not a proposal merely possibility) it
would reuse the header.

> >
> > http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/src/hal_diag.c_sec1
> >
> > What is the reason for renaming of hal_stm32_serial_init() into
> > hal_stm32_serial_init_channel()
> I'm not sure now :) but as I suppose it doesn't work.
> BTW.
> Definition of function is static void hal_stm32_serial_init(void) but function
> is called with argument
> &stm32_ser_channels[CYGNUM_HAL_VIRTUAL_VECTOR_CONSOLE_CHANNEL]
> IMHO in case of non virtual vector is initialized other serial port than I
> expect

I am referring (and link points) to changes in hal_stm32_diag_init()
See discussion on issue below.

>
> >
> > http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/src/stm32_misc.c_sec2
> >
> > Changes like this may look attractive but (as well as previous one) may (will)
> > break other user's applications. Conditional compilation may help but in this
> > case I suggest to keep old code, it's simple and provides working conditions
> > to all peripherals.
> I agree I have just added it because in case of non virtual vector isn't such
> function.

I am referring (and link points) to changes in hal_variant_init(). Once these
functions were published they are being (as are) used in applications not known
to me or you. Such changes may make many users unhappy and are unacceptable.


>
> > http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/src/stm32_misc.c_sec3
> >
> > Is this necessary?
> Yes RCC (clock generation) module for CL has another PLL stage which is
> required to achieve proper clock for eth module.    
>

My question is referring to cyg_uint32 hal_arch_default_isr(). Why do you need
it?

> BTW
> I have further bad news : in STM32F2xx family RCC module is different than
> STM32f105/7 family.
>

Regarding this (and in general), take in consideration future extensions when
you design system software.

> >
> > http://bugs.ecos.sourceware.org/attachment.cgi?id=1396&action=diff#ecos_clear/packages/hal/cortexm/stm32/var/current/src/stm32_misc.c_sec4
> >
> > I'm referring to following:
> > -    hal_stm32_sysclk /= 2;
> > +    hal_stm32_sysclk = CYGARC_HAL_CORTEXM_STM32_INTERNAL_CLOCK / 2;
> >
> > If it works don't fix it :)
> It was made assumption that internal clock is the same as input clock but it's
> wrong for CL controller.

OK so this is needed (sorry :).
Then you can add it, but again, take care not to break backward compatibility.
Preprocessor may help you.

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

[bugzilla-daemon]

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

--- Comment #16 from Ilija Kocho <[hidden email]> 2011-10-12 22:15:09 BST ---
(In reply to comment #14)
> Hello Ilija,
>
> (In reply to comment #12)
> > (In reply to comment #6)
> > > Created an attachment (id=1395)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1395) [details]
[details] [details]

> > > Platform for Propox MMstm32f107 module
> >
> > First to remind you to my remark in comment #4
> >
> > At present the user will see F103ZE which is unlikely to be the target device.
> Please consider that such situation will always happen if further target will
> be introduced.
>
> > Platform may require the "STM variany in use" (CYGHWR_HAL_CORTEXM_STM32) to be
> > a specific device, or a set of eligible devices. If you opt for second case
> > just make sure that configtool resolves with the one found on Propox board.
> IMHO Either platform should provide proper STM32 variant or this option should
> be remove - nobody is interested in this option.
>
> Please review this option considering targets existing STM32F100/1/2/3 further
> like my STM32F105/7 and possible STM32F2xx, STM32F4xx and give me "order" how I
> should introduce STM32F105/7 family.
>

I am not in a position to "order" you but I can give you some hints. Here is a
hal_cortexm_stm32_mmstm32f107.cdl diff snippet.

     implements  CYGINT_DEVS_ETH_CORTEXM_STM32_REMAP_PINS
     implements  CYGINT_DEVS_SPI_CORTEXM_STM32_BUS3_REMAP_PINS
+
+    requires {
+        (CYGHWR_HAL_CORTEXM_STM32 == "F107VB") ||
+        (CYGHWR_HAL_CORTEXM_STM32 == "F107VC") ||
+        (CYGHWR_HAL_CORTEXM_STM32 == "F105VB") ||
+        (CYGHWR_HAL_CORTEXM_STM32 == "F105VC")
+    }
+
     requires      { is_active(CYGPKG_DEVS_ETH_PHY) implies
                     (1 == CYGHWR_DEVS_ETH_PHY_DP8384X) }

You can at your option put a single or multiple devices in "requires".
Configtool will resolve (conflict) with first one. Other listed devices shall
be "allowed" and rest of devices (off the list) shall raise conflict.

Note: If you opt for multiple devices they should be ones that the controller
on Propox board can emulate.

Regarding F2 devices let's leave them for the time being. It will be another
platform anyway.

>
> > Why JTAG startup? AFAIK Connectivity line desn't feature FSMC (or am I out of
> > date?). I guess SRAM startup should work (but test it first).
> You are absolute right. JTAG startup doesn't have sens and I remove it.
>
> > Regarding ROM startup there's one issue: RAM region is required in order to
> > build RedBoot. True, there's not much use of RedBoot on device with 64KiB RAM
> > but somebody would like to try. Solution is simple rename SRAM region (nor
> > section) into RAM.
> I follow other cortex-m target like lm3s where is:
> - only one startup ROM
> - only SRAM section

I am refering to SRAM memory region, not .sram section.

>
> BTW. I've built RedBoot successful with only SRAM section.

You probably do it without GDB support. Then I was unclear, sorry, I should
have said "RAM is required by GDB support".

When I reconsider, this is a small memory device, perhaps we could live without
it.

>
> If it's possible I would like to shrink amount of startups only to ROM.

ROM startup is enough.


P.S.
As a general advice, please take care about backward compatibility - it is very
important. If able, please do regressions on "STM3210E EVAL board" (or
equivalent). If you don't have hardware at least check the building process.

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.

[bugzilla-daemon]

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

--- Comment #17 from Ilija Kocho <[hidden email]> 2011-10-16 15:11:36 BST ---
(In reply to comment #13)
> (In reply to comment #11)
> > (In reply to comment #7)
> > And: (this is my personal only)
> > #if CYGINT_HAL_CORTEXM_STM32_CL != 0
> > or
> > #if CYGINT_HAL_CORTEXM_STM32_CL
> > is more common than >0.
> OK I check it.

Even better would be to change cdl_interface CYGINT_HAL_CORTEXM_STM32_CL flavor
to bool and then use

#ifdef CYGINT_HAL_CORTEXM_STM32_CL
#ifndef CYGINT_HAL_CORTEXM_STM32_CL

The advantage is that for non-Connectivity-Line devices
CYGINT_HAL_CORTEXM_STM32_CL shall not exist.

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.

[bugzilla-daemon]

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

Ilija Kocho <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         OS/Version|ARM                         |Cortex-M

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

[bugzilla-daemon]

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

--- Comment #19 from Ilija Kocho <[hidden email]> 2011-10-22 13:21:29 BST ---
(In reply to comment #5)
> Created an attachment (id=1394)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1394) [details]
> Ethernet driver for STM32 CL
>
> Ethernet driver for STM32 Connectivity Line

Some comments on Ethernet driver:

MII and RMII interfaces are mutually exclusive and CDL should reflect this. A
cdl_option with legal values "RMII" "MII" would provide it and in addition make
it visible in configtool.


"Remap pins" could be also visible if it were cdl_option.


Regarding pins, some addition to my statement in Comment #11. Since pins are
being provided by HAL, they should be defined in HAL (unlike other Ethernet
definitions such as registers, etc.). Preferable place is plf_io.h rather than
var_io.h. because other chips (present or future) may have different pin
mapping.
On the other hand, the pin functions - once assigned to Ethernet (although pins
are provided provided by HAL) belong to Ethernet so their naming should reflect
this Here is a plf_io.h snuppet:

plf_io.h snippet --------------------------------

#define CYGHWR_IO_ETH_STM32MAC_MII_COL \
        CYGHWR_HAL_STM32_GPIO(A, 3,  IN , FLOATING)
...
-------------------------------------------------

Also if_stm32.c
Could CYGHWR_HAL_STM32_GPIO_SET(CYGHWR_...); lines be replaced by a loop?
And in order to avoid specifying HAL specific macros in a device driver, a new
macro can be defined CYGHWR_IO_ETH_STM32MAC_PIN(...).

Note: In macro names above I arbitrarily put "STM32MAC" segment. Probably there
is a more appropriate name for this Ethernet controller.

CYGNUM_DEVS_ETH_CORTEXM_STM32_RX_BUFS: Is there a range of legal values?
BTW other Ethernet devices that I have seen also provide configuration option
for TX_BUFFS.Is this fixed on STM32 Ethernet controller?


TCP/IP Checksum generation and check.
FYI lwIP is aware of such hardware features
http://sourceware.org/ml/ecos-discuss/2011-07/msg00017.html
and it would be good if they are implemented.

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.