[Bug 1000761] eCos support for MPC5xxx MCUs

[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=1000761

--- Comment #67 from Ilija Kocho <ilijak@siva.com.mk> 2012-10-30 13:25:52 GMT ---
(In reply to comment #64)

Hi Stefan

> Hi Ilija,
> I am a bit confused about your two last posts.
> 
> Our intention was to split the drivers into a generic part, which is put into
> the dev section and the device dependent definitions, which I did put into the
> HAL section. The rational was, that the MPC5xxx family spans now over 3 device
> technologies 130, 90, 55 nm and more than 8 years and that obviously a number
> of things have changed despite some generic base architecture. Therefore our
> attempt to split the drivers in a way like this.

Technologies change, new features are being added, but it usually doesn't
change the basic device structure. New flags/fields are introduced in
previously unused/reserved bits. Even when new registers are added they appear
in positions that don't break binary compatibility with the legacy devices.

> Are you proposing, that we should remove those device dependent definitions
> from the HAL and have a separate folder for each device in the dev section ?

On the contrary I am proposing reduction of number of files. The structures and
definitions that, at present, have multiple copies across platforms should be
consolidated by single instances that shall live either under variant or with
respective device drivers.

Device drivers would keep headers for their respective hardware: eSCI, FlexCAN,
DSPI, Ethernet, etc. Here they can be shared by multiple
architectures/families: MPC5xxx, Kinetis, Coldfire, etc.

Variant would accommodate devices that belong to the HAL: clocks, interrupt
controller, port control and so on.

Note: There are hardware components that belong to HAL, but are shared among
multiple architectures, for example eDMA. Their sub-tree is packages/hal/misc.

Normal places for HAL structures and definitions is var_io.h (itself or files,
living in the same directory with, and being included by var_io.h). For
interrupt-related definitions, such as interrupt vectors, there is var_intr.h.

Now we come to platform specific data. Here come [almost always] pin (port)
definitions and [sometimes] base addresses. It is important to note that base
addresses are usually same across the complete family and should be defined in
variant, but sometimes they may differ. Sometimes it is useful to define some
default value in variant, with provision for platform override.

> 
> Notably the following 3 files exist in every platform: 
> - var_regs.h: Register definitions in the eCos style. Of course there is
> overlap with the device header file, since certain definitions exist in both
> files, but this file is done in the way used in many other eCOS platforms and
> the other file is reused from the Freescale website.

> - mpc5*.h: This is the device definition header file for each device take
> directly from our website. I tried to use that unmodified, however in some
> instances I needed to make small adjustments, so that the common drivers find
> common definitions. Since the spirit was to try to use those files unmodified
> from the website, that are also the only files (at least that was my
> attention), where there is a different copyright, since otherwise I would need
> to modify those files.

There are 2 important issues with mpc5*.h files:
  - Their copyright message: IANAL and please do not consider this as legal
advice, but IMO that the copyright banner on these files is not acceptable.
  - Redundancy: Same structures and definitions are being copied over and over
and some of them are not even being used. Actually they are the ones that
should be consolidated as described above. Even in present sources, some
structures and/or definitions have their replacements in device driver headers,
such as eSCI and CAN, so should have already been removed.

> - plf_intr.h: Interrupt Mapping for each device. This must be different for
> each device. There are of course common sections accross multiple files, but
> not two will be identical.
> 
> Can you please help me to understand, what I should change now ? I would not
> like to remove the support for some devices to have less files to review - that
> sounds wrong to me. I am still struggling to understand what you expect here. 

1. Consolidate HAL structures and definitions under variant level. This usually
gives structures and definitions that are superset (union) of individual
devices. If some devices have radically different hardware they may lend under
their respective platform (feel free to point eventual such case in this bug -
we can discuss it before you proceed with implementation).

2. The same for device drivers. Please note that we already have such files for
eSCI and FlexCAN. Also there are devices not yet supported by eCos. If they do
not belong to HAL, you can safely omit them from headers (at least for the time
being).

3. See which base address definitions shall lend under variant and which under
platform(s).

4. Once you have accomplished Steps 1. and 2., mpc5*.h files shall be redundant
and you can remove them.


> I
> know that you are pushing to reuse the eCos drivers from other peripherals and
> we will try to do that on some, but e.g. the new Flexcan driver does not work
> yet at all (although I have already invested quite some time in it).

Regarding FlexCAN, I guess, basically you need to do the same thing that you
have already done with eSCI. FlexCAN should be slightly easier than eSCI
because CAN has no HAL drivers. Maybe it could help if we open a separate bug
for FlexCan.

Should you have additional questions, feel free to ask.
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.