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

[bugzilla-daemon at bugs dot ecos dot sourceware dot org]

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 <ilijak@siva.com.mk> 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.